fix: set defaults for AWS CP and Worker instanceType (#504)

Moved d2iq-labs#51 Addressed the pending review comments **What problem does this PR solve?:** - sets defaults for aws nodes CP: m5.xlarge and Workers: m5.2xlarge - sets PLACEHOLDER value as defaults in AWSMachineTemplates because instanceType field is required by CAPA. - sets instance type as required field in the schema since CAREN will be using the default if not provided by the user. update existing unit tests to use default schema **NOTE** Once this PR is finalized, I will create follow up PR to set instance profile and some other changes in the API types that would ensure that it creates variable schema from the object instead of creating new one. **Which issue(s) this PR fixes:** Fixes # #485 **How Has This Been Tested?:** Tried to test locally using make e2e-test E2E_LABEL='provider:AWS && cni:Cilium' but local environment timed outs during initializations. Looking into fixing local environment (colima +docker +kind on mac) Tested manually be creating AWS cluster. aws-quick-start cluster class has following ``` instanceType: default: m5.xlarge description: The AWS instance type to use for the cluster Machines type: string required: - instanceType ``` and workers ``` instanceType: default: m5.2xlarge description: The AWS instance type to use for the cluster Machines type: string required: - instanceType ``` Created AWS cluster without adding instanceType variable, CAREN patched AWSMachineTemplate with default instanceType. ** Special notes for your reviewer:** The approach taken in this PR will be used as guide to set default for other variables.
nutanix-cloud-native · Apr 15, 2024 · 64e6e72 · 64e6e72
1 parent 094f26a
commit 64e6e72
Show file tree

Hide file tree

Showing 15 changed files with 57 additions and 23 deletions.
diff --git a/api/v1alpha1/aws_node_types.go b/api/v1alpha1/aws_node_types.go
@@ -5,7 +5,15 @@ package v1alpha1
 
 import (
 	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/utils/ptr"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+
+	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables"
+)
+
+const (
+	AWSControlPlaneInstanceType InstanceType = "m5.xlarge"
+	AWSWorkerInstanceType       InstanceType = "m5.2xlarge"
 )
 
 type AWSNodeSpec struct {
@@ -24,6 +32,18 @@ type AWSNodeSpec struct {
 	AdditionalSecurityGroups AdditionalSecurityGroup `json:"additionalSecurityGroups,omitempty"`
 }
 
+func NewAWSControlPlaneNodeSpec() *AWSNodeSpec {
+	return &AWSNodeSpec{
+		InstanceType: ptr.To(AWSControlPlaneInstanceType),
+	}
+}
+
+func NewAWSWorkerNodeSpec() *AWSNodeSpec {
+	return &AWSNodeSpec{
+		InstanceType: ptr.To(AWSWorkerInstanceType),
+	}
+}
+
 type AdditionalSecurityGroup []SecurityGroup
 
 type SecurityGroup struct {
@@ -49,17 +69,18 @@ func (AdditionalSecurityGroup) VariableSchema() clusterv1.VariableSchema {
 	}
 }
 
-func (AWSNodeSpec) VariableSchema() clusterv1.VariableSchema {
+func (a AWSNodeSpec) VariableSchema() clusterv1.VariableSchema {
 	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"},
 		},
 	}
 }
@@ -77,11 +98,12 @@ func (IAMInstanceProfile) VariableSchema() clusterv1.VariableSchema {
 
 type InstanceType string
 
-func (InstanceType) VariableSchema() clusterv1.VariableSchema {
+func (i InstanceType) 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(i),
 		},
 	}
 }

