Visitar URL original
Brought back the missing changes · NetApp/cloudstack@0b8a4a8 · GitHub
Skip to content

Commit 0b8a4a8

Browse files
sandeeplocharlasandeeplocharla
committed
Brought back the missing changes
1 parent 949a9b6 commit 0b8a4a8

4 files changed

Lines changed: 93 additions & 53 deletions

File tree

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ public DataStore initialize(Map<String, Object> dsInfos) {
145145
}
146146
Aggregate aggregate;
147147
try {
148-
aggregate = storageStrategy.chooseAggregate(capacityBytes);
148+
aggregate = storageStrategy.chooseAggregate(storageStrategy.getAggregates(), capacityBytes);
149149
} catch (Exception e) {
150150
logger.error("Exception occurred while choosing aggregate for pool: " + storagePoolName, e);
151151
throw new CloudRuntimeException("Failed to choose ONTAP aggregate for pool: " + storagePoolName

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.apache.cloudstack.storage.service;
2121

22+
import java.util.ArrayList;
2223
import java.util.HashMap;
2324
import java.util.LinkedHashSet;
2425
import java.util.List;
@@ -322,11 +323,23 @@ public String getResolvedSvmUuid() {
322323
return resolvedSvmUuid;
323324
}
324325

326+
/**
327+
* Aggregates eligible for new FlexVol creation, populated by {@link #connect(boolean)} with aggregate validation enabled.
328+
*/
329+
public List<Aggregate> getAggregates() {
330+
return aggregates;
331+
}
332+
333+
public void setAggregates(List<Aggregate> aggregates) {
334+
this.aggregates = aggregates;
335+
}
336+
325337
private void validateAndSelectAggregatesForVolumeCreation(String authHeader, String svmName, List<Aggregate> aggrs) {
326338
if (aggrs == null || aggrs.isEmpty()) {
327339
logger.error("No aggregates are assigned to SVM " + svmName);
328340
throw new CloudRuntimeException("No aggregates are assigned to SVM " + svmName);
329341
}
342+
List<Aggregate> eligibleAggregates = new ArrayList<>();
330343
for (Aggregate aggr : aggrs) {
331344
logger.debug("Found aggregate: " + aggr.getName() + " with UUID: " + aggr.getUuid());
332345
Aggregate aggrResp = aggregateFeignClient.getAggregateByUUID(authHeader, aggr.getUuid(),
@@ -346,27 +359,28 @@ private void validateAndSelectAggregatesForVolumeCreation(String authHeader, Str
346359
continue;
347360
}
348361
logger.info("Selected aggregate: " + aggr.getName() + " for volume operations.");
349-
this.aggregates = List.of(aggr);
362+
eligibleAggregates.add(aggr);
350363
}
351-
if (this.aggregates == null || this.aggregates.isEmpty()) {
364+
if (eligibleAggregates.isEmpty()) {
352365
logger.error("No suitable aggregates found on SVM " + svmName + " for volume creation.");
353366
throw new CloudRuntimeException("No suitable aggregates found on SVM " + svmName + " for volume creation.");
354367
}
368+
setAggregates(eligibleAggregates);
355369
}
356370

357371
// Common methods like create/delete etc., should be here
358372

359373
/**
360-
* Selects the best aggregate for a volume of the given size from candidates populated by
361-
* {@link #connect(boolean)} with aggregate validation enabled.
374+
* Selects the best aggregate for a volume of the given size from the given candidate aggregates.
362375
*
363376
* <p>Picks the online aggregate with the largest available block space that can fit
364377
* {@code size}. The returned aggregate includes node information for LIF affinity.</p>
365378
*
379+
* @param aggregates candidate aggregates, for example {@link #getAggregates()}
366380
* @param size requested volume size in bytes
367381
* @return the chosen aggregate detail response
368382
*/
369-
public Aggregate chooseAggregate(Long size) {
383+
public Aggregate chooseAggregate(List<Aggregate> aggregates, Long size) {
370384
String svmName = storage.getSvmName();
371385
if (aggregates == null || aggregates.isEmpty()) {
372386
logger.error("No aggregates available to create volume on SVM " + svmName);

‎plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ void setUp() {
141141
Aggregate.Node node = new Aggregate.Node();
142142
node.setName("node-a");
143143
aggregate.setNode(node);
144-
when(storageStrategy.chooseAggregate(any())).thenReturn(aggregate);
144+
when(storageStrategy.chooseAggregate(any(), any())).thenReturn(aggregate);
145145
when(storageStrategy.getNetworkInterface(any())).thenReturn(
146146
Map.of(OntapStorageConstants.DATA_LIF, "testNetworkInterface"));
147147

@@ -739,7 +739,7 @@ public void testInitialize_positiveWithDetailAssertions() {
739739

740740
// Verify LIF selection completes before FlexVol creation
741741
InOrder inOrder = inOrder(storageStrategy);
742-
inOrder.verify(storageStrategy).chooseAggregate(any());
742+
inOrder.verify(storageStrategy).chooseAggregate(any(), any());
743743
inOrder.verify(storageStrategy).getNetworkInterface(any());
744744
inOrder.verify(storageStrategy).createStorageVolume(any(), any(), any());
745745

‎plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java‎

Lines changed: 71 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,39 @@ public void testConnect_positive() {
258258
verify(svmFeignClient, times(1)).getSvmResponse(anyMap(), anyString());
259259
}
260260

261+
@Test
262+
public void testConnect_collectsAllEligibleAggregates() {
263+
Svm svm = new Svm();
264+
svm.setName("svm1");
265+
svm.setState(OntapStorageConstants.RUNNING);
266+
svm.setNfsEnabled(true);
267+
268+
Aggregate aggregate1 = new Aggregate();
269+
aggregate1.setName("aggr1");
270+
aggregate1.setUuid("aggr-uuid-1");
271+
Aggregate aggregate2 = new Aggregate();
272+
aggregate2.setName("aggr2");
273+
aggregate2.setUuid("aggr-uuid-2");
274+
svm.setAggregates(List.of(aggregate1, aggregate2));
275+
276+
OntapResponse<Svm> svmResponse = new OntapResponse<>();
277+
svmResponse.setRecords(List.of(svm));
278+
279+
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
280+
when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap()))
281+
.thenReturn(buildAggregate("aggr1", "aggr-uuid-1", 10000000000.0));
282+
when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-2"), anyMap()))
283+
.thenReturn(buildAggregate("aggr2", "aggr-uuid-2", 20000000000.0));
284+
285+
assertTrue(storageStrategy.connect());
286+
287+
List<Aggregate> aggregates = storageStrategy.getAggregates();
288+
assertNotNull(aggregates);
289+
assertEquals(2, aggregates.size());
290+
assertEquals("aggr-uuid-1", aggregates.get(0).getUuid());
291+
assertEquals("aggr-uuid-2", aggregates.get(1).getUuid());
292+
}
293+
261294
@Test
262295
public void testConnect_operationsOnly_skipsAggregateValidation() {
263296
Svm svm = new Svm();
@@ -560,14 +593,11 @@ public void testConnect_invalidCredentials() {
560593

561594
@Test
562595
public void testChooseAggregate_positive() {
563-
setupSuccessfulConnect();
564-
storageStrategy.connect();
565-
566596
Aggregate aggregateDetail = buildAggregate("aggr1", "aggr-uuid-1", 10000000000.0, "node-a");
567597
when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap()))
568598
.thenReturn(aggregateDetail);
569599

570-
Aggregate result = storageStrategy.chooseAggregate(5000000000L);
600+
Aggregate result = storageStrategy.chooseAggregate(candidateAggregates(), 5000000000L);
571601

572602
assertNotNull(result);
573603
assertEquals("aggr1", result.getName());
@@ -576,37 +606,52 @@ public void testChooseAggregate_positive() {
576606
}
577607

578608
@Test
579-
public void testChooseAggregate_invalidSize() {
580-
setupSuccessfulConnect();
581-
storageStrategy.connect();
609+
public void testChooseAggregate_picksLargestAvailable() {
610+
Aggregate candidate1 = new Aggregate();
611+
candidate1.setName("aggr1");
612+
candidate1.setUuid("aggr-uuid-1");
613+
Aggregate candidate2 = new Aggregate();
614+
candidate2.setName("aggr2");
615+
candidate2.setUuid("aggr-uuid-2");
616+
617+
when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap()))
618+
.thenReturn(buildAggregate("aggr1", "aggr-uuid-1", 10000000000.0, "node-a"));
619+
when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-2"), anyMap()))
620+
.thenReturn(buildAggregate("aggr2", "aggr-uuid-2", 20000000000.0, "node-b"));
582621

622+
Aggregate result = storageStrategy.chooseAggregate(List.of(candidate1, candidate2), 5000000000L);
623+
624+
assertEquals("aggr-uuid-2", result.getUuid());
625+
assertEquals("node-b", result.getNode().getName());
626+
}
627+
628+
@Test
629+
public void testChooseAggregate_invalidSize() {
583630
Exception ex = assertThrows(CloudRuntimeException.class,
584-
() -> storageStrategy.chooseAggregate(-1L));
631+
() -> storageStrategy.chooseAggregate(candidateAggregates(), -1L));
585632
assertTrue(ex.getMessage().contains("Invalid volume size"));
586633
}
587634

588635
@Test
589636
public void testChooseAggregate_nullSize() {
590-
setupSuccessfulConnect();
591-
storageStrategy.connect();
592-
593637
Exception ex = assertThrows(CloudRuntimeException.class,
594-
() -> storageStrategy.chooseAggregate(null));
638+
() -> storageStrategy.chooseAggregate(candidateAggregates(), null));
595639
assertTrue(ex.getMessage().contains("Invalid volume size"));
596640
}
597641

598642
@Test
599643
public void testChooseAggregate_noAggregates() {
600644
Exception ex = assertThrows(CloudRuntimeException.class,
601-
() -> storageStrategy.chooseAggregate(5000000000L));
645+
() -> storageStrategy.chooseAggregate(null, 5000000000L));
646+
assertTrue(ex.getMessage().contains("No aggregates available"));
647+
648+
ex = assertThrows(CloudRuntimeException.class,
649+
() -> storageStrategy.chooseAggregate(List.of(), 5000000000L));
602650
assertTrue(ex.getMessage().contains("No aggregates available"));
603651
}
604652

605653
@Test
606654
public void testChooseAggregate_aggregateNotOnline() {
607-
setupSuccessfulConnect();
608-
storageStrategy.connect();
609-
610655
Aggregate aggregateDetail = new Aggregate();
611656
aggregateDetail.setName("aggr1");
612657
aggregateDetail.setUuid("aggr-uuid-1");
@@ -616,39 +661,40 @@ public void testChooseAggregate_aggregateNotOnline() {
616661
.thenReturn(aggregateDetail);
617662

618663
Exception ex = assertThrows(CloudRuntimeException.class,
619-
() -> storageStrategy.chooseAggregate(5000000000L));
664+
() -> storageStrategy.chooseAggregate(candidateAggregates(), 5000000000L));
620665
assertTrue(ex.getMessage().contains("No suitable aggregates found"));
621666
}
622667

623668
@Test
624669
public void testChooseAggregate_insufficientSpace() {
625-
setupSuccessfulConnect();
626-
storageStrategy.connect();
627-
628670
Aggregate aggregateDetail = buildAggregate("aggr1", "aggr-uuid-1", 1000000.0, "node-a");
629671

630672
when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap()))
631673
.thenReturn(aggregateDetail);
632674

633675
Exception ex = assertThrows(CloudRuntimeException.class,
634-
() -> storageStrategy.chooseAggregate(5000000000L));
676+
() -> storageStrategy.chooseAggregate(candidateAggregates(), 5000000000L));
635677
assertTrue(ex.getMessage().contains("No suitable aggregates found"));
636678
}
637679

638680
@Test
639681
public void testChooseAggregate_missingNode() {
640-
setupSuccessfulConnect();
641-
storageStrategy.connect();
642-
643682
Aggregate aggregateDetail = buildAggregate("aggr1", "aggr-uuid-1", 10000000000.0);
644683
when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap()))
645684
.thenReturn(aggregateDetail);
646685

