From b91c06976ba41eb7940b30e00ed8f96937bc3ed5 Mon Sep 17 00:00:00 2001 From: sbadiger Date: Tue, 22 Aug 2023 11:43:02 -0700 Subject: [PATCH 01/10] make launchtemplates default Signed-off-by: sbadiger --- api/v1alpha1/instancegroup_types.go | 2 +- controllers/instancegroup_controller.go | 29 +++++++++++++++---------- main.go | 28 ++++++++++++++---------- 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/api/v1alpha1/instancegroup_types.go b/api/v1alpha1/instancegroup_types.go index 9eaee389..9d1bdb14 100644 --- a/api/v1alpha1/instancegroup_types.go +++ b/api/v1alpha1/instancegroup_types.go @@ -419,7 +419,7 @@ func (s *EKSSpec) Validate() error { } if s.Type != LaunchConfiguration && s.Type != LaunchTemplate { - s.Type = LaunchConfiguration + s.Type = LaunchTemplate } if s.Type == LaunchConfiguration { diff --git a/controllers/instancegroup_controller.go b/controllers/instancegroup_controller.go index 881a2f38..85d918a2 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 + SetScalingConfigurationToLaunchConfig bool } type InstanceGroupAuthenticator struct { @@ -201,6 +202,10 @@ func (r *InstanceGroupReconciler) Reconcile(ctxt context.Context, req ctrl.Reque return ctrl.Result{}, errors.Wrapf(err, "provisioner %v reconcile failed", provisionerKind) } + if r.SetScalingConfigurationToLaunchConfig{ + input.InstanceGroup.Spec.EKSSpec.Type = v1alpha1.LaunchConfiguration + } + if err = HandleReconcileRequest(ctx); err != nil { ctx.SetState(v1alpha1.ReconcileErr) r.PatchStatus(input.InstanceGroup, statusPatch) diff --git a/main.go b/main.go index bdc4f38c..1c4dbaff 100644 --- a/main.go +++ b/main.go @@ -75,6 +75,7 @@ func main() { maxAPIRetries int configRetention int err error + setScalingConfigurationToLaunchConfig bool ) flag.IntVar(&maxParallel, "max-workers", 5, "The number of maximum parallel reconciles") @@ -87,6 +88,8 @@ 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.BoolVar(&setScalingConfigurationToLaunchConfig, "scaling-configuration-to-launchconfig", false, "By default ASGs will have launchtemplates. Set this flag to true if launchconfigs needs to be the default.") + flag.Parse() ctrl.SetLogger(zap.New(zap.UseDevMode(true))) @@ -150,18 +153,19 @@ func main() { } 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, + SetScalingConfigurationToLaunchConfig: setScalingConfigurationToLaunchConfig, Auth: &controllers.InstanceGroupAuthenticator{ Aws: awsWorker, Kubernetes: kube, From 646946888da49da3f309b0372b01500c1b56473d Mon Sep 17 00:00:00 2001 From: sbadiger Date: Tue, 22 Aug 2023 11:43:44 -0700 Subject: [PATCH 02/10] remove additional new lint Signed-off-by: sbadiger --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index 1c4dbaff..b0966d20 100644 --- a/main.go +++ b/main.go @@ -182,4 +182,4 @@ func main() { setupLog.Error(err, "problem running manager") os.Exit(1) } -} +} \ No newline at end of file From 7ef7798d4aa842eddd628d25d4f0587afdf7ea0a Mon Sep 17 00:00:00 2001 From: sbadiger Date: Tue, 22 Aug 2023 11:45:33 -0700 Subject: [PATCH 03/10] indentation Signed-off-by: sbadiger --- controllers/instancegroup_controller.go | 28 +++++++------- main.go | 49 ++++++++++++------------- 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/controllers/instancegroup_controller.go b/controllers/instancegroup_controller.go index 85d918a2..2d7988ff 100644 --- a/controllers/instancegroup_controller.go +++ b/controllers/instancegroup_controller.go @@ -43,19 +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 - SetScalingConfigurationToLaunchConfig 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 + SetScalingConfigurationToLaunchConfig bool } type InstanceGroupAuthenticator struct { @@ -202,7 +202,7 @@ func (r *InstanceGroupReconciler) Reconcile(ctxt context.Context, req ctrl.Reque return ctrl.Result{}, errors.Wrapf(err, "provisioner %v reconcile failed", provisionerKind) } - if r.SetScalingConfigurationToLaunchConfig{ + if r.SetScalingConfigurationToLaunchConfig { input.InstanceGroup.Spec.EKSSpec.Type = v1alpha1.LaunchConfiguration } diff --git a/main.go b/main.go index b0966d20..f2656e5b 100644 --- a/main.go +++ b/main.go @@ -65,16 +65,16 @@ 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 setScalingConfigurationToLaunchConfig bool ) @@ -90,7 +90,6 @@ func main() { 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.BoolVar(&setScalingConfigurationToLaunchConfig, "scaling-configuration-to-launchconfig", false, "By default ASGs will have launchtemplates. Set this flag to true if launchconfigs needs to be the default.") - flag.Parse() ctrl.SetLogger(zap.New(zap.UseDevMode(true))) @@ -153,19 +152,19 @@ func main() { } 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, - SetScalingConfigurationToLaunchConfig: setScalingConfigurationToLaunchConfig, + 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, + SetScalingConfigurationToLaunchConfig: setScalingConfigurationToLaunchConfig, Auth: &controllers.InstanceGroupAuthenticator{ Aws: awsWorker, Kubernetes: kube, @@ -182,4 +181,4 @@ func main() { setupLog.Error(err, "problem running manager") os.Exit(1) } -} \ No newline at end of file +} From 5ff855c189f8b0a5011160c727dad5e0ada54e04 Mon Sep 17 00:00:00 2001 From: sbadiger Date: Tue, 22 Aug 2023 23:07:10 -0700 Subject: [PATCH 04/10] update default config type Signed-off-by: sbadiger --- controllers/instancegroup_controller.go | 13 ++++++++----- main.go | 6 +++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/controllers/instancegroup_controller.go b/controllers/instancegroup_controller.go index 2d7988ff..7d199e9a 100644 --- a/controllers/instancegroup_controller.go +++ b/controllers/instancegroup_controller.go @@ -55,7 +55,7 @@ type InstanceGroupReconciler struct { ConfigRetention int Metrics *common.MetricsCollector DisableWinClusterInjection bool - SetScalingConfigurationToLaunchConfig bool + SetDefaultConfigurationToLaunchConfig bool } type InstanceGroupAuthenticator struct { @@ -195,6 +195,13 @@ func (r *InstanceGroupReconciler) Reconcile(ctxt context.Context, req ctrl.Reque ctx = eksfargate.New(input) } + // for igs without any config type mentioned, allow the default to be set to launchconfig. + if r.SetDefaultConfigurationToLaunchConfig { + if input.InstanceGroup.EKSSpec.Type != v1alpha1.LaunchConfiguration && input.InstanceGroup.EKSSpec.Type != v1alpha1.LaunchTemplate { + input.InstanceGroup.EKSSpec.Type = v1alpha1.LaunchConfiguration + } + } + if err = input.InstanceGroup.Validate(); err != nil { ctx.SetState(v1alpha1.ReconcileErr) r.PatchStatus(input.InstanceGroup, statusPatch) @@ -202,10 +209,6 @@ func (r *InstanceGroupReconciler) Reconcile(ctxt context.Context, req ctrl.Reque return ctrl.Result{}, errors.Wrapf(err, "provisioner %v reconcile failed", provisionerKind) } - if r.SetScalingConfigurationToLaunchConfig { - input.InstanceGroup.Spec.EKSSpec.Type = v1alpha1.LaunchConfiguration - } - if err = HandleReconcileRequest(ctx); err != nil { ctx.SetState(v1alpha1.ReconcileErr) r.PatchStatus(input.InstanceGroup, statusPatch) diff --git a/main.go b/main.go index f2656e5b..adc93cb9 100644 --- a/main.go +++ b/main.go @@ -75,7 +75,7 @@ func main() { maxAPIRetries int configRetention int err error - setScalingConfigurationToLaunchConfig bool + setDefaultConfigurationToLaunchConfig bool ) flag.IntVar(&maxParallel, "max-workers", 5, "The number of maximum parallel reconciles") @@ -88,7 +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.BoolVar(&setScalingConfigurationToLaunchConfig, "scaling-configuration-to-launchconfig", false, "By default ASGs will have launchtemplates. Set this flag to true if launchconfigs needs to be the default.") + flag.BoolVar(&setDefaultConfigurationToLaunchConfig, "scaling-configuration-to-launchconfig", false, "By default ASGs will have launchtemplates. Set this flag to true if launchconfigs needs to be the default.") flag.Parse() ctrl.SetLogger(zap.New(zap.UseDevMode(true))) @@ -164,7 +164,7 @@ func main() { Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("instancegroup"), MaxParallel: maxParallel, - SetScalingConfigurationToLaunchConfig: setScalingConfigurationToLaunchConfig, + SetDefaultConfigurationToLaunchConfig: setDefaultConfigurationToLaunchConfig, Auth: &controllers.InstanceGroupAuthenticator{ Aws: awsWorker, Kubernetes: kube, From 54e96b692349bffd1e3a60380c41b72627a63e8f Mon Sep 17 00:00:00 2001 From: sbadiger Date: Tue, 22 Aug 2023 23:17:31 -0700 Subject: [PATCH 05/10] update spec Signed-off-by: sbadiger --- controllers/instancegroup_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/instancegroup_controller.go b/controllers/instancegroup_controller.go index 7d199e9a..f4902688 100644 --- a/controllers/instancegroup_controller.go +++ b/controllers/instancegroup_controller.go @@ -197,8 +197,8 @@ func (r *InstanceGroupReconciler) Reconcile(ctxt context.Context, req ctrl.Reque // for igs without any config type mentioned, allow the default to be set to launchconfig. if r.SetDefaultConfigurationToLaunchConfig { - if input.InstanceGroup.EKSSpec.Type != v1alpha1.LaunchConfiguration && input.InstanceGroup.EKSSpec.Type != v1alpha1.LaunchTemplate { - input.InstanceGroup.EKSSpec.Type = v1alpha1.LaunchConfiguration + if input.InstanceGroup.Spec.EKSSpec.Type != v1alpha1.LaunchConfiguration && input.InstanceGroup.Spec.EKSSpec.Type != v1alpha1.LaunchTemplate { + input.InstanceGroup.Spec.EKSSpec.Type = v1alpha1.LaunchConfiguration } } From 48e2cf0615a2f4b57a52123720c04efcba6722d9 Mon Sep 17 00:00:00 2001 From: sbadiger Date: Wed, 30 Aug 2023 13:49:25 -0700 Subject: [PATCH 06/10] refactor the default scaling config code Signed-off-by: sbadiger --- api/v1alpha1/instancegroup_types.go | 18 +++++-- api/v1alpha1/instancegroup_types_test.go | 2 +- api/v1alpha1/zz_generated.deepcopy.go | 20 ++++++++ controllers/instancegroup_controller.go | 36 ++++++-------- controllers/provisioners/eks/upgrade_test.go | 2 +- main.go | 51 ++++++++++---------- 6 files changed, 79 insertions(+), 50 deletions(-) diff --git a/api/v1alpha1/instancegroup_types.go b/api/v1alpha1/instancegroup_types.go index 9d1bdb14..b011921b 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 @@ -420,6 +429,9 @@ func (s *EKSSpec) Validate() error { if s.Type != LaunchConfiguration && s.Type != LaunchTemplate { 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 index 6b6e1be2..5a85134f 100644 --- a/api/v1alpha1/instancegroup_types_test.go +++ b/api/v1alpha1/instancegroup_types_test.go @@ -25,7 +25,7 @@ type EksUnitTest struct { } func (u *EksUnitTest) Run(t *testing.T) string { - err := u.InstanceGroup.Validate() + err := u.InstanceGroup.Validate(&ValidationOverrides{}) if err == nil { return aws.StringValue(nil) } else { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 83dce64f..576601b7 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 f4902688..76e6d86b 100644 --- a/controllers/instancegroup_controller.go +++ b/controllers/instancegroup_controller.go @@ -43,19 +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 - SetDefaultConfigurationToLaunchConfig 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 { @@ -196,13 +196,9 @@ func (r *InstanceGroupReconciler) Reconcile(ctxt context.Context, req ctrl.Reque } // for igs without any config type mentioned, allow the default to be set to launchconfig. - if r.SetDefaultConfigurationToLaunchConfig { - if input.InstanceGroup.Spec.EKSSpec.Type != v1alpha1.LaunchConfiguration && input.InstanceGroup.Spec.EKSSpec.Type != v1alpha1.LaunchTemplate { - input.InstanceGroup.Spec.EKSSpec.Type = v1alpha1.LaunchConfiguration - } - } - - if err = input.InstanceGroup.Validate(); err != nil { + 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 index 12bb7362..c5f2d213 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 diff --git a/main.go b/main.go index adc93cb9..65497fce 100644 --- a/main.go +++ b/main.go @@ -65,17 +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 - setDefaultConfigurationToLaunchConfig bool + 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") @@ -88,7 +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.BoolVar(&setDefaultConfigurationToLaunchConfig, "scaling-configuration-to-launchconfig", false, "By default ASGs will have launchtemplates. Set this flag to true if launchconfigs needs to be the default.") + flag.StringVar(&defaultScalingConfiguration, "", string(instancemgrv1alpha1.LaunchTemplate), "By default ASGs will have launchtemplates. Set this flag to true if launchconfigs needs to be the default.") flag.Parse() ctrl.SetLogger(zap.New(zap.UseDevMode(true))) @@ -151,20 +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, - SetDefaultConfigurationToLaunchConfig: setDefaultConfigurationToLaunchConfig, + 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, From 1b6fb88642c200451863dcc715ab56241a36b55e Mon Sep 17 00:00:00 2001 From: sbadiger Date: Wed, 30 Aug 2023 14:54:35 -0700 Subject: [PATCH 07/10] add unit tests Signed-off-by: sbadiger --- Makefile | 2 +- api/v1alpha1/instancegroup_types_test.go | 78 ++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 28e6b234..5e14cedb 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_test.go b/api/v1alpha1/instancegroup_types_test.go index 5a85134f..5a79390f 100644 --- a/api/v1alpha1/instancegroup_types_test.go +++ b/api/v1alpha1/instancegroup_types_test.go @@ -22,6 +22,7 @@ import ( type EksUnitTest struct { InstanceGroup *InstanceGroup + Overrides *ValidationOverrides } func (u *EksUnitTest) Run(t *testing.T) string { @@ -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-fargate", "managed", nil, nil, basicFargateSpec()), + overrides: &ValidationOverrides{ + scalingConfigurationOverride: &launchconfiguration, + }, + }, + want: LaunchConfiguration, + }, + { + name: "no default overrides", + args: args{ + instancegroup: MockInstanceGroup("eks-fargate", "managed", nil, nil, basicFargateSpec()), + overrides: &ValidationOverrides{}, + }, + want: LaunchTemplate, + }, + { + name: "override default to launchtemplate", + args: args{ + instancegroup: MockInstanceGroup("eks-fargate", "managed", nil, 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 From 0b28e6e5a17e4b609ec73888f40b4475c125f4fd Mon Sep 17 00:00:00 2001 From: sbadiger Date: Thu, 31 Aug 2023 11:32:00 -0700 Subject: [PATCH 08/10] fix unit tests Signed-off-by: sbadiger --- api/v1alpha1/instancegroup_types_test.go | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/api/v1alpha1/instancegroup_types_test.go b/api/v1alpha1/instancegroup_types_test.go index 5a79390f..82e3607e 100644 --- a/api/v1alpha1/instancegroup_types_test.go +++ b/api/v1alpha1/instancegroup_types_test.go @@ -26,7 +26,7 @@ type EksUnitTest struct { } func (u *EksUnitTest) Run(t *testing.T) string { - err := u.InstanceGroup.Validate(&ValidationOverrides{}) + err := u.InstanceGroup.Validate(u.Overrides) if err == nil { return aws.StringValue(nil) } else { @@ -480,7 +480,7 @@ func TestScalingConfigOverride(t *testing.T) { { name: "override default to launchconfig instead of launchtemplate", args: args{ - instancegroup: MockInstanceGroup("eks-fargate", "managed", nil, nil, basicFargateSpec()), + instancegroup: MockInstanceGroup("eks", "managed", MockEKSSpec(), nil, basicFargateSpec()), overrides: &ValidationOverrides{ scalingConfigurationOverride: &launchconfiguration, }, @@ -490,15 +490,15 @@ func TestScalingConfigOverride(t *testing.T) { { name: "no default overrides", args: args{ - instancegroup: MockInstanceGroup("eks-fargate", "managed", nil, nil, basicFargateSpec()), - overrides: &ValidationOverrides{}, + instancegroup: MockInstanceGroup("eks", "managed", MockEKSSpec(), nil, basicFargateSpec()), + overrides: &ValidationOverrides{}, }, want: LaunchTemplate, }, { name: "override default to launchtemplate", args: args{ - instancegroup: MockInstanceGroup("eks-fargate", "managed", nil, nil, basicFargateSpec()), + instancegroup: MockInstanceGroup("eks", "managed", MockEKSSpec(), nil, basicFargateSpec()), overrides: &ValidationOverrides{ scalingConfigurationOverride: &launchtemplate, }, @@ -585,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", + }, + } +} From 8311a89beb3ae7e05f4117ec8071ad6d4d725751 Mon Sep 17 00:00:00 2001 From: sbadiger Date: Thu, 31 Aug 2023 11:32:51 -0700 Subject: [PATCH 09/10] indent Signed-off-by: sbadiger --- api/v1alpha1/instancegroup_types_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1alpha1/instancegroup_types_test.go b/api/v1alpha1/instancegroup_types_test.go index 82e3607e..8021b6f3 100644 --- a/api/v1alpha1/instancegroup_types_test.go +++ b/api/v1alpha1/instancegroup_types_test.go @@ -491,7 +491,7 @@ func TestScalingConfigOverride(t *testing.T) { name: "no default overrides", args: args{ instancegroup: MockInstanceGroup("eks", "managed", MockEKSSpec(), nil, basicFargateSpec()), - overrides: &ValidationOverrides{}, + overrides: &ValidationOverrides{}, }, want: LaunchTemplate, }, From d823aaa7b40597e1ccfa7967c6f2307fd4e4fc26 Mon Sep 17 00:00:00 2001 From: sbadiger Date: Thu, 31 Aug 2023 11:47:47 -0700 Subject: [PATCH 10/10] update defaultScalingConfiguration flag description Signed-off-by: sbadiger --- controllers/instancegroup_controller.go | 2 +- main.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/instancegroup_controller.go b/controllers/instancegroup_controller.go index 76e6d86b..47412ce3 100644 --- a/controllers/instancegroup_controller.go +++ b/controllers/instancegroup_controller.go @@ -195,7 +195,7 @@ func (r *InstanceGroupReconciler) Reconcile(ctxt context.Context, req ctrl.Reque ctx = eksfargate.New(input) } - // for igs without any config type mentioned, allow the default to be set to launchconfig. + // for igs without any config type mentioned, allow overriding the default. overrides := v1alpha1.NewValidationOverrides(r.DefaultScalingConfiguration) if err = input.InstanceGroup.Validate(overrides); err != nil { diff --git a/main.go b/main.go index 65497fce..caa606db 100644 --- a/main.go +++ b/main.go @@ -88,7 +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 launchtemplates. Set this flag to true if launchconfigs needs to be the default.") + 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)))