Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support of snapshot copy to StorPool primary storage in different zones #9478

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/src/main/java/com/cloud/storage/VolumeApiService.java
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ public interface VolumeApiService {

Volume detachVolumeFromVM(DetachVolumeCmd cmd);

Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm, Snapshot.LocationType locationType, boolean asyncBackup, Map<String, String> tags, List<Long> zoneIds)
Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm, Snapshot.LocationType locationType, boolean asyncBackup, Map<String, String> tags, List<Long> zoneIds, List<Long> poolIds)
throws ResourceAllocationException;

Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType, List<Long> zoneIds) throws ResourceAllocationException;
Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType, List<Long> zoneIds, List<Long> poolIds) throws ResourceAllocationException;

Volume updateVolume(long volumeId, String path, String state, Long storageId,
Boolean displayVolume, Boolean deleteProtection,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,7 @@ public class ApiConstants {

public static final String ZONE_ID_LIST = "zoneids";
public static final String DESTINATION_ZONE_ID_LIST = "destzoneids";
public static final String STORAGE_ID_LIST = "storageids";
public static final String ADMIN = "admin";
public static final String CHECKSUM_PARAMETER_PREFIX_DESCRIPTION = "The parameter containing the checksum will be considered a MD5sum if it is not prefixed\n"
+ " and just a plain ascii/utf8 representation of a hexadecimal string. If it is required to\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@

package org.apache.cloudstack.api.command.user.snapshot;

import java.util.ArrayList;
import java.util.List;

import com.cloud.dc.DataCenter;
import com.cloud.event.EventTypes;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.exception.StorageUnavailableException;
import com.cloud.storage.Snapshot;
import com.cloud.user.Account;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiCommandResourceType;
Expand All @@ -31,26 +35,23 @@
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.command.user.UserCmd;
import org.apache.cloudstack.api.response.SnapshotResponse;
import org.apache.cloudstack.api.response.StoragePoolResponse;
import org.apache.cloudstack.api.response.ZoneResponse;
import org.apache.cloudstack.context.CallContext;
import org.apache.commons.collections.CollectionUtils;

import com.cloud.dc.DataCenter;
import com.cloud.event.EventTypes;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.exception.StorageUnavailableException;
import com.cloud.storage.Snapshot;
import com.cloud.user.Account;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.util.ArrayList;
import java.util.List;

@APICommand(name = "copySnapshot", description = "Copies a snapshot from one zone to another.",
responseObject = SnapshotResponse.class, responseView = ResponseObject.ResponseView.Restricted,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.19.0",
authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
public class CopySnapshotCmd extends BaseAsyncCmd implements UserCmd {
public static final Logger logger = LogManager.getLogger(CopySnapshotCmd.class.getName());
private Snapshot snapshot;

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand Down Expand Up @@ -84,6 +85,15 @@
"Do not specify destzoneid and destzoneids together, however one of them is required.")
protected List<Long> destZoneIds;

@Parameter(name = ApiConstants.STORAGE_ID_LIST,
type=CommandType.LIST,
collectionType = CommandType.UUID,
entityType = StoragePoolResponse.class,
required = false,
description = "A comma-separated list of IDs of the storage pools in other zones in which the snapshot will be made available. " +
slavkap marked this conversation as resolved.
Show resolved Hide resolved
"The snapshot will always be made available in the zone in which the volume is present. Currently supported for StorPool only")
protected List<Long> storagePoolIds;

/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
Expand All @@ -106,7 +116,11 @@
destIds.add(destZoneId);
return destIds;
}
return null;
return new ArrayList<>();

Check warning on line 119 in api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java#L119

Added line #L119 was not covered by tests
}

public List<Long> getStoragePoolIds() {
return storagePoolIds;

Check warning on line 123 in api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java#L122-L123

Added lines #L122 - L123 were not covered by tests
}

@Override
Expand Down Expand Up @@ -152,7 +166,7 @@
@Override
public void execute() throws ResourceUnavailableException {
try {
if (destZoneId == null && CollectionUtils.isEmpty(destZoneIds))
if (destZoneId == null && CollectionUtils.isEmpty(destZoneIds) && CollectionUtils.isEmpty(storagePoolIds))
throw new ServerApiException(ApiErrorCode.PARAM_ERROR,
"Either destzoneid or destzoneids parameters have to be specified.");

Expand All @@ -161,7 +175,7 @@
"Both destzoneid and destzoneids cannot be specified at the same time.");

CallContext.current().setEventDetails(getEventDescription());
Snapshot snapshot = _snapshotService.copySnapshot(this);
snapshot = _snapshotService.copySnapshot(this);

if (snapshot != null) {
SnapshotResponse response = _queryService.listSnapshot(this);
Expand All @@ -177,6 +191,13 @@
logger.warn("Exception: ", ex);
throw new ServerApiException(ApiErrorCode.RESOURCE_ALLOCATION_ERROR, ex.getMessage());
}
}

public Snapshot getSnapshot() {
return snapshot;
}

Check warning on line 198 in api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java#L196-L198

Added lines #L196 - L198 were not covered by tests

public void setSnapshot(Snapshot snapshot) {
this.snapshot = snapshot;

Check warning on line 201 in api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java#L200-L201

Added lines #L200 - L201 were not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.cloudstack.api.response.DomainResponse;
import org.apache.cloudstack.api.response.SnapshotPolicyResponse;
import org.apache.cloudstack.api.response.SnapshotResponse;
import org.apache.cloudstack.api.response.StoragePoolResponse;
import org.apache.cloudstack.api.response.VolumeResponse;
import org.apache.cloudstack.api.response.ZoneResponse;
import org.apache.commons.collections.MapUtils;
Expand Down Expand Up @@ -99,6 +100,15 @@
since = "4.19.0")
protected List<Long> zoneIds;

@Parameter(name = ApiConstants.STORAGE_ID_LIST,
type=CommandType.LIST,
collectionType = CommandType.UUID,
entityType = StoragePoolResponse.class,
description = "A comma-separated list of IDs of the storage pools in other zones in which the snapshot will be made available. " +
"The snapshot will always be made available in the zone in which the volume is present.",
since = "4.20.0")
protected List<Long> storagePoolIds;

private String syncObjectType = BaseAsyncCmd.snapshotHostSyncObject;

// ///////////////////////////////////////////////////
Expand Down Expand Up @@ -161,6 +171,10 @@
return zoneIds;
}

