Skip to content

Commit 4d1377c

Browse files
CSTACKEX-200: CG create must have SVM name and respective validations
1 parent 2724dbb commit 4d1377c

2 files changed

Lines changed: 75 additions & 5 deletions

File tree

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -708,7 +708,8 @@ void createVmSnapshotsViaTemporaryCg(VMSnapshot vmSnapshot, UserVm userVm,
708708
try {
709709
logger.info("takeVMSnapshot: [CG:Create] Creating temporary consistency group [{}] for VM [{}] over {} FlexVol(s)",
710710
tempCgName, userVm.getInstanceName(), flexVolGroups.size());
711-
tempCgUuid = createTemporaryConsistencyGroup(snapshotClient, storageStrategy, authHeader, tempCgName, flexVolGroups.keySet());
711+
tempCgUuid = createTemporaryConsistencyGroup(snapshotClient, storageStrategy, authHeader, tempCgName,
712+
resolveConsistencyGroupSvmName(flexVolGroups), flexVolGroups.keySet());
712713

713714
logger.info("takeVMSnapshot: [CG:Start] Starting phase-1 snapshot [{}] for temporary consistency group [{}]",
714715
snapshotNameBase, tempCgUuid);
@@ -793,26 +794,55 @@ String buildTemporaryConsistencyGroupName(VMSnapshot vmSnapshot) {
793794
"cg" + vmSnapshot.getId());
794795
}
795796

797+
/**
798+
* ONTAP consistency groups are scoped to a single SVM. All participating FlexVols must belong to it.
799+
*/
800+
String resolveConsistencyGroupSvmName(Map<String, FlexVolGroupInfo> flexVolGroups) {
801+
String svmName = null;
802+
for (FlexVolGroupInfo group : flexVolGroups.values()) {
803+
String candidate = group.poolDetails.get(OntapStorageConstants.SVM_NAME);
804+
if (candidate == null || candidate.trim().isEmpty()) {
805+
throw new CloudRuntimeException("SVM name not found in pool details for pool [" + group.poolId + "]");
806+
}
807+
if (svmName == null) {
808+
svmName = candidate;
809+
} else if (!svmName.equals(candidate)) {
810+
throw new CloudRuntimeException("ONTAP consistency groups require all VM volumes on the same SVM; found ["
811+
+ svmName + "] and [" + candidate + "]");
812+
}
813+
}
814+
return svmName;
815+
}
816+
796817
/**
797818
* Creates a temporary consistency group for the involved FlexVol UUIDs and returns its UUID.
798819
*/
799820
String createTemporaryConsistencyGroup(SnapshotFeignClient client, StorageStrategy storageStrategy,
800-
String authHeader, String cgName, Set<String> flexVolUuids) {
821+
String authHeader, String cgName, String svmName,
822+
Set<String> flexVolUuids) {
823+
if (svmName == null || svmName.trim().isEmpty()) {
824+
throw new CloudRuntimeException("SVM name is required to create ONTAP consistency group [" + cgName + "]");
825+
}
826+
801827
List<Map<String, Object>> volumeRefs = new ArrayList<>();
802828
for (String flexVolUuid : flexVolUuids) {
803829
Map<String, Object> volumeRef = new LinkedHashMap<>();
804830
volumeRef.put("uuid", flexVolUuid);
805831
volumeRefs.add(volumeRef);
806832
}
807833

834+
Map<String, Object> svmRef = new LinkedHashMap<>();
835+
svmRef.put("name", svmName);
836+
808837
Map<String, Object> payload = new LinkedHashMap<>();
809838
payload.put("name", cgName);
839+
payload.put("svm", svmRef);
810840
payload.put("volumes", volumeRefs);
811841

812842
JobResponse response = client.createConsistencyGroup(authHeader, payload);
813843
storageStrategy.pollJobIfPresent(response, "create temporary consistency group " + cgName);
814844

815-
String cgUuid = resolveConsistencyGroupUuidByName(client, authHeader, cgName);
845+
String cgUuid = resolveConsistencyGroupUuidByName(client, authHeader, cgName, svmName);
816846
if (cgUuid == null || cgUuid.isEmpty()) {
817847
throw new CloudRuntimeException("Unable to resolve temporary consistency group UUID for [" + cgName + "]");
818848
}
@@ -852,11 +882,13 @@ void deleteTemporaryConsistencyGroup(SnapshotFeignClient client, StorageStrategy
852882
}
853883

854884
/**
855-
* Resolves consistency group UUID by name.
885+
* Resolves consistency group UUID by name within the given SVM.
856886
*/
857-
String resolveConsistencyGroupUuidByName(SnapshotFeignClient client, String authHeader, String cgName) {
887+
String resolveConsistencyGroupUuidByName(SnapshotFeignClient client, String authHeader,
888+
String cgName, String svmName) {
858889
Map<String, Object> query = new HashMap<>();
859890
query.put("name", cgName);
891+
query.put("svm.name", svmName);
860892
query.put("fields", "uuid,name");
861893
OntapResponse<Map<String, Object>> response = client.getConsistencyGroups(authHeader, query);
862894
if (response == null || response.getRecords() == null || response.getRecords().isEmpty()) {

plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,44 @@ void testGroupVolumesByFlexVol_VolumeNotFound_ThrowsException() {
551551
() -> strategy.groupVolumesByFlexVol(Collections.singletonList(volumeTO1)));
552552
}
553553

554+
@Test
555+
void testCreateTemporaryConsistencyGroup_includesSvmName() {
556+
SnapshotFeignClient client = mock(SnapshotFeignClient.class);
557+
StorageStrategy storageStrategy = mock(StorageStrategy.class);
558+
when(client.createConsistencyGroup(any(), any())).thenReturn(createJobResponse("job-cg-create"));
559+
OntapResponse<Map<String, Object>> cgResponse = new OntapResponse<>();
560+
Map<String, Object> cgRecord = new HashMap<>();
561+
cgRecord.put("uuid", "cg-uuid-1");
562+
cgResponse.setRecords(Collections.singletonList(cgRecord));
563+
when(client.getConsistencyGroups(any(), any())).thenReturn(cgResponse);
564+
565+
String cgUuid = strategy.createTemporaryConsistencyGroup(client, storageStrategy, "auth",
566+
"cg-name", "vs0", java.util.Set.of("flexvol-uuid-1", "flexvol-uuid-2"));
567+
568+
assertEquals("cg-uuid-1", cgUuid);
569+
org.mockito.ArgumentCaptor<Map<String, Object>> payloadCaptor = org.mockito.ArgumentCaptor.forClass(Map.class);
570+
verify(client).createConsistencyGroup(eq("auth"), payloadCaptor.capture());
571+
Map<String, Object> payload = payloadCaptor.getValue();
572+
assertEquals("cg-name", payload.get("name"));
573+
@SuppressWarnings("unchecked")
574+
Map<String, Object> svm = (Map<String, Object>) payload.get("svm");
575+
assertEquals("vs0", svm.get("name"));
576+
}
577+
578+
@Test
579+
void testResolveConsistencyGroupSvmName_rejectsDifferentSvms() {
580+
Map<String, OntapVMSnapshotStrategy.FlexVolGroupInfo> groups = new HashMap<>();
581+
Map<String, String> poolDetails1 = new HashMap<>();
582+
poolDetails1.put(OntapStorageConstants.SVM_NAME, "vs0");
583+
groups.put("flexvol-uuid-1", new OntapVMSnapshotStrategy.FlexVolGroupInfo(poolDetails1, POOL_ID_1));
584+
585+
Map<String, String> poolDetails2 = new HashMap<>();
586+
poolDetails2.put(OntapStorageConstants.SVM_NAME, "vs1");
587+
groups.put("flexvol-uuid-2", new OntapVMSnapshotStrategy.FlexVolGroupInfo(poolDetails2, POOL_ID_2));
588+
589+
assertThrows(CloudRuntimeException.class, () -> strategy.resolveConsistencyGroupSvmName(groups));
590+
}
591+
554592
// ══════════════════════════════════════════════════════════════════════════
555593
// Tests: FlexVolSnapshotDetail parse/toString
556594
// ══════════════════════════════════════════════════════════════════════════

0 commit comments

Comments
 (0)