Skip to content

Commit 0f4ce8a

Browse files
authored
fix: skip syncing controlplane's infrastructure templates for EKS Clusterclass (#1366)
**What problem does this PR solve?**: This bug was found when integrating EKS provider with NKP UI. The Managed Kubernetes providers like EKS and AKS will not have controlplane infrastructure provider reference. This PR copies controlplane infrastructure provider template only if they exists. Tested with unit tests: Before fix the test failed with: ``` Starting the test environment manager I1027 17:01:27.066340 8612 controller.go:204] "Starting EventSource" controller="syncclusterclass" controllerGroup="" controllerKind="Namespace" source="kind source: *v1beta1.ClusterClass" I1027 17:01:27.066340 8612 controller.go:204] "Starting EventSource" controller="syncclusterclass" controllerGroup="" controllerKind="Namespace" source="kind source: *v1.Namespace" I1027 17:01:27.169218 8612 controller.go:239] "Starting Controller" controller="syncclusterclass" controllerGroup="" controllerKind="Namespace" I1027 17:01:27.169236 8612 controller.go:248] "Starting workers" controller="syncclusterclass" controllerGroup="" controllerKind="Namespace" worker count=1 E1027 17:01:29.548837 8612 controller.go:347] "Reconciler error" err="failed to copy source ClusterClass source/test-5r8wr or its referenced Templates to namespace target-kx6ls: error processing references: failed to get reference: failed to get GenericBootstrapConfigTemplate test-5r8wr/source: failed to retrieve GenericBootstrapConfigTemplate source/test-5r8wr: GenericBootstrapConfigTemplate.bootstrap.cluster.x-k8s.io \"test-5r8wr\" not found" controller="syncclusterclass" controllerGroup="" controllerKind="Namespace" Namespace="target-kx6ls" namespace="" name="target-kx6ls" reconcileID="8ca531d5-2634-4a4c-9b96-fe34a9245399" E1027 17:01:29.554044 8612 controller.go:347] "Reconciler error" err="failed to copy source ClusterClass source/test-5r8wr or its referenced Templates to namespace target-kx6ls: error processing references: failed to get reference: failed to get GenericInfrastructureClusterTemplate test-5r8wr/source: failed to retrieve GenericInfrastructureClusterTemplate source/test-5r8wr: GenericInfrastructureClusterTemplate.infrastructure.cluster.x-k8s.io \"test-5r8wr\" not found" controller="syncclusterclass" controllerGroup="" controllerKind="Namespace" Namespace="target-kx6ls" namespace="" name="target-kx6ls" reconcileID="840dc3e9-594e-43f1-83c4-ab549d8c11f1" E1027 17:01:29.562716 8612 signal_unix.go:925] "Observed a panic" controller="syncclusterclass" controllerGroup="" controllerKind="Namespace" Namespace="target-87wdz" namespace="" name="target-87wdz" reconcileID="fb7cbaa9-cf65-4cce-87b3-d773403de9d2" panic="runtime error: invalid memory address or nil pointer dereference" panicGoValue="\"invalid memory address or nil pointer dereference\"" stacktrace=< goroutine 298 [running]: k8s.io/apimachinery/pkg/util/runtime.logPanic({0x10299b1d0, 0x1400099bb00}, {0x1026c5800, 0x103940090}) /Users/shalinpatel/go/pkg/mod/k8s.io/apimachinery@v0.32.9/pkg/util/runtime/runtime.go:107 +0x94 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Reconcile.func1() /Users/shalinpatel/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.20.4/pkg/internal/controller/controller.go:108 +0xdc panic({0x1026c5800?, 0x103940090?}) /Users/shalinpatel/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.1.darwin-arm64/src/runtime/panic.go:783 +0x120 github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/controllers/namespacesync.walkReferences({0x10299b1d0, 0x1400099bb00}, 0x140009e6008, 0x14000ff3aa0) /Users/shalinpatel/gitrepos/cluster-api-runtime-extensions-nutanix/pkg/controllers/namespacesync/references.go:63 +0x44 github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/controllers/namespacesync.copyClusterClassAndTemplates({0x10299b1d0, 0x1400099bb00}, {0x14b31e140, 0x14001082f30}, {0x14d490230, 0x14001083a70}, 0x14000ff3b38?, {0x14001100340, 0xc}) /Users/shalinpatel/gitrepos/cluster-api-runtime-extensions-nutanix/pkg/controllers/namespacesync/copy.go:25 +0xb4 github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/controllers/namespacesync.(*Reconciler).Reconcile(0x14000e53800, {0x10299b1d0, 0x1400099bb00}, {{{0x0?, 0x14000ff3c38?}, {0x14001100340?, 0x14000ff3c58?}}}) /Users/shalinpatel/gitrepos/cluster-api-runtime-extensions-nutanix/pkg/controllers/namespacesync/controller.go:134 +0x134 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Reconcile(0x140005e1240?, {0x10299b1d0?, 0x1400099bb00?}, {{{0x0?, 0x0?}, {0x14001100340?, 0x0?}}}) /Users/shalinpatel/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.20.4/pkg/internal/controller/controller.go:119 +0x8c sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).reconcileHandler(0x1029b3160, {0x10299b208, 0x14000035270}, {{{0x0, 0x0}, {0x14001100340, 0xc}}}, 0x0) /Users/shalinpatel/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.20.4/pkg/internal/controller/controller.go:334 +0x280 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).processNextWorkItem(0x1029b3160, {0x10299b208, 0x14000035270}) /Users/shalinpatel/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.20.4/pkg/internal/controller/controller.go:294 +0x16c sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Start.func2.2() /Users/shalinpatel/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.20.4/pkg/internal/controller/controller.go:255 +0x74 created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Start.func2 in goroutine 313 /Users/shalinpatel/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.20.4/pkg/internal/controller/controller.go:251 +0x4e4 ``` **Which issue(s) this PR fixes**: Fixes # **How Has This Been Tested?**: <!-- Please describe the tests that you ran to verify your changes. Provide output from the tests and any manual steps needed to replicate the tests. --> **Special notes for your reviewer**: <!-- Use this to provide any additional information to the reviewers. This may include: - Best way to review the PR. - Where the author wants the most review attention on. - etc. -->
1 parent b9bb371 commit 0f4ce8a

