Skip to content

Commit 154a769

Browse files
pavelmaliypavelmaliy
authored
fix update status logic (#68)
1 parent aa592f8 commit 154a769

File tree

3 files changed

+32
-68
lines changed

3 files changed

+32
-68
lines changed

‎controllers/base_controller.go‎

Lines changed: 6 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -115,50 +115,14 @@ func (r *BaseReconciler) removeFinalizer(ctx context.Context, object servicesv1a
115115
return nil
116116
}
117117

118-
func (r *BaseReconciler) updateStatusWithRetries(ctx context.Context, object servicesv1alpha1.SAPBTPResource, log logr.Logger) error {
119-
logFailedAttempt := func(retries int, err error) {
120-
log.Info(fmt.Sprintf("failed to update status of %s attempt #%v, %s", object.GetControllerName(), retries, err.Error()))
121-
}
122-
123-
log.Info(fmt.Sprintf("updating %s status with retries", object.GetControllerName()))
124-
var err error
125-
if err = r.Status().Update(ctx, object); err != nil {
126-
logFailedAttempt(1, err)
127-
for i := 2; i <= 3; i++ {
128-
if err = r.updateStatus(ctx, object, log); err == nil {
129-
break
130-
}
131-
logFailedAttempt(i, err)
132-
}
133-
}
134-
135-
if err != nil {
136-
log.Error(err, fmt.Sprintf("failed to update status of %s giving up!!", object.GetControllerName()))
137-
return err
138-
}
139-
140-
log.Info(fmt.Sprintf("updated %s status in k8s", object.GetControllerName()))
141-
return nil
142-
}
143-
144-
func (r *BaseReconciler) updateStatus(ctx context.Context, object servicesv1alpha1.SAPBTPResource, log logr.Logger) error {
145-
status := object.GetStatus()
146-
if err := r.Get(ctx, apimachinerytypes.NamespacedName{Name: object.GetName(), Namespace: object.GetNamespace()}, object); err != nil {
147-
log.Error(err, fmt.Sprintf("failed to fetch latest %s, unable to update status", object.GetControllerName()))
148-
return err
149-
}
150-
clonedObj := object.DeepClone()
151-
clonedObj.SetStatus(status)
152-
if err := r.Status().Update(ctx, clonedObj); err != nil {
153-
return err
154-
}
155-
return nil
118+
func (r *BaseReconciler) updateStatus(ctx context.Context, object servicesv1alpha1.SAPBTPResource) error {
119+
return r.Status().Update(ctx, object)
156120
}
157121

158-
func (r *BaseReconciler) init(ctx context.Context, log logr.Logger, obj servicesv1alpha1.SAPBTPResource) error {
122+
func (r *BaseReconciler) init(ctx context.Context, obj servicesv1alpha1.SAPBTPResource) error {
159123
obj.SetReady(metav1.ConditionFalse)
160124
setInProgressConditions(smTypes.CREATE, "Pending", obj)
161-
if err := r.updateStatusWithRetries(ctx, obj, log); err != nil {
125+
if err := r.updateStatus(ctx, obj); err != nil {
162126
return err
163127
}
164128
return nil
@@ -321,7 +285,7 @@ func (r *BaseReconciler) markAsNonTransientError(ctx context.Context, operationT
321285
log.Info(fmt.Sprintf("operation %s of %s encountered a non transient error %s, giving up operation :(", operationType, object.GetControllerName(), nonTransientErr.Error()))
322286
}
323287
object.SetObservedGeneration(object.GetGeneration())
324-
err := r.updateStatusWithRetries(ctx, object, log)
288+
err := r.updateStatus(ctx, object)
325289
if err != nil {
326290
return ctrl.Result{}, err
327291
}
@@ -334,7 +298,7 @@ func (r *BaseReconciler) markAsNonTransientError(ctx context.Context, operationT
334298
func (r *BaseReconciler) markAsTransientError(ctx context.Context, operationType smTypes.OperationCategory, transientErr error, object servicesv1alpha1.SAPBTPResource, log logr.Logger) (ctrl.Result, error) {
335299
setInProgressConditions(operationType, transientErr.Error(), object)
336300
log.Info(fmt.Sprintf("operation %s of %s encountered a transient error %s, retrying operation :)", operationType, object.GetControllerName(), transientErr.Error()))
337-
if err := r.updateStatusWithRetries(ctx, object, log); err != nil {
301+
if err := r.updateStatus(ctx, object); err != nil {
338302
return ctrl.Result{}, err
339303
}
340304
return ctrl.Result{}, transientErr

‎controllers/servicebinding_controller.go‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
6868
serviceBinding = serviceBinding.DeepCopy()
6969

7070
if len(serviceBinding.GetConditions()) == 0 {
71-
err := r.init(ctx, log, serviceBinding)
71+
err := r.init(ctx, serviceBinding)
7272
if err != nil {
7373
return ctrl.Result{}, err
7474
}
@@ -107,7 +107,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
107107
}
108108

109109
setBlockedCondition(instanceErr.Error(), serviceBinding)
110-
if err := r.updateStatusWithRetries(ctx, serviceBinding, log); err != nil {
110+
if err := r.updateStatus(ctx, serviceBinding); err != nil {
111111
return ctrl.Result{}, err
112112
}
113113

@@ -119,7 +119,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
119119

120120
setInProgressConditions(smTypes.CREATE, fmt.Sprintf("creation in progress, waiting for service instance '%s' to be ready", serviceBinding.Spec.ServiceInstanceName),
121121
serviceBinding)
122-
if err := r.updateStatusWithRetries(ctx, serviceBinding, log); err != nil {
122+
if err := r.updateStatus(ctx, serviceBinding); err != nil {
123123
return ctrl.Result{}, err
124124
}
125125

@@ -134,7 +134,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
134134
err := r.validateSecretNameIsAvailable(ctx, serviceBinding)
135135
if err != nil {
136136
setBlockedCondition(err.Error(), serviceBinding)
137-
return ctrl.Result{}, r.updateStatusWithRetries(ctx, serviceBinding, log)
137+
return ctrl.Result{}, r.updateStatus(ctx, serviceBinding)
138138
}
139139

140140
binding, err := r.getBindingForRecovery(smClient, serviceBinding, log)
@@ -154,7 +154,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
154154
}
155155
r.resyncBindingStatus(serviceBinding, binding, serviceInstance.Status.InstanceID)
156156

157-
return ctrl.Result{}, r.updateStatusWithRetries(ctx, serviceBinding, log)
157+
return ctrl.Result{}, r.updateStatus(ctx, serviceBinding)
158158
}
159159
if serviceBinding.Status.Ready != metav1.ConditionTrue {
160160
return r.createBinding(ctx, smClient, serviceInstance, serviceBinding, log)
@@ -205,7 +205,7 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s
205205
serviceBinding.Status.OperationURL = operationURL
206206
serviceBinding.Status.OperationType = smTypes.CREATE
207207
setInProgressConditions(smTypes.CREATE, "", serviceBinding)
208-
if err := r.updateStatusWithRetries(ctx, serviceBinding, log); err != nil {
208+
if err := r.updateStatus(ctx, serviceBinding); err != nil {
209209
log.Error(err, "unable to update ServiceBinding status")
210210
return ctrl.Result{}, err
211211
}
@@ -223,7 +223,7 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s
223223
setSuccessConditions(smTypes.CREATE, serviceBinding)
224224
log.Info("Updating binding", "bindingID", smBinding.ID)
225225

226-
return ctrl.Result{}, r.updateStatusWithRetries(ctx, serviceBinding, log)
226+
return ctrl.Result{}, r.updateStatus(ctx, serviceBinding)
227227
}
228228

229229
func (r *ServiceBindingReconciler) delete(ctx context.Context, smClient sm.Client, serviceBinding *v1alpha1.ServiceBinding, log logr.Logger) (ctrl.Result, error) {
@@ -238,7 +238,7 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, smClient sm.Clien
238238
log.Info("binding exists in SM continue with deletion")
239239
serviceBinding.Status.BindingID = smBinding.ID
240240
setInProgressConditions(smTypes.DELETE, "delete after recovery", serviceBinding)
241-
return ctrl.Result{}, r.updateStatusWithRetries(ctx, serviceBinding, log)
241+
return ctrl.Result{}, r.updateStatus(ctx, serviceBinding)
242242
}
243243

244244
// make sure there's no secret stored for the binding
@@ -266,7 +266,7 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, smClient sm.Clien
266266
serviceBinding.Status.OperationURL = operationURL
267267
serviceBinding.Status.OperationType = smTypes.DELETE
268268
setInProgressConditions(smTypes.DELETE, "", serviceBinding)
269-
if err := r.updateStatusWithRetries(ctx, serviceBinding, log); err != nil {
269+
if err := r.updateStatus(ctx, serviceBinding); err != nil {
270270
return ctrl.Result{}, err
271271
}
272272
return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, nil
@@ -291,7 +291,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, smClient sm.Client,
291291
freshStatus.BindingID = serviceBinding.Status.BindingID
292292
}
293293
serviceBinding.Status = freshStatus
294-
if err := r.updateStatusWithRetries(ctx, serviceBinding, log); err != nil {
294+
if err := r.updateStatus(ctx, serviceBinding); err != nil {
295295
log.Error(err, "failed to update status during polling")
296296
}
297297
return ctrl.Result{}, statusErr
@@ -312,7 +312,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, smClient sm.Client,
312312
if serviceBinding.Status.OperationType == smTypes.DELETE {
313313
serviceBinding.Status.OperationURL = ""
314314
serviceBinding.Status.OperationType = ""
315-
if err := r.updateStatusWithRetries(ctx, serviceBinding, log); err != nil {
315+
if err := r.updateStatus(ctx, serviceBinding); err != nil {
316316
log.Error(err, "unable to update ServiceBinding status")
317317
return ctrl.Result{}, err
318318
}
@@ -345,7 +345,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, smClient sm.Client,
345345
serviceBinding.Status.OperationURL = ""
346346
serviceBinding.Status.OperationType = ""
347347

348-
return ctrl.Result{}, r.updateStatusWithRetries(ctx, serviceBinding, log)
348+
return ctrl.Result{}, r.updateStatus(ctx, serviceBinding)
349349
}
350350

351351
func (r *ServiceBindingReconciler) SetOwner(ctx context.Context, serviceInstance *v1alpha1.ServiceInstance, serviceBinding *v1alpha1.ServiceBinding, log logr.Logger) error {
@@ -528,7 +528,7 @@ func (r *ServiceBindingReconciler) getBindingForRecovery(smClient sm.Client, ser
528528
func (r *ServiceBindingReconciler) removeBindingFromKubernetes(ctx context.Context, serviceBinding *v1alpha1.ServiceBinding, log logr.Logger) (ctrl.Result, error) {
529529
serviceBinding.Status.BindingID = ""
530530
setSuccessConditions(smTypes.DELETE, serviceBinding)
531-
if err := r.updateStatusWithRetries(ctx, serviceBinding, log); err != nil {
531+
if err := r.updateStatus(ctx, serviceBinding); err != nil {
532532
return ctrl.Result{}, err
533533
}
534534

@@ -589,7 +589,7 @@ func (r *ServiceBindingReconciler) maintain(ctx context.Context, binding *v1alph
589589

590590
if shouldUpdateStatus {
591591
log.Info(fmt.Sprintf("maintanance required for binding %s", binding.Name))
592-
return ctrl.Result{}, r.updateStatusWithRetries(ctx, binding, log)
592+
return ctrl.Result{}, r.updateStatus(ctx, binding)
593593
}
594594
return ctrl.Result{}, nil
595595
}

‎controllers/serviceinstance_controller.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
6161
serviceInstance = serviceInstance.DeepCopy()
6262

6363
if len(serviceInstance.GetConditions()) == 0 {
64-
err := r.init(ctx, log, serviceInstance)
64+
err := r.init(ctx, serviceInstance)
6565
if err != nil {
6666
return ctrl.Result{}, err
6767
}
@@ -100,7 +100,7 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
100100
if instance != nil {
101101
log.Info(fmt.Sprintf("found existing instance in SM with id %s, updating status", instance.ID))
102102
r.resyncInstanceStatus(serviceInstance, instance)
103-
return ctrl.Result{}, r.updateStatusWithRetries(ctx, serviceInstance, log)
103+
return ctrl.Result{}, r.updateStatus(ctx, serviceInstance)
104104
}
105105

106106
//if instance was not recovered then create new instance
@@ -126,7 +126,7 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, smClient sm.Client
126126
freshStatus.InstanceID = serviceInstance.Status.InstanceID
127127
}
128128
serviceInstance.Status = freshStatus
129-
if err := r.updateStatusWithRetries(ctx, serviceInstance, log); err != nil {
129+
if err := r.updateStatus(ctx, serviceInstance); err != nil {
130130
log.Error(err, "failed to update status during polling")
131131
}
132132
return ctrl.Result{}, statusErr
@@ -143,7 +143,7 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, smClient sm.Client
143143
if serviceInstance.Status.OperationType == smTypes.DELETE {
144144
serviceInstance.Status.OperationURL = ""
145145
serviceInstance.Status.OperationType = ""
146-
if err := r.updateStatusWithRetries(ctx, serviceInstance, log); err != nil {
146+
if err := r.updateStatus(ctx, serviceInstance); err != nil {
147147
return ctrl.Result{}, err
148148
}
149149
errMsg := "async deprovisioning operation failed"
@@ -168,7 +168,7 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, smClient sm.Client
168168
serviceInstance.Status.OperationURL = ""
169169
serviceInstance.Status.OperationType = ""
170170

171-
return ctrl.Result{}, r.updateStatusWithRetries(ctx, serviceInstance, log)
171+
return ctrl.Result{}, r.updateStatus(ctx, serviceInstance)
172172
}
173173

174174
func (r *ServiceInstanceReconciler) createInstance(ctx context.Context, smClient sm.Client, serviceInstance *v1alpha1.ServiceInstance, log logr.Logger) (ctrl.Result, error) {
@@ -206,7 +206,7 @@ func (r *ServiceInstanceReconciler) createInstance(ctx context.Context, smClient
206206
serviceInstance.Status.OperationURL = operationURL
207207
serviceInstance.Status.OperationType = smTypes.CREATE
208208
setInProgressConditions(smTypes.CREATE, "", serviceInstance)
209-
if err := r.updateStatusWithRetries(ctx, serviceInstance, log); err != nil {
209+
if err := r.updateStatus(ctx, serviceInstance); err != nil {
210210
return ctrl.Result{}, err
211211
}
212212

@@ -216,7 +216,7 @@ func (r *ServiceInstanceReconciler) createInstance(ctx context.Context, smClient
216216
serviceInstance.Status.InstanceID = smInstanceID
217217
serviceInstance.Status.Ready = metav1.ConditionTrue
218218
setSuccessConditions(smTypes.CREATE, serviceInstance)
219-
return ctrl.Result{}, r.updateStatusWithRetries(ctx, serviceInstance, log)
219+
return ctrl.Result{}, r.updateStatus(ctx, serviceInstance)
220220
}
221221

222222
func (r *ServiceInstanceReconciler) updateInstance(ctx context.Context, smClient sm.Client, serviceInstance *v1alpha1.ServiceInstance, log logr.Logger) (ctrl.Result, error) {
@@ -248,15 +248,15 @@ func (r *ServiceInstanceReconciler) updateInstance(ctx context.Context, smClient
248248
serviceInstance.Status.OperationType = smTypes.UPDATE
249249
setInProgressConditions(smTypes.UPDATE, "", serviceInstance)
250250

251-
if err := r.updateStatusWithRetries(ctx, serviceInstance, log); err != nil {
251+
if err := r.updateStatus(ctx, serviceInstance); err != nil {
252252
return ctrl.Result{}, err
253253
}
254254

255255
return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, nil
256256
}
257257
log.Info("Instance updated successfully")
258258
setSuccessConditions(smTypes.UPDATE, serviceInstance)
259-
return ctrl.Result{}, r.updateStatusWithRetries(ctx, serviceInstance, log)
259+
return ctrl.Result{}, r.updateStatus(ctx, serviceInstance)
260260
}
261261

262262
func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, smClient sm.Client, serviceInstance *v1alpha1.ServiceInstance, log logr.Logger) (ctrl.Result, error) {
@@ -272,7 +272,7 @@ func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, smClient
272272
log.Info("instance exists in SM continue with deletion")
273273
serviceInstance.Status.InstanceID = smInstance.ID
274274
setInProgressConditions(smTypes.DELETE, "delete after recovery", serviceInstance)
275-
return ctrl.Result{}, r.updateStatusWithRetries(ctx, serviceInstance, log)
275+
return ctrl.Result{}, r.updateStatus(ctx, serviceInstance)
276276
}
277277
log.Info("instance does not exists in SM, removing finalizer")
278278
return ctrl.Result{}, r.removeFinalizer(ctx, serviceInstance, v1alpha1.FinalizerName, log)
@@ -291,7 +291,7 @@ func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, smClient
291291
serviceInstance.Status.OperationType = smTypes.DELETE
292292
setInProgressConditions(smTypes.DELETE, "", serviceInstance)
293293

294-
if err := r.updateStatusWithRetries(ctx, serviceInstance, log); err != nil {
294+
if err := r.updateStatus(ctx, serviceInstance); err != nil {
295295
return ctrl.Result{}, err
296296
}
297297

@@ -300,7 +300,7 @@ func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, smClient
300300
log.Info("Instance was deleted successfully")
301301
serviceInstance.Status.InstanceID = ""
302302
setSuccessConditions(smTypes.DELETE, serviceInstance)
303-
if err := r.updateStatusWithRetries(ctx, serviceInstance, log); err != nil {
303+
if err := r.updateStatus(ctx, serviceInstance); err != nil {
304304
return ctrl.Result{}, err
305305
}
306306

0 commit comments

Comments
 (0)