Merge pull request #5928 from sbueringer/pr-md-variables

✨ ClusterClass: implement MachineDeployment variable overrides

Author: k8s-ci-robot

api/v1alpha4/conversion.go

@@ -42,6 +42,7 @@ func (src *Cluster) ConvertTo(dstRaw conversion.Hub) error {
 		if restored.Spec.Topology.Workers != nil {
 			for i := range restored.Spec.Topology.Workers.MachineDeployments {
 				dst.Spec.Topology.Workers.MachineDeployments[i].FailureDomain = restored.Spec.Topology.Workers.MachineDeployments[i].FailureDomain
+				dst.Spec.Topology.Workers.MachineDeployments[i].Variables = restored.Spec.Topology.Workers.MachineDeployments[i].Variables
 			}
 		}
 	}

api/v1alpha4/zz_generated.conversion.go

@@ -154,6 +154,10 @@ type MachineDeploymentTopology struct {
 	// of this value.
 	// +optional
 	Replicas *int32 `json:"replicas,omitempty"`
+
+	// Variables can be used to customize the MachineDeployment through patches.
+	// +optional
+	Variables *MachineDeploymentVariables `json:"variables,omitempty"`
 }
 
 // ClusterVariable can be used to customize the Cluster through
@@ -173,6 +177,13 @@ type ClusterVariable struct {
 	Value apiextensionsv1.JSON `json:"value"`
 }
 
+// MachineDeploymentVariables can be used to provide variables for a specific MachineDeployment.
+type MachineDeploymentVariables struct {
+	// Overrides can be used to override Cluster level variables.
+	// +optional
+	Overrides []ClusterVariable `json:"overrides,omitempty"`
+}
+
 // ANCHOR_END: ClusterSpec
 
 // ANCHOR: ClusterNetwork

api/v1beta1/cluster_types.go

@@ -968,6 +968,40 @@ spec:
                                 of this value.
                               format: int32
                               type: integer
+                            variables:
+                              description: Variables can be used to customize the
+                                MachineDeployment through patches.
+                              properties:
+                                overrides:
+                                  description: Overrides can be used to override Cluster
+                                    level variables.
+                                  items:
+                                    description: ClusterVariable can be used to customize
+                                      the Cluster through patches. It must comply
+                                      to the corresponding ClusterClassVariable defined
+                                      in the ClusterClass.
+                                    properties:
+                                      name:
+                                        description: Name of the variable.
+                                        type: string
+                                      value:
+                                        description: 'Value of the variable. Note:
+                                          the value will be validated against the
+                                          schema of the corresponding ClusterClassVariable
+                                          from the ClusterClass. Note: We have to
+                                          use apiextensionsv1.JSON instead of a custom
+                                          JSON type, because controller-tools has
+                                          a hard-coded schema for apiextensionsv1.JSON
+                                          which cannot be produced by another type
+                                          via controller-tools, i.e. it is not possible
+                                          to have no type field. Ref: https://github.com/kubernetes-sigs/controller-tools/blob/d0e03a142d0ecdd5491593e941ee1d6b5d91dba6/pkg/crd/known_types.go#L106-L111'
+                                        x-kubernetes-preserve-unknown-fields: true
+                                    required:
+                                    - name
+                                    - value
+                                    type: object
+                                  type: array
+                              type: object
                           required:
                           - class
                           - name

api/v1beta1/zz_generated.deepcopy.go

