Merge pull request #10337 from chrischdi/pr-fix-cc-variables

🐛 Fix ClusterClass variables status & RuntimeExtension and add test coverage

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"`
 }

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
 		}
 	}

internal/controllers/clusterclass/clusterclass_controller_test.go

@@ -117,6 +117,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()
 
@@ -132,7 +140,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 {
@@ -183,6 +191,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)
@@ -387,7 +398,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)
 
@@ -401,6 +411,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",
@@ -433,6 +451,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",
+								},
+							},
 						},
 					},
 				},
@@ -489,6 +515,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",
+							},
+						},
 					},
 				},
 			},
@@ -504,6 +538,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",
@@ -526,6 +568,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",
+								},
+							},
 						},
 					},
 				},
@@ -591,6 +641,7 @@ func TestReconciler_reconcileVariables(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(*testing.T) {
+			g := NewWithT(t)
 			fakeRuntimeClient := fakeruntimeclient.NewRuntimeClientBuilder().
 				WithCallExtensionResponses(
 					map[string]runtimehooksv1.ResponseObject{

test/extension/handlers/topologymutation/handler.go

@@ -439,6 +439,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",
+				},
+			},
 		},
 	}
 }