From 509daead3284bd67017795bf4c5fb9d059ca91fc Mon Sep 17 00:00:00 2001 From: Vince Prignano Date: Thu, 22 Jul 2021 09:18:23 -0700 Subject: [PATCH] Align ClusterClass ControlPlane with the rest of the fields This commits introduces ControlPlaneClass and allows users to set metadata fields for all control plane objects. It also adds fields to set infrastructure reference for Machine objects and clarifies what contracts the field expects from the control plane provider. Signed-off-by: Vince Prignano --- api/v1alpha4/clusterclass_types.go | 19 +++- api/v1alpha4/clusterclass_webhook_test.go | 106 +++++++++++++----- api/v1alpha4/zz_generated.deepcopy.go | 22 ++++ .../cluster.x-k8s.io_clusterclasses.yaml | 89 +++++++++++++++ ...56-cluster-class-and-managed-topologies.md | 35 ++++-- 5 files changed, 232 insertions(+), 39 deletions(-) diff --git a/api/v1alpha4/clusterclass_types.go b/api/v1alpha4/clusterclass_types.go index 0bf6dfdc56e0..6cc34cb0ce0e 100644 --- a/api/v1alpha4/clusterclass_types.go +++ b/api/v1alpha4/clusterclass_types.go @@ -44,7 +44,7 @@ type ClusterClassSpec struct { // ControlPlane is a reference to a local struct that holds the details // for provisioning the Control Plane for the Cluster. - ControlPlane LocalObjectTemplate `json:"controlPlane,omitempty"` + ControlPlane ControlPlaneClass `json:"controlPlane,omitempty"` // Workers describes the worker nodes for the cluster. // It is a collection of node types which can be used to create @@ -53,6 +53,23 @@ type ClusterClassSpec struct { Workers WorkersClass `json:"workers,omitempty"` } +// ControlPlaneClass defines the class for the control plane. +type ControlPlaneClass struct { + Metadata ObjectMeta `json:"metadata,omitempty"` + + // LocalObjectTemplate contains the reference to the control plane provider. + LocalObjectTemplate `json:",inline"` + + // MachineTemplate defines the metadata and infrastructure information + // for control plane machines. + // + // This field is supported if and only if the control plane provider template + // referenced above is Machine based and supports setting replicas. + // + // +optional + MachineInfrastructure *LocalObjectTemplate `json:"machineInfrastructure,omitempty"` +} + // WorkersClass is a collection of deployment classes. type WorkersClass struct { // MachineDeployments is a list of machine deployment classes that can be used to create diff --git a/api/v1alpha4/clusterclass_webhook_test.go b/api/v1alpha4/clusterclass_webhook_test.go index ed3535cce536..a969ff2fc406 100644 --- a/api/v1alpha4/clusterclass_webhook_test.go +++ b/api/v1alpha4/clusterclass_webhook_test.go @@ -45,7 +45,9 @@ func TestClusterClassDefaultNamespaces(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -96,7 +98,9 @@ func TestClusterClassValidationFeatureGated(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -120,7 +124,9 @@ func TestClusterClassValidationFeatureGated(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -140,7 +146,9 @@ func TestClusterClassValidationFeatureGated(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -202,7 +210,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -233,7 +243,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: refInAnotherNamespace}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -257,7 +269,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: refInAnotherNamespace}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: refInAnotherNamespace}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -281,7 +295,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -305,7 +321,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -329,7 +347,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -360,7 +380,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -380,7 +402,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -405,7 +429,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -431,7 +457,9 @@ func TestClusterClassValidation(t *testing.T) { Name: "bazx", Namespace: "default", }}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -456,7 +484,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -477,12 +507,14 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: &corev1.ObjectReference{ - APIVersion: "foox", - Kind: "barx", - Name: "bazx", - Namespace: "default", - }}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: &corev1.ObjectReference{ + APIVersion: "foox", + Kind: "barx", + Name: "bazx", + Namespace: "default", + }}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -507,7 +539,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -528,7 +562,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -558,7 +594,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -578,7 +616,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -609,7 +649,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -629,7 +671,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -660,7 +704,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { @@ -687,7 +733,9 @@ func TestClusterClassValidation(t *testing.T) { }, Spec: ClusterClassSpec{ Infrastructure: LocalObjectTemplate{Ref: ref}, - ControlPlane: LocalObjectTemplate{Ref: ref}, + ControlPlane: ControlPlaneClass{ + LocalObjectTemplate: LocalObjectTemplate{Ref: ref}, + }, Workers: WorkersClass{ MachineDeployments: []MachineDeploymentClass{ { diff --git a/api/v1alpha4/zz_generated.deepcopy.go b/api/v1alpha4/zz_generated.deepcopy.go index 81ede96092ef..87229abce7f9 100644 --- a/api/v1alpha4/zz_generated.deepcopy.go +++ b/api/v1alpha4/zz_generated.deepcopy.go @@ -345,6 +345,28 @@ func (in Conditions) DeepCopy() Conditions { return *out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ControlPlaneClass) DeepCopyInto(out *ControlPlaneClass) { + *out = *in + in.Metadata.DeepCopyInto(&out.Metadata) + in.LocalObjectTemplate.DeepCopyInto(&out.LocalObjectTemplate) + if in.MachineInfrastructure != nil { + in, out := &in.MachineInfrastructure, &out.MachineInfrastructure + *out = new(LocalObjectTemplate) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ControlPlaneClass. +func (in *ControlPlaneClass) DeepCopy() *ControlPlaneClass { + if in == nil { + return nil + } + out := new(ControlPlaneClass) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ControlPlaneTopology) DeepCopyInto(out *ControlPlaneTopology) { *out = *in diff --git a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml index d40fefbe4a89..28df5710cb0d 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml @@ -45,6 +45,95 @@ spec: description: ControlPlane is a reference to a local struct that holds the details for provisioning the Control Plane for the Cluster. properties: + machineInfrastructure: + description: "MachineTemplate defines the metadata and infrastructure + information for control plane machines. \n This field is supported + if and only if the control plane provider template referenced + above is Machine based and supports setting replicas." + properties: + ref: + description: Ref is a required reference to a custom resource + offered by a provider. + properties: + apiVersion: + description: API version of the referent. + type: string + fieldPath: + description: 'If referring to a piece of an object instead + of an entire object, this string should contain a valid + JSON/Go field access statement, such as desiredState.manifest.containers[2]. + For example, if the object reference is to a container + within a pod, this would take on a value like: "spec.containers{name}" + (where "name" refers to the name of the container that + triggered the event) or if no container name is specified + "spec.containers[2]" (container with index 2 in this + pod). This syntax is chosen only to have some well-defined + way of referencing a part of an object. TODO: this design + is not final and this field is subject to change in + the future.' + type: string + kind: + description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + type: string + namespace: + description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/' + type: string + resourceVersion: + description: 'Specific resourceVersion to which this reference + is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency' + type: string + uid: + description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' + type: string + type: object + required: + - ref + type: object + metadata: + description: "ObjectMeta is metadata that all persisted resources + must have, which includes all objects users must create. This + is a copy of customizable fields from metav1.ObjectMeta. \n + ObjectMeta is embedded in `Machine.Spec`, `MachineDeployment.Template` + and `MachineSet.Template`, which are not top-level Kubernetes + objects. Given that metav1.ObjectMeta has lots of special cases + and read-only fields which end up in the generated CRD validation, + having it as a subset simplifies the API and some issues that + can impact user experience. \n During the [upgrade to controller-tools@v2](https://github.com/kubernetes-sigs/cluster-api/pull/1054) + for v1alpha2, we noticed a failure would occur running Cluster + API test suite against the new CRDs, specifically `spec.metadata.creationTimestamp + in body must be of type string: \"null\"`. The investigation + showed that `controller-tools@v2` behaves differently than its + previous version when handling types from [metav1](k8s.io/apimachinery/pkg/apis/meta/v1) + package. \n In more details, we found that embedded (non-top + level) types that embedded `metav1.ObjectMeta` had validation + properties, including for `creationTimestamp` (metav1.Time). + The `metav1.Time` type specifies a custom json marshaller that, + when IsZero() is true, returns `null` which breaks validation + because the field isn't marked as nullable. \n In future versions, + controller-tools@v2 might allow overriding the type and validation + for embedded types. When that happens, this hack should be revisited." + properties: + annotations: + additionalProperties: + type: string + description: 'Annotations is an unstructured key value map + stored with a resource that may be set by external tools + to store and retrieve arbitrary metadata. They are not queryable + and should be preserved when modifying objects. More info: + http://kubernetes.io/docs/user-guide/annotations' + type: object + labels: + additionalProperties: + type: string + description: 'Map of string keys and values that can be used + to organize and categorize (scope and select) objects. May + match selectors of replication controllers and services. + More info: http://kubernetes.io/docs/user-guide/labels' + type: object + type: object ref: description: Ref is a required reference to a custom resource offered by a provider. diff --git a/docs/proposals/202105256-cluster-class-and-managed-topologies.md b/docs/proposals/202105256-cluster-class-and-managed-topologies.md index 8b1f60d3724e..c2c2b3a27188 100644 --- a/docs/proposals/202105256-cluster-class-and-managed-topologies.md +++ b/docs/proposals/202105256-cluster-class-and-managed-topologies.md @@ -161,7 +161,7 @@ type ClusterClassSpec struct { // ControlPlane is a reference to a local struct that holds the details // for provisioning the Control Plane for the Cluster. - ControlPlane LocalObjectTemplate `json:"controlPlane,omitempty"` + ControlPlane ControlPlaneClass `json:"controlPlane,omitempty"` // Workers describes the worker nodes for the cluster. // It is a collection of node types which can be used to create @@ -170,6 +170,23 @@ type ClusterClassSpec struct { Workers WorkersClass `json:"workers,omitempty"` } +// ControlPlaneClass defines the class for the control plane. +type ControlPlaneClass struct { + Metadata ObjectMeta `json:"metadata,omitempty"` + + // LocalObjectTemplate contains the reference to the control plane provider. + LocalObjectTemplate `json:",inline"` + + // MachineTemplate defines the metadata and infrastructure information + // for control plane machines. + // + // This field is supported if and only if the control plane provider template + // referenced above is Machine based and supports setting replicas. + // + // +optional + MachineInfrastructure *LocalObjectTemplate `json:"machineInfrastructure,omitempty"` +} + // WorkersClass is a collection of deployment classes. type WorkersClass struct { // MachineDeployments is a list of machine deployment classes that can be used to create @@ -251,7 +268,7 @@ type LocalObjectTemplate struct { // ControlPlaneTopology specifies the parameters for the control plane nodes in the cluster. type ControlPlaneTopology struct { Metadata ObjectMeta `json:"metadata,omitempty"` - + // The number of control plane nodes. Replicas int `json:"replicas"` } @@ -264,7 +281,7 @@ type LocalObjectTemplate struct { type WorkersTopology struct { // MachineDeployments is a list of machine deployment in the cluster. MachineDeployments []MachineDeploymentTopology `json:"machineDeployments,omitempty"` - } + } ``` 1. The `MachineDeploymentTopology` object represents a single set of worker nodes of the topology. ```golang @@ -272,18 +289,18 @@ type LocalObjectTemplate struct { // This set of nodes is managed by a MachineDeployment object whose lifecycle is managed by the Cluster controller. type MachineDeploymentTopology struct { Metadata ObjectMeta `json:"metadata,omitempty"` - + // Class is the name of the MachineDeploymentClass used to create the set of worker nodes. // This should match one of the deployment classes defined in the ClusterClass object // mentioned in the `Cluster.Spec.Class` field. Class string `json:"class"` - + // Name is the unique identifier for this MachineDeploymentTopology. // The value is used with other unique identifiers to create a MachineDeployment's Name // (e.g. cluster's name, etc). In case the name is greater than the allowed maximum length, // the values are hashed together. Name string `json:"name"` - + // The number of worker nodes belonging to this set. // If the value is nil, the MachineDeployment is created without the number of Replicas (defaulting to zero) // and it's assumed that an external entity (like cluster autoscaler) is responsible for the management @@ -297,7 +314,7 @@ type LocalObjectTemplate struct { ##### ClusterClass - For object creation: - (defaulting) if namespace field is empty for a reference, default it to `metadata.Namespace` - - all the reference must be in the same namespace of `metadata.Namespace` + - all the reference must be in the same namespace of `metadata.Namespace` - `spec.workers.machineDeployments[i].class` field must be unique within a ClusterClass. - For object updates: - all the reference must be in the same namespace of `metadata.Namespace` @@ -309,9 +326,9 @@ type LocalObjectTemplate struct { - `spec.topology` and `spec.infrastructureRef` cannot be simultaneously set. - `spec.topology` and `spec.controlPlaneRef` cannot be simultaneously set. - If `spec.topology` is set, `spec.topology.class` cannot be empty. - - If `spec.topology` is set, `spec.topology.version` cannot be empty and must be a valid semver. + - If `spec.topology` is set, `spec.topology.version` cannot be empty and must be a valid semver. - `spec.topology.workers.machineDeployments[i].name` field must be unique within a Cluster - + - For object updates: - If `spec.topology.class` is set, it cannot be unset or modified. - `spec.topology.version` cannot be unset and must be a valid semver, if being updated.