@@ -176,6 +176,13 @@ func ControlPlane(controlPlaneTopology *clusterv1.ControlPlaneTopology, controlP
 func MachineDeployment(mdTopology *clusterv1.MachineDeploymentTopology, md *clusterv1.MachineDeployment) (VariableMap, error) {
 	variables := VariableMap{}
 
+	// Add variables overrides for the MachineDeployment.
+	if mdTopology.Variables != nil {
+		for _, variable := range mdTopology.Variables.Overrides {
+			variables[variable.Name] = variable.Value
+		}
+	}
+
 	// Construct builtin variable.
 	builtin := Builtins{
 		MachineDeployment: &MachineDeploymentBuiltins{

config/crd/bases/cluster.x-k8s.io_clusters.yaml

@@ -49,8 +49,8 @@ func TestGlobal(t *testing.T) {
 						Value: toJSON("8"),
 					},
 					{
-						// This is blocked by a webhook, but let's make sure we overwrite
-						// the user-defined variable with the builtin variable anyway.
+						// This is blocked by a webhook, but let's make sure that the user-defined
+						// variable is overwritten by the builtin variable anyway.
 						Name:  "builtin",
 						Value: toJSON("8"),
 					},
@@ -137,6 +137,35 @@ func TestMachineDeployment(t *testing.T) {
 	}{
 		{
 			name: "Should calculate MachineDeployment variables",
+			mdTopology: &clusterv1.MachineDeploymentTopology{
+				Replicas: pointer.Int32(3),
+				Name:     "md-topology",
+				Class:    "md-class",
+				Variables: &clusterv1.MachineDeploymentVariables{
+					Overrides: []clusterv1.ClusterVariable{
+						{
+							Name:  "location",
+							Value: toJSON("\"us-central\""),
+						},
+						{
+							Name:  "cpu",
+							Value: toJSON("8"),
+						},
+					},
+				},
+			},
+			md: builder.MachineDeployment(metav1.NamespaceDefault, "md1").
+				WithReplicas(3).
+				WithVersion("v1.21.1").
+				Build(),
+			want: VariableMap{
+				"location":   toJSON("\"us-central\""),
+				"cpu":        toJSON("8"),
+				BuiltinsName: toJSON("{\"machineDeployment\":{\"version\":\"v1.21.1\",\"class\":\"md-class\",\"name\":\"md1\",\"topologyName\":\"md-topology\",\"replicas\":3}}"),
+			},
+		},
+		{
+			name: "Should calculate MachineDeployment variables (without overrides)",
 			mdTopology: &clusterv1.MachineDeploymentTopology{
 				Replicas: pointer.Int32(3),
 				Name:     "md-topology",
@@ -155,11 +184,25 @@ func TestMachineDeployment(t *testing.T) {
 			mdTopology: &clusterv1.MachineDeploymentTopology{
 				Name:  "md-topology",
 				Class: "md-class",
+				Variables: &clusterv1.MachineDeploymentVariables{
+					Overrides: []clusterv1.ClusterVariable{
+						{
+							Name:  "location",
+							Value: toJSON("\"us-central\""),
+						},
+						{
+							Name:  "cpu",
+							Value: toJSON("8"),
+						},
+					},
+				},
 			},
 			md: builder.MachineDeployment(metav1.NamespaceDefault, "md1").
 				WithVersion("v1.21.1").
 				Build(),
 			want: VariableMap{
+				"location":   toJSON("\"us-central\""),
+				"cpu":        toJSON("8"),
 				BuiltinsName: toJSON("{\"machineDeployment\":{\"version\":\"v1.21.1\",\"class\":\"md-class\",\"name\":\"md1\",\"topologyName\":\"md-topology\"}}"),
 			},
 		},

internal/controllers/topology/cluster/patches/variables/variables.go

@@ -157,9 +157,10 @@ func (c *ClusterTopologyBuilder) Build() *clusterv1.Topology {
 
 // MachineDeploymentTopologyBuilder holds the values needed to create a testable MachineDeploymentTopology.
 type MachineDeploymentTopologyBuilder struct {
-	class    string
-	name     string
-	replicas *int32
+	class     string
+	name      string
+	replicas  *int32
+	variables []clusterv1.ClusterVariable
 }
 
 // MachineDeploymentTopology returns a builder used to create a testable MachineDeploymentTopology.
@@ -181,13 +182,27 @@ func (m *MachineDeploymentTopologyBuilder) WithReplicas(replicas int32) *Machine
 	return m
 }
 
+// WithVariables adds variables used as the MachineDeploymentTopology variables value.
+func (m *MachineDeploymentTopologyBuilder) WithVariables(variables ...clusterv1.ClusterVariable) *MachineDeploymentTopologyBuilder {
+	m.variables = variables
+	return m
+}
+
 // Build returns a testable MachineDeploymentTopology with any values passed to the builder.
 func (m *MachineDeploymentTopologyBuilder) Build() clusterv1.MachineDeploymentTopology {
-	return clusterv1.MachineDeploymentTopology{
+	md := clusterv1.MachineDeploymentTopology{
 		Class:    m.class,
 		Name:     m.name,
 		Replicas: m.replicas,
 	}
+
+	if len(m.variables) > 0 {
+		md.Variables = &clusterv1.MachineDeploymentVariables{
+			Overrides: m.variables,
+		}
+	}
+
+	return md
 }
 
 // ClusterClassBuilder holds the variables and objects required to build a clusterv1.ClusterClass.

internal/controllers/topology/cluster/patches/variables/variables_test.go

@@ -29,15 +29,32 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
-// DefaultClusterVariables defaults variables which do not exist in clusterVariable, if they
-// have a default value in the corresponding schema in clusterClassVariable.
+// DefaultClusterVariables defaults ClusterVariables.
 func DefaultClusterVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, fldPath *field.Path) ([]clusterv1.ClusterVariable, field.ErrorList) {
+	return defaultClusterVariables(clusterVariables, clusterClassVariables, true, fldPath)
+}
+
+// DefaultMachineDeploymentVariables defaults MachineDeploymentVariables.
+func DefaultMachineDeploymentVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, fldPath *field.Path) ([]clusterv1.ClusterVariable, field.ErrorList) {
+	return defaultClusterVariables(clusterVariables, clusterClassVariables, false, fldPath)
+}
+
+// defaultClusterVariables defaults variables.
+// If they do not exist yet, they are created if createVariables is set.
+func defaultClusterVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, createVariables bool, fldPath *field.Path) ([]clusterv1.ClusterVariable, field.ErrorList) {
 	var allErrs field.ErrorList
 
 	// Build maps for easier and faster access.
 	clusterVariablesMap := getClusterVariablesMap(clusterVariables)
 	clusterClassVariablesMap := getClusterClassVariablesMap(clusterClassVariables)
 
+	// Validate that all variables in the Cluster are defined in the ClusterClass.
+	// Note: If we don't validate this, we would get a nil pointer dereference below.
+	allErrs = append(allErrs, validateClusterVariablesDefined(clusterVariables, clusterClassVariablesMap, fldPath)...)
+	if len(allErrs) > 0 {
+		return nil, allErrs
+	}
+
 	// allVariables is used to get a full correctly ordered list of variables.
 	allVariables := []string{}
 	// Add any ClusterVariables that already exist.
@@ -60,7 +77,7 @@ func DefaultClusterVariables(clusterVariables []clusterv1.ClusterVariable, clust
 		clusterClassVariable := clusterClassVariablesMap[variableName]
 		clusterVariable := clusterVariablesMap[variableName]
 
-		defaultedClusterVariable, errs := defaultClusterVariable(clusterVariable, clusterClassVariable, fldPath.Index(i))
+		defaultedClusterVariable, errs := defaultClusterVariable(clusterVariable, clusterClassVariable, fldPath.Index(i), createVariables)
 		if len(errs) > 0 {
 			allErrs = append(allErrs, errs...)
 			continue
@@ -84,10 +101,16 @@ func DefaultClusterVariables(clusterVariables []clusterv1.ClusterVariable, clust
 }
 
 // defaultClusterVariable defaults a clusterVariable based on the default value in the clusterClassVariable.
-func defaultClusterVariable(clusterVariable *clusterv1.ClusterVariable, clusterClassVariable *clusterv1.ClusterClassVariable, fldPath *field.Path) (*clusterv1.ClusterVariable, field.ErrorList) {
-	// Return if the variable does not exist yet and there is no top-level default value.
-	if clusterVariable == nil && clusterClassVariable.Schema.OpenAPIV3Schema.Default == nil {
-		return nil, nil
+func defaultClusterVariable(clusterVariable *clusterv1.ClusterVariable, clusterClassVariable *clusterv1.ClusterClassVariable, fldPath *field.Path, createVariable bool) (*clusterv1.ClusterVariable, field.ErrorList) {
+	if clusterVariable == nil {
+		// Return if the variable does not exist yet and createVariable is false.
+		if !createVariable {
+			return nil, nil
+		}
+		// Return if the variable does not exist yet and there is no top-level default value.
+		if clusterClassVariable.Schema.OpenAPIV3Schema.Default == nil {
+			return nil, nil
+		}
 	}
 
 	// Convert schema to Kubernetes APIExtensions schema.