add call to return if machine exists and consider requeue-after error… #546
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
|
/lgtm |
k8s-ci-robot
added
the
lgtm
pkg/controller/machine/controller.go
Outdated
| } | ||
| return reconcile.Result{}, nil | ||
| return reconcile.Result{}, err |
There was a problem hiding this comment.
why return err here? can this ever not-be nil? seems like we return on line 158 if err != nil, and the other assignments shadow this.
There was a problem hiding this comment.
It can be not nil:
if line 175 returns error and is not a requeueErr error then line 182 must return the error (or nil)
There was a problem hiding this comment.
Correct me if I am wrong - but doesn't line 175 shadow the err value, so you will never get that assignment:
if err := r.create(m)
There was a problem hiding this comment.
oh yeh, I think you are right! thanks! I'll change it
|
lgtm for reference - make it look more like this again |
| @@ -168,14 +168,18 @@ func (r *ReconcileMachine) Reconcile(request reconcile.Request) (reconcile.Resul | |||
| return reconcile.Result{Requeue: true, RequeueAfter: requeueErr.RequeueAfter}, nil | |||
| } | |||
| } | |||
| return reconcile.Result{}, err | |||
| } | |||
| // Machine resource created. Machine does not yet exist. | |||
| glog.Infof("Reconciling machine object %v triggers idempotent create.", m.ObjectMeta.Name) | |||
| if err := r.create(m); err != nil { | |||
There was a problem hiding this comment.
if we want to return this, we should not shadow it
if err = r.create(m)
There was a problem hiding this comment.
fwiw, we may not want to shadow the update either
k8s-ci-robot
removed
the
lgtm
enxebre
force-pushed
the
controller-fix
branch
from
46c1d32 to
84cffd9
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/test pull-cluster-api-test |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre, roberthbailey 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
…-uri Update generation script w correct CABPK image
… on create
What this PR does / why we need it:
returnafter an update is triggered for the machine controllerWhich issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #547
Special notes for your reviewer:
Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
Release note: