Skip to content

Commit

Permalink
Merge pull request #10341 from chrischdi/pr-1-6-cp-10337
Browse files Browse the repository at this point in the history
[release-1.6] 🐛 Fix ClusterClass variables status & RuntimeExtension and add test coverage
  • Loading branch information
k8s-ci-robot committed Mar 28, 2024
2 parents ec3998b + 61bc954 commit 693f406
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 4 deletions.
7 changes: 7 additions & 0 deletions api/v1beta1/clusterclass_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"`
}
Expand Down
1 change: 1 addition & 0 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion api/v1beta1/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion internal/controllers/clusterclass/clusterclass_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ func addNewStatusVariable(variable clusterv1.ClusterClassVariable, from string)
{
From: from,
Required: variable.Required,
Metadata: variable.Metadata,
Schema: variable.Schema,
},
}}
Expand All @@ -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)
Expand All @@ -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
}
}
Expand Down
55 changes: 53 additions & 2 deletions internal/controllers/clusterclass/clusterclass_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)

Expand All @@ -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",
Expand Down Expand Up @@ -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",
},
},
},
},
},
Expand Down Expand Up @@ -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",
},
},
},
},
},
Expand All @@ -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",
Expand All @@ -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",
},
},
},
},
},
Expand Down Expand Up @@ -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{
Expand Down
8 changes: 8 additions & 0 deletions test/extension/handlers/topologymutation/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
},
}
}

0 comments on commit 693f406

Please sign in to comment.