Skip to content

Commit a21faa0

Browse files
Danil-GrigorevDanil-Grigorev
authored
🐛 Use namespace of the reference on external.Get (#11361)
* Use namespace of the reference on Get Signed-off-by: Danil-Grigorev <[email protected]> * Updating tests Signed-off-by: Danil-Grigorev <[email protected]> * Update references usage across the code Signed-off-by: Danil-Grigorev <[email protected]> * Ensure refrence namespace is populated in MD Signed-off-by: Danil-Grigorev <[email protected]> * Use kref for logging Signed-off-by: Danil-Grigorev <[email protected]> * Ensure ref NS in MS is set, and ignored in hash Signed-off-by: Danil-Grigorev <[email protected]> * Double-check and populate ns for MS and MD template Signed-off-by: Danil-Grigorev <[email protected]> * Review: log messages Signed-off-by: Danil-Grigorev <[email protected]> --------- Signed-off-by: Danil-Grigorev <[email protected]>
1 parent c739d0a commit a21faa0

37 files changed

+245
-80
lines changed

‎bootstrap/util/configowner.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ func getConfigOwner(ctx context.Context, c client.Client, obj metav1.Object, get
176176

177177
// GetOwnerByRef finds and returns the owner by looking at the object reference.
178178
func GetOwnerByRef(ctx context.Context, c client.Client, ref *corev1.ObjectReference) (*ConfigOwner, error) {
179-
obj, err := external.Get(ctx, c, ref, ref.Namespace)
179+
obj, err := external.Get(ctx, c, ref)
180180
if err != nil {
181181
return nil, err
182182
}

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

+8-8
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func Discovery(ctx context.Context, c client.Client, namespace, name string, opt
8484
tree := NewObjectTree(cluster, options.toObjectTreeOptions())
8585

8686
// Adds cluster infra
87-
clusterInfra, err := external.Get(ctx, c, cluster.Spec.InfrastructureRef, cluster.Namespace)
87+
clusterInfra, err := external.Get(ctx, c, cluster.Spec.InfrastructureRef)
8888
if err != nil {
8989
return nil, errors.Wrap(err, "get InfraCluster reference from Cluster")
9090
}
@@ -95,7 +95,7 @@ func Discovery(ctx context.Context, c client.Client, namespace, name string, opt
9595
}
9696

9797
// Adds control plane
98-
controlPlane, err := external.Get(ctx, c, cluster.Spec.ControlPlaneRef, cluster.Namespace)
98+
controlPlane, err := external.Get(ctx, c, cluster.Spec.ControlPlaneRef)
9999
if err == nil {
100100
addControlPlane(cluster, controlPlane, tree, options)
101101
}
@@ -112,13 +112,13 @@ func Discovery(ctx context.Context, c client.Client, namespace, name string, opt
112112

113113
if visible {
114114
if (m.Spec.InfrastructureRef != corev1.ObjectReference{}) {
115-
if machineInfra, err := external.Get(ctx, c, &m.Spec.InfrastructureRef, cluster.Namespace); err == nil {
115+
if machineInfra, err := external.Get(ctx, c, &m.Spec.InfrastructureRef); err == nil {
116116
tree.Add(m, machineInfra, ObjectMetaName("MachineInfrastructure"), NoEcho(true))
117117
}
118118
}
119119

120120
if m.Spec.Bootstrap.ConfigRef != nil {
121-
if machineBootstrap, err := external.Get(ctx, c, m.Spec.Bootstrap.ConfigRef, cluster.Namespace); err == nil {
121+
if machineBootstrap, err := external.Get(ctx, c, m.Spec.Bootstrap.ConfigRef); err == nil {
122122
tree.Add(m, machineBootstrap, ObjectMetaName("BootstrapConfig"), NoEcho(true))
123123
}
124124
}
@@ -154,7 +154,7 @@ func Discovery(ctx context.Context, c client.Client, namespace, name string, opt
154154

155155
if len(machinePoolList.Items) > 0 { // Add MachinePool objects
156156
tree.Add(cluster, workers)
157-
addMachinePoolsToObjectTree(ctx, c, cluster.Namespace, workers, machinePoolList, machinesList, tree, addMachineFunc)
157+
addMachinePoolsToObjectTree(ctx, c, workers, machinePoolList, machinesList, tree, addMachineFunc)
158158
}
159159

160160
// Handles orphan machines.
@@ -275,17 +275,17 @@ func addMachineDeploymentToObjectTree(ctx context.Context, c client.Client, clus
275275
return nil
276276
}
277277

278-
func addMachinePoolsToObjectTree(ctx context.Context, c client.Client, namespace string, workers *unstructured.Unstructured, machinePoolList *expv1.MachinePoolList, machinesList *clusterv1.MachineList, tree *ObjectTree, addMachineFunc func(parent client.Object, m *clusterv1.Machine)) {
278+
func addMachinePoolsToObjectTree(ctx context.Context, c client.Client, workers *unstructured.Unstructured, machinePoolList *expv1.MachinePoolList, machinesList *clusterv1.MachineList, tree *ObjectTree, addMachineFunc func(parent client.Object, m *clusterv1.Machine)) {
279279
for i := range machinePoolList.Items {
280280
mp := &machinePoolList.Items[i]
281281
_, visible := tree.Add(workers, mp, GroupingObject(true))
282282

283283
if visible {
284-
if machinePoolBootstrap, err := external.Get(ctx, c, mp.Spec.Template.Spec.Bootstrap.ConfigRef, namespace); err == nil {
284+
if machinePoolBootstrap, err := external.Get(ctx, c, mp.Spec.Template.Spec.Bootstrap.ConfigRef); err == nil {
285285
tree.Add(mp, machinePoolBootstrap, ObjectMetaName("BootstrapConfig"), NoEcho(true))
286286
}
287287

288-
if machinePoolInfra, err := external.Get(ctx, c, &mp.Spec.Template.Spec.InfrastructureRef, namespace); err == nil {
288+
if machinePoolInfra, err := external.Get(ctx, c, &mp.Spec.Template.Spec.InfrastructureRef); err == nil {
289289
tree.Add(mp, machinePoolInfra, ObjectMetaName("MachinePoolInfrastructure"), NoEcho(true))
290290
}
291291
}

‎controllers/external/util.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,24 @@ import (
2525
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2626
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2727
"k8s.io/apiserver/pkg/storage/names"
28+
"k8s.io/klog/v2"
2829
"sigs.k8s.io/controller-runtime/pkg/client"
2930

3031
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3132
)
3233

3334
// Get uses the client and reference to get an external, unstructured object.
34-
func Get(ctx context.Context, c client.Reader, ref *corev1.ObjectReference, namespace string) (*unstructured.Unstructured, error) {
35+
func Get(ctx context.Context, c client.Reader, ref *corev1.ObjectReference) (*unstructured.Unstructured, error) {
3536
if ref == nil {
3637
return nil, errors.Errorf("cannot get object - object reference not set")
3738
}
3839
obj := new(unstructured.Unstructured)
3940
obj.SetAPIVersion(ref.APIVersion)
4041
obj.SetKind(ref.Kind)
4142
obj.SetName(ref.Name)
42-
key := client.ObjectKey{Name: obj.GetName(), Namespace: namespace}
43-
if err := c.Get(ctx, key, obj); err != nil {
44-
return nil, errors.Wrapf(err, "failed to retrieve %s external object %q/%q", obj.GetKind(), key.Namespace, key.Name)
43+
obj.SetNamespace(ref.Namespace)
44+
if err := c.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil {
45+
return nil, errors.Wrapf(err, "failed to retrieve %s %s", obj.GetKind(), klog.KRef(ref.Namespace, ref.Name))
4546
}
4647
return obj, nil
4748
}
@@ -54,7 +55,7 @@ func Delete(ctx context.Context, c client.Writer, ref *corev1.ObjectReference) e
5455
obj.SetName(ref.Name)
5556
obj.SetNamespace(ref.Namespace)
5657
if err := c.Delete(ctx, obj); err != nil {
57-
return errors.Wrapf(err, "failed to delete %s external object %q/%q", obj.GetKind(), obj.GetNamespace(), obj.GetName())
58+
return errors.Wrapf(err, "failed to delete %s %s", obj.GetKind(), klog.KRef(ref.Namespace, ref.Name))
5859
}
5960
return nil
6061
}
@@ -92,7 +93,7 @@ type CreateFromTemplateInput struct {
9293

9394
// CreateFromTemplate uses the client and the reference to create a new object from the template.
9495
func CreateFromTemplate(ctx context.Context, in *CreateFromTemplateInput) (*corev1.ObjectReference, error) {
95-
from, err := Get(ctx, in.Client, in.TemplateRef, in.Namespace)
96+
from, err := Get(ctx, in.Client, in.TemplateRef)
9697
if err != nil {
9798
return nil, err
9899
}

‎controllers/external/util_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestGetResourceFound(t *testing.T) {
6363
}
6464

6565
fakeClient := fake.NewClientBuilder().WithObjects(testResource.DeepCopy()).Build()
66-
got, err := Get(ctx, fakeClient, testResourceReference, metav1.NamespaceDefault)
66+
got, err := Get(ctx, fakeClient, testResourceReference)
6767
g.Expect(err).ToNot(HaveOccurred())
6868
g.Expect(got).To(BeComparableTo(testResource))
6969
}
@@ -79,7 +79,7 @@ func TestGetResourceNotFound(t *testing.T) {
7979
}
8080

8181
fakeClient := fake.NewClientBuilder().Build()
82-
_, err := Get(ctx, fakeClient, testResourceReference, metav1.NamespaceDefault)
82+
_, err := Get(ctx, fakeClient, testResourceReference)
8383
g.Expect(err).To(HaveOccurred())
8484
g.Expect(apierrors.IsNotFound(errors.Cause(err))).To(BeTrue())
8585
}

‎controlplane/kubeadm/internal/control_plane.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ func (c *ControlPlane) UpToDateMachines() collections.Machines {
250250
func getInfraResources(ctx context.Context, cl client.Client, machines collections.Machines) (map[string]*unstructured.Unstructured, error) {
251251
result := map[string]*unstructured.Unstructured{}
252252
for _, m := range machines {
253-
infraObj, err := external.Get(ctx, cl, &m.Spec.InfrastructureRef, m.Namespace)
253+
infraObj, err := external.Get(ctx, cl, &m.Spec.InfrastructureRef)
254254
if err != nil {
255255
if apierrors.IsNotFound(errors.Cause(err)) {
256256
continue

‎controlplane/kubeadm/internal/controllers/controller_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1399,7 +1399,7 @@ kubernetesVersion: metav1.16.1
13991399
machine := machineList.Items[0]
14001400
g.Expect(machine.Name).To(HavePrefix(kcp.Name))
14011401
// Newly cloned infra objects should have the infraref annotation.
1402-
infraObj, err := external.Get(ctx, r.Client, &machine.Spec.InfrastructureRef, machine.Spec.InfrastructureRef.Namespace)
1402+
infraObj, err := external.Get(ctx, r.Client, &machine.Spec.InfrastructureRef)
14031403
g.Expect(err).ToNot(HaveOccurred())
14041404
g.Expect(infraObj.GetAnnotations()).To(HaveKeyWithValue(clusterv1.TemplateClonedFromNameAnnotation, genericInfrastructureMachineTemplate.GetName()))
14051405
g.Expect(infraObj.GetAnnotations()).To(HaveKeyWithValue(clusterv1.TemplateClonedFromGroupKindAnnotation, genericInfrastructureMachineTemplate.GroupVersionKind().GroupKind().String()))

‎controlplane/kubeadm/internal/controllers/helpers.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileExternalReference(ctx context.C
151151
return err
152152
}
153153

154-
obj, err := external.Get(ctx, r.Client, ref, controlPlane.Cluster.Namespace)
154+
obj, err := external.Get(ctx, r.Client, ref)
155155
if err != nil {
156156
if apierrors.IsNotFound(err) {
157157
controlPlane.InfraMachineTemplateIsNotFound = true

‎controlplane/kubeadm/internal/controllers/helpers_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ func TestCloneConfigsAndGenerateMachine(t *testing.T) {
386386
g.Expect(m.Name).NotTo(BeEmpty())
387387
g.Expect(m.Name).To(HavePrefix(kcp.Name + namingTemplateKey))
388388

389-
infraObj, err := external.Get(ctx, r.Client, &m.Spec.InfrastructureRef, m.Spec.InfrastructureRef.Namespace)
389+
infraObj, err := external.Get(ctx, r.Client, &m.Spec.InfrastructureRef)
390390
g.Expect(err).ToNot(HaveOccurred())
391391
g.Expect(infraObj.GetAnnotations()).To(HaveKeyWithValue(clusterv1.TemplateClonedFromNameAnnotation, genericInfrastructureMachineTemplate.GetName()))
392392
g.Expect(infraObj.GetAnnotations()).To(HaveKeyWithValue(clusterv1.TemplateClonedFromGroupKindAnnotation, genericInfrastructureMachineTemplate.GroupVersionKind().GroupKind().String()))
@@ -469,7 +469,7 @@ func TestCloneConfigsAndGenerateMachineFail(t *testing.T) {
469469
Status: corev1.ConditionFalse,
470470
Severity: clusterv1.ConditionSeverityError,
471471
Reason: controlplanev1.InfrastructureTemplateCloningFailedReason,
472-
Message: "failed to retrieve GenericMachineTemplate external object \"default\"/\"something_invalid\": genericmachinetemplates.generic.io \"something_invalid\" not found",
472+
Message: "failed to retrieve GenericMachineTemplate default/something_invalid: genericmachinetemplates.generic.io \"something_invalid\" not found",
473473
}))
474474
}
475475

‎exp/internal/controllers/machinepool_controller.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -341,10 +341,10 @@ func (r *MachinePoolReconciler) reconcileDeleteExternal(ctx context.Context, mac
341341
continue
342342
}
343343

344-
obj, err := external.Get(ctx, r.Client, ref, machinePool.Namespace)
344+
obj, err := external.Get(ctx, r.Client, ref)
345345
if err != nil && !apierrors.IsNotFound(errors.Cause(err)) {
346346
return false, errors.Wrapf(err, "failed to get %s %q for MachinePool %q in namespace %q",
347-
ref.GroupVersionKind(), ref.Name, machinePool.Name, machinePool.Namespace)
347+
ref.GroupVersionKind(), ref.Name, machinePool.Name, ref.Namespace)
348348
}
349349
if obj != nil {
350350
objects = append(objects, obj)

‎exp/internal/controllers/machinepool_controller_phases.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,11 @@ func (r *MachinePoolReconciler) reconcileExternal(ctx context.Context, m *expv1.
113113
return external.ReconcileOutput{}, err
114114
}
115115

116-
obj, err := external.Get(ctx, r.Client, ref, m.Namespace)
116+
obj, err := external.Get(ctx, r.Client, ref)
117117
if err != nil {
118118
if apierrors.IsNotFound(errors.Cause(err)) {
119119
return external.ReconcileOutput{}, errors.Wrapf(err, "could not find %v %q for MachinePool %q in namespace %q, requeuing",
120-
ref.GroupVersionKind(), ref.Name, m.Name, m.Namespace)
120+
ref.GroupVersionKind(), ref.Name, m.Name, ref.Namespace)
121121
}
122122
return external.ReconcileOutput{}, err
123123
}

‎exp/internal/controllers/machinepool_controller_phases_test.go

+17-2
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,14 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
7878
APIVersion: builder.BootstrapGroupVersion.String(),
7979
Kind: builder.TestBootstrapConfigKind,
8080
Name: "bootstrap-config1",
81+
Namespace: metav1.NamespaceDefault,
8182
},
8283
},
8384
InfrastructureRef: corev1.ObjectReference{
8485
APIVersion: builder.InfrastructureGroupVersion.String(),
8586
Kind: builder.TestInfrastructureMachineTemplateKind,
8687
Name: "infra-config1",
88+
Namespace: metav1.NamespaceDefault,
8789
},
8890
},
8991
},
@@ -808,6 +810,7 @@ func TestReconcileMachinePoolBootstrap(t *testing.T) {
808810
APIVersion: builder.BootstrapGroupVersion.String(),
809811
Kind: builder.TestBootstrapConfigKind,
810812
Name: "bootstrap-config1",
813+
Namespace: metav1.NamespaceDefault,
811814
},
812815
},
813816
},
@@ -949,6 +952,7 @@ func TestReconcileMachinePoolBootstrap(t *testing.T) {
949952
APIVersion: builder.BootstrapGroupVersion.String(),
950953
Kind: builder.TestBootstrapConfigKind,
951954
Name: "bootstrap-config1",
955+
Namespace: metav1.NamespaceDefault,
952956
},
953957
DataSecretName: ptr.To("data"),
954958
},
@@ -1032,6 +1036,7 @@ func TestReconcileMachinePoolBootstrap(t *testing.T) {
10321036
APIVersion: builder.BootstrapGroupVersion.String(),
10331037
Kind: builder.TestBootstrapConfigKind,
10341038
Name: "bootstrap-config1",
1039+
Namespace: metav1.NamespaceDefault,
10351040
},
10361041
DataSecretName: ptr.To("data"),
10371042
},
@@ -1107,12 +1112,14 @@ func TestReconcileMachinePoolInfrastructure(t *testing.T) {
11071112
APIVersion: builder.BootstrapGroupVersion.String(),
11081113
Kind: builder.TestBootstrapConfigKind,
11091114
Name: "bootstrap-config1",
1115+
Namespace: metav1.NamespaceDefault,
11101116
},
11111117
},
11121118
InfrastructureRef: corev1.ObjectReference{
11131119
APIVersion: builder.InfrastructureGroupVersion.String(),
11141120
Kind: builder.TestInfrastructureMachineTemplateKind,
11151121
Name: "infra-config1",
1122+
Namespace: metav1.NamespaceDefault,
11161123
},
11171124
},
11181125
},
@@ -1186,12 +1193,14 @@ func TestReconcileMachinePoolInfrastructure(t *testing.T) {
11861193
APIVersion: builder.BootstrapGroupVersion.String(),
11871194
Kind: builder.TestBootstrapConfigKind,
11881195
Name: "bootstrap-config1",
1196+
Namespace: metav1.NamespaceDefault,
11891197
},
11901198
},
11911199
InfrastructureRef: corev1.ObjectReference{
11921200
APIVersion: builder.InfrastructureGroupVersion.String(),
11931201
Kind: builder.TestInfrastructureMachineTemplateKind,
11941202
Name: "infra-config1",
1203+
Namespace: metav1.NamespaceDefault,
11951204
},
11961205
},
11971206
},
@@ -1245,12 +1254,14 @@ func TestReconcileMachinePoolInfrastructure(t *testing.T) {
12451254
APIVersion: builder.BootstrapGroupVersion.String(),
12461255
Kind: builder.TestBootstrapConfigKind,
12471256
Name: "bootstrap-config1",
1257+
Namespace: metav1.NamespaceDefault,
12481258
},
12491259
},
12501260
InfrastructureRef: corev1.ObjectReference{
12511261
APIVersion: builder.InfrastructureGroupVersion.String(),
12521262
Kind: builder.TestInfrastructureMachineTemplateKind,
12531263
Name: "infra-config1",
1264+
Namespace: metav1.NamespaceDefault,
12541265
},
12551266
},
12561267
},
@@ -1437,7 +1448,7 @@ func TestReconcileMachinePoolMachines(t *testing.T) {
14371448
g.Expect(machineList.Items).To(HaveLen(2))
14381449
for i := range machineList.Items {
14391450
machine := &machineList.Items[i]
1440-
_, err := external.Get(ctx, r.Client, &machine.Spec.InfrastructureRef, machine.Namespace)
1451+
_, err := external.Get(ctx, r.Client, &machine.Spec.InfrastructureRef)
14411452
g.Expect(err).ToNot(HaveOccurred())
14421453
}
14431454
})
@@ -1508,7 +1519,7 @@ func TestReconcileMachinePoolMachines(t *testing.T) {
15081519
g.Expect(machineList.Items).To(HaveLen(2))
15091520
for i := range machineList.Items {
15101521
machine := &machineList.Items[i]
1511-
_, err := external.Get(ctx, r.Client, &machine.Spec.InfrastructureRef, machine.Namespace)
1522+
_, err := external.Get(ctx, r.Client, &machine.Spec.InfrastructureRef)
15121523
g.Expect(err).ToNot(HaveOccurred())
15131524
}
15141525
})
@@ -1755,12 +1766,14 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
17551766
APIVersion: builder.BootstrapGroupVersion.String(),
17561767
Kind: builder.TestBootstrapConfigKind,
17571768
Name: "bootstrap-config1",
1769+
Namespace: ns.Name,
17581770
},
17591771
},
17601772
InfrastructureRef: corev1.ObjectReference{
17611773
APIVersion: builder.InfrastructureGroupVersion.String(),
17621774
Kind: builder.TestInfrastructureMachineTemplateKind,
17631775
Name: "infra-config1",
1776+
Namespace: ns.Name,
17641777
},
17651778
},
17661779
},
@@ -2181,12 +2194,14 @@ func getMachinePool(replicas int, mpName, clusterName, nsName string) expv1.Mach
21812194
APIVersion: builder.BootstrapGroupVersion.String(),
21822195
Kind: builder.GenericBootstrapConfigKind,
21832196
Name: "bootstrap-config1",
2197+
Namespace: nsName,
21842198
},
21852199
},
21862200
InfrastructureRef: corev1.ObjectReference{
21872201
APIVersion: builder.InfrastructureGroupVersion.String(),
21882202
Kind: builder.GenericInfrastructureMachineKind,
21892203
Name: "infra-config1",
2204+
Namespace: nsName,
21902205
},
21912206
},
21922207
},

0 commit comments

Comments
 (0)