Skip to content

Commit c680942

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #8207 from sbueringer/pr-ssa-caching
✨ SSA: Implement request caching
2 parents 7ee0cd4 + 539760a commit c680942

22 files changed

+570
-132
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ type KubeadmControlPlaneReconciler struct {
8686
// support SSA. This flag should be dropped after all affected tests are migrated
8787
// to envtest.
8888
disableInPlacePropagation bool
89+
ssaCache ssa.Cache
8990
}
9091

9192
func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
@@ -113,6 +114,7 @@ func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mg
113114

114115
r.controller = c
115116
r.recorder = mgr.GetEventRecorderFor("kubeadm-control-plane-controller")
117+
r.ssaCache = ssa.NewCache()
116118

117119
if r.managementCluster == nil {
118120
if r.Tracker == nil {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1574,7 +1574,7 @@ func TestKubeadmControlPlaneReconciler_syncMachines(t *testing.T) {
15741574

15751575
// Run syncMachines to clean up managed fields and have proper field ownership
15761576
// for Machines, InfrastructureMachines and KubeadmConfigs.
1577-
reconciler := &KubeadmControlPlaneReconciler{Client: env}
1577+
reconciler := &KubeadmControlPlaneReconciler{Client: env, ssaCache: ssa.NewCache()}
15781578
g.Expect(reconciler.syncMachines(ctx, controlPlane)).To(Succeed())
15791579

15801580
// The inPlaceMutatingMachine should have cleaned up managed fields.

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

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
"sigs.k8s.io/cluster-api/controllers/external"
3939
controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
4040
"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal"
41+
"sigs.k8s.io/cluster-api/internal/util/ssa"
4142
"sigs.k8s.io/cluster-api/util"
4243
"sigs.k8s.io/cluster-api/util/certs"
4344
"sigs.k8s.io/cluster-api/util/conditions"
@@ -305,12 +306,8 @@ func (r *KubeadmControlPlaneReconciler) updateExternalObject(ctx context.Context
305306
// Update annotations
306307
updatedObject.SetAnnotations(kcp.Spec.MachineTemplate.ObjectMeta.Annotations)
307308

308-
patchOptions := []client.PatchOption{
309-
client.ForceOwnership,
310-
client.FieldOwner(kcpManagerName),
311-
}
312-
if err := r.Client.Patch(ctx, updatedObject, client.Apply, patchOptions...); err != nil {
313-
return errors.Wrapf(err, "failed to update %s", klog.KObj(obj))
309+
if err := ssa.Patch(ctx, r.Client, kcpManagerName, updatedObject); err != nil {
310+
return errors.Wrapf(err, "failed to update %s", obj.GetObjectKind().GroupVersionKind().Kind)
314311
}
315312
return nil
316313
}
@@ -320,12 +317,8 @@ func (r *KubeadmControlPlaneReconciler) createMachine(ctx context.Context, kcp *
320317
if err != nil {
321318
return errors.Wrap(err, "failed to create Machine: failed to compute desired Machine")
322319
}
323-
patchOptions := []client.PatchOption{
324-
client.ForceOwnership,
325-
client.FieldOwner(kcpManagerName),
326-
}
327-
if err := r.Client.Patch(ctx, machine, client.Apply, patchOptions...); err != nil {
328-
return errors.Wrap(err, "failed to create Machine: apply failed")
320+
if err := ssa.Patch(ctx, r.Client, kcpManagerName, machine); err != nil {
321+
return errors.Wrap(err, "failed to create Machine")
329322
}
330323
// Remove the annotation tracking that a remediation is in progress (the remediation completed when
331324
// the replacement machine has been created above).
@@ -342,12 +335,10 @@ func (r *KubeadmControlPlaneReconciler) updateMachine(ctx context.Context, machi
342335
if err != nil {
343336
return nil, errors.Wrap(err, "failed to update Machine: failed to compute desired Machine")
344337
}
345-
patchOptions := []client.PatchOption{
346-
client.ForceOwnership,
347-
client.FieldOwner(kcpManagerName),
348-
}
349-
if err := r.Client.Patch(ctx, updatedMachine, client.Apply, patchOptions...); err != nil {
350-
return nil, errors.Wrap(err, "failed to update Machine: apply failed")
338+
339+
err = ssa.Patch(ctx, r.Client, kcpManagerName, updatedMachine, ssa.WithCachingProxy{Cache: r.ssaCache, Original: machine})
340+
if err != nil {
341+
return nil, errors.Wrap(err, "failed to update Machine")
351342
}
352343
return updatedMachine, nil
353344
}

‎internal/controllers/machine/machine_controller.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import (
4646
"sigs.k8s.io/cluster-api/controllers/external"
4747
"sigs.k8s.io/cluster-api/controllers/noderefutil"
4848
"sigs.k8s.io/cluster-api/controllers/remote"
49+
"sigs.k8s.io/cluster-api/internal/util/ssa"
4950
"sigs.k8s.io/cluster-api/util"
5051
"sigs.k8s.io/cluster-api/util/annotations"
5152
"sigs.k8s.io/cluster-api/util/collections"
@@ -58,6 +59,10 @@ import (
5859
const (
5960
// controllerName defines the controller used when creating clients.
6061
controllerName = "machine-controller"
62+
63+
// machineManagerName is the manager name used for Server-Side-Apply (SSA) operations
64+
// in the Machine controller.
65+
machineManagerName = "capi-machine"
6166
)
6267

6368
var (
@@ -91,6 +96,7 @@ type Reconciler struct {
9196
// nodeDeletionRetryTimeout determines how long the controller will retry deleting a node
9297
// during a single reconciliation.
9398
nodeDeletionRetryTimeout time.Duration
99+
ssaCache ssa.Cache
94100
}
95101

96102
func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
@@ -134,6 +140,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
134140
r.externalTracker = external.ObjectTracker{
135141
Controller: controller,
136142
}
143+
r.ssaCache = ssa.NewCache()
137144
return nil
138145
}
139146

‎internal/controllers/machine/machine_controller_noderef.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3434
"sigs.k8s.io/cluster-api/api/v1beta1/index"
3535
"sigs.k8s.io/cluster-api/controllers/noderefutil"
36+
"sigs.k8s.io/cluster-api/internal/util/ssa"
3637
"sigs.k8s.io/cluster-api/util"
3738
"sigs.k8s.io/cluster-api/util/annotations"
3839
"sigs.k8s.io/cluster-api/util/conditions"
@@ -124,13 +125,10 @@ func (r *Reconciler) reconcileNode(ctx context.Context, cluster *clusterv1.Clust
124125
}
125126
}
126127

127-
options := []client.PatchOption{
128-
client.FieldOwner("capi-machine"),
129-
client.ForceOwnership,
130-
}
131-
nodePatch := unstructuredNode(node.Name, node.UID, getManagedLabels(machine.Labels))
132-
if err := remoteClient.Patch(ctx, nodePatch, client.Apply, options...); err != nil {
133-
return ctrl.Result{}, errors.Wrap(err, "failed to apply patch label to the node")
128+
updatedNode := unstructuredNode(node.Name, node.UID, getManagedLabels(machine.Labels))
129+
err = ssa.Patch(ctx, remoteClient, machineManagerName, updatedNode, ssa.WithCachingProxy{Cache: r.ssaCache, Original: node})
130+
if err != nil {
131+
return ctrl.Result{}, errors.Wrap(err, "failed to apply labels to Node")
134132
}
135133

136134
// Do the remaining node health checks, then set the node health to true if all checks pass.

‎internal/controllers/machine/machine_controller_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
"sigs.k8s.io/cluster-api/api/v1beta1/index"
4343
"sigs.k8s.io/cluster-api/controllers/remote"
4444
"sigs.k8s.io/cluster-api/internal/test/builder"
45+
"sigs.k8s.io/cluster-api/internal/util/ssa"
4546
"sigs.k8s.io/cluster-api/util"
4647
"sigs.k8s.io/cluster-api/util/conditions"
4748
"sigs.k8s.io/cluster-api/util/patch"
@@ -726,8 +727,9 @@ func TestReconcileRequest(t *testing.T) {
726727
).WithIndex(&corev1.Node{}, index.NodeProviderIDField, index.NodeByProviderID).Build()
727728

728729
r := &Reconciler{
729-
Client: clientFake,
730-
Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), clientFake, scheme.Scheme, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
730+
Client: clientFake,
731+
Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), clientFake, scheme.Scheme, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
732+
ssaCache: ssa.NewCache(),
731733
}
732734

733735
result, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: util.ObjectKey(&tc.machine)})
@@ -972,8 +974,9 @@ func TestMachineConditions(t *testing.T) {
972974
Build()
973975

974976
r := &Reconciler{
975-
Client: clientFake,
976-
Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), clientFake, scheme.Scheme, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
977+
Client: clientFake,
978+
Tracker: remote.NewTestClusterCacheTracker(logr.New(log.NullLogSink{}), clientFake, scheme.Scheme, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
979+
ssaCache: ssa.NewCache(),
977980
}
978981

979982
_, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: util.ObjectKey(&machine)})

‎internal/controllers/machinedeployment/machinedeployment_controller.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ type Reconciler struct {
7070
WatchFilterValue string
7171

7272
recorder record.EventRecorder
73+
ssaCache ssa.Cache
7374
}
7475

7576
func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
@@ -106,6 +107,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
106107
}
107108

108109
r.recorder = mgr.GetEventRecorderFor("machinedeployment-controller")
110+
r.ssaCache = ssa.NewCache()
109111
return nil
110112
}
111113

‎internal/controllers/machinedeployment/machinedeployment_sync.go

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ import (
4040

4141
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
4242
"sigs.k8s.io/cluster-api/internal/controllers/machinedeployment/mdutil"
43+
"sigs.k8s.io/cluster-api/internal/util/hash"
44+
"sigs.k8s.io/cluster-api/internal/util/ssa"
4345
"sigs.k8s.io/cluster-api/util"
4446
"sigs.k8s.io/cluster-api/util/conditions"
4547
"sigs.k8s.io/cluster-api/util/patch"
@@ -153,11 +155,8 @@ func (r *Reconciler) updateMachineSet(ctx context.Context, deployment *clusterv1
153155
}
154156

155157
// Update the MachineSet to propagate in-place mutable fields from the MachineDeployment.
156-
patchOptions := []client.PatchOption{
157-
client.ForceOwnership,
158-
client.FieldOwner(machineDeploymentManagerName),
159-
}
160-
if err := r.Client.Patch(ctx, updatedMS, client.Apply, patchOptions...); err != nil {
158+
err = ssa.Patch(ctx, r.Client, machineDeploymentManagerName, updatedMS, ssa.WithCachingProxy{Cache: r.ssaCache, Original: ms})
159+
if err != nil {
161160
r.recorder.Eventf(deployment, corev1.EventTypeWarning, "FailedUpdate", "Failed to update MachineSet %s: %v", klog.KObj(updatedMS), err)
162161
return nil, errors.Wrapf(err, "failed to update MachineSet %s", klog.KObj(updatedMS))
163162
}
@@ -178,11 +177,7 @@ func (r *Reconciler) createMachineSetAndWait(ctx context.Context, deployment *cl
178177
}
179178

180179
// Create the MachineSet.
181-
patchOptions := []client.PatchOption{
182-
client.ForceOwnership,
183-
client.FieldOwner(machineDeploymentManagerName),
184-
}
185-
if err = r.Client.Patch(ctx, newMS, client.Apply, patchOptions...); err != nil {
180+
if err := ssa.Patch(ctx, r.Client, machineDeploymentManagerName, newMS); err != nil {
186181
r.recorder.Eventf(deployment, corev1.EventTypeWarning, "FailedCreate", "Failed to create MachineSet %s: %v", klog.KObj(newMS), err)
187182
return nil, errors.Wrapf(err, "failed to create new MachineSet %s", klog.KObj(newMS))
188183
}
@@ -237,7 +232,7 @@ func (r *Reconciler) computeDesiredMachineSet(deployment *clusterv1.MachineDeplo
237232
// As a result, we use the hash of the machine template while ignoring all in-place mutable fields, i.e. the
238233
// machine template with only fields that could trigger a rollout for the machine-template-hash, making it
239234
// independent of the changes to any in-place mutable fields.
240-
templateHash, err := mdutil.ComputeSpewHash(mdutil.MachineTemplateDeepCopyRolloutFields(&deployment.Spec.Template))
235+
templateHash, err := hash.Compute(mdutil.MachineTemplateDeepCopyRolloutFields(&deployment.Spec.Template))
241236
if err != nil {
242237
return nil, errors.Wrap(err, "failed to compute desired MachineSet: failed to compute machine template hash")
243238
}

‎internal/controllers/machinedeployment/mdutil/util.go

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,10 @@ package mdutil
1919

2020
import (
2121
"fmt"
22-
"hash"
23-
"hash/fnv"
2422
"sort"
2523
"strconv"
2624
"strings"
2725

28-
"github.com/davecgh/go-spew/spew"
2926
"github.com/go-logr/logr"
3027
"github.com/pkg/errors"
3128
corev1 "k8s.io/api/core/v1"
@@ -688,33 +685,6 @@ func CloneSelectorAndAddLabel(selector *metav1.LabelSelector, labelKey, labelVal
688685
return newSelector
689686
}
690687

691-
// SpewHashObject writes specified object to hash using the spew library
692-
// which follows pointers and prints actual values of the nested objects
693-
// ensuring the hash does not change when a pointer changes.
694-
func SpewHashObject(hasher hash.Hash, objectToWrite interface{}) error {
695-
hasher.Reset()
696-
printer := spew.ConfigState{
697-
Indent: " ",
698-
SortKeys: true,
699-
DisableMethods: true,
700-
SpewKeys: true,
701-
}
702-
703-
if _, err := printer.Fprintf(hasher, "%#v", objectToWrite); err != nil {
704-
return fmt.Errorf("failed to write object to hasher")
705-
}
706-
return nil
707-
}
708-
709-
// ComputeSpewHash computes the hash of a MachineTemplateSpec using the spew library.
710-
func ComputeSpewHash(objectToWrite interface{}) (uint32, error) {
711-
machineTemplateSpecHasher := fnv.New32a()
712-
if err := SpewHashObject(machineTemplateSpecHasher, objectToWrite); err != nil {
713-
return 0, err
714-
}
715-
return machineTemplateSpecHasher.Sum32(), nil
716-
}
717-
718688
// GetDeletingMachineCount gets the number of machines that are in the process of being deleted
719689
// in a machineList.
720690
func GetDeletingMachineCount(machineList *clusterv1.MachineList) int32 {

‎internal/controllers/machinehealthcheck/machinehealthcheck_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2245,7 +2245,7 @@ func TestGetMaxUnhealthy(t *testing.T) {
22452245
func ownerReferenceForCluster(ctx context.Context, g *WithT, c *clusterv1.Cluster) metav1.OwnerReference {
22462246
// Fetch the cluster to populate the UID
22472247
cc := &clusterv1.Cluster{}
2248-
g.Expect(env.GetClient().Get(ctx, util.ObjectKey(c), cc)).To(Succeed())
2248+
g.Expect(env.Get(ctx, util.ObjectKey(c), cc)).To(Succeed())
22492249

22502250
return metav1.OwnerReference{
22512251
APIVersion: clusterv1.GroupVersion.String(),

‎internal/controllers/machineset/machineset_controller.go

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ type Reconciler struct {
8585
// WatchFilterValue is the label value used to filter events prior to reconciliation.
8686
WatchFilterValue string
8787

88+
ssaCache ssa.Cache
8889
recorder record.EventRecorder
8990
}
9091

@@ -122,6 +123,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
122123
}
123124

124125
r.recorder = mgr.GetEventRecorderFor("machineset-controller")
126+
r.ssaCache = ssa.NewCache()
125127
return nil
126128
}
127129

@@ -393,11 +395,8 @@ func (r *Reconciler) syncMachines(ctx context.Context, machineSet *clusterv1.Mac
393395

394396
// Update Machine to propagate in-place mutable fields from the MachineSet.
395397
updatedMachine := r.computeDesiredMachine(machineSet, m)
396-
patchOptions := []client.PatchOption{
397-
client.ForceOwnership,
398-
client.FieldOwner(machineSetManagerName),
399-
}
400-
if err := r.Client.Patch(ctx, updatedMachine, client.Apply, patchOptions...); err != nil {
398+
err := ssa.Patch(ctx, r.Client, machineSetManagerName, updatedMachine, ssa.WithCachingProxy{Cache: r.ssaCache, Original: m})
399+
if err != nil {
401400
log.Error(err, "failed to update Machine", "Machine", klog.KObj(updatedMachine))
402401
return errors.Wrapf(err, "failed to update Machine %q", klog.KObj(updatedMachine))
403402
}
@@ -529,11 +528,7 @@ func (r *Reconciler) syncReplicas(ctx context.Context, ms *clusterv1.MachineSet,
529528
machine.Spec.InfrastructureRef = *infraRef
530529

531530
// Create the Machine.
532-
patchOptions := []client.PatchOption{
533-
client.ForceOwnership,
534-
client.FieldOwner(machineSetManagerName),
535-
}
536-
if err := r.Client.Patch(ctx, machine, client.Apply, patchOptions...); err != nil {
531+
if err := ssa.Patch(ctx, r.Client, machineSetManagerName, machine); err != nil {
537532
log.Error(err, "Error while creating a machine")
538533
r.recorder.Eventf(ms, corev1.EventTypeWarning, "FailedCreate", "Failed to create machine: %v", err)
539534
errs = append(errs, err)
@@ -676,11 +671,7 @@ func (r *Reconciler) updateExternalObject(ctx context.Context, obj client.Object
676671
updatedObject.SetLabels(machineLabelsFromMachineSet(machineSet))
677672
updatedObject.SetAnnotations(machineAnnotationsFromMachineSet(machineSet))
678673

679-
patchOptions := []client.PatchOption{
680-
client.ForceOwnership,
681-
client.FieldOwner(machineSetManagerName),
682-
}
683-
if err := r.Client.Patch(ctx, updatedObject, client.Apply, patchOptions...); err != nil {
674+
if err := ssa.Patch(ctx, r.Client, machineSetManagerName, updatedObject, ssa.WithCachingProxy{Cache: r.ssaCache, Original: obj}); err != nil {
684675
return errors.Wrapf(err, "failed to update %s", klog.KObj(obj))
685676
}
686677
return nil

‎internal/controllers/machineset/machineset_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1175,7 +1175,7 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) {
11751175

11761176
// Run syncMachines to clean up managed fields and have proper field ownership
11771177
// for Machines, InfrastructureMachines and BootstrapConfigs.
1178-
reconciler := &Reconciler{Client: env}
1178+
reconciler := &Reconciler{Client: env, ssaCache: ssa.NewCache()}
11791179
g.Expect(reconciler.syncMachines(ctx, ms, machines)).To(Succeed())
11801180

11811181
// The inPlaceMutatingMachine should have cleaned up managed fields.

‎internal/controllers/topology/cluster/cluster_controller.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import (
4747
"sigs.k8s.io/cluster-api/internal/hooks"
4848
tlog "sigs.k8s.io/cluster-api/internal/log"
4949
runtimeclient "sigs.k8s.io/cluster-api/internal/runtime/client"
50+
"sigs.k8s.io/cluster-api/internal/util/ssa"
5051
"sigs.k8s.io/cluster-api/internal/webhooks"
5152
"sigs.k8s.io/cluster-api/util"
5253
"sigs.k8s.io/cluster-api/util/annotations"
@@ -118,7 +119,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
118119
r.patchEngine = patches.NewEngine(r.RuntimeClient)
119120
r.recorder = mgr.GetEventRecorderFor("topology/cluster")
120121
if r.patchHelperFactory == nil {
121-
r.patchHelperFactory = serverSideApplyPatchHelperFactory(r.Client)
122+
r.patchHelperFactory = serverSideApplyPatchHelperFactory(r.Client, ssa.NewCache())
122123
}
123124
return nil
124125
}
@@ -394,9 +395,9 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
394395
}
395396

396397
// serverSideApplyPatchHelperFactory makes use of managed fields provided by server side apply and is used by the controller.
397-
func serverSideApplyPatchHelperFactory(c client.Client) structuredmerge.PatchHelperFactoryFunc {
398+
func serverSideApplyPatchHelperFactory(c client.Client, ssaCache ssa.Cache) structuredmerge.PatchHelperFactoryFunc {
398399
return func(ctx context.Context, original, modified client.Object, opts ...structuredmerge.HelperOption) (structuredmerge.PatchHelper, error) {
399-
return structuredmerge.NewServerSidePatchHelper(ctx, original, modified, c, opts...)
400+
return structuredmerge.NewServerSidePatchHelper(ctx, original, modified, c, ssaCache, opts...)
400401
}
401402
}
402403

0 commit comments

Comments
 (0)