Skip to content

Commit 5adeb34

Browse files
bavarianbidibavarianbidi
committed
handle nil pointer in clusterctl describe
if a cluster as an empy bootstrap.configref in the machinedeployment (which is allowed), a nil pointer will raise if clusterctl describe cluster is called with --show-templates. Also add a second type of a fake machinedeployment which has a secret reference in the boostrap object instead of a config ref Signed-off-by: Mario Constanti <[email protected]>
1 parent 664c9c9 commit 5adeb34

File tree

5 files changed

+283
-66
lines changed

5 files changed

+283
-66
lines changed

‎cmd/clusterctl/client/cluster/mover_test.go

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,54 @@ var moveTests = []struct {
248248
},
249249
wantErr: false,
250250
},
251+
{
252+
name: "Cluster with MachineDeployment with a static bootstrap config",
253+
fields: moveTestsFields{
254+
objs: test.NewFakeCluster("ns1", "cluster1").
255+
WithMachineDeployments(
256+
test.NewFakeMachineDeployment("md1").
257+
WithStaticBootstrapConfig().
258+
WithMachineSets(
259+
test.NewFakeMachineSet("ms1").
260+
WithStaticBootstrapConfig().
261+
WithMachines(
262+
test.NewFakeMachine("m1").
263+
WithStaticBootstrapConfig(),
264+
test.NewFakeMachine("m2").
265+
WithStaticBootstrapConfig(),
266+
),
267+
),
268+
).Objs(),
269+
},
270+
wantMoveGroups: [][]string{
271+
{ // group 1
272+
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
273+
},
274+
{ // group 2 (objects with ownerReferences in group 1)
275+
// owned by Clusters
276+
"/v1, Kind=Secret, ns1/cluster1-ca",
277+
"/v1, Kind=Secret, ns1/cluster1-kubeconfig",
278+
"cluster.x-k8s.io/v1beta1, Kind=MachineDeployment, ns1/md1",
279+
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureCluster, ns1/cluster1",
280+
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureMachineTemplate, ns1/md1",
281+
},
282+
{ // group 3 (objects with ownerReferences in group 1,2)
283+
// owned by MachineDeployments
284+
"cluster.x-k8s.io/v1beta1, Kind=MachineSet, ns1/ms1",
285+
},
286+
{ // group 4 (objects with ownerReferences in group 1,2,3)
287+
// owned by MachineSets
288+
"cluster.x-k8s.io/v1beta1, Kind=Machine, ns1/m1",
289+
"cluster.x-k8s.io/v1beta1, Kind=Machine, ns1/m2",
290+
},
291+
{ // group 5 (objects with ownerReferences in group 1,2,3,4)
292+
// owned by Machines
293+
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureMachine, ns1/m1",
294+
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureMachine, ns1/m2",
295+
},
296+
},
297+
wantErr: false,
298+
},
251299
{
252300
name: "Cluster with Control Plane",
253301
fields: moveTestsFields{
@@ -825,7 +873,7 @@ func Test_objectMover_restoreTargetObject(t *testing.T) {
825873
oTo.SetKind(node.identity.Kind)
826874

827875
if err := csTo.Get(ctx, key, oTo); err != nil {
828-
t.Errorf("error = %v when checking for %v created in target cluster", err, key)
876+
t.Errorf("error = %v when checking for %s %v created in target cluster", err, oTo.GetKind(), key)
829877
continue
830878
}
831879

@@ -853,7 +901,7 @@ func Test_objectMover_restoreTargetObject(t *testing.T) {
853901
oAfter.SetKind(node.identity.Kind)
854902

855903
if err := csAfter.Get(ctx, keyAfter, oAfter); err != nil {
856-
t.Errorf("error = %v when checking for %v created in target cluster", err, key)
904+
t.Errorf("error = %v when checking for %s %v created in target cluster", err, oAfter.GetKind(), key)
857905
continue
858906
}
859907

@@ -1076,7 +1124,7 @@ func Test_objectMover_fromDirectory(t *testing.T) {
10761124
oTo.SetKind(node.identity.Kind)
10771125

10781126
if err := csTo.Get(ctx, key, oTo); err != nil {
1079-
t.Errorf("error = %v when checking for %v created in target cluster", err, key)
1127+
t.Errorf("error = %v when checking for %s %v created in target cluster", err, oTo.GetKind(), key)
10801128
continue
10811129
}
10821130
}
@@ -1164,7 +1212,7 @@ func Test_objectMover_move_dryRun(t *testing.T) {
11641212
oFrom.SetKind(node.identity.Kind)
11651213

11661214
if err := csFrom.Get(ctx, key, oFrom); err != nil {
1167-
t.Errorf("error = %v when checking for %v kept in source cluster", err, key)
1215+
t.Errorf("error = %v when checking for %s %v kept in source cluster", err, oFrom.GetKind(), key)
11681216
continue
11691217
}
11701218

@@ -1176,11 +1224,11 @@ func Test_objectMover_move_dryRun(t *testing.T) {
11761224
err := csTo.Get(ctx, key, oTo)
11771225
if err == nil {
11781226
if oFrom.GetNamespace() != "" {
1179-
t.Errorf("%v created in target cluster which should not", key)
1227+
t.Errorf("%s %v created in target cluster which should not", oFrom.GetKind(), key)
11801228
continue
11811229
}
11821230
} else if !apierrors.IsNotFound(err) {
1183-
t.Errorf("error = %v when checking for %v should not created ojects in target cluster", err, key)
1231+
t.Errorf("error = %v when checking for %s %v should not created ojects in target cluster", err, oFrom.GetKind(), key)
11841232
continue
11851233
}
11861234
}
@@ -1240,11 +1288,11 @@ func Test_objectMover_move(t *testing.T) {
12401288
err := csFrom.Get(ctx, key, oFrom)
12411289
if err == nil {
12421290
if !node.isGlobal && !node.isGlobalHierarchy {
1243-
t.Errorf("%v not deleted in source cluster", key)
1291+
t.Errorf("%s %v not deleted in source cluster", oFrom.GetKind(), key)
12441292
continue
12451293
}
12461294
} else if !apierrors.IsNotFound(err) {
1247-
t.Errorf("error = %v when checking for %v deleted in source cluster", err, key)
1295+
t.Errorf("error = %v when checking for %s %v deleted in source cluster", err, oFrom.GetKind(), key)
12481296
continue
12491297
}
12501298

@@ -1254,7 +1302,7 @@ func Test_objectMover_move(t *testing.T) {
12541302
oTo.SetKind(node.identity.Kind)
12551303

12561304
if err := csTo.Get(ctx, key, oTo); err != nil {
1257-
t.Errorf("error = %v when checking for %v created in target cluster", err, key)
1305+
t.Errorf("error = %v when checking for %s %v created in target cluster", err, oFrom.GetKind(), key)
12581306
continue
12591307
}
12601308
}
@@ -1349,11 +1397,11 @@ func Test_objectMover_move_with_Mutator(t *testing.T) {
13491397
err := csFrom.Get(ctx, key, oFrom)
13501398
if err == nil {
13511399
if !node.isGlobal && !node.isGlobalHierarchy {
1352-
t.Errorf("%v not deleted in source cluster", key)
1400+
t.Errorf("%s %v not deleted in source cluster", oFrom.GetKind(), key)
13531401
continue
13541402
}
13551403
} else if !apierrors.IsNotFound(err) {
1356-
t.Errorf("error = %v when checking for %v deleted in source cluster", err, key)
1404+
t.Errorf("error = %v when checking for %s %v deleted in source cluster", err, oFrom.GetKind(), key)
13571405
continue
13581406
}
13591407

@@ -1366,7 +1414,7 @@ func Test_objectMover_move_with_Mutator(t *testing.T) {
13661414
}
13671415

13681416
if err := csTo.Get(ctx, key, oTo); err != nil {
1369-
t.Errorf("error = %v when checking for %v created in target cluster", err, key)
1417+
t.Errorf("error = %v when checking for %s %v created in target cluster", err, oFrom.GetKind(), key)
13701418
continue
13711419
}
13721420
if fields, knownKind := updateKnownKinds[oTo.GetKind()]; knownKind {

‎cmd/clusterctl/client/cluster/objectgraph_test.go

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ func assertGraph(t *testing.T, got *objectGraph, want wantGraph) {
252252

253253
g := NewWithT(t)
254254

255-
g.Expect(got.uidToNode).To(HaveLen(len(want.nodes)), "the number of nodes in the objectGraph doesn't match the number of expected nodes")
255+
g.Expect(got.uidToNode).To(HaveLen(len(want.nodes)), "the number of nodes in the objectGraph doesn't match the number of expected nodes - got: %d expected: %d", len(got.uidToNode), len(want.nodes))
256256

257257
for uid, wantNode := range want.nodes {
258258
gotNode, ok := got.uidToNode[types.UID(uid)]
@@ -810,6 +810,83 @@ var objectGraphsTests = []struct {
810810
},
811811
},
812812
},
813+
{
814+
name: "Cluster with MachineDeployment without a BootstrapConfigRef",
815+
args: objectGraphTestArgs{
816+
objs: test.NewFakeCluster("ns1", "cluster1").
817+
WithMachineDeployments(
818+
test.NewFakeMachineDeployment("md1").
819+
WithStaticBootstrapConfig().
820+
WithMachineSets(
821+
test.NewFakeMachineSet("ms1").
822+
WithMachines(
823+
test.NewFakeMachine("m1"),
824+
),
825+
),
826+
).Objs(),
827+
},
828+
want: wantGraph{
829+
nodes: map[string]wantGraphItem{
830+
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1": {
831+
forceMove: true,
832+
forceMoveHierarchy: true,
833+
},
834+
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureCluster, ns1/cluster1": {
835+
owners: []string{
836+
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
837+
},
838+
},
839+
"/v1, Kind=Secret, ns1/cluster1-ca": {
840+
softOwners: []string{
841+
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", // NB. this secret is not linked to the cluster through owner ref
842+
},
843+
},
844+
"/v1, Kind=Secret, ns1/cluster1-kubeconfig": {
845+
owners: []string{
846+
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
847+
},
848+
},
849+
850+
"cluster.x-k8s.io/v1beta1, Kind=MachineDeployment, ns1/md1": {
851+
owners: []string{
852+
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
853+
},
854+
},
855+
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureMachineTemplate, ns1/md1": {
856+
owners: []string{
857+
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
858+
},
859+
},
860+
861+
"cluster.x-k8s.io/v1beta1, Kind=MachineSet, ns1/ms1": {
862+
owners: []string{
863+
"cluster.x-k8s.io/v1beta1, Kind=MachineDeployment, ns1/md1",
864+
},
865+
},
866+
867+
"cluster.x-k8s.io/v1beta1, Kind=Machine, ns1/m1": {
868+
owners: []string{
869+
"cluster.x-k8s.io/v1beta1, Kind=MachineSet, ns1/ms1",
870+
},
871+
},
872+
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureMachine, ns1/m1": {
873+
owners: []string{
874+
"cluster.x-k8s.io/v1beta1, Kind=Machine, ns1/m1",
875+
},
876+
},
877+
"bootstrap.cluster.x-k8s.io/v1beta1, Kind=GenericBootstrapConfig, ns1/m1": {
878+
owners: []string{
879+
"cluster.x-k8s.io/v1beta1, Kind=Machine, ns1/m1",
880+
},
881+
},
882+
"/v1, Kind=Secret, ns1/m1": {
883+
owners: []string{
884+
"bootstrap.cluster.x-k8s.io/v1beta1, Kind=GenericBootstrapConfig, ns1/m1",
885+
},
886+
},
887+
},
888+
},
889+
},
813890
{
814891
name: "Cluster with Control Plane",
815892
args: objectGraphTestArgs{

‎cmd/clusterctl/client/tree/discovery.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,11 @@ func addMachineDeploymentToObjectTree(ctx context.Context, c client.Client, clus
241241
templateParent = md
242242
}
243243

244-
bootstrapTemplateRefObject := ObjectReferenceObject(md.Spec.Template.Spec.Bootstrap.ConfigRef)
245-
tree.Add(templateParent, bootstrapTemplateRefObject, ObjectMetaName("BootstrapConfigTemplate"))
244+
// md.Spec.Template.Spec.Bootstrap.ConfigRef is optional
245+
if md.Spec.Template.Spec.Bootstrap.ConfigRef != nil {
246+
bootstrapTemplateRefObject := ObjectReferenceObject(md.Spec.Template.Spec.Bootstrap.ConfigRef)
247+
tree.Add(templateParent, bootstrapTemplateRefObject, ObjectMetaName("BootstrapConfigTemplate"))
248+
}
246249

247250
machineTemplateRefObject := ObjectReferenceObject(&md.Spec.Template.Spec.InfrastructureRef)
248251
tree.Add(templateParent, machineTemplateRefObject, ObjectMetaName("MachineInfrastructureTemplate"))

‎cmd/clusterctl/client/tree/discovery_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,17 @@ func Test_Discovery(t *testing.T) {
397397
test.NewFakeInfrastructureTemplate("md1"),
398398
),
399399
).
400+
WithMachineDeployments(
401+
test.NewFakeMachineDeployment("md2").
402+
WithStaticBootstrapConfig().
403+
WithMachineSets(
404+
test.NewFakeMachineSet("ms2").
405+
WithMachines(
406+
test.NewFakeMachine("m3"),
407+
test.NewFakeMachine("m4"),
408+
),
409+
),
410+
).
400411
Objs(),
401412
},
402413
wantTree: map[string][]string{
@@ -418,6 +429,7 @@ func Test_Discovery(t *testing.T) {
418429
// Workers should have a machine deployment
419430
"virtual.cluster.x-k8s.io/v1beta1, Kind=WorkerGroup, ns1/Workers": {
420431
"cluster.x-k8s.io/v1beta1, Kind=MachineDeployment, ns1/md1",
432+
"cluster.x-k8s.io/v1beta1, Kind=MachineDeployment, ns1/md2",
421433
},
422434
// Machine deployment should have a group of machines (grouping) and templates group
423435
"cluster.x-k8s.io/v1beta1, Kind=MachineDeployment, ns1/md1": {
@@ -433,6 +445,17 @@ func Test_Discovery(t *testing.T) {
433445
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureMachineTemplate, ns1/md1": {},
434446
// MachineDeployment BootstrapConfigRef should be a leaf
435447
"bootstrap.cluster.x-k8s.io/v1beta1, Kind=GenericBootstrapConfigTemplate, ns1/md1": {},
448+
// Machine deployment should have a group of machines (grouping) and templates group
449+
"cluster.x-k8s.io/v1beta1, Kind=MachineDeployment, ns1/md2": {
450+
"virtual.cluster.x-k8s.io/v1beta1, Kind=MachineGroup, ns1/zzz_",
451+
"virtual.cluster.x-k8s.io/v1beta1, Kind=TemplateGroup, ns1/md2",
452+
},
453+
// MachineDeployment TemplateGroup using static bootstrap will only have InfrastructureRef
454+
"virtual.cluster.x-k8s.io/v1beta1, Kind=TemplateGroup, ns1/md2": {
455+
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureMachineTemplate, ns1/md2",
456+
},
457+
// MachineDeployment InfrastructureRef should be a leaf
458+
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureMachineTemplate, ns1/md2": {},
436459
// ControlPlane TemplateGroup should have a InfrastructureRef
437460
"virtual.cluster.x-k8s.io/v1beta1, Kind=TemplateGroup, ns1/cp": {
438461
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureMachineTemplate, ns1/cp",

0 commit comments

Comments
 (0)