Skip to content

Commit 426d2ad

Browse files
nojnhuhnojnhuh
committed
✨ Use caching read for bootstrap config owner
1 parent 8150380 commit 426d2ad

File tree

4 files changed

+173
-116
lines changed

4 files changed

+173
-116
lines changed

‎bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func (r *KubeadmConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
158158
}
159159

160160
// Look up the owner of this kubeadm config if there is one
161-
configOwner, err := bsutil.GetConfigOwner(ctx, r.Client, config)
161+
configOwner, err := bsutil.GetConfigOwnerFromCache(ctx, r.Client, config)
162162
if apierrors.IsNotFound(err) {
163163
// Could not find the owner yet, this is not an error and will rereconcile when the owner gets set.
164164
return ctrl.Result{}, nil

‎bootstrap/util/configowner.go

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ import (
2424
corev1 "k8s.io/api/core/v1"
2525
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2626
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
27+
"k8s.io/apimachinery/pkg/runtime"
2728
"k8s.io/apimachinery/pkg/runtime/schema"
29+
"k8s.io/apimachinery/pkg/types"
2830
"sigs.k8s.io/controller-runtime/pkg/client"
2931

3032
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -123,8 +125,20 @@ func (co ConfigOwner) KubernetesVersion() string {
123125
return version
124126
}
125127

126-
// GetConfigOwner returns the Unstructured object owning the current resource.
128+
// GetConfigOwner returns the Unstructured object owning the current resource
129+
// using the uncached unstructured client. For performance-sensitive uses,
130+
// consider GetConfigOwnerCached.
127131
func GetConfigOwner(ctx context.Context, c client.Client, obj metav1.Object) (*ConfigOwner, error) {
132+
return getConfigOwner(ctx, c, obj, GetOwnerByRef)
133+
}
134+
135+
// GetConfigOwnerFromCache returns the Unstructured object owning the current
136+
// resource. The implementation ensures a typed client is used, so the objects are read from the cache.
137+
func GetConfigOwnerFromCache(ctx context.Context, c client.Client, obj metav1.Object) (*ConfigOwner, error) {
138+
return getConfigOwner(ctx, c, obj, GetOwnerByRefFromCache)
139+
}
140+
141+
func getConfigOwner(ctx context.Context, c client.Client, obj metav1.Object, getFn func(context.Context, client.Client, *corev1.ObjectReference) (*ConfigOwner, error)) (*ConfigOwner, error) {
128142
allowedGKs := []schema.GroupKind{
129143
{
130144
Group: clusterv1.GroupVersion.Group,
@@ -148,7 +162,7 @@ func GetConfigOwner(ctx context.Context, c client.Client, obj metav1.Object) (*C
148162

149163
for _, gk := range allowedGKs {
150164
if refGVK.Group == gk.Group && refGVK.Kind == gk.Kind {
151-
return GetOwnerByRef(ctx, c, &corev1.ObjectReference{
165+
return getFn(ctx, c, &corev1.ObjectReference{
152166
APIVersion: ref.APIVersion,
153167
Kind: ref.Kind,
154168
Name: ref.Name,
@@ -168,3 +182,33 @@ func GetOwnerByRef(ctx context.Context, c client.Client, ref *corev1.ObjectRefer
168182
}
169183
return &ConfigOwner{obj}, nil
170184
}
185+
186+
// GetOwnerByRefFromCache finds and returns the owner by looking at the object
187+
// reference. The implementation ensures a typed client is used, so the objects are read from the cache.
188+
func GetOwnerByRefFromCache(ctx context.Context, c client.Client, ref *corev1.ObjectReference) (*ConfigOwner, error) {
189+
obj, err := c.Scheme().New(ref.GroupVersionKind())
190+
if err != nil {
191+
return nil, errors.Wrapf(err, "failed to construct object of type %s", ref.GroupVersionKind())
192+
}
193+
metaObj, ok := obj.(client.Object)
194+
if !ok {
195+
return nil, errors.Errorf("expected owner reference to refer to a client.Object, is actually %T", obj)
196+
}
197+
key := types.NamespacedName{
198+
Namespace: ref.Namespace,
199+
Name: ref.Name,
200+
}
201+
err = c.Get(ctx, key, metaObj)
202+
if err != nil {
203+
return nil, err
204+
}
205+
206+
content, err := runtime.DefaultUnstructuredConverter.ToUnstructured(metaObj)
207+
if err != nil {
208+
return nil, err
209+
}
210+
u := unstructured.Unstructured{}
211+
u.SetUnstructuredContent(content)
212+
213+
return &ConfigOwner{&u}, nil
214+
}

‎bootstrap/util/configowner_test.go

Lines changed: 125 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package util
1818

1919
import (
20+
"context"
2021
"testing"
2122

2223
. "github.com/onsi/gomega"
@@ -25,6 +26,7 @@ import (
2526
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2627
"k8s.io/apimachinery/pkg/runtime"
2728
"k8s.io/utils/pointer"
29+
"sigs.k8s.io/controller-runtime/pkg/client"
2830
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2931

3032
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -34,136 +36,146 @@ import (
3436
)
3537

3638
func TestGetConfigOwner(t *testing.T) {
37-
t.Run("should get the owner when present (Machine)", func(t *testing.T) {
38-
g := NewWithT(t)
39-
myMachine := &clusterv1.Machine{
40-
ObjectMeta: metav1.ObjectMeta{
41-
Name: "my-machine",
42-
Namespace: metav1.NamespaceDefault,
43-
Labels: map[string]string{
44-
clusterv1.MachineControlPlaneLabel: "",
39+
doTests := func(t *testing.T, getFn func(context.Context, client.Client, metav1.Object) (*ConfigOwner, error)) {
40+
t.Helper()
41+
42+
t.Run("should get the owner when present (Machine)", func(t *testing.T) {
43+
g := NewWithT(t)
44+
myMachine := &clusterv1.Machine{
45+
ObjectMeta: metav1.ObjectMeta{
46+
Name: "my-machine",
47+
Namespace: metav1.NamespaceDefault,
48+
Labels: map[string]string{
49+
clusterv1.MachineControlPlaneLabel: "",
50+
},
4551
},
46-
},
47-
Spec: clusterv1.MachineSpec{
48-
ClusterName: "my-cluster",
49-
Bootstrap: clusterv1.Bootstrap{
50-
DataSecretName: pointer.String("my-data-secret"),
52+
Spec: clusterv1.MachineSpec{
53+
ClusterName: "my-cluster",
54+
Bootstrap: clusterv1.Bootstrap{
55+
DataSecretName: pointer.String("my-data-secret"),
56+
},
57+
Version: pointer.String("v1.19.6"),
5158
},
52-
Version: pointer.String("v1.19.6"),
53-
},
54-
Status: clusterv1.MachineStatus{
55-
InfrastructureReady: true,
56-
},
57-
}
59+
Status: clusterv1.MachineStatus{
60+
InfrastructureReady: true,
61+
},
62+
}
5863

59-
c := fake.NewClientBuilder().WithObjects(myMachine).Build()
60-
obj := &bootstrapv1.KubeadmConfig{
61-
ObjectMeta: metav1.ObjectMeta{
62-
OwnerReferences: []metav1.OwnerReference{
63-
{
64-
Kind: "Machine",
65-
APIVersion: clusterv1.GroupVersion.String(),
66-
Name: "my-machine",
64+
c := fake.NewClientBuilder().WithObjects(myMachine).Build()
65+
obj := &bootstrapv1.KubeadmConfig{
66+
ObjectMeta: metav1.ObjectMeta{
67+
OwnerReferences: []metav1.OwnerReference{
68+
{
69+
Kind: "Machine",
70+
APIVersion: clusterv1.GroupVersion.String(),
71+
Name: "my-machine",
72+
},
6773
},
74+
Namespace: metav1.NamespaceDefault,
75+
Name: "my-resource-owned-by-machine",
6876
},
69-
Namespace: metav1.NamespaceDefault,
70-
Name: "my-resource-owned-by-machine",
71-
},
72-
}
73-
configOwner, err := GetConfigOwner(ctx, c, obj)
74-
g.Expect(err).NotTo(HaveOccurred())
75-
g.Expect(configOwner).ToNot(BeNil())
76-
g.Expect(configOwner.ClusterName()).To(BeEquivalentTo("my-cluster"))
77-
g.Expect(configOwner.IsInfrastructureReady()).To(BeTrue())
78-
g.Expect(configOwner.IsControlPlaneMachine()).To(BeTrue())
79-
g.Expect(configOwner.IsMachinePool()).To(BeFalse())
80-
g.Expect(configOwner.KubernetesVersion()).To(Equal("v1.19.6"))
81-
g.Expect(*configOwner.DataSecretName()).To(BeEquivalentTo("my-data-secret"))
82-
})
77+
}
78+
configOwner, err := getFn(ctx, c, obj)
79+
g.Expect(err).NotTo(HaveOccurred())
80+
g.Expect(configOwner).ToNot(BeNil())
81+
g.Expect(configOwner.ClusterName()).To(BeEquivalentTo("my-cluster"))
82+
g.Expect(configOwner.IsInfrastructureReady()).To(BeTrue())
83+
g.Expect(configOwner.IsControlPlaneMachine()).To(BeTrue())
84+
g.Expect(configOwner.IsMachinePool()).To(BeFalse())
85+
g.Expect(configOwner.KubernetesVersion()).To(Equal("v1.19.6"))
86+
g.Expect(*configOwner.DataSecretName()).To(BeEquivalentTo("my-data-secret"))
87+
})
8388

84-
t.Run("should get the owner when present (MachinePool)", func(t *testing.T) {
85-
_ = feature.MutableGates.Set("MachinePool=true")
89+
t.Run("should get the owner when present (MachinePool)", func(t *testing.T) {
90+
_ = feature.MutableGates.Set("MachinePool=true")
8691

87-
g := NewWithT(t)
88-
myPool := &expv1.MachinePool{
89-
ObjectMeta: metav1.ObjectMeta{
90-
Name: "my-machine-pool",
91-
Namespace: metav1.NamespaceDefault,
92-
Labels: map[string]string{
93-
clusterv1.MachineControlPlaneLabel: "",
92+
g := NewWithT(t)
93+
myPool := &expv1.MachinePool{
94+
ObjectMeta: metav1.ObjectMeta{
95+
Name: "my-machine-pool",
96+
Namespace: metav1.NamespaceDefault,
97+
Labels: map[string]string{
98+
clusterv1.MachineControlPlaneLabel: "",
99+
},
94100
},
95-
},
96-
Spec: expv1.MachinePoolSpec{
97-
ClusterName: "my-cluster",
98-
Template: clusterv1.MachineTemplateSpec{
99-
Spec: clusterv1.MachineSpec{
100-
Version: pointer.String("v1.19.6"),
101+
Spec: expv1.MachinePoolSpec{
102+
ClusterName: "my-cluster",
103+
Template: clusterv1.MachineTemplateSpec{
104+
Spec: clusterv1.MachineSpec{
105+
Version: pointer.String("v1.19.6"),
106+
},
101107
},
102108
},
103-
},
104-
Status: expv1.MachinePoolStatus{
105-
InfrastructureReady: true,
106-
},
107-
}
109+
Status: expv1.MachinePoolStatus{
110+
InfrastructureReady: true,
111+
},
112+
}
108113

109-
c := fake.NewClientBuilder().WithObjects(myPool).Build()
110-
obj := &bootstrapv1.KubeadmConfig{
111-
ObjectMeta: metav1.ObjectMeta{
112-
OwnerReferences: []metav1.OwnerReference{
113-
{
114-
Kind: "MachinePool",
115-
APIVersion: expv1.GroupVersion.String(),
116-
Name: "my-machine-pool",
114+
c := fake.NewClientBuilder().WithObjects(myPool).Build()
115+
obj := &bootstrapv1.KubeadmConfig{
116+
ObjectMeta: metav1.ObjectMeta{
117+
OwnerReferences: []metav1.OwnerReference{
118+
{
119+
Kind: "MachinePool",
120+
APIVersion: expv1.GroupVersion.String(),
121+
Name: "my-machine-pool",
122+
},
117123
},
124+
Namespace: metav1.NamespaceDefault,
125+
Name: "my-resource-owned-by-machine-pool",
118126
},
119-
Namespace: metav1.NamespaceDefault,
120-
Name: "my-resource-owned-by-machine-pool",
121-
},
122-
}
123-
configOwner, err := GetConfigOwner(ctx, c, obj)
124-
g.Expect(err).NotTo(HaveOccurred())
125-
g.Expect(configOwner).ToNot(BeNil())
126-
g.Expect(configOwner.ClusterName()).To(BeEquivalentTo("my-cluster"))
127-
g.Expect(configOwner.IsInfrastructureReady()).To(BeTrue())
128-
g.Expect(configOwner.IsControlPlaneMachine()).To(BeFalse())
129-
g.Expect(configOwner.IsMachinePool()).To(BeTrue())
130-
g.Expect(configOwner.KubernetesVersion()).To(Equal("v1.19.6"))
131-
g.Expect(configOwner.DataSecretName()).To(BeNil())
132-
})
127+
}
128+
configOwner, err := getFn(ctx, c, obj)
129+
g.Expect(err).NotTo(HaveOccurred())
130+
g.Expect(configOwner).ToNot(BeNil())
131+
g.Expect(configOwner.ClusterName()).To(BeEquivalentTo("my-cluster"))
132+
g.Expect(configOwner.IsInfrastructureReady()).To(BeTrue())
133+
g.Expect(configOwner.IsControlPlaneMachine()).To(BeFalse())
134+
g.Expect(configOwner.IsMachinePool()).To(BeTrue())
135+
g.Expect(configOwner.KubernetesVersion()).To(Equal("v1.19.6"))
136+
g.Expect(configOwner.DataSecretName()).To(BeNil())
137+
})
133138

134-
t.Run("return an error when not found", func(t *testing.T) {
135-
g := NewWithT(t)
136-
c := fake.NewClientBuilder().Build()
137-
obj := &bootstrapv1.KubeadmConfig{
138-
ObjectMeta: metav1.ObjectMeta{
139-
OwnerReferences: []metav1.OwnerReference{
140-
{
141-
Kind: "Machine",
142-
APIVersion: clusterv1.GroupVersion.String(),
143-
Name: "my-machine",
139+
t.Run("return an error when not found", func(t *testing.T) {
140+
g := NewWithT(t)
141+
c := fake.NewClientBuilder().Build()
142+
obj := &bootstrapv1.KubeadmConfig{
143+
ObjectMeta: metav1.ObjectMeta{
144+
OwnerReferences: []metav1.OwnerReference{
145+
{
146+
Kind: "Machine",
147+
APIVersion: clusterv1.GroupVersion.String(),
148+
Name: "my-machine",
149+
},
144150
},
151+
Namespace: metav1.NamespaceDefault,
152+
Name: "my-resource-owned-by-machine",
145153
},
146-
Namespace: metav1.NamespaceDefault,
147-
Name: "my-resource-owned-by-machine",
148-
},
149-
}
150-
_, err := GetConfigOwner(ctx, c, obj)
151-
g.Expect(err).To(HaveOccurred())
152-
})
154+
}
155+
_, err := getFn(ctx, c, obj)
156+
g.Expect(err).To(HaveOccurred())
157+
})
153158

154-
t.Run("return nothing when there is no owner", func(t *testing.T) {
155-
g := NewWithT(t)
156-
c := fake.NewClientBuilder().Build()
157-
obj := &bootstrapv1.KubeadmConfig{
158-
ObjectMeta: metav1.ObjectMeta{
159-
OwnerReferences: []metav1.OwnerReference{},
160-
Namespace: metav1.NamespaceDefault,
161-
Name: "my-resource-owned-by-machine",
162-
},
163-
}
164-
configOwner, err := GetConfigOwner(ctx, c, obj)
165-
g.Expect(err).NotTo(HaveOccurred())
166-
g.Expect(configOwner).To(BeNil())
159+
t.Run("return nothing when there is no owner", func(t *testing.T) {
160+
g := NewWithT(t)
161+
c := fake.NewClientBuilder().Build()
162+
obj := &bootstrapv1.KubeadmConfig{
163+
ObjectMeta: metav1.ObjectMeta{
164+
OwnerReferences: []metav1.OwnerReference{},
165+
Namespace: metav1.NamespaceDefault,
166+
Name: "my-resource-owned-by-machine",
167+
},
168+
}
169+
configOwner, err := getFn(ctx, c, obj)
170+
g.Expect(err).NotTo(HaveOccurred())
171+
g.Expect(configOwner).To(BeNil())
172+
})
173+
}
174+
t.Run("uncached", func(t *testing.T) {
175+
doTests(t, GetConfigOwner)
176+
})
177+
t.Run("cached", func(t *testing.T) {
178+
doTests(t, GetConfigOwnerFromCache)
167179
})
168180
}
169181

‎docs/book/src/developer/providers/migrations/v1.4-to-v1.5.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ maintainers of providers and consumers of our Go API.
3636
- clusterctl move is adding the new annotation `clusterctl.cluster.x-k8s.io/delete-for-move` before object deletion.
3737
- Providers running CAPI release-0.3 clusterctl upgrade tests should set `WorkloadKubernetesVersion` field to the maximum workload cluster kubernetes version supported by the old providers in `ClusterctlUpgradeSpecInput`. For more information, please see: https://github.com/kubernetes-sigs/cluster-api/pull/8518#issuecomment-1508064859
3838
- Introduced function `CollectInfrastructureLogs` at the `ClusterLogCollector` interface in `test/framework/cluster_proxy.go` to allow collecting infrastructure related logs during tests.
39+
- A `GetConfigOwnerFromCache` function has been added to the `sigs.k8s.io./cluster-api/bootstrap/util` package. It is equivalent to `GetConfigOwner` except that it uses the cached typed client instead of the uncached unstructured client, so `GetConfigOwnerFromCache` is expected to be more performant.
3940

4041
### Suggested changes for providers
4142

0 commit comments

Comments
 (0)