From f7e9a4f0f79cebe3d43a0a23498516499c6bef3d Mon Sep 17 00:00:00 2001 From: Jitendra Kumar Date: Wed, 23 Dec 2020 18:30:50 +0530 Subject: [PATCH] Heading: [#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. --- .../controllers/InstanceTypeController.java | 11 +++++ .../com/yugabyte/yw/models/InstanceType.java | 1 + .../InstanceTypeControllerTest.java | 45 +++++++++++++++---- .../universes/UniverseForm/ClusterFields.js | 5 ++- 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/managed/src/main/java/com/yugabyte/yw/controllers/InstanceTypeController.java b/managed/src/main/java/com/yugabyte/yw/controllers/InstanceTypeController.java index 0fe0ccea3308..670c9eeac8eb 100644 --- a/managed/src/main/java/com/yugabyte/yw/controllers/InstanceTypeController.java +++ b/managed/src/main/java/com/yugabyte/yw/controllers/InstanceTypeController.java @@ -50,6 +50,17 @@ public Result list(UUID customerUUID, UUID providerUUID, List 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 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"); diff --git a/managed/src/main/java/com/yugabyte/yw/models/InstanceType.java b/managed/src/main/java/com/yugabyte/yw/models/InstanceType.java index 1e515b620225..de11421ba4da 100644 --- a/managed/src/main/java/com/yugabyte/yw/models/InstanceType.java +++ b/managed/src/main/java/com/yugabyte/yw/models/InstanceType.java @@ -210,6 +210,7 @@ public static class InstanceTypeDetails { public List volumeDetailsList; public PublicCloudConstants.Tenancy tenancy; + public Integer volumeCount= DEFAULT_VOLUME_COUNT; public InstanceTypeDetails() { volumeDetailsList = new LinkedList<>(); diff --git a/managed/src/test/java/com/yugabyte/yw/controllers/InstanceTypeControllerTest.java b/managed/src/test/java/com/yugabyte/yw/controllers/InstanceTypeControllerTest.java index d9e793dfe3a4..4db1c2d13bb3 100644 --- a/managed/src/test/java/com/yugabyte/yw/controllers/InstanceTypeControllerTest.java +++ b/managed/src/test/java/com/yugabyte/yw/controllers/InstanceTypeControllerTest.java @@ -101,7 +101,8 @@ private JsonNode doDeleteInstanceTypeAndVerify(UUID providerUUID, String instanc return Json.parse(contentAsString(result)); } - private Map setUpValidInstanceTypes(int numInstanceTypes) { + private Map setUpValidInstanceTypes(int numInstanceTypes, + String instanceTypesCode) { Map instanceTypes = new HashMap<>(numInstanceTypes); for (int i = 0; i < numInstanceTypes; ++i) { InstanceType.VolumeDetails volDetails = new InstanceType.VolumeDetails(); @@ -109,8 +110,10 @@ private Map setUpValidInstanceTypes(int numInstanceTypes) 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)); } @@ -134,23 +137,38 @@ public void testListEmptyInstanceTypeWithValidProviderUUID() { @Test public void testListInstanceTypeWithValidProviderUUID() { - Map instanceTypes = setUpValidInstanceTypes(2); + Map instanceTypes = setUpValidInstanceTypes(2, "c3.i"); JsonNode json = doListInstanceTypesAndVerify(awsProvider.uuid, OK); checkListResponse(instanceTypes, json); } + @Test + public void testListc4InstanceTypeVolumeCountWithValidProviderUUID() { + Map instanceTypes = setUpValidInstanceTypes(1, "c4.i"); + JsonNode json = doListInstanceTypesAndVerify(awsProvider.uuid, OK); + checkListVolumeCount(instanceTypes, json); + } + + @Test + public void testListc5InstanceTypeVolumeCountWithValidProviderUUID() { + Map instanceTypes = setUpValidInstanceTypes(1, "c5.i"); + JsonNode json = doListInstanceTypesAndVerify(awsProvider.uuid, OK); + checkListVolumeCount(instanceTypes, json); + } + @Test public void testListInstanceTypeWithValidProviderUUID_filtered_ignoreNoCloud() { - Map instanceTypes = setUpValidInstanceTypes(2); + Map 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 instanceTypes = setUpValidInstanceTypes(2); + Map instanceTypes = setUpValidInstanceTypes(2, "c3.i"); RuntimeException thrown = new RuntimeException("cloud exception"); CloudAPI mockCloudAPI = mock(CloudAPI.class); when(mockCloudAPIFactory.get(any())).thenReturn(mockCloudAPI); @@ -162,7 +180,7 @@ public void testListInstanceTypeWithValidProviderUUID_filtered_ignoreCloudExcept @Test public void testListInstanceTypeWithValidProviderUUID_filtered_allOffered() { - Map instanceTypes = setUpValidInstanceTypes(2); + Map instanceTypes = setUpValidInstanceTypes(2, "c3.i"); ImmutableSet everywhere = ImmutableSet.of("zone1", "zone2"); Map> allInstancesEverywhere = ImmutableMap.of( "c3.i0", everywhere, @@ -185,7 +203,7 @@ public void testListInstanceTypeWithValidProviderUUID_filtered_allOffered() { @Test public void testListInstanceTypeWithValidProviderUUID_filtered_disjointOffered() { - Map instanceTypes = setUpValidInstanceTypes(2); + Map instanceTypes = setUpValidInstanceTypes(2, "c3.i"); Map> cloudResponse = ImmutableMap.of( "c3.i0", ImmutableSet.of("zone1"), "c3.i1", ImmutableSet.of("zone2")); @@ -207,7 +225,7 @@ public void testListInstanceTypeWithValidProviderUUID_filtered_disjointOffered() @Test public void testListInstanceTypeWithValidProviderUUID_filtered_someNotOffered() { - Map instanceTypes = setUpValidInstanceTypes(2); + Map instanceTypes = setUpValidInstanceTypes(2, "c3.i"); Map> cloudResponse = ImmutableMap.of( "c3.i1", ImmutableSet.of("zone2", "zone1")); CloudAPI mockCloudAPI = mock(CloudAPI.class); @@ -248,6 +266,8 @@ private void checkListResponse(Map 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); } @@ -256,6 +276,15 @@ private void checkListResponse(Map instanceTypes, JsonNode assertAuditEntry(0, customer.uuid); } + private void checkListVolumeCount(Map 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(); diff --git a/managed/ui/src/components/universes/UniverseForm/ClusterFields.js b/managed/ui/src/components/universes/UniverseForm/ClusterFields.js index 519bfc849aa6..82f003e214e6 100644 --- a/managed/ui/src/components/universes/UniverseForm/ClusterFields.js +++ b/managed/ui/src/components/universes/UniverseForm/ClusterFields.js @@ -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 @@ -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' @@ -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);