From 22be39abb244dfe26ff6021369f66d2bd03d66fd Mon Sep 17 00:00:00 2001 From: Shalin Patel Date: Fri, 5 Apr 2024 13:02:51 -0700 Subject: [PATCH 1/2] fix: set defaults for AWS CP and Worker instanceType --- api/v1alpha1/aws_node_types.go | 28 +++++++++++++++++-- api/v1alpha1/clusterconfig_types.go | 16 +++++++---- api/v1alpha1/node_types.go | 8 +++++- .../aws-cluster-class.yaml | 4 +-- .../aws/kustomization.yaml.tmpl | 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 +- 15 files changed, 57 insertions(+), 23 deletions(-) diff --git a/api/v1alpha1/aws_node_types.go b/api/v1alpha1/aws_node_types.go index 82009af18..393679111 100644 --- 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/d2iq-labs/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(string(i)), }, } } diff --git a/api/v1alpha1/clusterconfig_types.go b/api/v1alpha1/clusterconfig_types.go index b5863bd06..e0bd7882d 100644 --- 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 index f2d3c6bf3..7455da6cf 100644 --- 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 index ca25ad7bd..5d2f6650c 100644 --- 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 index aed3dfcb1..07363aeab 100644 --- 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 index cf423de25..e80b72282 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.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 index a30fa3dc2..63eb78802 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 fda5a069c..059f4f0ed 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 d63969340..f5aea0919 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 983899b4a..040ca4384 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.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 index aab5ed024..2aa82b7a2 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 6ff7926ea..295fb3f12 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 0ebb296e2..27e634de3 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 518be1553..3b0975211 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.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 index b9252a9e1..8bd661665 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.NodeConfigSpec{AWS: &v1alpha1.AWSNodeSpec{}}.VariableSchema()), + ptr.To(v1alpha1.NewAWSWorkerConfigSpec().VariableSchema()), false, NewVariable, capitest.VariableTestDef{ From bce383ca31dad21931f16172b60ea5862dbaeaaa Mon Sep 17 00:00:00 2001 From: Shalin Patel Date: Thu, 11 Apr 2024 12:59:47 -0700 Subject: [PATCH 2/2] fix: marshal instance type using generic function --- api/v1alpha1/aws_node_types.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/aws_node_types.go b/api/v1alpha1/aws_node_types.go index 393679111..81fc41ac2 100644 --- a/api/v1alpha1/aws_node_types.go +++ b/api/v1alpha1/aws_node_types.go @@ -8,7 +8,7 @@ import ( "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "github.com/d2iq-labs/cluster-api-runtime-extensions-nutanix/api/variables" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables" ) const ( @@ -103,7 +103,7 @@ func (i InstanceType) VariableSchema() clusterv1.VariableSchema { OpenAPIV3Schema: clusterv1.JSONSchemaProps{ Type: "string", Description: "The AWS instance type to use for the cluster Machines", - Default: variables.MustMarshal(string(i)), + Default: variables.MustMarshal(i), }, } }