647686
Exception ex = assertThrows(CloudRuntimeException.class,
648-
() -> storageStrategy.chooseAggregate(5000000000L));
687+
() -> storageStrategy.chooseAggregate(candidateAggregates(), 5000000000L));
649688
assertTrue(ex.getMessage().contains("does not have a node name"));
650689
}
651690

691+
private List<Aggregate> candidateAggregates() {
692+
Aggregate candidate = new Aggregate();
693+
candidate.setName("aggr1");
694+
candidate.setUuid("aggr-uuid-1");
695+
return List.of(candidate);
696+
}
697+
652698
// ========== createStorageVolume() Tests ==========
653699

654700
@Test
@@ -1235,26 +1281,6 @@ public void testGetNetworkInterface_nfs_tier1Down_tier2Used() {
12351281

12361282
// ========== Helper Methods ==========
12371283

1238-
private void setupSuccessfulConnect() {
1239-
Svm svm = new Svm();
1240-
svm.setName("svm1");
1241-
svm.setState(OntapStorageConstants.RUNNING);
1242-
svm.setNfsEnabled(true);
1243-
1244-
Aggregate aggregate = new Aggregate();
1245-
aggregate.setName("aggr1");
1246-
aggregate.setUuid("aggr-uuid-1");
1247-
svm.setAggregates(List.of(aggregate));
1248-
1249-
OntapResponse<Svm> svmResponse = new OntapResponse<>();
1250-
svmResponse.setRecords(List.of(svm));
1251-
1252-
when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse);
1253-
1254-
Aggregate aggregateDetail = buildAggregate("aggr1", "aggr-uuid-1", 10000000000.0, "node-a");
1255-
when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"), anyMap())).thenReturn(aggregateDetail);
1256-
}
1257-
12581284
private void setupSuccessfulJobCreation() {
12591285
Job job = new Job();
12601286
job.setUuid("job-uuid-1");

0 commit comments

Comments
 (0)