feat: support overriding default scaling configuration (#385)

* make launchtemplates default Signed-off-by: sbadiger <[email protected]> * remove additional new lint Signed-off-by: sbadiger <[email protected]> * indentation Signed-off-by: sbadiger <[email protected]> * update default config type Signed-off-by: sbadiger <[email protected]> * update spec Signed-off-by: sbadiger <[email protected]> * refactor the default scaling config code Signed-off-by: sbadiger <[email protected]> * add unit tests Signed-off-by: sbadiger <[email protected]> * fix unit tests Signed-off-by: sbadiger <[email protected]> * indent Signed-off-by: sbadiger <[email protected]> * update defaultScalingConfiguration flag description Signed-off-by: sbadiger <[email protected]> --------- Signed-off-by: sbadiger <[email protected]>
keikoproj · Aug 31, 2023 · d1e5113 · d1e5113
1 parent 7c2bf62
commit d1e5113
Show file tree

Hide file tree

Showing 7 changed files with 174 additions and 42 deletions.
diff --git a/Makefile b/Makefile
@@ -38,7 +38,7 @@ all: check-go test clean manager
 # Run tests
 .PHONY: test
 test: generate fmt vet manifests
-	go test -v ./controllers/... ./api/... -coverprofile coverage.txt
+	go test ./controllers/... ./api/... -coverprofile coverage.txt
 
 .PHONY: bdd
 bdd:

diff --git a/api/v1alpha1/instancegroup_types.go b/api/v1alpha1/instancegroup_types.go
@@ -368,6 +368,15 @@ type InstanceGroupStatus struct {
 
 type InstanceGroupConditionType string
 
+type ValidationOverrides struct {
+	scalingConfigurationOverride *ScalingConfigurationType
+}
+
+func NewValidationOverrides(defaultScalingConfiguration *ScalingConfigurationType) *ValidationOverrides {
+	return &ValidationOverrides{
+		scalingConfigurationOverride: defaultScalingConfiguration,
+	}
+}
 func NewInstanceGroupCondition(cType InstanceGroupConditionType, status corev1.ConditionStatus) InstanceGroupCondition {
 	return InstanceGroupCondition{
 		Type:   cType,
@@ -409,7 +418,7 @@ func (ig *InstanceGroup) Locked() bool {
 	return false
 }
 
-func (s *EKSSpec) Validate() error {
+func (s *EKSSpec) Validate(overrides *ValidationOverrides) error {
 	var (
 		configuration = s.EKSConfiguration
 		configType    = s.Type
@@ -419,7 +428,10 @@ func (s *EKSSpec) Validate() error {
 	}
 
 	if s.Type != LaunchConfiguration && s.Type != LaunchTemplate {
-		s.Type = LaunchConfiguration
+		s.Type = LaunchTemplate
+		if overrides.scalingConfigurationOverride != nil {
+			s.Type = *overrides.scalingConfigurationOverride
+		}
 	}
 
 	if s.Type == LaunchConfiguration {
@@ -700,7 +712,7 @@ func (m *MixedInstancesPolicySpec) Validate() error {
 	return nil
 }
 
-func (ig *InstanceGroup) Validate() error {
+func (ig *InstanceGroup) Validate(overrides *ValidationOverrides) error {
 	s := ig.Spec
 
 	if !common.ContainsEqualFold(Provisioners, s.Provisioner) {
@@ -735,7 +747,7 @@ func (ig *InstanceGroup) Validate() error {
 		config := ig.GetEKSConfiguration()
 		spec := ig.GetEKSSpec()
 
-		if err := spec.Validate(); err != nil {
+		if err := spec.Validate(overrides); err != nil {
 			return err
 		}
 

diff --git a/api/v1alpha1/instancegroup_types_test.go b/api/v1alpha1/instancegroup_types_test.go
@@ -22,10 +22,11 @@ import (
 
 type EksUnitTest struct {
 	InstanceGroup *InstanceGroup
+	Overrides     *ValidationOverrides
 }
 
 func (u *EksUnitTest) Run(t *testing.T) string {
-	err := u.InstanceGroup.Validate()
+	err := u.InstanceGroup.Validate(u.Overrides)
 	if err == nil {
 		return aws.StringValue(nil)
 	} else {
@@ -34,12 +35,15 @@ func (u *EksUnitTest) Run(t *testing.T) string {
 }
 
 func TestInstanceGroupSpecValidate(t *testing.T) {
+	launchconfiguration := LaunchConfiguration
 	type args struct {
 		instancegroup *InstanceGroup
+		overrides     *ValidationOverrides
 	}
 	testFunction := func(t *testing.T, args args) string {
 		testCase := EksUnitTest{
 			InstanceGroup: args.instancegroup,
+			Overrides:     args.overrides,
 		}
 		return testCase.Run(t)
 	}
@@ -432,6 +436,16 @@ func TestInstanceGroupSpecValidate(t *testing.T) {
 			},
 			want: "validation failed, HostResourceGroupArn must be a valid dedicated HostResourceGroup ARN",
 		},
+		{
+			name: "default to launch config instead of launch template",
+			args: args{
+				instancegroup: MockInstanceGroup("eks-fargate", "managed", nil, nil, basicFargateSpec()),
+				overrides: &ValidationOverrides{
+					scalingConfigurationOverride: &launchconfiguration,
+				},
+			},
+			want: "",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -444,6 +458,70 @@ func TestInstanceGroupSpecValidate(t *testing.T) {
 	}
 }
 
+func TestScalingConfigOverride(t *testing.T) {
+	launchconfiguration := LaunchConfiguration
+	launchtemplate := LaunchTemplate
+	type args struct {
+		instancegroup *InstanceGroup
+		overrides     *ValidationOverrides
+	}
+	testFunction := func(t *testing.T, args args) string {
+		testCase := EksUnitTest{
+			InstanceGroup: args.instancegroup,
+			Overrides:     args.overrides,
+		}
+		return testCase.Run(t)
+	}
+	tests := []struct {
+		name string
+		args args
+		want ScalingConfigurationType
+	}{
+		{
+			name: "override default to launchconfig instead of launchtemplate",
+			args: args{
+				instancegroup: MockInstanceGroup("eks", "managed", MockEKSSpec(), nil, basicFargateSpec()),
+				overrides: &ValidationOverrides{
+					scalingConfigurationOverride: &launchconfiguration,
+				},
+			},
+			want: LaunchConfiguration,
+		},
+		{
+			name: "no default overrides",
+			args: args{
+				instancegroup: MockInstanceGroup("eks", "managed", MockEKSSpec(), nil, basicFargateSpec()),
+				overrides:     &ValidationOverrides{},
+			},
+			want: LaunchTemplate,
+		},
+		{
+			name: "override default to launchtemplate",
+			args: args{
+				instancegroup: MockInstanceGroup("eks", "managed", MockEKSSpec(), nil, basicFargateSpec()),
+				overrides: &ValidationOverrides{
+					scalingConfigurationOverride: &launchtemplate,
+				},
+			},
+			want: LaunchTemplate,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := testFunction(t, tt.args)
+			if err != "" {
+				t.Errorf("error:%v", err)
+			}
+			got := tt.args.instancegroup.Spec.EKSSpec.Type
+			if got != tt.want {
+				t.Errorf("%v: got %v, want %v", tt.name, got, tt.want)
+			}
+
+		})
+
+	}
+}
+
 func TestLockedAnnotation(t *testing.T) {
 	tests := []struct {
 		name       string
@@ -507,3 +585,17 @@ func MockInstanceGroup(provisioner, strategy string, eksSpec *EKSSpec, eksManage
 	}
 
 }
+
+func MockEKSSpec() *EKSSpec {
+	return &EKSSpec{
+		Type: "invalid-scaling-config",
+		EKSConfiguration: &EKSConfiguration{
+			EksClusterName:     "sample-cluster",
+			Subnets:            []string{"subnet-1111111", "subnet-222222"},
+			NodeSecurityGroups: []string{"sg-sample-1", "sg-sample-2"},
+			Image:              "sample-ami",
+			InstanceType:       "sample-instance",
+			KeyPairName:        "sample-key-pair",
+		},
+	}
+}
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/controllers/instancegroup_controller.go b/controllers/instancegroup_controller.go
@@ -43,18 +43,19 @@ import (
 // InstanceGroupReconciler reconciles an InstanceGroup object
 type InstanceGroupReconciler struct {
 	client.Client
-	SpotRecommendationTime     float64
-	ConfigNamespace            string
-	NodeRelabel                bool
-	Log                        logr.Logger
-	MaxParallel                int
-	Auth                       *InstanceGroupAuthenticator
-	ConfigMap                  *corev1.ConfigMap
-	Namespaces                 map[string]corev1.Namespace
-	NamespacesLock             *sync.RWMutex
-	ConfigRetention            int
-	Metrics                    *common.MetricsCollector
-	DisableWinClusterInjection bool
+	SpotRecommendationTime      float64
+	ConfigNamespace             string
+	NodeRelabel                 bool
+	Log                         logr.Logger
+	MaxParallel                 int
+	Auth                        *InstanceGroupAuthenticator
+	ConfigMap                   *corev1.ConfigMap
+	Namespaces                  map[string]corev1.Namespace
+	NamespacesLock              *sync.RWMutex
+	ConfigRetention             int
+	Metrics                     *common.MetricsCollector
+	DisableWinClusterInjection  bool
+	DefaultScalingConfiguration *v1alpha1.ScalingConfigurationType
 }
 
 type InstanceGroupAuthenticator struct {
@@ -194,7 +195,10 @@ func (r *InstanceGroupReconciler) Reconcile(ctxt context.Context, req ctrl.Reque
 		ctx = eksfargate.New(input)
 	}
 
-	if err = input.InstanceGroup.Validate(); err != nil {
+	// for igs without any config type mentioned, allow overriding the default.
+	overrides := v1alpha1.NewValidationOverrides(r.DefaultScalingConfiguration)
+
+	if err = input.InstanceGroup.Validate(overrides); err != nil {
 		ctx.SetState(v1alpha1.ReconcileErr)
 		r.PatchStatus(input.InstanceGroup, statusPatch)
 		r.Metrics.IncFail(instanceGroup.NamespacedName(), ErrorReasonValidationFailed)

diff --git a/controllers/provisioners/eks/upgrade_test.go b/controllers/provisioners/eks/upgrade_test.go
@@ -96,7 +96,7 @@ func TestUpgradeCRDStrategyValidation(t *testing.T) {
 		t.Logf("#%v - %+v", i, tc.input)
 		var errOccured bool
 		ig.SetUpgradeStrategy(tc.input)
-		err := ig.Validate()
+		err := ig.Validate(&v1alpha1.ValidationOverrides{})
 		if err != nil {
 			t.Log(err)
 			errOccured = true

diff --git a/main.go b/main.go
@@ -65,16 +65,17 @@ func main() {
 	printVersion()
 
 	var (
-		metricsAddr                string
-		configNamespace            string
-		spotRecommendationTime     float64
-		enableLeaderElection       bool
-		nodeRelabel                bool
-		disableWinClusterInjection bool
-		maxParallel                int
-		maxAPIRetries              int
-		configRetention            int
-		err                        error
+		metricsAddr                 string
+		configNamespace             string
+		spotRecommendationTime      float64
+		enableLeaderElection        bool
+		nodeRelabel                 bool
+		disableWinClusterInjection  bool
+		maxParallel                 int
+		maxAPIRetries               int
+		configRetention             int
+		err                         error
+		defaultScalingConfiguration string
 	)
 
 	flag.IntVar(&maxParallel, "max-workers", 5, "The number of maximum parallel reconciles")
@@ -87,6 +88,7 @@ func main() {
 		"Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.")
 	flag.BoolVar(&nodeRelabel, "node-relabel", true, "relabel nodes as they join with kubernetes.io/role label via controller")
 	flag.BoolVar(&disableWinClusterInjection, "disable-windows-cluster-ca-injection", false, "Setting this to true will cause the ClusterCA and Endpoint to not be injected for Windows nodes")
+	flag.StringVar(&defaultScalingConfiguration, "", string(instancemgrv1alpha1.LaunchTemplate), "By default ASGs will have LaunchTemplate. Set this string to either 'LaunchConfiguration' or 'LaunchTemplate' to enforce defaults.")
 
 	flag.Parse()
 	ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
@@ -149,19 +151,21 @@ func main() {
 		setupLog.Info("instance-manager configmap does not exist, will not load defaults/boundaries")
 	}
 
+	defaultScalingConfigurationType := instancemgrv1alpha1.ScalingConfigurationType(defaultScalingConfiguration)
 	err = (&controllers.InstanceGroupReconciler{
-		Metrics:                    controllerCollector,
-		ConfigMap:                  cm,
-		ConfigRetention:            configRetention,
-		SpotRecommendationTime:     spotRecommendationTime,
-		ConfigNamespace:            configNamespace,
-		Namespaces:                 make(map[string]corev1.Namespace),
-		NamespacesLock:             &sync.RWMutex{},
-		NodeRelabel:                nodeRelabel,
-		DisableWinClusterInjection: disableWinClusterInjection,
-		Client:                     mgr.GetClient(),
-		Log:                        ctrl.Log.WithName("controllers").WithName("instancegroup"),
-		MaxParallel:                maxParallel,
+		Metrics:                     controllerCollector,
+		ConfigMap:                   cm,
+		ConfigRetention:             configRetention,
+		SpotRecommendationTime:      spotRecommendationTime,
+		ConfigNamespace:             configNamespace,
+		Namespaces:                  make(map[string]corev1.Namespace),
+		NamespacesLock:              &sync.RWMutex{},
+		NodeRelabel:                 nodeRelabel,
+		DisableWinClusterInjection:  disableWinClusterInjection,
+		Client:                      mgr.GetClient(),
+		Log:                         ctrl.Log.WithName("controllers").WithName("instancegroup"),
+		MaxParallel:                 maxParallel,
+		DefaultScalingConfiguration: &defaultScalingConfigurationType,
 		Auth: &controllers.InstanceGroupAuthenticator{
 			Aws:        awsWorker,
 			Kubernetes: kube,