Skip to content

Commit

Permalink
Revert "Remove UseSmallerMachineTypes feature flag (kyma-project#882)"
Browse files Browse the repository at this point in the history 
This reverts commit 53acbd7.
  • Loading branch information
@MarekMichali
MarekMichali committed Jun 25, 2024
1 parent 2a2ccba commit a974208
Show file tree
Hide file tree
Showing 27 changed files with 1,151 additions and 78 deletions.
2 changes: 1 addition & 1 deletion cmd/broker/broker_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func NewBrokerSuiteTestWithConfig(t *testing.T, cfg *Config, version ...string)
DefaultGardenerShootPurpose: "testing",
DefaultTrialProvider: internal.AWS,
EnableShootAndSeedSameRegion: cfg.Provisioner.EnableShootAndSeedSameRegion,
}, defaultKymaVer, map[string]string{"cf-eu10": "europe", "cf-us10": "us"}, cfg.FreemiumProviders, defaultOIDCValues())
}, defaultKymaVer, map[string]string{"cf-eu10": "europe", "cf-us10": "us"}, cfg.FreemiumProviders, defaultOIDCValues(), cfg.Broker.UseSmallerMachineTypes)

storageCleanup, db, err := GetStorageForE2ETests()
assert.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion cmd/broker/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ func main() {

oidcDefaultValues, err := runtime.ReadOIDCDefaultValuesFromYAML(cfg.SkrOidcDefaultValuesYAMLFilePath)
fatalOnError(err, logs)
inputFactory, err := input.NewInputBuilderFactory(configProvider, cfg.Provisioner, cfg.KymaVersion, regions, cfg.FreemiumProviders, oidcDefaultValues)
inputFactory, err := input.NewInputBuilderFactory(configProvider, cfg.Provisioner, cfg.KymaVersion, regions, cfg.FreemiumProviders, oidcDefaultValues, cfg.Broker.UseSmallerMachineTypes)
fatalOnError(err, logs)

edpClient := edp.NewClient(cfg.EDP)
Expand Down
54 changes: 45 additions & 9 deletions cmd/broker/provisioning_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func TestCatalog(t *testing.T) {

func TestProvisioning_HappyPath(t *testing.T) {
// given
suite := NewProvisioningSuite(t, false, "")
suite := NewProvisioningSuite(t, false, "", false)
defer suite.TearDown()

// when
Expand Down Expand Up @@ -847,6 +847,7 @@ func TestProvisioning_ClusterParameters(t *testing.T) {
region string
multiZone bool
controlPlaneFailureTolerance string
useSmallerMachineTypes bool

expectedZonesCount *int
expectedProvider string
Expand All @@ -859,6 +860,17 @@ func TestProvisioning_ClusterParameters(t *testing.T) {
"Regular trial": {
planID: broker.TrialPlanID,

expectedMinimalNumberOfNodes: 1,
expectedMaximumNumberOfNodes: 1,
expectedMachineType: "Standard_D4s_v5",
expectedProvider: "azure",
expectedSharedSubscription: true,
expectedSubscriptionHyperscalerType: hyperscaler.Azure(),
},
"Regular trial with smaller machines": {
planID: broker.TrialPlanID,
useSmallerMachineTypes: true,

expectedMinimalNumberOfNodes: 1,
expectedMaximumNumberOfNodes: 1,
expectedMachineType: "Standard_D2s_v5",
Expand All @@ -870,6 +882,18 @@ func TestProvisioning_ClusterParameters(t *testing.T) {
planID: broker.FreemiumPlanID,
platformProvider: internal.AWS,

expectedMinimalNumberOfNodes: 1,
expectedMaximumNumberOfNodes: 1,
expectedProvider: "aws",
expectedSharedSubscription: false,
expectedMachineType: "m5.xlarge",
expectedSubscriptionHyperscalerType: hyperscaler.AWS(),
},
"Freemium aws with smaller machines": {
planID: broker.FreemiumPlanID,
platformProvider: internal.AWS,
useSmallerMachineTypes: true,

expectedMinimalNumberOfNodes: 1,
expectedMaximumNumberOfNodes: 1,
expectedProvider: "aws",
Expand All @@ -881,6 +905,18 @@ func TestProvisioning_ClusterParameters(t *testing.T) {
planID: broker.FreemiumPlanID,
platformProvider: internal.Azure,

expectedMinimalNumberOfNodes: 1,
expectedMaximumNumberOfNodes: 1,
expectedProvider: "azure",
expectedSharedSubscription: false,
expectedMachineType: "Standard_D4s_v5",
expectedSubscriptionHyperscalerType: hyperscaler.Azure(),
},
"Freemium azure with smaller machines": {
planID: broker.FreemiumPlanID,
platformProvider: internal.Azure,
useSmallerMachineTypes: true,

expectedMinimalNumberOfNodes: 1,
expectedMaximumNumberOfNodes: 1,
expectedProvider: "azure",
Expand Down Expand Up @@ -972,7 +1008,7 @@ func TestProvisioning_ClusterParameters(t *testing.T) {
} {
t.Run(tn, func(t *testing.T) {
// given
suite := NewProvisioningSuite(t, tc.multiZone, tc.controlPlaneFailureTolerance)
suite := NewProvisioningSuite(t, tc.multiZone, tc.controlPlaneFailureTolerance, tc.useSmallerMachineTypes)
defer suite.TearDown()

// when
Expand Down Expand Up @@ -1010,7 +1046,7 @@ func TestProvisioning_OIDCValues(t *testing.T) {

t.Run("should apply default OIDC values when OIDC object is nil", func(t *testing.T) {
// given
suite := NewProvisioningSuite(t, false, "")
suite := NewProvisioningSuite(t, false, "", false)
defer suite.TearDown()
defaultOIDC := fixture.FixOIDCConfigDTO()
expectedOIDC := gqlschema.OIDCConfigInput{
Expand Down Expand Up @@ -1041,7 +1077,7 @@ func TestProvisioning_OIDCValues(t *testing.T) {

t.Run("should apply default OIDC values when all OIDC object's fields are empty", func(t *testing.T) {
// given
suite := NewProvisioningSuite(t, false, "")
suite := NewProvisioningSuite(t, false, "", false)
defer suite.TearDown()
defaultOIDC := fixture.FixOIDCConfigDTO()
expectedOIDC := gqlschema.OIDCConfigInput{
Expand Down Expand Up @@ -1075,7 +1111,7 @@ func TestProvisioning_OIDCValues(t *testing.T) {

t.Run("should apply provided OIDC configuration", func(t *testing.T) {
// given
suite := NewProvisioningSuite(t, false, "")
suite := NewProvisioningSuite(t, false, "", false)
defer suite.TearDown()
providedOIDC := internal.OIDCConfigDTO{
ClientID: "fake-client-id-1",
Expand Down Expand Up @@ -1114,7 +1150,7 @@ func TestProvisioning_OIDCValues(t *testing.T) {

t.Run("should apply default OIDC values on empty OIDC params from input", func(t *testing.T) {
// given
suite := NewProvisioningSuite(t, false, "")
suite := NewProvisioningSuite(t, false, "", false)
defer suite.TearDown()
providedOIDC := internal.OIDCConfigDTO{
ClientID: "fake-client-id-1",
Expand Down Expand Up @@ -1152,7 +1188,7 @@ func TestProvisioning_OIDCValues(t *testing.T) {
func TestProvisioning_RuntimeAdministrators(t *testing.T) {
t.Run("should use UserID as default value for admins list", func(t *testing.T) {
// given
suite := NewProvisioningSuite(t, false, "")
suite := NewProvisioningSuite(t, false, "", false)
defer suite.TearDown()
options := RuntimeOptions{
UserID: "fake-user-id",
Expand All @@ -1178,7 +1214,7 @@ func TestProvisioning_RuntimeAdministrators(t *testing.T) {

t.Run("should apply new admins list", func(t *testing.T) {
// given
suite := NewProvisioningSuite(t, false, "")
suite := NewProvisioningSuite(t, false, "", false)
defer suite.TearDown()
options := RuntimeOptions{
UserID: "fake-user-id",
Expand All @@ -1205,7 +1241,7 @@ func TestProvisioning_RuntimeAdministrators(t *testing.T) {

t.Run("should apply empty admin value (list is not empty)", func(t *testing.T) {
// given
suite := NewProvisioningSuite(t, false, "")
suite := NewProvisioningSuite(t, false, "", false)
defer suite.TearDown()
options := RuntimeOptions{
UserID: "fake-user-id",
Expand Down
6 changes: 3 additions & 3 deletions cmd/broker/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func NewOrchestrationSuite(t *testing.T, additionalKymaVersions []string) *Orche
ProvisioningTimeout: time.Minute,
URL: "http://localhost",
DefaultGardenerShootPurpose: "testing",
}, kymaVer, map[string]string{"cf-eu10": "europe"}, cfg.FreemiumProviders, oidcDefaults)
}, kymaVer, map[string]string{"cf-eu10": "europe"}, cfg.FreemiumProviders, oidcDefaults, cfg.Broker.UseSmallerMachineTypes)
require.NoError(t, err)

gardenerClient := gardener.NewDynamicFakeClient()
Expand Down Expand Up @@ -551,7 +551,7 @@ func (s *ProvisioningSuite) TearDown() {
}
}

func NewProvisioningSuite(t *testing.T, multiZoneCluster bool, controlPlaneFailureTolerance string) *ProvisioningSuite {
func NewProvisioningSuite(t *testing.T, multiZoneCluster bool, controlPlaneFailureTolerance string, useSmallerMachineTypes bool) *ProvisioningSuite {
defer func() {
if r := recover(); r != nil {
err := cleanupContainer()
Expand Down Expand Up @@ -594,7 +594,7 @@ func NewProvisioningSuite(t *testing.T, multiZoneCluster bool, controlPlaneFailu
DefaultGardenerShootPurpose: "testing",
MultiZoneCluster: multiZoneCluster,
ControlPlaneFailureTolerance: controlPlaneFailureTolerance,
}, defaultKymaVer, map[string]string{"cf-eu10": "europe"}, cfg.FreemiumProviders, oidcDefaults)
}, defaultKymaVer, map[string]string{"cf-eu10": "europe"}, cfg.FreemiumProviders, oidcDefaults, useSmallerMachineTypes)
require.NoError(t, err)

server := avs.NewMockAvsServer(t)
Expand Down
2 changes: 1 addition & 1 deletion cmd/broker/update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2082,7 +2082,7 @@ func TestUpdateMachineType(t *testing.T) {
rs, err := suite.db.RuntimeStates().ListByRuntimeID(i.RuntimeID)
assert.NoError(t, err, "runtime states after provisioning")
assert.Equal(t, 1, len(rs), "runtime states after provisioning")
assert.Equal(t, "m6i.large", rs[0].ClusterConfig.MachineType, "after provisioning")
assert.Equal(t, "m5.xlarge", rs[0].ClusterConfig.MachineType, "after provisioning")

// when patch to change machine type

Expand Down
3 changes: 2 additions & 1 deletion internal/broker/broker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ type Config struct {
TrialDocsURL string `envconfig:"default="`
EnableShootAndSeedSameRegion bool `envconfig:"default=false"`

Binding BindingConfig
Binding BindingConfig
UseSmallerMachineTypes bool `envconfig:"default=false"`
}

type ServicesConfig map[string]Service
Expand Down
2 changes: 1 addition & 1 deletion internal/broker/instance_create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ func (b *ProvisionEndpoint) determineLicenceType(planId string) *string {

func (b *ProvisionEndpoint) validator(details *domain.ProvisionDetails, provider internal.CloudProvider, ctx context.Context) (JSONSchemaValidator, error) {
platformRegion, _ := middleware.RegionFromContext(ctx)
plans := Plans(b.plansConfig, provider, b.config.IncludeAdditionalParamsInSchema, euaccess.IsEURestrictedAccess(platformRegion), b.config.EnableShootAndSeedSameRegion, b.convergedCloudRegionsProvider.GetRegions(platformRegion))
plans := Plans(b.plansConfig, provider, b.config.IncludeAdditionalParamsInSchema, euaccess.IsEURestrictedAccess(platformRegion), b.config.UseSmallerMachineTypes, b.config.EnableShootAndSeedSameRegion, b.convergedCloudRegionsProvider.GetRegions(platformRegion))
plan := plans[details.PlanID]
schema := string(Marshal(plan.Schemas.Instance.Create.Parameters))

Expand Down
2 changes: 1 addition & 1 deletion internal/broker/instance_update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ func (b *UpdateEndpoint) isKyma2(instance *internal.Instance) (bool, string, err
func (b *UpdateEndpoint) getJsonSchemaValidator(provider internal.CloudProvider, planID string, platformRegion string) (JSONSchemaValidator, error) {
// shootAndSeedSameRegion is never enabled for update
b.log.Printf("region is: %s", platformRegion)
plans := Plans(b.plansConfig, provider, b.config.IncludeAdditionalParamsInSchema, euaccess.IsEURestrictedAccess(platformRegion), false, b.convergedCloudRegionsProvider.GetRegions(platformRegion))
plans := Plans(b.plansConfig, provider, b.config.IncludeAdditionalParamsInSchema, euaccess.IsEURestrictedAccess(platformRegion), b.config.UseSmallerMachineTypes, false, b.convergedCloudRegionsProvider.GetRegions(platformRegion))
plan := plans[planID]
schema := string(Marshal(plan.Schemas.Instance.Update.Parameters))

Expand Down
21 changes: 20 additions & 1 deletion internal/broker/plans.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,20 @@ func SapConvergedCloudMachinesDisplay() map[string]string {
}
}

func removeMachinesNamesFromList(machinesNames []string, machinesNamesToRemove ...string) []string {
for i, machineName := range machinesNames {
for _, machineNameToRemove := range machinesNamesToRemove {
if machineName == machineNameToRemove {
copy(machinesNames[i:], machinesNames[i+1:])
machinesNames[len(machinesNames)-1] = ""
machinesNames = machinesNames[:len(machinesNames)-1]
}
}
}

return machinesNames
}

func requiredSchemaProperties() []string {
return []string{"name", "region"}
}
Expand Down Expand Up @@ -450,7 +464,7 @@ func unmarshalSchema(schema *RootSchema) *map[string]interface{} {

// Plans is designed to hold plan defaulting logic
// keep internal/hyperscaler/azure/config.go in sync with any changes to available zones
func Plans(plans PlansConfig, provider internal.CloudProvider, includeAdditionalParamsInSchema bool, euAccessRestricted bool, shootAndSeedFeatureFlag bool, sapConvergedCloudRegions []string) map[string]domain.ServicePlan {
func Plans(plans PlansConfig, provider internal.CloudProvider, includeAdditionalParamsInSchema bool, euAccessRestricted bool, useSmallerMachineTypes bool, shootAndSeedFeatureFlag bool, sapConvergedCloudRegions []string) map[string]domain.ServicePlan {
awsMachineNames := AwsMachinesNames()
awsMachinesDisplay := AwsMachinesDisplay()
awsRegionsDisplay := AWSRegionsDisplay()
Expand All @@ -463,6 +477,11 @@ func Plans(plans PlansConfig, provider internal.CloudProvider, includeAdditional
gcpMachinesDisplay := GcpMachinesDisplay()
gcpRegionsDisplay := GcpRegionsDisplay()

if !useSmallerMachineTypes {
azureLiteMachinesNames = removeMachinesNamesFromList(azureLiteMachinesNames, "Standard_D2s_v5")
delete(azureLiteMachinesDisplay, "Standard_D2s_v5")
}

awsSchema := AWSSchema(awsMachinesDisplay, awsRegionsDisplay, awsMachineNames, includeAdditionalParamsInSchema, false, euAccessRestricted, shootAndSeedFeatureFlag)
// awsHASchema := AWSHASchema(awsMachinesDisplay, awsMachines, includeAdditionalParamsInSchema, false)
azureSchema := AzureSchema(azureMachinesDisplay, azureRegionsDisplay, azureMachinesNames, includeAdditionalParamsInSchema, false, euAccessRestricted, shootAndSeedFeatureFlag)
Expand Down
40 changes: 37 additions & 3 deletions internal/broker/plans_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ import (
)

func TestSchemaGenerator(t *testing.T) {
azureLiteMachineNamesReduced := AzureLiteMachinesNames()
azureLiteMachinesDisplayReduced := AzureLiteMachinesDisplay()

azureLiteMachineNamesReduced = removeMachinesNamesFromList(azureLiteMachineNamesReduced, "Standard_D2s_v5")
delete(azureLiteMachinesDisplayReduced, "Standard_D2s_v5")

tests := []struct {
name string
generator func(map[string]string, map[string]string, []string, bool, bool) *map[string]interface{}
Expand Down Expand Up @@ -94,6 +100,20 @@ func TestSchemaGenerator(t *testing.T) {
fileOIDC: "azure-lite-schema-additional-params.json",
updateFileOIDC: "update-azure-lite-schema-additional-params.json",
},
{
name: "AzureLite reduced schema is correct",
generator: func(machinesDisplay, regionsDisplay map[string]string, machines []string, additionalParams, update bool) *map[string]interface{} {
return AzureLiteSchema(machinesDisplay, regionsDisplay, machines, additionalParams, update, false, false)
},
machineTypes: azureLiteMachineNamesReduced,
machineTypesDisplay: azureLiteMachinesDisplayReduced,
regionDisplay: AzureRegionsDisplay(false),
path: "azure",
file: "azure-lite-schema-reduced.json",
updateFile: "update-azure-lite-schema-reduced.json",
fileOIDC: "azure-lite-schema-additional-params-reduced.json",
updateFileOIDC: "update-azure-lite-schema-additional-params-reduced.json",
},
{
name: "AzureLite schema with EU access restriction is correct",
generator: func(machinesDisplay, regionsDisplay map[string]string, machines []string, additionalParams, update bool) *map[string]interface{} {
Expand All @@ -108,6 +128,20 @@ func TestSchemaGenerator(t *testing.T) {
fileOIDC: "azure-lite-schema-additional-params-eu.json",
updateFileOIDC: "update-azure-lite-schema-additional-params.json",
},
{
name: "AzureLite reduced schema with EU access restriction is correct",
generator: func(machinesDisplay, regionsDisplay map[string]string, machines []string, additionalParams, update bool) *map[string]interface{} {
return AzureLiteSchema(machinesDisplay, regionsDisplay, machines, additionalParams, update, true, false)
},
machineTypes: azureLiteMachineNamesReduced,
machineTypesDisplay: azureLiteMachinesDisplayReduced,
regionDisplay: AzureRegionsDisplay(true),
path: "azure",
file: "azure-lite-schema-eu-reduced.json",
updateFile: "update-azure-lite-schema-reduced.json",
fileOIDC: "azure-lite-schema-additional-params-eu-reduced.json",
updateFileOIDC: "update-azure-lite-schema-additional-params-reduced.json",
},
{
name: "Freemium schema is correct",
generator: func(machinesDisplay, regionsDisplay map[string]string, machines []string, additionalParams, update bool) *map[string]interface{} {
Expand Down Expand Up @@ -235,7 +269,7 @@ func TestSapConvergedSchema(t *testing.T) {
regions := []string{"region1", "region2"}

// when
schema := Plans(nil, "", false, false, false, regions)
schema := Plans(nil, "", false, false, false, false, regions)
convergedSchema, found := schema[SapConvergedCloudPlanID]
schemaRegionsCreate := convergedSchema.Schemas.Instance.Create.Parameters["properties"].(map[string]interface{})["region"].(map[string]interface{})["enum"]

Expand All @@ -250,15 +284,15 @@ func TestSapConvergedSchema(t *testing.T) {
regions := []string{}

// when
schema := Plans(nil, "", false, false, false, regions)
schema := Plans(nil, "", false, false, false, false, regions)
_, found := schema[SapConvergedCloudPlanID]

// then
assert.NotNil(t, schema)
assert.False(t, found)

// when
schema = Plans(nil, "", false, false, false, nil)
schema = Plans(nil, "", false, false, false, false, nil)
_, found = schema[SapConvergedCloudPlanID]

// then
Expand Down
1 change: 1 addition & 0 deletions internal/broker/services.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func (b *ServicesEndpoint) Services(ctx context.Context) ([]domain.Service, erro
provider,
b.cfg.IncludeAdditionalParamsInSchema,
euaccess.IsEURestrictedAccess(platformRegion),
b.cfg.UseSmallerMachineTypes,
b.cfg.EnableShootAndSeedSameRegion,
b.convergedCloudRegionsProvider.GetRegions(platformRegion),
) {
Expand Down
Loading

0 comments on commit a974208

Please sign in to comment.