diff --git a/api/v1alpha1/clusterconfig_types.go b/api/v1alpha1/clusterconfig_types.go
@@ -57,16 +57,13 @@ type ClusterConfigSpec struct {
 
 func (s ClusterConfigSpec) VariableSchema() clusterv1.VariableSchema { //nolint:gocritic,lll // Passed by value for no potential side-effect.
 	clusterConfigProps := GenericClusterConfig{}.VariableSchema()
-
 	switch {
 	case s.AWS != nil:
 		maps.Copy(
 			clusterConfigProps.OpenAPIV3Schema.Properties,
 			map[string]clusterv1.JSONSchemaProps{
-				AWSVariableName: AWSSpec{}.VariableSchema().OpenAPIV3Schema,
-				"controlPlane": NodeConfigSpec{
-					AWS: &AWSNodeSpec{},
-				}.VariableSchema().OpenAPIV3Schema,
+				AWSVariableName: s.AWS.VariableSchema().OpenAPIV3Schema,
+				"controlPlane":  s.ControlPlane.VariableSchema().OpenAPIV3Schema,
 			},
 		)
 	case s.Docker != nil:
@@ -94,6 +91,15 @@ func (s ClusterConfigSpec) VariableSchema() clusterv1.VariableSchema { //nolint:
 	return clusterConfigProps
 }
 
+func NewAWSClusterConfigSpec() *ClusterConfigSpec {
+	return &ClusterConfigSpec{
+		AWS: &AWSSpec{},
+		ControlPlane: &NodeConfigSpec{
+			AWS: NewAWSControlPlaneNodeSpec(),
+		},
+	}
+}
+
 // GenericClusterConfig defines the generic cluster configdesired.
 type GenericClusterConfig struct {
 	// +optional

diff --git a/api/v1alpha1/node_types.go b/api/v1alpha1/node_types.go
@@ -41,7 +41,7 @@ func (s NodeConfigSpec) VariableSchema() clusterv1.VariableSchema {
 		maps.Copy(
 			nodeConfigProps.OpenAPIV3Schema.Properties,
 			map[string]clusterv1.JSONSchemaProps{
-				AWSVariableName: AWSNodeSpec{}.VariableSchema().OpenAPIV3Schema,
+				AWSVariableName: s.AWS.VariableSchema().OpenAPIV3Schema,
 			},
 		)
 	case s.Docker != nil:
@@ -63,6 +63,12 @@ func (s NodeConfigSpec) VariableSchema() clusterv1.VariableSchema {
 	return nodeConfigProps
 }
 
+func NewAWSWorkerConfigSpec() *NodeConfigSpec {
+	return &NodeConfigSpec{
+		AWS: NewAWSWorkerNodeSpec(),
+	}
+}
+
 type GenericNodeConfig struct{}
 
 func (GenericNodeConfig) VariableSchema() clusterv1.VariableSchema {

diff --git a/charts/cluster-api-runtime-extensions-nutanix/defaultclusterclasses/aws-cluster-class.yaml b/charts/cluster-api-runtime-extensions-nutanix/defaultclusterclasses/aws-cluster-class.yaml
@@ -106,7 +106,7 @@ spec:
   template:
     spec:
       iamInstanceProfile: control-plane.cluster-api-provider-aws.sigs.k8s.io
-      instanceType: m5.xlarge
+      instanceType: PLACEHOLDER
       sshKeyName: ""
 ---
 apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
@@ -119,7 +119,7 @@ spec:
   template:
     spec:
       iamInstanceProfile: nodes.cluster-api-provider-aws.sigs.k8s.io
-      instanceType: m5.2xlarge
+      instanceType: PLACEHOLDER
       sshKeyName: ""
 ---
 apiVersion: bootstrap.cluster.x-k8s.io/v1beta1

diff --git a/hack/examples/overlays/clusterclasses/aws/kustomization.yaml.tmpl b/hack/examples/overlays/clusterclasses/aws/kustomization.yaml.tmpl
@@ -51,11 +51,11 @@ patches:
     patch: |-
       - op: "add"
         path: "/spec/template/spec/instanceType"
-        value: "m5.2xlarge"
+        value: "PLACEHOLDER"
   - target:
       kind: AWSMachineTemplate
       name: quick-start-control-plane
     patch: |-
       - op: "add"
         path: "/spec/template/spec/instanceType"
-        value: "m5.xlarge"
+        value: "PLACEHOLDER"
diff --git 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.ClusterConfigSpec{AWS: &v1alpha1.AWSSpec{}}.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
@@ -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
@@ -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
@@ -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
@@ -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/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
@@ -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
@@ -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
@@ -43,7 +43,7 @@ func (h *awsWorkerConfigVariableHandler) DiscoverVariables(
 	resp.Variables = append(resp.Variables, clusterv1.ClusterClassVariable{
 		Name:     workerconfig.MetaVariableName,
 		Required: false,
-		Schema:   v1alpha1.NodeConfigSpec{AWS: &v1alpha1.AWSNodeSpec{}}.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
@@ -17,7 +17,7 @@ func TestVariableValidation(t *testing.T) {
 	capitest.ValidateDiscoverVariables(
 		t,
 		workerconfig.MetaVariableName,
-		ptr.To(v1alpha1.NodeConfigSpec{AWS: &v1alpha1.AWSNodeSpec{}}.VariableSchema()),
+		ptr.To(v1alpha1.NewAWSWorkerConfigSpec().VariableSchema()),
 		false,
 		NewVariable,
 		capitest.VariableTestDef{