Skip to content

Commit

Permalink
Heading:
Browse files Browse the repository at this point in the history
[#5919] Platform: change default storage option for AWS provider to 1x250GB
Description:
We wanted to show default volume count to 1 on the UI for all the instance types starting with "c5./c4."
Testing:
When we select aws provider while creating universe, volume count was showing more than one for some instances types starting with ""c5./c4.". Now with this fix it will show one one.
Added Unit test as well.
  • Loading branch information
Jitendra Kumar committed Dec 28, 2020
1 parent fbd6dbf commit f7e9a4f
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,17 @@ public Result list(UUID customerUUID, UUID providerUUID, List<String> zoneCodes)
try {
instanceTypesMap = InstanceType.findByProvider(provider).stream()
.collect(toMap(InstanceType::getInstanceTypeCode, identity()));
// Setting up the default volumeCount to 1 for instanceTypes starting with c5./c4.
// TODO: Need to revert this changes
for (Map.Entry<String, InstanceType> entry : instanceTypesMap.entrySet()) {
if ((provider.code.equals("aws") && (entry.getKey().startsWith("c5.") ||
entry.getKey().startsWith("c4.")))) {
entry.getValue().instanceTypeDetails.volumeCount = 1;
} else {
entry.getValue().instanceTypeDetails.volumeCount =
entry.getValue().instanceTypeDetails.volumeDetailsList.size();
}
}
} catch (Exception e) {
LOG.error("Unable to list Instance types {}:{} in DB.", providerUUID, e.getMessage());
return ApiResponse.error(INTERNAL_SERVER_ERROR, "Unable to list InstanceType");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ public static class InstanceTypeDetails {

public List<VolumeDetails> volumeDetailsList;
public PublicCloudConstants.Tenancy tenancy;
public Integer volumeCount= DEFAULT_VOLUME_COUNT;

public InstanceTypeDetails() {
volumeDetailsList = new LinkedList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,19 @@ private JsonNode doDeleteInstanceTypeAndVerify(UUID providerUUID, String instanc
return Json.parse(contentAsString(result));
}

private Map<String, InstanceType> setUpValidInstanceTypes(int numInstanceTypes) {
private Map<String, InstanceType> setUpValidInstanceTypes(int numInstanceTypes,
String instanceTypesCode) {
Map<String, InstanceType> instanceTypes = new HashMap<>(numInstanceTypes);
for (int i = 0; i < numInstanceTypes; ++i) {
InstanceType.VolumeDetails volDetails = new InstanceType.VolumeDetails();
volDetails.volumeSizeGB = 100;
volDetails.volumeType = InstanceType.VolumeType.EBS;
InstanceTypeDetails instanceDetails = new InstanceTypeDetails();
instanceDetails.volumeDetailsList.add(volDetails);
instanceDetails.volumeDetailsList.add(volDetails);
instanceDetails.setDefaultMountPaths();
String code = "c3.i" + i;
instanceDetails.volumeCount = 1;
String code = instanceTypesCode + i;
instanceTypes.put(code,
InstanceType.upsert(awsProvider.code, code, 2, 10.5, instanceDetails));
}
Expand All @@ -134,23 +137,38 @@ public void testListEmptyInstanceTypeWithValidProviderUUID() {

@Test
public void testListInstanceTypeWithValidProviderUUID() {
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2);
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2, "c3.i");
JsonNode json = doListInstanceTypesAndVerify(awsProvider.uuid, OK);
checkListResponse(instanceTypes, json);
}

@Test
public void testListc4InstanceTypeVolumeCountWithValidProviderUUID() {
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(1, "c4.i");
JsonNode json = doListInstanceTypesAndVerify(awsProvider.uuid, OK);
checkListVolumeCount(instanceTypes, json);
}

@Test
public void testListc5InstanceTypeVolumeCountWithValidProviderUUID() {
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(1, "c5.i");
JsonNode json = doListInstanceTypesAndVerify(awsProvider.uuid, OK);
checkListVolumeCount(instanceTypes, json);
}

@Test
public void testListInstanceTypeWithValidProviderUUID_filtered_ignoreNoCloud() {
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2);
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2, "c3.i");
when(mockCloudAPIFactory.get(any())).thenReturn(null);
JsonNode json = doListFilteredInstanceTypeAndVerify(
awsProvider.uuid, OK, "zone1", "zone2");
checkListResponse(instanceTypes, json);
}


@Test
public void testListInstanceTypeWithValidProviderUUID_filtered_ignoreCloudException() {
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2);
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2, "c3.i");
RuntimeException thrown = new RuntimeException("cloud exception");
CloudAPI mockCloudAPI = mock(CloudAPI.class);
when(mockCloudAPIFactory.get(any())).thenReturn(mockCloudAPI);
Expand All @@ -162,7 +180,7 @@ public void testListInstanceTypeWithValidProviderUUID_filtered_ignoreCloudExcept

@Test
public void testListInstanceTypeWithValidProviderUUID_filtered_allOffered() {
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2);
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2, "c3.i");
ImmutableSet<String> everywhere = ImmutableSet.of("zone1", "zone2");
Map<String, Set<String>> allInstancesEverywhere = ImmutableMap.of(
"c3.i0", everywhere,
Expand All @@ -185,7 +203,7 @@ public void testListInstanceTypeWithValidProviderUUID_filtered_allOffered() {

@Test
public void testListInstanceTypeWithValidProviderUUID_filtered_disjointOffered() {
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2);
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2, "c3.i");
Map<String, Set<String>> cloudResponse = ImmutableMap.of(
"c3.i0", ImmutableSet.of("zone1"),
"c3.i1", ImmutableSet.of("zone2"));
Expand All @@ -207,7 +225,7 @@ public void testListInstanceTypeWithValidProviderUUID_filtered_disjointOffered()

@Test
public void testListInstanceTypeWithValidProviderUUID_filtered_someNotOffered() {
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2);
Map<String, InstanceType> instanceTypes = setUpValidInstanceTypes(2, "c3.i");
Map<String, Set<String>> cloudResponse = ImmutableMap.of(
"c3.i1", ImmutableSet.of("zone2", "zone1"));
CloudAPI mockCloudAPI = mock(CloudAPI.class);
Expand Down Expand Up @@ -248,6 +266,8 @@ private void checkListResponse(Map<String, InstanceType> instanceTypes, JsonNode
JsonNode jsonDetails = detailsListNode.get(0);
assertThat(jsonDetails.get("volumeSizeGB").asInt(), allOf(notNullValue(),
equalTo(targetDetails.volumeSizeGB)));
assertThat(itdNode.get("volumeCount").asInt(), allOf(notNullValue(),
equalTo(itd.volumeDetailsList.size())));
assertValue(jsonDetails, "volumeType", targetDetails.volumeType.toString());
assertValue(jsonDetails, "mountPath", targetDetails.mountPath);
}
Expand All @@ -256,6 +276,15 @@ private void checkListResponse(Map<String, InstanceType> instanceTypes, JsonNode
assertAuditEntry(0, customer.uuid);
}

private void checkListVolumeCount(Map<String, InstanceType> instanceTypes, JsonNode json) {
assertEquals(1, json.size());
JsonNode instance = json.path(0);
InstanceType expectedInstanceType =
instanceTypes.get(instance.path("instanceTypeCode").asText());
assertThat(instance.get("instanceTypeDetails").get("volumeCount").asInt(), allOf(notNullValue(),
equalTo(expectedInstanceType.instanceTypeDetails.volumeCount)));
}

@Test
public void testCreateInstanceTypeWithInvalidProviderUUID() {
ObjectNode instanceTypeJson = Json.newObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,7 @@ export default class ClusterFields extends Component {
});
const volumesList = instanceTypeSelectedData.instanceTypeDetails.volumeDetailsList;
const volumeDetail = volumesList[0];
const volumeCount = instanceTypeSelectedData.instanceTypeDetails.volumeCount;
let mountPoints = null;
if (instanceTypeSelectedData.providerCode === 'onprem') {
mountPoints = instanceTypeSelectedData.instanceTypeDetails.volumeDetailsList
Expand All @@ -608,7 +609,7 @@ export default class ClusterFields extends Component {
if (volumeDetail) {
const deviceInfo = {
volumeSize: volumeDetail.volumeSizeGB,
numVolumes: volumesList.length,
numVolumes: volumeCount,
mountPoints: mountPoints,
storageType:
volumeDetail.volumeType === 'EBS'
Expand All @@ -618,7 +619,7 @@ export default class ClusterFields extends Component {
diskIops: null
};
updateFormField(`${clusterType}.volumeSize`, volumeDetail.volumeSizeGB);
updateFormField(`${clusterType}.numVolumes`, volumesList.length);
updateFormField(`${clusterType}.numVolumes`, volumeCount);
updateFormField(`${clusterType}.diskIops`, volumeDetail.diskIops);
updateFormField(`${clusterType}.storageType`, volumeDetail.storageType);
updateFormField(`${clusterType}.mountPoints`, mountPoints);
Expand Down

0 comments on commit f7e9a4f

Please sign in to comment.