Minimize number of unnecessary logs #298
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/S
Edwinhr716
changed the title
| @@ -476,8 +479,16 @@ func (r *LeaderWorkerSetReconciler) updateStatus(ctx context.Context, lws *leade | |||
| if err != nil { | |||
| return false, err | |||
| } | |||
| // Get the most up to date lws object to merge the status with | |||
There was a problem hiding this comment.
This is forcing an update, I think this is dangerous to do.
A better option is to check if the error is a conflict, in which case we requeue the key and return without printing an error, we should do that here:
There was a problem hiding this comment.
So switch it back to status().Update()?
There was a problem hiding this comment.
yes, and return ctrl.Result{Requeue: true}, nil when the error is a conflict
There was a problem hiding this comment.
Makes sense. Will also change it so that it doesn't log the error if there is a conflict
ahg-g
added
the
tide/merge-method-squash
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahg-g, Edwinhr716 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
What type of PR is this?
What this PR does / why we need it
Minimize the number of unnecessary logs that get emitted
Which issue(s) this PR fixes
Fixes #
Special notes for your reviewer
Does this PR introduce a user-facing change?