Skip to content

Commit 545eb37

Browse files
author
Eric Stroczynski
authored
internal/{helm,ansible}/controller: use correct context in client.Client calls (#4474)
internal/{helm,ansible}/controller: use Reconcile()'s ctx argument in Client method calls Signed-off-by: Eric Stroczynski <[email protected]>
1 parent b05a68b commit 545eb37

File tree

2 files changed

+49
-38
lines changed

2 files changed

+49
-38
lines changed

‎internal/ansible/controller/reconcile.go‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func (r *AnsibleOperatorReconciler) Reconcile(ctx context.Context, request recon
8787
duration, err := time.ParseDuration(ds)
8888
if err != nil {
8989
// Should attempt to update to a failed condition
90-
errmark := r.markError(u, request.NamespacedName,
90+
errmark := r.markError(ctx, request.NamespacedName, u,
9191
fmt.Sprintf("Unable to parse reconcile period annotation: %v", err))
9292
if errmark != nil {
9393
logger.Error(errmark, "Unable to mark error annotation")
@@ -129,7 +129,7 @@ func (r *AnsibleOperatorReconciler) Reconcile(ctx context.Context, request recon
129129
}
130130

131131
if r.ManageStatus {
132-
errmark := r.markRunning(u, request.NamespacedName)
132+
errmark := r.markRunning(ctx, request.NamespacedName, u)
133133
if errmark != nil {
134134
logger.Error(errmark, "Unable to update the status to mark cr as running")
135135
return reconcileResult, errmark
@@ -145,7 +145,7 @@ func (r *AnsibleOperatorReconciler) Reconcile(ctx context.Context, request recon
145145

146146
kc, err := kubeconfig.Create(ownerRef, "http://localhost:8888", u.GetNamespace())
147147
if err != nil {
148-
errmark := r.markError(u, request.NamespacedName, "Unable to run reconciliation")
148+
errmark := r.markError(ctx, request.NamespacedName, u, "Unable to run reconciliation")
149149
if errmark != nil {
150150
logger.Error(errmark, "Unable to mark error to run reconciliation")
151151
}
@@ -159,7 +159,7 @@ func (r *AnsibleOperatorReconciler) Reconcile(ctx context.Context, request recon
159159
}()
160160
result, err := r.Runner.Run(ident, u, kc.Name())
161161
if err != nil {
162-
errmark := r.markError(u, request.NamespacedName, "Unable to run reconciliation")
162+
errmark := r.markError(ctx, request.NamespacedName, u, "Unable to run reconciliation")
163163
if errmark != nil {
164164
logger.Error(errmark, "Unable to mark error to run reconciliation")
165165
}
@@ -219,7 +219,7 @@ func (r *AnsibleOperatorReconciler) Reconcile(ctx context.Context, request recon
219219
eventErr := errors.New("did not receive playbook_on_stats event")
220220
stdout, err := result.Stdout()
221221
if err != nil {
222-
errmark := r.markError(u, request.NamespacedName, "Failed to get ansible-runner stdout")
222+
errmark := r.markError(ctx, request.NamespacedName, u, "Failed to get ansible-runner stdout")
223223
if errmark != nil {
224224
logger.Error(errmark, "Unable to mark error to run reconciliation")
225225
}
@@ -256,14 +256,14 @@ func (r *AnsibleOperatorReconciler) Reconcile(ctx context.Context, request recon
256256
}
257257
}
258258
u.SetFinalizers(finalizers)
259-
err := r.Client.Update(context.TODO(), u)
259+
err := r.Client.Update(ctx, u)
260260
if err != nil {
261261
logger.Error(err, "Failed to remove finalizer")
262262
return reconcileResult, err
263263
}
264264
}
265265
if r.ManageStatus {
266-
errmark := r.markDone(u, request.NamespacedName, statusEvent, failureMessages)
266+
errmark := r.markDone(ctx, request.NamespacedName, u, statusEvent, failureMessages)
267267
if errmark != nil {
268268
logger.Error(errmark, "Failed to mark status done")
269269
}
@@ -299,11 +299,10 @@ func (r *AnsibleOperatorReconciler) printAnsibleResult(result runner.RunResult)
299299
}
300300
}
301301

302-
func (r *AnsibleOperatorReconciler) markRunning(u *unstructured.Unstructured,
303-
namespacedName types.NamespacedName) error {
302+
func (r *AnsibleOperatorReconciler) markRunning(ctx context.Context, nn types.NamespacedName, u *unstructured.Unstructured) error {
304303

305304
// Get the latest resource to prevent updating a stale status.
306-
if err := r.APIReader.Get(context.TODO(), namespacedName, u); err != nil {
305+
if err := r.APIReader.Get(ctx, nn, u); err != nil {
307306
return err
308307
}
309308
crStatus := getStatus(u)
@@ -327,19 +326,20 @@ func (r *AnsibleOperatorReconciler) markRunning(u *unstructured.Unstructured,
327326
ansiblestatus.SetCondition(&crStatus, *c)
328327
u.Object["status"] = crStatus.GetJSONMap()
329328

330-
return r.Client.Status().Update(context.TODO(), u)
329+
return r.Client.Status().Update(ctx, u)
331330
}
332331

333332
// markError - used to alert the user to the issues during the validation of a reconcile run.
334333
// i.e Annotations that could be incorrect
335-
func (r *AnsibleOperatorReconciler) markError(u *unstructured.Unstructured, namespacedName types.NamespacedName,
334+
func (r *AnsibleOperatorReconciler) markError(ctx context.Context, nn types.NamespacedName, u *unstructured.Unstructured,
336335
failureMessage string) error {
336+
337337
logger := logf.Log.WithName("markError")
338338
// Immediately update metrics with failed reconciliation, since Get()
339339
// may fail.
340340
metrics.ReconcileFailed(r.GVK.String())
341341
// Get the latest resource to prevent updating a stale status.
342-
if err := r.APIReader.Get(context.TODO(), namespacedName, u); err != nil {
342+
if err := r.APIReader.Get(ctx, nn, u); err != nil {
343343
if apierrors.IsNotFound(err) {
344344
logger.Info("Resource not found, assuming it was deleted")
345345
return nil
@@ -365,14 +365,15 @@ func (r *AnsibleOperatorReconciler) markError(u *unstructured.Unstructured, name
365365
// This needs the status subresource to be enabled by default.
366366
u.Object["status"] = crStatus.GetJSONMap()
367367

368-
return r.Client.Status().Update(context.TODO(), u)
368+
return r.Client.Status().Update(ctx, u)
369369
}
370370

371-
func (r *AnsibleOperatorReconciler) markDone(u *unstructured.Unstructured, namespacedName types.NamespacedName,
371+
func (r *AnsibleOperatorReconciler) markDone(ctx context.Context, nn types.NamespacedName, u *unstructured.Unstructured,
372372
statusEvent eventapi.StatusJobEvent, failureMessages eventapi.FailureMessages) error {
373+
373374
logger := logf.Log.WithName("markDone")
374375
// Get the latest resource to prevent updating a stale status.
375-
if err := r.APIReader.Get(context.TODO(), namespacedName, u); err != nil {
376+
if err := r.APIReader.Get(ctx, nn, u); err != nil {
376377
if apierrors.IsNotFound(err) {
377378
logger.Info("Resource not found, assuming it was deleted")
378379
return nil
@@ -415,7 +416,7 @@ func (r *AnsibleOperatorReconciler) markDone(u *unstructured.Unstructured, names
415416
// This needs the status subresource to be enabled by default.
416417
u.Object["status"] = crStatus.GetJSONMap()
417418

418-
return r.Client.Status().Update(context.TODO(), u)
419+
return r.Client.Status().Update(ctx, u)
419420
}
420421

421422
func contains(l []string, s string) bool {

‎internal/helm/controller/reconcile.go‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
110110
Reason: types.ReasonUninstallError,
111111
Message: err.Error(),
112112
})
113-
_ = r.updateResourceStatus(o, status)
113+
if err := r.updateResourceStatus(ctx, o, status); err != nil {
114+
log.Error(err, "Failed to update status after uninstall release failure")
115+
}
114116
return reconcile.Result{}, err
115117
}
116118
status.RemoveCondition(types.ConditionReleaseFailed)
@@ -129,13 +131,13 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
129131
})
130132
status.DeployedRelease = nil
131133
}
132-
if err := r.updateResourceStatus(o, status); err != nil {
134+
if err := r.updateResourceStatus(ctx, o, status); err != nil {
133135
log.Info("Failed to update CR status")
134136
return reconcile.Result{}, err
135137
}
136138

137139
controllerutil.RemoveFinalizer(o, finalizer)
138-
if err := r.updateResource(o); err != nil {
140+
if err := r.updateResource(ctx, o); err != nil {
139141
log.Info("Failed to remove CR uninstall finalizer")
140142
return reconcile.Result{}, err
141143
}
@@ -144,7 +146,7 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
144146
// deletion here will guarantee that the next reconciliation
145147
// will see that the CR has been deleted and that there's
146148
// nothing left to do.
147-
if err := r.waitForDeletion(o); err != nil {
149+
if err := r.waitForDeletion(ctx, o); err != nil {
148150
log.Info("Failed waiting for CR deletion")
149151
return reconcile.Result{}, err
150152
}
@@ -165,7 +167,9 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
165167
Reason: types.ReasonReconcileError,
166168
Message: err.Error(),
167169
})
168-
_ = r.updateResourceStatus(o, status)
170+
if err := r.updateResourceStatus(ctx, o, status); err != nil {
171+
log.Error(err, "Failed to update status after sync release failure")
172+
}
169173
return reconcile.Result{}, err
170174
}
171175
status.RemoveCondition(types.ConditionIrreconcilable)
@@ -184,14 +188,16 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
184188
Reason: types.ReasonInstallError,
185189
Message: err.Error(),
186190
})
187-
_ = r.updateResourceStatus(o, status)
191+
if err := r.updateResourceStatus(ctx, o, status); err != nil {
192+
log.Error(err, "Failed to update status after insatll release failure")
193+
}
188194
return reconcile.Result{}, err
189195
}
190196
status.RemoveCondition(types.ConditionReleaseFailed)
191197

192198
log.V(1).Info("Adding finalizer", "finalizer", finalizer)
193199
controllerutil.AddFinalizer(o, finalizer)
194-
if err := r.updateResource(o); err != nil {
200+
if err := r.updateResource(ctx, o); err != nil {
195201
log.Info("Failed to add CR uninstall finalizer")
196202
return reconcile.Result{}, err
197203
}
@@ -222,14 +228,14 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
222228
Name: installedRelease.Name,
223229
Manifest: installedRelease.Manifest,
224230
}
225-
err = r.updateResourceStatus(o, status)
231+
err = r.updateResourceStatus(ctx, o, status)
226232
return reconcile.Result{RequeueAfter: r.ReconcilePeriod}, err
227233
}
228234

229235
if !contains(o.GetFinalizers(), finalizer) {
230236
log.V(1).Info("Adding finalizer", "finalizer", finalizer)
231237
controllerutil.AddFinalizer(o, finalizer)
232-
if err := r.updateResource(o); err != nil {
238+
if err := r.updateResource(ctx, o); err != nil {
233239
log.Info("Failed to add CR uninstall finalizer")
234240
return reconcile.Result{}, err
235241
}
@@ -250,7 +256,9 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
250256
Reason: types.ReasonUpgradeError,
251257
Message: err.Error(),
252258
})
253-
_ = r.updateResourceStatus(o, status)
259+
if err := r.updateResourceStatus(ctx, o, status); err != nil {
260+
log.Error(err, "Failed to update status after sync release failure")
261+
}
254262
return reconcile.Result{}, err
255263
}
256264
status.RemoveCondition(types.ConditionReleaseFailed)
@@ -281,7 +289,7 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
281289
Name: upgradedRelease.Name,
282290
Manifest: upgradedRelease.Manifest,
283291
}
284-
err = r.updateResourceStatus(o, status)
292+
err = r.updateResourceStatus(ctx, o, status)
285293
return reconcile.Result{RequeueAfter: r.ReconcilePeriod}, err
286294
}
287295

@@ -302,7 +310,9 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
302310
Reason: types.ReasonReconcileError,
303311
Message: err.Error(),
304312
})
305-
_ = r.updateResourceStatus(o, status)
313+
if err := r.updateResourceStatus(ctx, o, status); err != nil {
314+
log.Error(err, "Failed to update status after reconcile release failure")
315+
}
306316
return reconcile.Result{}, err
307317
}
308318
status.RemoveCondition(types.ConditionIrreconcilable)
@@ -333,7 +343,7 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
333343
Name: expectedRelease.Name,
334344
Manifest: expectedRelease.Manifest,
335345
}
336-
err = r.updateResourceStatus(o, status)
346+
err = r.updateResourceStatus(ctx, o, status)
337347
return reconcile.Result{RequeueAfter: r.ReconcilePeriod}, err
338348
}
339349

@@ -355,34 +365,34 @@ func hasHelmUpgradeForceAnnotation(o *unstructured.Unstructured) bool {
355365
return value
356366
}
357367

358-
func (r HelmOperatorReconciler) updateResource(o client.Object) error {
368+
func (r HelmOperatorReconciler) updateResource(ctx context.Context, o client.Object) error {
359369
return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
360-
return r.Client.Update(context.TODO(), o)
370+
return r.Client.Update(ctx, o)
361371
})
362372
}
363373

364-
func (r HelmOperatorReconciler) updateResourceStatus(o *unstructured.Unstructured, status *types.HelmAppStatus) error {
374+
func (r HelmOperatorReconciler) updateResourceStatus(ctx context.Context, o *unstructured.Unstructured, status *types.HelmAppStatus) error {
365375
return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
366376
o.Object["status"] = status
367-
return r.Client.Status().Update(context.TODO(), o)
377+
return r.Client.Status().Update(ctx, o)
368378
})
369379
}
370380

371-
func (r HelmOperatorReconciler) waitForDeletion(o client.Object) error {
381+
func (r HelmOperatorReconciler) waitForDeletion(ctx context.Context, o client.Object) error {
372382
key := client.ObjectKeyFromObject(o)
373383

374-
ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
384+
tctx, cancel := context.WithTimeout(ctx, time.Second*5)
375385
defer cancel()
376386
return wait.PollImmediateUntil(time.Millisecond*10, func() (bool, error) {
377-
err := r.Client.Get(ctx, key, o)
387+
err := r.Client.Get(tctx, key, o)
378388
if apierrors.IsNotFound(err) {
379389
return true, nil
380390
}
381391
if err != nil {
382392
return false, err
383393
}
384394
return false, nil
385-
}, ctx.Done())
395+
}, tctx.Done())
386396
}
387397

388398
func contains(l []string, s string) bool {

0 commit comments

Comments
 (0)