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.go b/api/v1alpha1/instancegroup_types.go index 9eaee389..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 @@ -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 index 6b6e1be2..8021b6f3 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/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 881a2f38..47412ce3 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/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 bdc4f38c..caa606db 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,