Skip to content

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

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
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 @@ -113,10 +113,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, Boolean useStorageReplication)
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, Boolean useStorageReplication) throws ResourceAllocationException;

Volume updateVolume(long volumeId, String path, String state, Long storageId,
Boolean displayVolume, Boolean deleteProtection,
Expand Down
4 changes: 4 additions & 0 deletions api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,9 @@ public class ApiConstants {
public static final String SNAPSHOT_POLICY_ID = "snapshotpolicyid";
public static final String SNAPSHOT_TYPE = "snapshottype";
public static final String SNAPSHOT_QUIESCEVM = "quiescevm";

public static final String USE_STORAGE_REPLICATION = "usestoragereplication";

public static final String SOURCE_CIDR_LIST = "sourcecidrlist";
public static final String SOURCE_ZONE_ID = "sourcezoneid";
public static final String SSL_VERIFICATION = "sslverification";
Expand Down Expand Up @@ -1063,6 +1066,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,19 @@
"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,
authorized = RoleType.Admin,
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. Currently supported for StorPool only")
protected List<Long> storagePoolIds;

@Parameter (name = ApiConstants.USE_STORAGE_REPLICATION, type=CommandType.BOOLEAN, required = false, description = "This parameter enables the option the snapshot to be copied to supported primary storage")
protected Boolean useStorageReplication;

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

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

Check warning on line 128 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#L126-L128

Added lines #L126 - L128 were not covered by tests

public Boolean useStorageReplication() {
if (useStorageReplication == null) {
return false;
}
return useStorageReplication;

Check warning on line 134 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#L134

Added line #L134 was not covered by tests
}

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

Expand All @@ -161,7 +186,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 +202,13 @@
logger.warn("Exception: ", ex);
throw new ServerApiException(ApiErrorCode.RESOURCE_ALLOCATION_ERROR, ex.getMessage());
}
}

public Snapshot getSnapshot() {
return snapshot;
}

Check warning on line 209 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#L207-L209

Added lines #L207 - L209 were not covered by tests

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

Check warning on line 212 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#L211-L212

Added lines #L211 - L212 were not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.List;
import java.util.Map;

import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiCommandResourceType;
import org.apache.cloudstack.api.ApiConstants;
Expand All @@ -32,6 +33,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 +101,19 @@
since = "4.19.0")
protected List<Long> zoneIds;

@Parameter(name = ApiConstants.STORAGE_ID_LIST,
type=CommandType.LIST,
collectionType = CommandType.UUID,
entityType = StoragePoolResponse.class,
authorized = RoleType.Admin,
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;

@Parameter (name = ApiConstants.USE_STORAGE_REPLICATION, type=CommandType.BOOLEAN, required = false, description = "This parameter enables the option the snapshot to be copied to supported primary storage")
protected Boolean useStorageReplication;

private String syncObjectType = BaseAsyncCmd.snapshotHostSyncObject;

// ///////////////////////////////////////////////////
Expand Down Expand Up @@ -161,6 +176,17 @@
return zoneIds;
}

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

public Boolean useStorageReplication() {
if (useStorageReplication == null) {
return false;
}
return useStorageReplication;

Check warning on line 187 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#L187

Added line #L187 was not covered by tests
}

// ///////////////////////////////////////////////////
// ///////////// API Implementation///////////////////
// ///////////////////////////////////////////////////
Expand Down Expand Up @@ -209,7 +235,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(), useStorageReplication());

Check warning on line 238 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#L238

Added line #L238 was not covered by tests
if (snapshot != null) {
setEntityId(snapshot.getId());
setEntityUuid(snapshot.getUuid());
Expand All @@ -223,7 +249,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(), useStorageReplication());

if (snapshot != null) {
SnapshotResponse response = _responseGenerator.createSnapshotResponse(snapshot);
Expand All @@ -243,7 +269,7 @@
}
}

private Snapshot.LocationType getLocationType() {
public Snapshot.LocationType getLocationType() {

if (Snapshot.LocationType.values() == null || Snapshot.LocationType.values().length == 0 || locationType == null) {
return null;
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), Mockito.anyBoolean())).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(), Mockito.anyBoolean())).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 @@ -87,7 +87,12 @@ public void testGetDestZoneIdWithBothParams() {

@Test (expected = ServerApiException.class)
public void testExecuteWrongNoParams() {
UUIDManager uuidManager = Mockito.mock(UUIDManager.class);
SnapshotApiService snapshotApiService = Mockito.mock(SnapshotApiService.class);
final CopySnapshotCmd cmd = new CopySnapshotCmd();
cmd._uuidMgr = uuidManager;
cmd._snapshotService = snapshotApiService;

try {
cmd.execute();
} catch (ResourceUnavailableException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,13 @@
/**
* 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
/**
* indicates that this driver supports the option to create a template from the back-end snapshot
*/
CAN_CREATE_TEMPLATE_FROM_SNAPSHOT

Check warning on line 51 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#L51

Added line #L51 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);
}
Loading
Loading