From 61bc954ffe67898081478ba77b20383c60d0c4a6 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Tue, 26 Mar 2024 15:18:53 +0100 Subject: [PATCH] Fix ClusterClass variables status & RuntimeExtension and add test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Stefan Büringer buringerst@vmware.com --- api/v1beta1/clusterclass_types.go | 7 +++ api/v1beta1/zz_generated.deepcopy.go | 1 + api/v1beta1/zz_generated.openapi.go | 9 ++- .../cluster.x-k8s.io_clusterclasses.yaml | 20 +++++++ .../clusterclass/clusterclass_controller.go | 4 +- .../clusterclass_controller_test.go | 55 ++++++++++++++++++- .../handlers/topologymutation/handler.go | 8 +++ 7 files changed, 100 insertions(+), 4 deletions(-) diff --git a/api/v1beta1/clusterclass_types.go b/api/v1beta1/clusterclass_types.go index e2733d287c1d..28156e2c2e2f 100644 --- a/api/v1beta1/clusterclass_types.go +++ b/api/v1beta1/clusterclass_types.go @@ -385,6 +385,7 @@ type ClusterClassVariable struct { // Metadata is the metadata of a variable. // It can be used to add additional data for higher level tools to // a ClusterClassVariable. + // +optional Metadata ClusterClassVariableMetadata `json:"metadata,omitempty"` // Schema defines the schema of the variable. @@ -754,6 +755,12 @@ type ClusterClassStatusVariableDefinition struct { // required, this will be specified inside the schema. Required bool `json:"required"` + // Metadata is the metadata of a variable. + // It can be used to add additional data for higher level tools to + // a ClusterClassVariable. + // +optional + Metadata ClusterClassVariableMetadata `json:"metadata,omitempty"` + // Schema defines the schema of the variable. Schema VariableSchema `json:"schema"` } diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index cb2844d15497..cd8e2982e3d9 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -273,6 +273,7 @@ func (in *ClusterClassStatusVariable) DeepCopy() *ClusterClassStatusVariable { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterClassStatusVariableDefinition) DeepCopyInto(out *ClusterClassStatusVariableDefinition) { *out = *in + in.Metadata.DeepCopyInto(&out.Metadata) in.Schema.DeepCopyInto(&out.Schema) } diff --git a/api/v1beta1/zz_generated.openapi.go b/api/v1beta1/zz_generated.openapi.go index 4e0b14696f35..b03bc1ff81d9 100644 --- a/api/v1beta1/zz_generated.openapi.go +++ b/api/v1beta1/zz_generated.openapi.go @@ -546,6 +546,13 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_ClusterClassStatusVariableDefiniti Format: "", }, }, + "metadata": { + SchemaProps: spec.SchemaProps{ + Description: "Metadata is the metadata of a variable. It can be used to add additional data for higher level tools to a ClusterClassVariable.", + Default: map[string]interface{}{}, + Ref: ref("sigs.k8s.io/cluster-api/api/v1beta1.ClusterClassVariableMetadata"), + }, + }, "schema": { SchemaProps: spec.SchemaProps{ Description: "Schema defines the schema of the variable.", @@ -558,7 +565,7 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_ClusterClassStatusVariableDefiniti }, }, Dependencies: []string{ - "sigs.k8s.io/cluster-api/api/v1beta1.VariableSchema"}, + "sigs.k8s.io/cluster-api/api/v1beta1.ClusterClassVariableMetadata", "sigs.k8s.io/cluster-api/api/v1beta1.VariableSchema"}, } } diff --git a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml index 940a47cb961c..c34fb8ec2aaf 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml @@ -1758,6 +1758,26 @@ spec: the ClusterClass for variables discovered from a DiscoverVariables runtime extensions. type: string + metadata: + description: Metadata is the metadata of a variable. It + can be used to add additional data for higher level + tools to a ClusterClassVariable. + properties: + annotations: + additionalProperties: + type: string + description: Annotations is an unstructured key value + map that can be used to store and retrieve arbitrary + metadata. They are not queryable. + type: object + labels: + additionalProperties: + type: string + description: Map of string keys and values that can + be used to organize and categorize (scope and select) + variables. + type: object + type: object required: description: 'Required specifies if the variable is required. Note: this applies to the variable as a whole and thus diff --git a/internal/controllers/clusterclass/clusterclass_controller.go b/internal/controllers/clusterclass/clusterclass_controller.go index a2823ae6a9fb..e18c7bb0d383 100644 --- a/internal/controllers/clusterclass/clusterclass_controller.go +++ b/internal/controllers/clusterclass/clusterclass_controller.go @@ -325,6 +325,7 @@ func addNewStatusVariable(variable clusterv1.ClusterClassVariable, from string) { From: from, Required: variable.Required, + Metadata: variable.Metadata, Schema: variable.Schema, }, }} @@ -335,6 +336,7 @@ func addDefinitionToExistingStatusVariable(variable clusterv1.ClusterClassVariab newVariableDefinition := clusterv1.ClusterClassStatusVariableDefinition{ From: from, Required: variable.Required, + Metadata: variable.Metadata, Schema: variable.Schema, } combinedVariable.Definitions = append(existingVariable.Definitions, newVariableDefinition) @@ -343,7 +345,7 @@ func addDefinitionToExistingStatusVariable(variable clusterv1.ClusterClassVariab // If definitions already conflict, no need to check. if !combinedVariable.DefinitionsConflict { currentDefinition := combinedVariable.Definitions[0] - if !(currentDefinition.Required == newVariableDefinition.Required && reflect.DeepEqual(currentDefinition.Schema, newVariableDefinition.Schema)) { + if !(currentDefinition.Required == newVariableDefinition.Required && reflect.DeepEqual(currentDefinition.Schema, newVariableDefinition.Schema) && reflect.DeepEqual(currentDefinition.Metadata, newVariableDefinition.Metadata)) { combinedVariable.DefinitionsConflict = true } } diff --git a/internal/controllers/clusterclass/clusterclass_controller_test.go b/internal/controllers/clusterclass/clusterclass_controller_test.go index 1c972f1ab1f2..131e842ad04e 100644 --- a/internal/controllers/clusterclass/clusterclass_controller_test.go +++ b/internal/controllers/clusterclass/clusterclass_controller_test.go @@ -105,6 +105,14 @@ func TestClusterClassReconciler_reconcile(t *testing.T) { Type: "integer", }, }, + Metadata: clusterv1.ClusterClassVariableMetadata{ + Labels: map[string]string{ + "some-label": "some-label-value", + }, + Annotations: map[string]string{ + "some-annotation": "some-annotation-value", + }, + }, }). Build() @@ -119,7 +127,7 @@ func TestClusterClassReconciler_reconcile(t *testing.T) { } for _, obj := range initObjs { - g.Expect(env.Create(ctx, obj)).To(Succeed()) + g.Expect(env.CreateAndWait(ctx, obj)).To(Succeed()) } defer func() { for _, obj := range initObjs { @@ -168,6 +176,9 @@ func assertStatusVariables(actualClusterClass *clusterv1.ClusterClass) error { if !reflect.DeepEqual(specVar.Schema, statusVarDefinition.Schema) { return errors.Errorf("ClusterClass status variable %s schema does not match. Expected %v. Got %v", specVar.Name, specVar.Schema, statusVarDefinition.Schema) } + if !reflect.DeepEqual(specVar.Metadata, statusVarDefinition.Metadata) { + return errors.Errorf("ClusterClass status variable %s metadata does not match. Expected %v. Got %v", specVar.Name, specVar.Metadata, statusVarDefinition.Metadata) + } } if !found { return errors.Errorf("ClusterClass does not have status for variable %s", specVar.Name) @@ -323,7 +334,6 @@ func isOwnerReferenceEqual(a, b metav1.OwnerReference) bool { func TestReconciler_reconcileVariables(t *testing.T) { defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.RuntimeSDK, true)() - g := NewWithT(t) catalog := runtimecatalog.New() _ = runtimehooksv1.AddToCatalog(catalog) @@ -337,6 +347,14 @@ func TestReconciler_reconcileVariables(t *testing.T) { Type: "integer", }, }, + Metadata: clusterv1.ClusterClassVariableMetadata{ + Labels: map[string]string{ + "some-label": "some-label-value", + }, + Annotations: map[string]string{ + "some-annotation": "some-annotation-value", + }, + }, }, { Name: "memory", @@ -369,6 +387,14 @@ func TestReconciler_reconcileVariables(t *testing.T) { Type: "integer", }, }, + Metadata: clusterv1.ClusterClassVariableMetadata{ + Labels: map[string]string{ + "some-label": "some-label-value", + }, + Annotations: map[string]string{ + "some-annotation": "some-annotation-value", + }, + }, }, }, }, @@ -425,6 +451,14 @@ func TestReconciler_reconcileVariables(t *testing.T) { Type: "string", }, }, + Metadata: clusterv1.ClusterClassVariableMetadata{ + Labels: map[string]string{ + "some-label": "some-label-value", + }, + Annotations: map[string]string{ + "some-annotation": "some-annotation-value", + }, + }, }, }, }, @@ -440,6 +474,14 @@ func TestReconciler_reconcileVariables(t *testing.T) { Type: "integer", }, }, + Metadata: clusterv1.ClusterClassVariableMetadata{ + Labels: map[string]string{ + "some-label": "some-label-value", + }, + Annotations: map[string]string{ + "some-annotation": "some-annotation-value", + }, + }, }, { From: "patch1", @@ -462,6 +504,14 @@ func TestReconciler_reconcileVariables(t *testing.T) { Type: "string", }, }, + Metadata: clusterv1.ClusterClassVariableMetadata{ + Labels: map[string]string{ + "some-label": "some-label-value", + }, + Annotations: map[string]string{ + "some-annotation": "some-annotation-value", + }, + }, }, }, }, @@ -527,6 +577,7 @@ func TestReconciler_reconcileVariables(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) fakeRuntimeClient := fakeruntimeclient.NewRuntimeClientBuilder(). WithCallExtensionResponses( map[string]runtimehooksv1.ResponseObject{ diff --git a/test/extension/handlers/topologymutation/handler.go b/test/extension/handlers/topologymutation/handler.go index 1ccf83c869dd..631ae3040b3a 100644 --- a/test/extension/handlers/topologymutation/handler.go +++ b/test/extension/handlers/topologymutation/handler.go @@ -432,6 +432,14 @@ func (h *ExtensionHandlers) DiscoverVariables(ctx context.Context, _ *runtimehoo Example: &apiextensionsv1.JSON{Raw: []byte(`"kindest"`)}, }, }, + Metadata: clusterv1.ClusterClassVariableMetadata{ + Labels: map[string]string{ + "objects": "DockerCluster", + }, + Annotations: map[string]string{ + "description": "Gets set at DockerCluster.Spec.Template.Spec.LoadBalancer.ImageRepository", + }, + }, }, } }