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

CLOUDSTACK-4858 Honors the snapshot.backup.rightafter configuration v… #1697

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -44,7 +44,10 @@
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
Expand Down Expand Up @@ -178,6 +181,8 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati
ClusterManager clusterManager;
@Inject
StorageManager storageMgr;
@Inject
StorageStrategyFactory _storageStrategyFactory;

private final StateMachine2<Volume.State, Volume.Event, Volume> _volStateMachine;
protected List<StoragePoolAllocator> _storagePoolAllocators;
Expand Down Expand Up @@ -383,6 +388,24 @@ public VolumeInfo createVolumeFromSnapshot(Volume volume, Snapshot snapshot, Use
DataStoreRole dataStoreRole = getDataStoreRole(snapshot);
SnapshotInfo snapInfo = snapshotFactory.getSnapshot(snapshot.getId(), dataStoreRole);


if(snapInfo == null && dataStoreRole == DataStoreRole.Image) {
// snapshot is not backed up to secondary, let's do that now.
snapInfo = snapshotFactory.getSnapshot(snapshot.getId(), DataStoreRole.Primary);

if (snapInfo == null) {
throw new CloudRuntimeException("Cannot find snapshot " + snapshot.getId());
}
// We need to copy the snapshot onto secondary.
SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.BACKUP);
snapshotStrategy.backupSnapshot(snapInfo);

// Attempt to grab it again.
snapInfo = snapshotFactory.getSnapshot(snapshot.getId(), dataStoreRole);
if (snapInfo == null) {
throw new CloudRuntimeException("Cannot find snapshot " + snapshot.getId() + " on secondary and could not create backup");
}
}
// don't try to perform a sync if the DataStoreRole of the snapshot is equal to DataStoreRole.Primary
if (!DataStoreRole.Primary.equals(dataStoreRole)) {
try {
Expand Down
Expand Up @@ -47,6 +47,7 @@ public SnapshotStateMachineManagerImpl() {
stateMachine.addTransition(Snapshot.State.BackingUp, Event.OperationFailed, Snapshot.State.Error);
stateMachine.addTransition(Snapshot.State.BackedUp, Event.DestroyRequested, Snapshot.State.Destroying);
stateMachine.addTransition(Snapshot.State.BackedUp, Event.CopyingRequested, Snapshot.State.Copying);
stateMachine.addTransition(Snapshot.State.BackedUp, Event.BackupToSecondary, Snapshot.State.BackingUp);
stateMachine.addTransition(Snapshot.State.Copying, Event.OperationSucceeded, Snapshot.State.BackedUp);
stateMachine.addTransition(Snapshot.State.Copying, Event.OperationFailed, Snapshot.State.BackedUp);
stateMachine.addTransition(Snapshot.State.Destroying, Event.OperationSucceeded, Snapshot.State.Destroyed);
Expand Down
Expand Up @@ -39,6 +39,7 @@
import org.apache.cloudstack.storage.datastore.PrimaryDataStoreImpl;
import org.apache.cloudstack.storage.to.SnapshotObjectTO;

import com.cloud.configuration.Config;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.hypervisor.Hypervisor;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
Expand Down Expand Up @@ -374,8 +375,24 @@ public SnapshotInfo takeSnapshot(SnapshotInfo snapshot) {

snapshot = result.getSnashot();
DataStore primaryStore = snapshot.getDataStore();
boolean backupFlag = Boolean.parseBoolean(configDao.getValue(Config.BackupSnapshotAfterTakingSnapshot.toString()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nathanejohnson in #1600 I've added a flag to the create snapshot API called locationType which can take primary or secondary as the parameter. We could use that here as well. Although for this PR, a global would suffice


SnapshotInfo backupedSnapshot = backupSnapshot(snapshot);
SnapshotInfo backupedSnapshot;
if(backupFlag) {
backupedSnapshot = backupSnapshot(snapshot);
} else {
// Fake it to get the transitions to fire in the proper order
s_logger.debug("skipping backup of snapshot due to configuration "+Config.BackupSnapshotAfterTakingSnapshot.toString());

SnapshotObject snapObj = (SnapshotObject)snapshot;
try {
snapObj.processEvent(Snapshot.Event.OperationNotPerformed);
} catch (NoTransitionException e) {
s_logger.debug("Failed to change state: " + snapshot.getId() + ": " + e.toString());
throw new CloudRuntimeException(e.toString());
}
backupedSnapshot = snapshot;
}

try {
SnapshotInfo parent = snapshot.getParent();
Expand Down
2 changes: 1 addition & 1 deletion server/src/com/cloud/configuration/Config.java
Expand Up @@ -512,7 +512,7 @@ public enum Config {
null),
SnapshotDeltaMax("Snapshots", SnapshotManager.class, Integer.class, "snapshot.delta.max", "16", "max delta snapshots between two full snapshots.", null),
BackupSnapshotAfterTakingSnapshot(
"Hidden",
"Snapshots",
SnapshotManager.class,
Boolean.class,
"snapshot.backup.rightafter",
Expand Down
Expand Up @@ -1018,7 +1018,13 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
DataStoreRole dataStoreRole = getDataStoreRole(snapshot, _snapshotStoreDao, dataStoreMgr);

SnapshotDataStoreVO snapshotStoreRef = _snapshotStoreDao.findBySnapshot(snapshotId, dataStoreRole);

if(snapshotStoreRef == null) {
// The snapshot was not backed up to secondary. Find the snap on primary
snapshotStoreRef = _snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
if(snapshotStoreRef == null) {
throw new CloudRuntimeException("Could not find snapshot");
}
}
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_CREATE, snapshot.getAccountId(), snapshot.getDataCenterId(), snapshotId, snapshot.getName(),
null, null, snapshotStoreRef.getPhysicalSize(), volume.getSize(), snapshot.getClass().getName(), snapshot.getUuid());

Expand Down
20 changes: 20 additions & 0 deletions server/src/com/cloud/template/TemplateManagerImpl.java
Expand Up @@ -75,7 +75,10 @@
import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager;
import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService;
Expand Down Expand Up @@ -254,6 +257,8 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
@Inject
private SnapshotDataFactory _snapshotFactory;
@Inject
StorageStrategyFactory _storageStrategyFactory;
@Inject
private TemplateService _tmpltSvr;
@Inject
private DataStoreManager _dataStoreMgr;
Expand Down Expand Up @@ -1493,6 +1498,21 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t
SnapshotInfo snapInfo = _snapshotFactory.getSnapshot(snapshotId, dataStoreRole);

if (dataStoreRole == DataStoreRole.Image) {
if (snapInfo == null) {
snapInfo = _snapshotFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
if(snapInfo == null) {
throw new CloudRuntimeException("Cannot find snapshot "+snapshotId);
}
// We need to copy the snapshot onto secondary.
SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.BACKUP);
snapshotStrategy.backupSnapshot(snapInfo);

// Attempt to grab it again.
snapInfo = _snapshotFactory.getSnapshot(snapshotId, dataStoreRole);
if(snapInfo == null) {
throw new CloudRuntimeException("Cannot find snapshot " + snapshotId + " on secondary and could not create backup");
}
}
DataStore snapStore = snapInfo.getDataStore();

if (snapStore != null) {
Expand Down
9 changes: 9 additions & 0 deletions server/test/com/cloud/template/TemplateManagerImplTest.java
Expand Up @@ -68,6 +68,7 @@
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager;
import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
Expand Down Expand Up @@ -161,6 +162,9 @@ public class TemplateManagerImplTest {
@Inject
SnapshotDao snapshotDao;

@Inject
StorageStrategyFactory storageStrategyFactory;

public class CustomThreadPoolExecutor extends ThreadPoolExecutor {
AtomicInteger ai = new AtomicInteger(0);
public CustomThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit,
Expand Down Expand Up @@ -453,6 +457,11 @@ public VMTemplateDao vmTemplateDao() {
return Mockito.mock(VMTemplateDao.class);
}

@Bean
public StorageStrategyFactory storageStrategyFactory() {
return Mockito.mock(StorageStrategyFactory.class);
}

@Bean
public VMTemplatePoolDao vmTemplatePoolDao() {
return Mockito.mock(VMTemplatePoolDao.class);
Expand Down