Skip to content

Commit

Permalink
Heading: (yugabyte#6792)
Browse files Browse the repository at this point in the history 
[yugabyte#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 only one.
Will have to reset instanceTypeDetailsJson to empty for older YW
instances.

Co-authored-by: Jitendra Kumar <[email protected]>
  • Loading branch information
@jitendra-12113
2 people authored and Alex Ball committed Mar 9, 2021
1 parent 8fdfcfb commit cd61595
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 27 deletions.
6 changes: 3 additions & 3 deletions managed/src/main/java/AppInit.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ public AppInit(Environment environment, Application application,
List<Provider> providerList = Provider.find.query().where().findList();
for (Provider provider : providerList) {
if (provider.code.equals("aws")) {
for (InstanceType instanceType : InstanceType.findByProvider(provider)) {
for (InstanceType instanceType : InstanceType.findByProvider(provider,
application.config())) {
if (instanceType.instanceTypeDetails != null &&
(instanceType.instanceTypeDetails.volumeDetailsList == null ||
instanceType.instanceTypeDetails.volumeDetailsList.isEmpty())) {
(instanceType.instanceTypeDetails.volumeDetailsList == null)) {
awsInitializer.initialize(provider.customerUUID, provider.uuid);
break;
}
Expand Down
2 changes: 1 addition & 1 deletion managed/src/main/java/com/yugabyte/yw/cloud/AWSInitializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ private void storeInstanceTypeInfoToDB() {
throw new UnsupportedOperationException(msg);
} else {
// TODO: hardcode me not?
volumeCount = 2;
volumeCount = 0;
volumeSizeGB = 250;
volumeType = VolumeType.EBS;
}
Expand Down
17 changes: 12 additions & 5 deletions managed/src/main/java/com/yugabyte/yw/controllers/InstanceTypeController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.inject.Inject;
import com.typesafe.config.Config;
import com.yugabyte.yw.cloud.CloudAPI;
import com.yugabyte.yw.cloud.PublicCloudConstants;
import com.yugabyte.yw.commissioner.Common;
Expand All @@ -26,12 +27,18 @@
public class InstanceTypeController extends AuthenticatedController {

public static final Logger LOG = LoggerFactory.getLogger(InstanceTypeController.class);
private final Config config;
private final FormFactory formFactory;
private final CloudAPI.Factory cloudAPIFactory;

// TODO: Remove this when we have HelperMethod in place to get Config details
@Inject
FormFactory formFactory;

@Inject
CloudAPI.Factory cloudAPIFactory;
public InstanceTypeController(Config config, FormFactory formFactory,
CloudAPI.Factory cloudAPIFactory) {
this.config = config;
this.formFactory = formFactory;
this.cloudAPIFactory = cloudAPIFactory;
}

/**
* GET endpoint for listing instance types
Expand All @@ -48,7 +55,7 @@ public Result list(UUID customerUUID, UUID providerUUID, List<String> zoneCodes)
}
Map<String, InstanceType> instanceTypesMap;
try {
instanceTypesMap = InstanceType.findByProvider(provider).stream()
instanceTypesMap = InstanceType.findByProvider(provider, config).stream()
.collect(toMap(InstanceType::getInstanceTypeCode, identity()));
} catch (Exception e) {
LOG.error("Unable to list Instance types {}:{} in DB.", providerUUID, e.getMessage());
Expand Down
38 changes: 27 additions & 11 deletions managed/src/main/java/com/yugabyte/yw/models/InstanceType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.google.common.collect.ImmutableList;
import com.yugabyte.yw.cloud.PublicCloudConstants;
import com.yugabyte.yw.commissioner.Common;
import com.typesafe.config.Config;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -29,6 +30,8 @@ public class InstanceType extends Model {

public static List<String> AWS_INSTANCE_PREFIXES_SUPPORTED = ImmutableList.of(
"m3.", "c5.", "c5d.", "c4.", "c3.", "i3.");
static final String YB_AWS_DEFAULT_VOLUME_COUNT_KEY = "yb.aws.default_volume_count";
static final String YB_AWS_DEFAULT_VOLUME_SIZE_GB_KEY = "yb.aws.default_volume_size_gb";

public enum VolumeType {
@EnumValue("EBS")
Expand Down Expand Up @@ -156,8 +159,8 @@ public static void resetInstanceTypeDetailsForProvider(Common.CloudType provider
/**
* Delete Instance Types corresponding to given provider
*/
public static void deleteInstanceTypesForProvider(Provider provider) {
for (InstanceType instanceType : findByProvider(provider)) {
public static void deleteInstanceTypesForProvider(Provider provider, Config config) {
for (InstanceType instanceType : findByProvider(provider, config)) {
instanceType.delete();
}
}
Expand All @@ -166,23 +169,37 @@ private static Predicate<InstanceType> supportedInstanceTypes(List<String> suppo
return p -> supportedPrefixes.stream().anyMatch(prefix -> p.getInstanceTypeCode().startsWith(prefix));
}

private static List<InstanceType> populateDefaultsIfEmpty(List<InstanceType> entries,
Config config) {
// For AWS, we would filter and show only supported instance prefixes
entries = entries.stream()
.filter(supportedInstanceTypes(AWS_INSTANCE_PREFIXES_SUPPORTED))
.collect(Collectors.toList());
for (InstanceType instanceType : entries) {
instanceType.instanceTypeDetails =
Json.fromJson(Json.parse(instanceType.instanceTypeDetailsJson), InstanceTypeDetails.class);
if (instanceType.instanceTypeDetails.volumeDetailsList.isEmpty()) {
instanceType.instanceTypeDetails.setVolumeDetailsList(
config.getInt(YB_AWS_DEFAULT_VOLUME_COUNT_KEY),
config.getInt(YB_AWS_DEFAULT_VOLUME_SIZE_GB_KEY), VolumeType.EBS);
}
}
return entries;
}

/**
* Query Helper to find supported instance types for a given cloud provider.
*/
public static List<InstanceType> findByProvider(Provider provider) {
public static List<InstanceType> findByProvider(Provider provider, Config config) {
List<InstanceType> entries = InstanceType.find.query().where()
.eq("provider_code", provider.code)
.eq("active", true)
.findList();
if (provider.code.equals("aws")) {
// For AWS, we would filter and show only supported instance prefixes
entries = entries.stream()
.filter(supportedInstanceTypes(AWS_INSTANCE_PREFIXES_SUPPORTED))
.collect(Collectors.toList());
entries = populateDefaultsIfEmpty(entries, config);
}

return entries.stream().map(entry -> InstanceType.get(entry.getProviderCode(),
entry.getInstanceTypeCode())).collect(Collectors.toList());

return entries;
}

public static InstanceType createWithMetadata(Provider provider, String instanceTypeCode,
Expand Down Expand Up @@ -251,6 +268,5 @@ public static InstanceTypeDetails createGCPInstanceTypeDetails(VolumeType volume
volumeType);
return instanceTypeDetails;
}

}
}
9 changes: 9 additions & 0 deletions managed/src/main/resources/reference.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ yb {
task_retention_duration = 120 days
}


aws {
# default volume count for aws instance types with EBS Only storage info
default_volume_count = 1

# default volume size for aws instance types with EBS Only storage info
default_volume_size_gb = 250
}

metrics.host="localhost"
metrics.url = "http://"${yb.metrics.host}":9090/api/v1"
storage.path="/opt/yugabyte"
Expand Down
45 changes: 38 additions & 7 deletions managed/src/test/java/com/yugabyte/yw/models/InstanceTypeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.core.AllOf.allOf;
import static org.junit.Assert.*;
import static org.mockito.Mockito.when;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -15,20 +16,28 @@

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.typesafe.config.Config;
import com.yugabyte.yw.common.ModelFactory;
import com.yugabyte.yw.models.InstanceType.VolumeType;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;

import com.yugabyte.yw.common.FakeDBApplication;
import play.libs.Json;

@RunWith(MockitoJUnitRunner.class)
public class InstanceTypeTest extends FakeDBApplication {
private Provider defaultProvider;
private Customer defaultCustomer;
private InstanceType.InstanceTypeDetails defaultDetails;

@Mock
Config mockConfig;

@Before
public void setUp() {
defaultCustomer = ModelFactory.testCustomer();
Expand All @@ -48,7 +57,7 @@ public void testCreate() {
assertEquals("aws", i1.getProviderCode());
assertEquals("foo", i1.getInstanceTypeCode());
}

@Test
public void testGetNonDefaultInstanceTypeDetails() {
int volumeCount = 3;
Expand All @@ -64,7 +73,7 @@ public void testGetNonDefaultInstanceTypeDetails() {
assertEquals(String.format("/mnt/d%d", i), v.mountPath);
}
}

@Test
public void testGetDefaultInstanceTypeDetails() {
InstanceType.InstanceTypeDetails itDetails =
Expand All @@ -90,7 +99,7 @@ public void testFindByProvider() {
instanceType.setActive(false);
instanceType.save();
InstanceType.upsert(newProvider.code, "bar", 2, 10.0, defaultDetails);
List<InstanceType> instanceTypeList = InstanceType.findByProvider(defaultProvider);
List<InstanceType> instanceTypeList = InstanceType.findByProvider(defaultProvider, mockConfig);
assertNotNull(instanceTypeList);
assertEquals(2, instanceTypeList.size());
Set<String> possibleTypes = new HashSet<>();
Expand All @@ -105,19 +114,41 @@ public void testFindByProvider() {
public void testFindByProviderWithUnSupportedInstances() {
InstanceType.upsert(defaultProvider.code, "t2.medium", 3, 10.0, defaultDetails);
InstanceType.upsert(defaultProvider.code, "c3.medium", 2, 10.0, defaultDetails);
List<InstanceType> instanceTypeList = InstanceType.findByProvider(defaultProvider);
List<InstanceType> instanceTypeList = InstanceType.findByProvider(defaultProvider, mockConfig);
assertNotNull(instanceTypeList);
assertEquals(1, instanceTypeList.size());
assertThat(instanceTypeList.get(0).getInstanceTypeCode(),
allOf(notNullValue(), equalTo("c3.medium")));
}


@Test
public void testFindByProviderWithEmptyInstanceTypeDetails() {
InstanceType.upsert(defaultProvider.code, "c5.medium", 3, 10.0,
new InstanceType.InstanceTypeDetails());
when(mockConfig.getInt(InstanceType.YB_AWS_DEFAULT_VOLUME_COUNT_KEY))
.thenReturn(1);
when(mockConfig.getInt(InstanceType.YB_AWS_DEFAULT_VOLUME_SIZE_GB_KEY))
.thenReturn(250);
List<InstanceType> instanceTypeList = InstanceType.findByProvider(defaultProvider, mockConfig);
assertNotNull(instanceTypeList);
InstanceType.VolumeDetails volumeDetails = instanceTypeList
.get(0)
.instanceTypeDetails
.volumeDetailsList
.get(0);
assertEquals(250, volumeDetails.volumeSizeGB.intValue());
assertEquals(InstanceType.VolumeType.EBS, volumeDetails.volumeType);
assertEquals(String.format("/mnt/d%d", 0), volumeDetails.mountPath);
assertThat(instanceTypeList.get(0).getInstanceTypeDetails().getVolumeDetailsList().size(),
allOf(notNullValue(), equalTo(1)));
}

@Test
public void testDeleteByProvider() {
Provider newProvider = ModelFactory.gcpProvider(defaultCustomer);
InstanceType.upsert(newProvider.code, "bar", 2, 10.0, defaultDetails);
InstanceType.deleteInstanceTypesForProvider(newProvider);
List<InstanceType> instanceTypeList = InstanceType.findByProvider(newProvider);
InstanceType.deleteInstanceTypesForProvider(newProvider, mockConfig);
List<InstanceType> instanceTypeList = InstanceType.findByProvider(newProvider, mockConfig);
assertEquals(0, instanceTypeList.size());
}

Expand Down

0 comments on commit cd61595

Please sign in to comment.