From 481a55aea933226b007fb78ade70fac592c2901e Mon Sep 17 00:00:00 2001 From: Shalin Patel Date: Tue, 9 Apr 2024 13:04:24 -0700 Subject: [PATCH] fix: set defaults for AWS CP and Worker instanceType --- api/v1alpha1/aws_node_types.go | 26 ++++--------------- api/v1alpha1/clusterconfig_types.go | 7 ++--- api/v1alpha1/node_types.go | 4 +-- pkg/handlers/aws/clusterconfig/variables.go | 2 +- .../aws/mutation/ami/variables_test.go | 2 +- .../variables_test.go | 2 +- .../iaminstanceprofile/variables_test.go | 2 +- .../mutation/instancetype/variables_test.go | 2 +- .../aws/mutation/network/variables_test.go | 2 +- .../aws/mutation/region/variables_test.go | 2 +- .../mutation/securitygroups/variables_test.go | 2 +- pkg/handlers/aws/workerconfig/variables.go | 2 +- .../aws/workerconfig/variables_test.go | 2 +- 13 files changed, 19 insertions(+), 38 deletions(-) diff --git a/api/v1alpha1/aws_node_types.go b/api/v1alpha1/aws_node_types.go index b4953c89a..04ee4f4b5 100644 --- a/api/v1alpha1/aws_node_types.go +++ b/api/v1alpha1/aws_node_types.go @@ -12,7 +12,7 @@ import ( const ( AWSControlPlaneInstanceType InstanceType = "m5.xlarge" - AWSWorkerInstanceType InstanceType = "m5.xlarge" + AWSWorkerInstanceType InstanceType = "m5.2xlarge" ) type AWSNodeSpec struct { @@ -57,32 +57,28 @@ func (AdditionalSecurityGroup) VariableSchema() clusterv1.VariableSchema { } func (a AWSNodeSpec) VariableSchema() clusterv1.VariableSchema { - instanceType := InstanceType("") - if a.InstanceType != nil { - instanceType = *a.InstanceType - } - return clusterv1.VariableSchema{ OpenAPIV3Schema: clusterv1.JSONSchemaProps{ Description: "AWS Node configuration", Type: "object", Properties: map[string]clusterv1.JSONSchemaProps{ "iamInstanceProfile": IAMInstanceProfile("").VariableSchema().OpenAPIV3Schema, - "instanceType": instanceType.VariableSchema().OpenAPIV3Schema, + "instanceType": a.InstanceType.VariableSchema().OpenAPIV3Schema, "ami": AMISpec{}.VariableSchema().OpenAPIV3Schema, "additionalSecurityGroups": AdditionalSecurityGroup{}.VariableSchema().OpenAPIV3Schema, }, + Required: []string{"instanceType"}, }, } } -func AWSControlPlaneNodeSpec() *AWSNodeSpec { +func NewAWSControlPlaneNodeSpec() *AWSNodeSpec { return &AWSNodeSpec{ InstanceType: ptr.To(AWSControlPlaneInstanceType), } } -func AWSWorkerNodeSpec() *AWSNodeSpec { +func NewAWSWorkerNodeSpec() *AWSNodeSpec { return &AWSNodeSpec{ InstanceType: ptr.To(AWSWorkerInstanceType), } @@ -111,18 +107,6 @@ func (i InstanceType) VariableSchema() clusterv1.VariableSchema { } } -type ControlPlaneInstanceType string - -func (ControlPlaneInstanceType) VariableSchema() clusterv1.VariableSchema { - return clusterv1.VariableSchema{ - OpenAPIV3Schema: clusterv1.JSONSchemaProps{ - Type: "string", - Description: "The AWS instance type to use for the cluster Machines", - Default: variables.MustMarshal("m5.xlarge"), - }, - } -} - type AMISpec struct { // ID is an explicit AMI to use. // +optional diff --git a/api/v1alpha1/clusterconfig_types.go b/api/v1alpha1/clusterconfig_types.go index 692611858..ed66a7a70 100644 --- a/api/v1alpha1/clusterconfig_types.go +++ b/api/v1alpha1/clusterconfig_types.go @@ -57,9 +57,6 @@ type ClusterConfigSpec struct { func (s ClusterConfigSpec) VariableSchema() clusterv1.VariableSchema { //nolint:gocritic,lll // Passed by value for no potential side-effect. clusterConfigProps := GenericClusterConfig{}.VariableSchema() - if s.ControlPlane == nil { - s.ControlPlane = &NodeConfigSpec{} - } switch { case s.AWS != nil: maps.Copy( @@ -94,11 +91,11 @@ func (s ClusterConfigSpec) VariableSchema() clusterv1.VariableSchema { //nolint: return clusterConfigProps } -func DefaultAWSClusterConfigSpec() *ClusterConfigSpec { +func NewAWSClusterConfigSpec() *ClusterConfigSpec { return &ClusterConfigSpec{ AWS: &AWSSpec{}, ControlPlane: &NodeConfigSpec{ - AWS: AWSControlPlaneNodeSpec(), + AWS: NewAWSControlPlaneNodeSpec(), }, } } diff --git a/api/v1alpha1/node_types.go b/api/v1alpha1/node_types.go index a7fe36317..7455da6cf 100644 --- a/api/v1alpha1/node_types.go +++ b/api/v1alpha1/node_types.go @@ -63,9 +63,9 @@ func (s NodeConfigSpec) VariableSchema() clusterv1.VariableSchema { return nodeConfigProps } -func DefaultAWSWorkerConfigSpec() *NodeConfigSpec { +func NewAWSWorkerConfigSpec() *NodeConfigSpec { return &NodeConfigSpec{ - AWS: AWSWorkerNodeSpec(), + AWS: NewAWSWorkerNodeSpec(), } } diff --git a/pkg/handlers/aws/clusterconfig/variables.go b/pkg/handlers/aws/clusterconfig/variables.go index af73f33d6..8a9e91dfe 100644 --- a/pkg/handlers/aws/clusterconfig/variables.go +++ b/pkg/handlers/aws/clusterconfig/variables.go @@ -43,7 +43,7 @@ func (h *awsClusterConfigVariableHandler) DiscoverVariables( resp.Variables = append(resp.Variables, clusterv1.ClusterClassVariable{ Name: clusterconfig.MetaVariableName, Required: true, - Schema: v1alpha1.DefaultAWSClusterConfigSpec().VariableSchema(), + Schema: v1alpha1.NewAWSClusterConfigSpec().VariableSchema(), }) resp.SetStatus(runtimehooksv1.ResponseStatusSuccess) } diff --git a/pkg/handlers/aws/mutation/ami/variables_test.go b/pkg/handlers/aws/mutation/ami/variables_test.go index 9bef2e4ce..8fd4cc0d6 100644 --- a/pkg/handlers/aws/mutation/ami/variables_test.go +++ b/pkg/handlers/aws/mutation/ami/variables_test.go @@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) { capitest.ValidateDiscoverVariables( t, clusterconfig.MetaVariableName, - ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()), + ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()), true, awsclusterconfig.NewVariable, capitest.VariableTestDef{ diff --git a/pkg/handlers/aws/mutation/controlplaneloadbalancer/variables_test.go b/pkg/handlers/aws/mutation/controlplaneloadbalancer/variables_test.go index 2439f8d77..f1c085b45 100644 --- a/pkg/handlers/aws/mutation/controlplaneloadbalancer/variables_test.go +++ b/pkg/handlers/aws/mutation/controlplaneloadbalancer/variables_test.go @@ -19,7 +19,7 @@ func TestVariableValidation(t *testing.T) { capitest.ValidateDiscoverVariables( t, clusterconfig.MetaVariableName, - ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()), + ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()), true, awsclusterconfig.NewVariable, capitest.VariableTestDef{ diff --git a/pkg/handlers/aws/mutation/iaminstanceprofile/variables_test.go b/pkg/handlers/aws/mutation/iaminstanceprofile/variables_test.go index e114728c8..e81535da8 100644 --- a/pkg/handlers/aws/mutation/iaminstanceprofile/variables_test.go +++ b/pkg/handlers/aws/mutation/iaminstanceprofile/variables_test.go @@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) { capitest.ValidateDiscoverVariables( t, clusterconfig.MetaVariableName, - ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()), + ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()), true, awsclusterconfig.NewVariable, capitest.VariableTestDef{ diff --git a/pkg/handlers/aws/mutation/instancetype/variables_test.go b/pkg/handlers/aws/mutation/instancetype/variables_test.go index f8070a692..0a08fa310 100644 --- a/pkg/handlers/aws/mutation/instancetype/variables_test.go +++ b/pkg/handlers/aws/mutation/instancetype/variables_test.go @@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) { capitest.ValidateDiscoverVariables( t, clusterconfig.MetaVariableName, - ptr.To(v1alpha1.DefaultAWSClusterConfigSpec().VariableSchema()), + ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()), true, awsclusterconfig.NewVariable, capitest.VariableTestDef{ diff --git a/pkg/handlers/aws/mutation/network/variables_test.go b/pkg/handlers/aws/mutation/network/variables_test.go index d64f8a1a1..581a2f83b 100644 --- a/pkg/handlers/aws/mutation/network/variables_test.go +++ b/pkg/handlers/aws/mutation/network/variables_test.go @@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) { capitest.ValidateDiscoverVariables( t, clusterconfig.MetaVariableName, - ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()), + ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()), true, awsclusterconfig.NewVariable, capitest.VariableTestDef{ diff --git a/pkg/handlers/aws/mutation/region/variables_test.go b/pkg/handlers/aws/mutation/region/variables_test.go index c7ab5e56a..38d8e3d52 100644 --- a/pkg/handlers/aws/mutation/region/variables_test.go +++ b/pkg/handlers/aws/mutation/region/variables_test.go @@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) { capitest.ValidateDiscoverVariables( t, clusterconfig.MetaVariableName, - ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()), + ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()), true, awsclusterconfig.NewVariable, capitest.VariableTestDef{ diff --git a/pkg/handlers/aws/mutation/securitygroups/variables_test.go b/pkg/handlers/aws/mutation/securitygroups/variables_test.go index 3125d5072..7eb9e20b3 100644 --- a/pkg/handlers/aws/mutation/securitygroups/variables_test.go +++ b/pkg/handlers/aws/mutation/securitygroups/variables_test.go @@ -18,7 +18,7 @@ func TestVariableValidation(t *testing.T) { capitest.ValidateDiscoverVariables( t, clusterconfig.MetaVariableName, - ptr.To(v1alpha1.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.VariableSchema()), + ptr.To(v1alpha1.NewAWSClusterConfigSpec().VariableSchema()), true, awsclusterconfig.NewVariable, capitest.VariableTestDef{ diff --git a/pkg/handlers/aws/workerconfig/variables.go b/pkg/handlers/aws/workerconfig/variables.go index 94ae07908..e2471d513 100644 --- a/pkg/handlers/aws/workerconfig/variables.go +++ b/pkg/handlers/aws/workerconfig/variables.go @@ -43,7 +43,7 @@ func (h *awsWorkerConfigVariableHandler) DiscoverVariables( resp.Variables = append(resp.Variables, clusterv1.ClusterClassVariable{ Name: workerconfig.MetaVariableName, Required: false, - Schema: v1alpha1.DefaultAWSWorkerConfigSpec().VariableSchema(), + Schema: v1alpha1.NewAWSWorkerConfigSpec().VariableSchema(), }) resp.SetStatus(runtimehooksv1.ResponseStatusSuccess) } diff --git a/pkg/handlers/aws/workerconfig/variables_test.go b/pkg/handlers/aws/workerconfig/variables_test.go index 377548cb2..b494bc430 100644 --- a/pkg/handlers/aws/workerconfig/variables_test.go +++ b/pkg/handlers/aws/workerconfig/variables_test.go @@ -17,7 +17,7 @@ func TestVariableValidation(t *testing.T) { capitest.ValidateDiscoverVariables( t, workerconfig.MetaVariableName, - ptr.To(v1alpha1.DefaultAWSWorkerConfigSpec().VariableSchema()), + ptr.To(v1alpha1.NewAWSWorkerConfigSpec().VariableSchema()), false, NewVariable, capitest.VariableTestDef{