Skip to content

Commit 0dd33b8

Browse files
committed
CM-1039: Thread context.Context from Reconcile() through controller helpers
Both istiocsr and trustmanager controllers stored a context.Context field on their Reconciler struct, initialized once in New(). The Reconcile() method receives a request-scoped context from controller-runtime but all helper methods used the stale struct field instead. This defeats cancellation and deadline propagation from the framework. Remove the ctx field from both Reconciler structs and thread the context parameter from Reconcile() through every helper method call chain.
1 parent 5608078 commit 0dd33b8

37 files changed

Lines changed: 240 additions & 233 deletions

pkg/controller/istiocsr/certificates.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package istiocsr
22

33
import (
4+
"context"
45
"fmt"
56
"maps"
67

@@ -16,7 +17,7 @@ import (
1617
"github.com/openshift/cert-manager-operator/pkg/operator/assets"
1718
)
1819

19-
func (r *Reconciler) createOrApplyCertificates(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string, istioCSRCreateRecon bool) error {
20+
func (r *Reconciler) createOrApplyCertificates(ctx context.Context, istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string, istioCSRCreateRecon bool) error {
2021
desired, err := r.getCertificateObject(istiocsr, resourceLabels)
2122
if err != nil {
2223
return fmt.Errorf("failed to generate certificate resource for creation in %s: %w", istiocsr.GetNamespace(), err)
@@ -25,7 +26,7 @@ func (r *Reconciler) createOrApplyCertificates(istiocsr *v1alpha1.IstioCSR, reso
2526
certificateName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
2627
r.log.V(4).Info("reconciling certificate resource", "name", certificateName)
2728
fetched := &certmanagerv1.Certificate{}
28-
exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
29+
exist, err := r.Exists(ctx, client.ObjectKeyFromObject(desired), fetched)
2930
if err != nil {
3031
return common.FromClientError(err, "failed to check %s certificate resource already exists", certificateName)
3132
}
@@ -36,7 +37,7 @@ func (r *Reconciler) createOrApplyCertificates(istiocsr *v1alpha1.IstioCSR, reso
3637
}
3738
if hasObjectChanged(desired, fetched) {
3839
r.log.V(1).Info("certificate has been modified, updating to desired state", "name", certificateName)
39-
if err := r.UpdateWithRetry(r.ctx, desired); err != nil {
40+
if err := r.UpdateWithRetry(ctx, desired); err != nil {
4041
return common.FromClientError(err, "failed to update %s certificate resource", certificateName)
4142
}
4243
r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "certificate resource %s reconciled back to desired state", certificateName)
@@ -46,7 +47,7 @@ func (r *Reconciler) createOrApplyCertificates(istiocsr *v1alpha1.IstioCSR, reso
4647
}
4748

4849
if !exist {
49-
if err := r.Create(r.ctx, desired); err != nil {
50+
if err := r.Create(ctx, desired); err != nil {
5051
return common.FromClientError(err, "failed to create %s certificate resource", certificateName)
5152
}
5253
r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "certificate resource %s created", certificateName)

pkg/controller/istiocsr/certificates_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ func TestCreateOrApplyCertificates(t *testing.T) {
223223
}, istiocsr); err != nil {
224224
t.Errorf("test error: %v", err)
225225
}
226-
err := r.createOrApplyCertificates(istiocsr, controllerDefaultResourceLabels, false)
226+
err := r.createOrApplyCertificates(context.Background(), istiocsr, controllerDefaultResourceLabels, false)
227227
if (tt.wantErr != "" || err != nil) && (err == nil || err.Error() != tt.wantErr) {
228228
t.Errorf("createOrApplyCertificates() err: %v, wantErr: %v", err, tt.wantErr)
229229
}

pkg/controller/istiocsr/controller.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ const RequestEnqueueLabelValue = "cert-manager-istio-csr"
3939
type Reconciler struct {
4040
common.CtrlClient
4141

42-
ctx context.Context
4342
eventRecorder record.EventRecorder
4443
log logr.Logger
4544
scheme *runtime.Scheme
@@ -58,7 +57,6 @@ func New(mgr ctrl.Manager) (*Reconciler, error) {
5857
}
5958
return &Reconciler{
6059
CtrlClient: c,
61-
ctx: context.Background(),
6260
eventRecorder: mgr.GetEventRecorderFor(ControllerName),
6361
log: ctrl.Log.WithName(ControllerName),
6462
scheme: mgr.GetScheme(),
@@ -178,7 +176,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
178176
if !istiocsr.DeletionTimestamp.IsZero() {
179177
r.log.V(1).Info("istiocsr.openshift.operator.io is marked for deletion", "namespace", req.NamespacedName)
180178

181-
if requeue, err := r.cleanUp(istiocsr); err != nil {
179+
if requeue, err := r.cleanUp(ctx, istiocsr); err != nil {
182180
return ctrl.Result{}, fmt.Errorf("clean up failed for %q istiocsr.openshift.operator.io instance deletion: %w", req.NamespacedName, err)
183181
} else if requeue {
184182
return ctrl.Result{RequeueAfter: defaultRequeueTime}, nil
@@ -197,25 +195,25 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
197195
return ctrl.Result{}, fmt.Errorf("failed to update %q istiocsr.openshift.operator.io with finalizers: %w", req.NamespacedName, err)
198196
}
199197

200-
return r.processReconcileRequest(istiocsr, req.NamespacedName)
198+
return r.processReconcileRequest(ctx, istiocsr, req.NamespacedName)
201199
}
202200

203-
func (r *Reconciler) processReconcileRequest(istiocsr *v1alpha1.IstioCSR, req types.NamespacedName) (ctrl.Result, error) {
201+
func (r *Reconciler) processReconcileRequest(ctx context.Context, istiocsr *v1alpha1.IstioCSR, req types.NamespacedName) (ctrl.Result, error) {
204202
istioCSRCreateRecon := false
205203
if !containsProcessedAnnotation(istiocsr) && reflect.DeepEqual(istiocsr.Status, v1alpha1.IstioCSRStatus{}) {
206204
r.log.V(1).Info("starting reconciliation of newly created istiocsr", "namespace", istiocsr.GetNamespace(), "name", istiocsr.GetName())
207205
istioCSRCreateRecon = true
208206
}
209207

210-
if err := r.disallowMultipleIstioCSRInstances(istiocsr); err != nil {
208+
if err := r.disallowMultipleIstioCSRInstances(ctx, istiocsr); err != nil {
211209
if common.IsMultipleInstanceError(err) {
212210
r.eventRecorder.Eventf(istiocsr, corev1.EventTypeWarning, "MultiIstioCSRInstance", "creation of multiple istiocsr instances is not supported, will not be processed")
213211
err = nil
214212
}
215213
return ctrl.Result{}, err
216214
}
217215

218-
reconcileErr := r.reconcileIstioCSRDeployment(istiocsr, istioCSRCreateRecon)
216+
reconcileErr := r.reconcileIstioCSRDeployment(ctx, istiocsr, istioCSRCreateRecon)
219217
if reconcileErr != nil {
220218
r.log.Error(reconcileErr, "failed to reconcile IstioCSR deployment", "request", req)
221219
}
@@ -225,7 +223,7 @@ func (r *Reconciler) processReconcileRequest(istiocsr *v1alpha1.IstioCSR, req ty
225223
reconcileErr,
226224
r.log.WithValues("namespace", istiocsr.GetNamespace(), "name", istiocsr.GetName()),
227225
func(prependErr error) error {
228-
return r.updateCondition(istiocsr, prependErr)
226+
return r.updateCondition(ctx, istiocsr, prependErr)
229227
},
230228
defaultRequeueTime,
231229
)
@@ -234,7 +232,7 @@ func (r *Reconciler) processReconcileRequest(istiocsr *v1alpha1.IstioCSR, req ty
234232
// cleanUp handles deletion of istiocsr.openshift.operator.io gracefully.
235233
//
236234
//nolint:unparam // error return is kept for future implementation
237-
func (r *Reconciler) cleanUp(istiocsr *v1alpha1.IstioCSR) (bool, error) {
235+
func (r *Reconciler) cleanUp(_ context.Context, istiocsr *v1alpha1.IstioCSR) (bool, error) {
238236
// TODO: For GA, handle cleaning up of resources created for installing istio-csr operand.
239237
// This might require a validation webhook to check for usage of service as GRPC endpoint in
240238
// any of OpenShift Service Mesh or Istiod deployments to avoid disruptions across cluster.

pkg/controller/istiocsr/controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,7 @@ func TestProcessReconcileRequest(t *testing.T) {
759759
}
760760
r.CtrlClient = mock
761761
istiocsr := tt.getIstioCSR()
762-
_, err := r.processReconcileRequest(istiocsr,
762+
_, err := r.processReconcileRequest(context.Background(), istiocsr,
763763
types.NamespacedName{Name: istiocsr.GetName(), Namespace: istiocsr.GetNamespace()})
764764
if (tt.wantErr != "" || err != nil) && (err == nil || err.Error() != tt.wantErr) {
765765
t.Errorf("processReconcileRequest() err: %v, wantErr: %v", err, tt.wantErr)

0 commit comments

Comments
 (0)