Repository: keikoproj/instance-manager
Pull request #385 (2023-08-22 22:22:57)

Make launchtemplates as a default scaling group configuration, instead of launchconfigurations.
The configurations for the AutoscalingGroups could be specified in either a launchConfiguration or a launchTemplate. launchtemplates has several benefits over launchconfigurations:
- ability to have increased number of scaling groups in a cluster. 
- support for mixed instance type in a ASG.
- support for newer instance type in ASG.


[aws blog-post](https://aws.amazon.com/blogs/compute/amazon-ec2-auto-scaling-will-no-longer-add-support-for-new-ec2-features-to-launch-configurations/)

>  AWS released [launch templates](https://aws.amazon.com/about-aws/whats-new/2017/11/introducing-launch-templates-for-amazon-ec2-instances/), the successor of launch configurations, as a way to streamline and simplify the launch process for [Auto Scaling](https://aws.amazon.com/ec2/autoscaling/), [Spot Fleet](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/spot-fleet.html), [Amazon EC2 Spot Instances](https://aws.amazon.com/ec2/spot/?cards.sort-by=item.additionalFields.startDateTime&cards.sort-order=asc&trk=ps_a134p000006vwG3AAI&trkCampaign=acq_paid_search_brand&sc_channel=PS&sc_campaign=acquisition_US&sc_publisher=Google&sc_category=Cloud%20Computing&sc_country=US&sc_geo=NAMER&sc_outcome=acq&sc_detail=ec2%20spot&sc_content=EC2%20Spot_e&sc_matchtype=e&sc_segment=517520538473&sc_medium=ACQ-P%7CPS-GO%7CBrand%7CDesktop%7CSU%7CCloud%20Computing%7CEC2%20Spot%7CUS%7CEN%7CText&s_kwcid=AL!4422!3!517520538473!e!!g!!ec2%20spot&ef_id=CjwKCAjwmK6IBhBqEiwAocMc8nuPFmpaTxCYXHIx2LzTmn2IjmsT8GUoWO53MOcvmJVS6wgVOUqxExoCL00QAvD_BwE:G:s&s_kwcid=AL!4422!3!517520538473!e!!g!!ec2%20spot), and [On-Demand Instances](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-on-demand-instances.html). Launch templates define the steps required to create an instance, by capturing instance parameters in a resource that can be used across multiple services. [Launch configurations](https://docs.aws.amazon.com/autoscaling/ec2/userguide/LaunchConfiguration.html) have continued to live alongside launch templates but haven’t benefitted from all of the features we’ve added to launch templates.
        
> Today, AWS is recommending that customers using [launch configurations](https://docs.aws.amazon.com/autoscaling/ec2/userguide/LaunchConfiguration.html) migrate to [launch templates](https://docs.aws.amazon.com/autoscaling/ec2/userguide/LaunchTemplates.html). We will continue to support and maintain launch configurations, but we will not be adding any new features to them. We will focus on adding new EC2 features to launch templates only. You can continue using launch configurations, and AWS is committed to supporting applications you have already built using them, but in order for you to take advantage of our most recent and upcoming releases, a migration to launch templates is recommended. Additionally, we plan to no longer support new instance types with launch configurations by the end of 2022. Our goal is to have all customers moved over to launch templates by then.



Additionally, launchconfigurations is being slowly phased out.
<img width="1709" alt="Screenshot 2023-08-22 at 3 18 17 PM" src="https://github.com/keikoproj/instance-manager/assets/7680410/576b0202-aaa1-476d-9d8c-afdffe6a2c62">


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/keikoproj/instance-manager/pull/385?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) Report
> Merging [#385](https://app.codecov.io/gh/keikoproj/instance-manager/pull/385?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) (2bf452f) into [master](https://app.codecov.io/gh/keikoproj/instance-manager/commit/7c2bf62d70523180a5f731f0480a9b604a03dcab?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) (7c2bf62) will **not change** coverage.
> The diff coverage is `0.00%`.

> :exclamation: Current head 2bf452f differs from pull request most recent head f4ae8e4. Consider uploading reports for the commit f4ae8e4 to get more accurate results

```diff
@@           Coverage Diff           @@
##           master     #385   +/-   ##
=======================================
  Coverage   51.37%   51.37%           
=======================================
  Files          33       33           
  Lines        4557     4557           
=======================================
  Hits         2341     2341           
  Misses       2069     2069           
  Partials      147      147           
```


| [Files Changed](https://app.codecov.io/gh/keikoproj/instance-manager/pull/385?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj) | Coverage Δ | |
|---|---|---|
| [api/v1alpha1/instancegroup\_types.go](https://app.codecov.io/gh/keikoproj/instance-manager/pull/385?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj#diff-YXBpL3YxYWxwaGExL2luc3RhbmNlZ3JvdXBfdHlwZXMuZ28=) | `18.69% <0.00%> (ø)` | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=keikoproj)


Patch:
diff --git a/Makefile b/Makefile
index 28e6b23..5e14ced 100644
--- 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
index 9eaee38..b011921 100644
--- 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/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
index 83dce64..576601b 100644
--- a/api/v1alpha1/zz_generated.deepcopy.go
+++ b/api/v1alpha1/zz_generated.deepcopy.go
@@ -668,6 +668,26 @@ func (in *UserDataStage) DeepCopy() *UserDataStage {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *ValidationOverrides) DeepCopyInto(out *ValidationOverrides) {
+	*out = *in
+	if in.scalingConfigurationOverride != nil {
+		in, out := &in.scalingConfigurationOverride, &out.scalingConfigurationOverride
+		*out = new(ScalingConfigurationType)
+		**out = **in
+	}
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ValidationOverrides.
+func (in *ValidationOverrides) DeepCopy() *ValidationOverrides {
+	if in == nil {
+		return nil
+	}
+	out := new(ValidationOverrides)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *WarmPoolSpec) DeepCopyInto(out *WarmPoolSpec) {
 	*out = *in
diff --git a/controllers/instancegroup_controller.go b/controllers/instancegroup_controller.go
index 881a2f3..47412ce 100644
--- 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/main.go b/main.go
index bdc4f38..caa606d 100644
--- 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,


Test patch:
diff --git a/api/v1alpha1/instancegroup_types_test.go b/api/v1alpha1/instancegroup_types_test.go
index 6b6e1be..8021b6f 100644
--- 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/controllers/provisioners/eks/upgrade_test.go b/controllers/provisioners/eks/upgrade_test.go
index 12bb736..c5f2d21 100644
--- 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