public List<Long> getStoragePoolIds() {
return storagePoolIds;
}

// ///////////////////////////////////////////////////
// ///////////// API Implementation///////////////////
// ///////////////////////////////////////////////////
Expand Down Expand Up @@ -209,7 +223,7 @@

@Override
public void create() throws ResourceAllocationException {
Snapshot snapshot = _volumeService.allocSnapshot(getVolumeId(), getPolicyId(), getSnapshotName(), getLocationType(), getZoneIds());
Snapshot snapshot = _volumeService.allocSnapshot(getVolumeId(), getPolicyId(), getSnapshotName(), getLocationType(), getZoneIds(), getStoragePoolIds());

Check warning on line 226 in api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java#L226

Added line #L226 was not covered by tests
if (snapshot != null) {
setEntityId(snapshot.getId());
setEntityUuid(snapshot.getUuid());
Expand All @@ -223,7 +237,7 @@
Snapshot snapshot;
try {
snapshot =
_volumeService.takeSnapshot(getVolumeId(), getPolicyId(), getEntityId(), _accountService.getAccount(getEntityOwnerId()), getQuiescevm(), getLocationType(), getAsyncBackup(), getTags(), getZoneIds());
_volumeService.takeSnapshot(getVolumeId(), getPolicyId(), getEntityId(), _accountService.getAccount(getEntityOwnerId()), getQuiescevm(), getLocationType(), getAsyncBackup(), getTags(), getZoneIds(), getStoragePoolIds());

if (snapshot != null) {
SnapshotResponse response = _responseGenerator.createSnapshotResponse(snapshot);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
// under the License.
package org.apache.cloudstack.api.command.user.snapshot;

import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.PermissionDeniedException;
import com.cloud.projects.Project;
import com.cloud.storage.Volume;
import com.cloud.storage.snapshot.SnapshotPolicy;
import com.cloud.user.Account;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiCommandResourceType;
Expand All @@ -30,16 +31,15 @@
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.response.SnapshotPolicyResponse;
import org.apache.cloudstack.api.response.StoragePoolResponse;
import org.apache.cloudstack.api.response.VolumeResponse;
import org.apache.cloudstack.api.response.ZoneResponse;
import org.apache.commons.collections.MapUtils;

import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.PermissionDeniedException;
import com.cloud.projects.Project;
import com.cloud.storage.Volume;
import com.cloud.storage.snapshot.SnapshotPolicy;
import com.cloud.user.Account;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

@APICommand(name = "createSnapshotPolicy", description = "Creates a snapshot policy for the account.", responseObject = SnapshotPolicyResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
Expand Down Expand Up @@ -83,6 +83,14 @@
"The snapshots will always be made available in the zone in which the volume is present.")
protected List<Long> zoneIds;

@Parameter(name = ApiConstants.STORAGE_ID_LIST,
type=CommandType.LIST,
collectionType = CommandType.UUID,
entityType = StoragePoolResponse.class,
description = "A comma-separated list of IDs of the storage pools in other zones in which the snapshot will be made available. " +
"The snapshot will always be made available in the zone in which the volume is present.",
since = "4.20.0")
protected List<Long> storagePoolIds;
/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
Expand Down Expand Up @@ -119,6 +127,8 @@
return zoneIds;
}

public List<Long> getStoragePoolIds() { return storagePoolIds; }

Check warning on line 130 in api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotPolicyCmd.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotPolicyCmd.java#L130

Added line #L130 was not covered by tests

/////////////////////////////////////////////////////
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,16 @@
// under the License.
package org.apache.cloudstack.api.response;

import java.util.LinkedHashSet;
import java.util.Set;

import com.cloud.serializer.Param;
import com.cloud.storage.snapshot.SnapshotPolicy;
import com.google.gson.annotations.SerializedName;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.BaseResponseWithTagInformation;
import org.apache.cloudstack.api.EntityReference;

import com.cloud.serializer.Param;
import com.cloud.storage.snapshot.SnapshotPolicy;
import com.google.gson.annotations.SerializedName;
import java.util.LinkedHashSet;
import java.util.Set;

@EntityReference(value = SnapshotPolicy.class)
public class SnapshotPolicyResponse extends BaseResponseWithTagInformation {
Expand Down Expand Up @@ -62,9 +61,14 @@
@Param(description = "The list of zones in which snapshot backup is scheduled", responseObject = ZoneResponse.class, since = "4.19.0")
protected Set<ZoneResponse> zones;

@SerializedName(ApiConstants.STORAGE)
@Param(description = "The list of pools in which snapshot backup is scheduled", responseObject = StoragePoolResponse.class, since = "4.20.0")
protected Set<StoragePoolResponse> storagePools;

public SnapshotPolicyResponse() {
tags = new LinkedHashSet<ResourceTagResponse>();
zones = new LinkedHashSet<>();
storagePools = new LinkedHashSet<>();

Check warning on line 71 in api/src/main/java/org/apache/cloudstack/api/response/SnapshotPolicyResponse.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/org/apache/cloudstack/api/response/SnapshotPolicyResponse.java#L71

Added line #L71 was not covered by tests
}

public String getId() {
Expand Down Expand Up @@ -130,4 +134,6 @@
public void setZones(Set<ZoneResponse> zones) {
this.zones = zones;
}

public void setStoragePools(Set<StoragePoolResponse> pools) { this.storagePools = pools; }

Check warning on line 138 in api/src/main/java/org/apache/cloudstack/api/response/SnapshotPolicyResponse.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/org/apache/cloudstack/api/response/SnapshotPolicyResponse.java#L138

Added line #L138 was not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public void testCreateSuccess() {
Snapshot snapshot = Mockito.mock(Snapshot.class);
try {
Mockito.when(volumeApiService.takeSnapshot(nullable(Long.class), nullable(Long.class), isNull(),
nullable(Account.class), nullable(Boolean.class), nullable(Snapshot.LocationType.class), nullable(Boolean.class), nullable(Map.class), nullable(List.class))).thenReturn(snapshot);
nullable(Account.class), nullable(Boolean.class), nullable(Snapshot.LocationType.class), nullable(Boolean.class), nullable(Map.class), nullable(List.class), nullable(List.class))).thenReturn(snapshot);

} catch (Exception e) {
Assert.fail("Received exception when success expected " + e.getMessage());
Expand Down Expand Up @@ -126,7 +126,7 @@ public void testCreateFailure() {

try {
Mockito.when(volumeApiService.takeSnapshot(nullable(Long.class), nullable(Long.class), nullable(Long.class),
nullable(Account.class), nullable(Boolean.class), nullable(Snapshot.LocationType.class), nullable(Boolean.class), any(), Mockito.anyList())).thenReturn(null);
nullable(Account.class), nullable(Boolean.class), nullable(Snapshot.LocationType.class), nullable(Boolean.class), any(), Mockito.anyList(), Mockito.anyList())).thenReturn(null);
} catch (Exception e) {
Assert.fail("Received exception when success expected " + e.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,9 @@
/**
* indicates that this driver supports reverting a volume to a snapshot state
*/
CAN_REVERT_VOLUME_TO_SNAPSHOT
CAN_REVERT_VOLUME_TO_SNAPSHOT,

Check warning on line 43 in engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java

View check run for this annotation

Codecov / codecov/patch

engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java#L43

Added line #L43 was not covered by tests
/**
* indicates that the driver supports copying snapshot between zones on pools of the same type
*/
CAN_COPY_SNAPSHOT_BETWEEN_ZONES_AND_SAME_POOL_TYPE

Check warning on line 47 in engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java

View check run for this annotation

Codecov / codecov/patch

engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java#L47

Added line #L47 was not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,6 @@ public interface SnapshotService {
AsyncCallFuture<SnapshotResult> copySnapshot(SnapshotInfo snapshot, String copyUrl, DataStore dataStore) throws ResourceUnavailableException;

AsyncCallFuture<CreateCmdResult> queryCopySnapshot(SnapshotInfo snapshot) throws ResourceUnavailableException;

AsyncCallFuture<SnapshotResult> copySnapshot(SnapshotInfo sourceSnapshot, SnapshotInfo destSnapshot, SnapshotStrategy strategy);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
// under the License.
package org.apache.cloudstack.engine.subsystem.api.storage;


import com.cloud.storage.Snapshot;
import org.apache.cloudstack.framework.async.AsyncCompletionCallback;

public interface SnapshotStrategy {

enum SnapshotOperation {
TAKE, BACKUP, DELETE, REVERT
TAKE, BACKUP, DELETE, REVERT, COPY
}

SnapshotInfo takeSnapshot(SnapshotInfo snapshot);
Expand All @@ -35,4 +37,7 @@
StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperation op);

void postSnapshotCreation(SnapshotInfo snapshot);

default void copySnapshot(DataObject snapshotSource, DataObject snapshotDest, AsyncCompletionCallback<CreateCmdResult> caller) {
}

Check warning on line 42 in engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java

View check run for this annotation

Codecov / codecov/patch

engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java#L41-L42

Added lines #L41 - L42 were not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@
private boolean quiesceVm;
private Snapshot.LocationType locationType;
private boolean asyncBackup;

private List<Long> poolIds;
private List<Long> zoneIds;

public VmWorkTakeVolumeSnapshot(long userId, long accountId, long vmId, String handlerName,
Long volumeId, Long policyId, Long snapshotId, boolean quiesceVm, Snapshot.LocationType locationType,
boolean asyncBackup, List<Long> zoneIds) {
boolean asyncBackup, List<Long> zoneIds, List<Long> poolIds) {
super(userId, accountId, vmId, handlerName);
this.volumeId = volumeId;
this.policyId = policyId;
Expand All @@ -44,6 +44,7 @@
this.locationType = locationType;
this.asyncBackup = asyncBackup;
this.zoneIds = zoneIds;
this.poolIds = poolIds;
}

public Long getVolumeId() {
Expand Down Expand Up @@ -71,4 +72,8 @@
public List<Long> getZoneIds() {
return zoneIds;
}

public List<Long> getPoolIds() {
return poolIds;
}

Check warning on line 78 in engine/components-api/src/main/java/com/cloud/vm/VmWorkTakeVolumeSnapshot.java

View check run for this annotation

Codecov / codecov/patch

engine/components-api/src/main/java/com/cloud/vm/VmWorkTakeVolumeSnapshot.java#L76-L78

Added lines #L76 - L78 were not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ public class VmWorkTakeVolumeSnapshotTest {
@Test
public void testVmWorkTakeVolumeSnapshotZoneIds() {
List<Long> zoneIds = List.of(10L, 20L);
List<Long> poolIds = List.of(10L, 20L);
VmWorkTakeVolumeSnapshot work = new VmWorkTakeVolumeSnapshot(1L, 1L, 1L, "handler",
1L, 1L, 1L, false, null, false, zoneIds);
1L, 1L, 1L, false, null, false, zoneIds, poolIds);
Assert.assertNotNull(work.getZoneIds());
Assert.assertEquals(zoneIds.size(), work.getZoneIds().size());
Assert.assertEquals(zoneIds.get(0), work.getZoneIds().get(0));
Expand Down
Loading
Loading