File tree

3 files changed

+319
-29
lines changed

3 files changed

+319
-29
lines changed

pkg/controllers/namespacesync/controller_test.go

Lines changed: 116 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -85,24 +85,40 @@ func TestReconcileNewClusterClass(t *testing.T) {
8585
targetNamespaces, err := createTargetNamespaces(3)
8686
g.Expect(err).ToNot(HaveOccurred())
8787

88-
sourceClusterClassName, _, cleanup, err := createUniqueClusterClassAndTemplates(
89-
sourceClusterClassNamespace,
90-
)
91-
g.Expect(err).ToNot(HaveOccurred())
92-
defer func() {
93-
g.Expect(cleanup()).To(Succeed())
94-
}()
95-
96-
for _, targetNamespace := range targetNamespaces {
97-
g.Eventually(func() error {
98-
return verifyClusterClassAndTemplates(
99-
env.Client,
100-
sourceClusterClassName,
101-
targetNamespace.Name,
102-
)
88+
testCases := []struct {
89+
name string
90+
create func(namespace string) (string, []client.Object, func() error, error)
91+
}{
92+
{
93+
name: "cluster class",
94+
create: createUniqueClusterClassAndTemplates,
10395
},
104-
timeout,
105-
).Should(Succeed())
96+
{
97+
name: "managed cluster class",
98+
create: createUniqueManagedClusterClassAndTemplates,
99+
},
100+
}
101+
102+
for _, tc := range testCases {
103+
t.Run(tc.name, func(t *testing.T) {
104+
g := NewWithT(t)
105+
sourceClusterClassName, _, cleanup, err := tc.create(sourceClusterClassNamespace)
106+
g.Expect(err).ToNot(HaveOccurred())
107+
defer func() {
108+
g.Expect(cleanup()).To(Succeed())
109+
}()
110+
for _, targetNamespace := range targetNamespaces {
111+
g.Eventually(func() error {
112+
return verifyClusterClassAndTemplates(
113+
env.Client,
114+
sourceClusterClassName,
115+
targetNamespace.Name,
116+
)
117+
},
118+
timeout,
119+
).Should(Succeed())
120+
}
121+
})
106122
}
107123
}
108124

@@ -292,6 +308,89 @@ func createClusterClassAndTemplates(
292308
return clusterClass.Name, templates, cleanup, nil
293309
}
294310

311+
func createUniqueManagedClusterClassAndTemplates(namespace string) (
312+
clusterClassName string,
313+
templates []client.Object,
314+
cleanup func() error,
315+
err error,
316+
) {
317+
return createManagedClusterClassAndTemplates(
318+
names.SimpleNameGenerator.GenerateName("test-managed-"),
319+
namespace,
320+
)
321+
}
322+
323+
// createManagedClusterClassAndTemplates creates a ClusterClass with a ControlPlane that does not have a MachineInfrastructure reference.
324+
func createManagedClusterClassAndTemplates(
325+
prefix,
326+
namespace string,
327+
) (
328+
clusterClassName string,
329+
templates []client.Object,
330+
cleanup func() error,
331+
err error,
332+
) {
333+
// The below objects are created in order to feed the reconcile loop all the information it needs to create a
334+
// full tree of ClusterClass objects (the objects should have owner references to the ClusterClass).
335+
336+
// Bootstrap templates for the workers.
337+
bootstrapTemplate := builder.BootstrapTemplate(namespace, prefix).Build()
338+
339+
// InfraMachineTemplates for the workers
340+
infraMachineTemplateWorker := builder.InfrastructureMachineTemplate(
341+
namespace,
342+
fmt.Sprintf("%s-worker", prefix),
343+
).Build()
344+
345+
// Control plane template.
346+
controlPlaneTemplate := builder.ControlPlaneTemplate(namespace, prefix).Build()
347+
348+
// InfraClusterTemplate.
349+
infraClusterTemplate := builder.InfrastructureClusterTemplate(namespace, prefix).Build()
350+
351+
// MachineDeploymentClasses that will be part of the ClusterClass.
352+
machineDeploymentClass := builder.MachineDeploymentClass(fmt.Sprintf("%s-worker", prefix)).
353+
WithBootstrapTemplate(bootstrapTemplate).
354+
WithInfrastructureTemplate(infraMachineTemplateWorker).
355+
Build()
356+
357+
// ClusterClass.
358+
clusterClass := builder.ClusterClass(namespace, prefix).
359+
WithInfrastructureClusterTemplate(infraClusterTemplate).
360+
WithControlPlaneTemplate(controlPlaneTemplate).
361+
WithWorkerMachineDeploymentClasses(*machineDeploymentClass).
362+
Build()
363+
364+
// Create the set of initObjects from the objects above to add to the API server when the test environment starts.
365+
366+
templates = []client.Object{
367+
bootstrapTemplate,
368+
infraMachineTemplateWorker,
369+
controlPlaneTemplate,
370+
infraClusterTemplate,
371+
}
372+
373+
for _, obj := range templates {
374+
if err := env.CreateAndWait(ctx, obj); err != nil {
375+
return "", nil, nil, err
376+
}
377+
}
378+
if err := env.CreateAndWait(ctx, clusterClass); err != nil {
379+
return "", nil, nil, err
380+
}
381+
382+
cleanup = func() error {
383+
for _, obj := range templates {
384+
if err := env.CleanupAndWait(ctx, obj); err != nil {
385+
return err
386+
}
387+
}
388+
return env.CleanupAndWait(ctx, clusterClass)
389+
}
390+
391+
return clusterClass.Name, templates, cleanup, nil
392+
}
393+
295394
func createTargetNamespaces(number int) ([]*corev1.Namespace, error) {
296395
targetNamespaces := []*corev1.Namespace{}
297396
for i := 0; i < number; i++ {

pkg/controllers/namespacesync/references.go

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,24 +57,36 @@ func walkReferences(
5757
ref *corev1.ObjectReference,
5858
) error,
5959
) error {
60-
for _, ref := range []*corev1.ObjectReference{
61-
cc.Spec.Infrastructure.Ref,
62-
cc.Spec.ControlPlane.Ref,
63-
cc.Spec.ControlPlane.MachineInfrastructure.Ref,
64-
} {
65-
if err := fn(ctx, ref); err != nil {
60+
if cc == nil {
61+
return nil
62+
}
63+
if cc.Spec.Infrastructure.Ref != nil {
64+
if err := fn(ctx, cc.Spec.Infrastructure.Ref); err != nil {
65+
return err
66+
}
67+
}
68+
69+
if cc.Spec.ControlPlane.Ref != nil {
70+
if err := fn(ctx, cc.Spec.ControlPlane.Ref); err != nil {
71+
return err
72+
}
73+
}
74+
75+
if cpInfra := cc.Spec.ControlPlane.MachineInfrastructure; cpInfra != nil && cpInfra.Ref != nil {
76+
if err := fn(ctx, cpInfra.Ref); err != nil {
6677
return err
6778
}
6879
}
6980

7081
for mdIdx := range cc.Spec.Workers.MachineDeployments {
7182
md := &cc.Spec.Workers.MachineDeployments[mdIdx]
72-
73-
for _, ref := range []*corev1.ObjectReference{
74-
md.Template.Infrastructure.Ref,
75-
md.Template.Bootstrap.Ref,
76-
} {
77-
if err := fn(ctx, ref); err != nil {
83+
if md.Template.Infrastructure.Ref != nil {
84+
if err := fn(ctx, md.Template.Infrastructure.Ref); err != nil {
85+
return err
86+
}
87+
}
88+
if md.Template.Bootstrap.Ref != nil {
89+
if err := fn(ctx, md.Template.Bootstrap.Ref); err != nil {
7890
return err
7991
}
8092
}
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
// Copyright 2024 Nutanix. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package namespacesync
5+
6+
import (
7+
"context"
8+
"testing"
9+
10+
. "github.com/onsi/gomega"
11+
corev1 "k8s.io/api/core/v1"
12+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
13+
14+
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/internal/test/builder"
15+
)
16+
17+
func TestWalkReferences(t *testing.T) {
18+
tests := []struct {
19+
name string
20+
clusterClass *clusterv1.ClusterClass
21+
}{
22+
{
23+
name: "nil ClusterClass should return nil without calling callback",
24+
clusterClass: nil,
25+
},
26+
{
27+
name: "empty ClusterClass with no template references",
28+
clusterClass: builder.ClusterClass("default", "test-cc").Build(),
29+
},
30+
{
31+
name: "ClusterClass with Infrastructure cluster template reference only",
32+
clusterClass: builder.ClusterClass("default", "test-cc").
33+
WithInfrastructureClusterTemplate(
34+
builder.InfrastructureClusterTemplate("default", "infra-template").Build(),
35+
).Build(),
36+
},
37+
{
38+
name: "ClusterClass with ControlPlane template reference only",
39+
clusterClass: builder.ClusterClass("default", "test-cc").
40+
WithControlPlaneTemplate(
41+
builder.ControlPlaneTemplate("default", "cp-template").Build(),
42+
).Build(),
43+
},
44+
{
45+
name: "ClusterClass with MachineInfrastructure template reference",
46+
clusterClass: builder.ClusterClass("default", "test-cc").
47+
WithControlPlaneInfrastructureMachineTemplate(
48+
builder.InfrastructureMachineTemplate("default", "cp-machine-template").Build(),
49+
).Build(),
50+
},
51+
{
52+
name: "ClusterClass with MachineDeployments template references",
53+
clusterClass: builder.ClusterClass("default", "test-cc").
54+
WithWorkerMachineDeploymentClasses(
55+
*builder.MachineDeploymentClass("worker-1").
56+
WithInfrastructureTemplate(
57+
builder.InfrastructureMachineTemplate("default", "worker-infra-template").Build(),
58+
).
59+
WithBootstrapTemplate(
60+
builder.BootstrapTemplate("default", "worker-bootstrap-template").Build(),
61+
).Build(),
62+
).Build(),
63+
},
64+
{
65+
name: "ClusterClass with MachineDeployments having nil Infrastructure template reference",
66+
clusterClass: builder.ClusterClass("default", "test-cc").
67+
WithWorkerMachineDeploymentClasses(
68+
*builder.MachineDeploymentClass("worker-1").
69+
WithBootstrapTemplate(
70+
builder.BootstrapTemplate("default", "worker-bootstrap-template").Build(),
71+
).Build(),
72+
).Build(),
73+
},
74+
{
75+
name: "ClusterClass with MachineDeployments having nil Bootstrap template reference",
76+
clusterClass: builder.ClusterClass("default", "test-cc").
77+
WithWorkerMachineDeploymentClasses(
78+
*builder.MachineDeploymentClass("worker-1").
79+
WithInfrastructureTemplate(
80+
builder.InfrastructureMachineTemplate("default", "worker-infra-template").Build(),
81+
).Build(),
82+
).Build(),
83+
},
84+
{
85+
name: "callback function returns error",
86+
clusterClass: builder.ClusterClass("default", "test-cc").
87+
WithInfrastructureClusterTemplate(
88+
builder.InfrastructureClusterTemplate("default", "infra-template").Build(),
89+
).Build(),
90+
},
91+
{
92+
name: "complete ClusterClass with all template references",
93+
clusterClass: builder.ClusterClass("default", "test-cc").
94+
WithInfrastructureClusterTemplate(
95+
builder.InfrastructureClusterTemplate("default", "infra-template").Build(),
96+
).
97+
WithControlPlaneTemplate(
98+
builder.ControlPlaneTemplate("default", "cp-template").Build(),
99+
).
100+
WithControlPlaneInfrastructureMachineTemplate(
101+
builder.InfrastructureMachineTemplate("default", "cp-machine-template").Build(),
102+
).
103+
WithWorkerMachineDeploymentClasses(
104+
*builder.MachineDeploymentClass("worker-1").
105+
WithInfrastructureTemplate(
106+
builder.InfrastructureMachineTemplate("default", "worker-infra-template").Build(),
107+
).
108+
WithBootstrapTemplate(
109+
builder.BootstrapTemplate("default", "worker-bootstrap-template").Build(),
110+
).Build(),
111+
*builder.MachineDeploymentClass("worker-2").
112+
WithInfrastructureTemplate(
113+
builder.InfrastructureMachineTemplate("default", "worker2-infra-template").Build(),
114+
).
115+
WithBootstrapTemplate(
116+
builder.BootstrapTemplate("default", "worker2-bootstrap-template").Build(),
117+
).Build(),
118+
).Build(),
119+
},
120+
}
121+
122+
for _, tt := range tests {
123+
t.Run(tt.name, func(t *testing.T) {
124+
t.Parallel()
125+
g := NewWithT(t)
126+
ctx := context.Background()
127+
var capturedRefs []*corev1.ObjectReference
128+
129+
callback := func(ctx context.Context, ref *corev1.ObjectReference) error {
130+
capturedRefs = append(capturedRefs, ref)
131+
return nil
132+
}
133+
134+
err := walkReferences(ctx, tt.clusterClass, callback)
135+
136+
g.Expect(err).ToNot(HaveOccurred())
137+
138+
// Verify that captured references match expected ones
139+
if tt.clusterClass != nil {
140+
expectedRefs := collectExpectedRefs(tt.clusterClass)
141+
g.Expect(capturedRefs).
142+
To(HaveLen(len(expectedRefs)), "Expected %d references, got %d", len(expectedRefs), len(capturedRefs))
143+
144+
for i, expectedRef := range expectedRefs {
145+
if expectedRef != nil {
146+
g.Expect(capturedRefs[i]).To(Equal(expectedRef), "Reference doesn't match")
147+
}
148+
}
149+
}
150+
})
151+
}
152+
}
153+
154+
func collectExpectedRefs(cc *clusterv1.ClusterClass) []*corev1.ObjectReference {
155+
var refs []*corev1.ObjectReference
156+
157+
if cc.Spec.Infrastructure.Ref != nil {
158+
refs = append(refs, cc.Spec.Infrastructure.Ref)
159+
}
160+
161+
if cc.Spec.ControlPlane.Ref != nil {
162+
refs = append(refs, cc.Spec.ControlPlane.Ref)
163+
}
164+
165+
if cpInfra := cc.Spec.ControlPlane.MachineInfrastructure; cpInfra != nil && cpInfra.Ref != nil {
166+
refs = append(refs, cpInfra.Ref)
167+
}
168+
169+
for _, md := range cc.Spec.Workers.MachineDeployments {
170+
if md.Template.Infrastructure.Ref != nil {
171+
refs = append(refs, md.Template.Infrastructure.Ref)
172+
}
173+
if md.Template.Bootstrap.Ref != nil {
174+
refs = append(refs, md.Template.Bootstrap.Ref)
175+
}
176+
}
177+
178+
return refs
179+
}

0 commit comments

Comments
 (0)