From 17c3ba733d1c4cbc17c447790b3f34ab2d385ae7 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Thu, 9 Jul 2026 14:03:10 +0200 Subject: [PATCH 1/2] Use patch for controller resource updates --- .../autoscalinglistener_controller.go | 12 ++++++------ .../ephemeralrunnerset_controller.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index 7d12f895d6..223cb56e46 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -178,7 +178,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. if shouldUpdate { log.Info("Updating listener service account") - if err := r.Update(ctx, updatedServiceAccount); err != nil { + if err := r.Patch(ctx, updatedServiceAccount, client.MergeFrom(&serviceAccount)); err != nil { log.Error(err, "Failed to update listener service account") return ctrl.Result{}, err } @@ -225,7 +225,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. } if shouldUpdate { log.Info("Updating listener role") - if err := r.Update(ctx, updatedRole); err != nil { + if err := r.Patch(ctx, updatedRole, client.MergeFrom(&listenerRole)); err != nil { log.Error(err, "Failed to update listener role") return ctrl.Result{}, err } @@ -264,7 +264,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. } if shouldUpdate { log.Info("Updating listener role binding") - if err := r.Update(ctx, updatedRoleBinding); err != nil { + if err := r.Patch(ctx, updatedRoleBinding, client.MergeFrom(&listenerRoleBinding)); err != nil { log.Error(err, "Failed to update listener role binding") return ctrl.Result{}, err } @@ -320,7 +320,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. } if shouldUpdate { log.Info("Updating listener proxy secret") - if err := r.Update(ctx, updatedProxySecret); err != nil { + if err := r.Patch(ctx, updatedProxySecret, client.MergeFrom(&proxySecret)); err != nil { log.Error(err, "Failed to update listener proxy secret") return ctrl.Result{}, err } @@ -408,7 +408,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. if shouldUpdate { log.Info("Updating listener config secret", "namespace", updatedSecret.Namespace, "name", updatedSecret.Name) - if err := r.Update(ctx, updatedSecret); err != nil { + if err := r.Patch(ctx, updatedSecret, client.MergeFrom(&listenerConfigSecret)); err != nil { return ctrl.Result{}, fmt.Errorf("failed to update listener config secret: %w", err) } return ctrl.Result{Requeue: true}, nil @@ -493,7 +493,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. if shouldUpdate { log.Info("Updating listener pod", "namespace", updatedPod.Namespace, "name", updatedPod.Name) - if err := r.Update(ctx, updatedPod); err != nil { + if err := r.Patch(ctx, updatedPod, client.MergeFrom(&listenerPod)); err != nil { log.Error(err, "Unable to update listener pod", "namespace", updatedPod.Namespace, "name", updatedPod.Name) return ctrl.Result{}, err } diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller.go b/controllers/actions.github.com/ephemeralrunnerset_controller.go index 381c9d29de..f362dd6443 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller.go @@ -514,7 +514,7 @@ func (r *EphemeralRunnerSetReconciler) reconcileEphemeralRunnerSetProxySecret(ct } if shouldUpdate { log.Info("Updating ephemeralRunnerSet proxy secret") - if err := r.Update(ctx, updatedProxySecret); err != nil { + if err := r.Patch(ctx, updatedProxySecret, client.MergeFrom(&proxySecret)); err != nil { return nil, false, fmt.Errorf("failed to update ephemeralRunnerSet proxy secret: %w", err) } return updatedProxySecret, true, nil From aa6adb080e584dab72cf3d017e0acc580467c391 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Thu, 9 Jul 2026 14:39:54 +0200 Subject: [PATCH 2/2] Avoid unnecessary deep copies before patching --- .../autoscalinglistener_controller.go | 134 +++++++++--------- .../ephemeralrunnerset_controller.go | 29 ++-- 2 files changed, 78 insertions(+), 85 deletions(-) diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index 223cb56e46..c0aa81114c 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -163,19 +163,18 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, err } - updatedServiceAccount := serviceAccount.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(serviceAccount.Labels, desiredServiceAccount.Labels) - if !maps.Equal(serviceAccount.Labels, desiredLabels) { - updatedServiceAccount.Labels = desiredLabels - shouldUpdate = true - } + labelsModified := !maps.Equal(serviceAccount.Labels, desiredLabels) desiredAnnotations := r.mergeAnnotations(serviceAccount.Annotations, desiredServiceAccount.Annotations) - if !maps.Equal(serviceAccount.Annotations, desiredAnnotations) { - updatedServiceAccount.Annotations = desiredAnnotations - shouldUpdate = true - } - if shouldUpdate { + annotationsModified := !maps.Equal(serviceAccount.Annotations, desiredAnnotations) + if labelsModified || annotationsModified { + updatedServiceAccount := serviceAccount.DeepCopy() + if labelsModified { + updatedServiceAccount.Labels = desiredLabels + } + if annotationsModified { + updatedServiceAccount.Annotations = desiredAnnotations + } log.Info("Updating listener service account") if err := r.Patch(ctx, updatedServiceAccount, client.MergeFrom(&serviceAccount)); err != nil { @@ -207,23 +206,22 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. switch { case err == nil: desiredRole := r.newScaleSetListenerRole(&autoscalingListener) - updatedRole := listenerRole.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(listenerRole.Labels, desiredRole.Labels) - if !maps.Equal(listenerRole.Labels, desiredLabels) { - updatedRole.Labels = desiredLabels - shouldUpdate = true - } + labelsModified := !maps.Equal(listenerRole.Labels, desiredLabels) desiredAnnotations := r.mergeAnnotations(listenerRole.Annotations, desiredRole.Annotations) - if !maps.Equal(listenerRole.Annotations, desiredAnnotations) { - updatedRole.Annotations = desiredAnnotations - shouldUpdate = true - } - if !reflect.DeepEqual(listenerRole.Rules, desiredRole.Rules) { - updatedRole.Rules = desiredRole.Rules - shouldUpdate = true - } - if shouldUpdate { + annotationsModified := !maps.Equal(listenerRole.Annotations, desiredAnnotations) + rulesModified := !reflect.DeepEqual(listenerRole.Rules, desiredRole.Rules) + if labelsModified || annotationsModified || rulesModified { + updatedRole := listenerRole.DeepCopy() + if labelsModified { + updatedRole.Labels = desiredLabels + } + if annotationsModified { + updatedRole.Annotations = desiredAnnotations + } + if rulesModified { + updatedRole.Rules = desiredRole.Rules + } log.Info("Updating listener role") if err := r.Patch(ctx, updatedRole, client.MergeFrom(&listenerRole)); err != nil { log.Error(err, "Failed to update listener role") @@ -250,19 +248,18 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. &listenerRole, &serviceAccount, ) - updatedRoleBinding := listenerRoleBinding.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(listenerRoleBinding.Labels, desiredRoleBinding.Labels) - if !maps.Equal(listenerRoleBinding.Labels, desiredLabels) { - updatedRoleBinding.Labels = desiredLabels - shouldUpdate = true - } + labelsModified := !maps.Equal(listenerRoleBinding.Labels, desiredLabels) desiredAnnotations := r.mergeAnnotations(listenerRoleBinding.Annotations, desiredRoleBinding.Annotations) - if !maps.Equal(listenerRoleBinding.Annotations, desiredAnnotations) { - updatedRoleBinding.Annotations = desiredAnnotations - shouldUpdate = true - } - if shouldUpdate { + annotationsModified := !maps.Equal(listenerRoleBinding.Annotations, desiredAnnotations) + if labelsModified || annotationsModified { + updatedRoleBinding := listenerRoleBinding.DeepCopy() + if labelsModified { + updatedRoleBinding.Labels = desiredLabels + } + if annotationsModified { + updatedRoleBinding.Annotations = desiredAnnotations + } log.Info("Updating listener role binding") if err := r.Patch(ctx, updatedRoleBinding, client.MergeFrom(&listenerRoleBinding)); err != nil { log.Error(err, "Failed to update listener role binding") @@ -306,19 +303,18 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. log.Error(err, "Failed to build desired listener proxy secret") return ctrl.Result{}, err } - updatedProxySecret := proxySecret.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(proxySecret.Labels, desiredListenerProxy.Labels) - if !maps.Equal(proxySecret.Labels, desiredLabels) { - updatedProxySecret.Labels = desiredLabels - shouldUpdate = true - } + labelsModified := !maps.Equal(proxySecret.Labels, desiredLabels) desiredAnnotations := r.mergeAnnotations(proxySecret.Annotations, desiredListenerProxy.Annotations) - if !maps.Equal(proxySecret.Annotations, desiredAnnotations) { - updatedProxySecret.Annotations = desiredAnnotations - shouldUpdate = true - } - if shouldUpdate { + annotationsModified := !maps.Equal(proxySecret.Annotations, desiredAnnotations) + if labelsModified || annotationsModified { + updatedProxySecret := proxySecret.DeepCopy() + if labelsModified { + updatedProxySecret.Labels = desiredLabels + } + if annotationsModified { + updatedProxySecret.Annotations = desiredAnnotations + } log.Info("Updating listener proxy secret") if err := r.Patch(ctx, updatedProxySecret, client.MergeFrom(&proxySecret)); err != nil { log.Error(err, "Failed to update listener proxy secret") @@ -393,20 +389,19 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. if err != nil { return ctrl.Result{}, fmt.Errorf("failed to build listener config secret: %w", err) } - updatedSecret := listenerConfigSecret.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(listenerConfigSecret.Labels, desiredSecret.Labels) - if !maps.Equal(listenerConfigSecret.Labels, desiredLabels) { - updatedSecret.Labels = desiredLabels - shouldUpdate = true - } + labelsModified := !maps.Equal(listenerConfigSecret.Labels, desiredLabels) desiredAnnotations := r.mergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations) - if !maps.Equal(listenerConfigSecret.Annotations, desiredAnnotations) { - updatedSecret.Annotations = desiredAnnotations - shouldUpdate = true - } + annotationsModified := !maps.Equal(listenerConfigSecret.Annotations, desiredAnnotations) - if shouldUpdate { + if labelsModified || annotationsModified { + updatedSecret := listenerConfigSecret.DeepCopy() + if labelsModified { + updatedSecret.Labels = desiredLabels + } + if annotationsModified { + updatedSecret.Annotations = desiredAnnotations + } log.Info("Updating listener config secret", "namespace", updatedSecret.Namespace, "name", updatedSecret.Name) if err := r.Patch(ctx, updatedSecret, client.MergeFrom(&listenerConfigSecret)); err != nil { return ctrl.Result{}, fmt.Errorf("failed to update listener config secret: %w", err) @@ -478,20 +473,19 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, nil } - updatedPod := listenerPod.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(listenerPod.Labels, desiredPod.Labels) - if !maps.Equal(listenerPod.Labels, desiredLabels) { - updatedPod.Labels = desiredLabels - shouldUpdate = true - } + labelsModified := !maps.Equal(listenerPod.Labels, desiredLabels) desiredAnnotations := r.mergeAnnotations(listenerPod.Annotations, desiredPod.Annotations) - if !maps.Equal(listenerPod.Annotations, desiredAnnotations) { - updatedPod.Annotations = desiredAnnotations - shouldUpdate = true - } + annotationsModified := !maps.Equal(listenerPod.Annotations, desiredAnnotations) - if shouldUpdate { + if labelsModified || annotationsModified { + updatedPod := listenerPod.DeepCopy() + if labelsModified { + updatedPod.Labels = desiredLabels + } + if annotationsModified { + updatedPod.Annotations = desiredAnnotations + } log.Info("Updating listener pod", "namespace", updatedPod.Namespace, "name", updatedPod.Name) if err := r.Patch(ctx, updatedPod, client.MergeFrom(&listenerPod)); err != nil { log.Error(err, "Unable to update listener pod", "namespace", updatedPod.Namespace, "name", updatedPod.Name) diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller.go b/controllers/actions.github.com/ephemeralrunnerset_controller.go index f362dd6443..92faae855f 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller.go @@ -496,23 +496,22 @@ func (r *EphemeralRunnerSetReconciler) reconcileEphemeralRunnerSetProxySecret(ct return nil, false, fmt.Errorf("failed to build desired ephemeralRunnerSet proxy secret: %w", err) } - updatedProxySecret := proxySecret.DeepCopy() - var shouldUpdate bool - if !maps.EqualFunc(proxySecret.Data, desiredRunnerSetProxy.Data, bytes.Equal) { - updatedProxySecret.Data = desiredRunnerSetProxy.Data - shouldUpdate = true - } + dataModified := !maps.EqualFunc(proxySecret.Data, desiredRunnerSetProxy.Data, bytes.Equal) desiredLabels := r.filterAndMergeLabels(proxySecret.Labels, desiredRunnerSetProxy.Labels) - if !maps.Equal(proxySecret.Labels, desiredLabels) { - updatedProxySecret.Labels = desiredLabels - shouldUpdate = true - } + labelsModified := !maps.Equal(proxySecret.Labels, desiredLabels) desiredAnnotations := r.mergeAnnotations(proxySecret.Annotations, desiredRunnerSetProxy.Annotations) - if !maps.Equal(proxySecret.Annotations, desiredAnnotations) { - updatedProxySecret.Annotations = desiredAnnotations - shouldUpdate = true - } - if shouldUpdate { + annotationsModified := !maps.Equal(proxySecret.Annotations, desiredAnnotations) + if dataModified || labelsModified || annotationsModified { + updatedProxySecret := proxySecret.DeepCopy() + if dataModified { + updatedProxySecret.Data = desiredRunnerSetProxy.Data + } + if labelsModified { + updatedProxySecret.Labels = desiredLabels + } + if annotationsModified { + updatedProxySecret.Annotations = desiredAnnotations + } log.Info("Updating ephemeralRunnerSet proxy secret") if err := r.Patch(ctx, updatedProxySecret, client.MergeFrom(&proxySecret)); err != nil { return nil, false, fmt.Errorf("failed to update ephemeralRunnerSet proxy secret: %w", err)