Fix the race condition by confirming creation/deletion of machine objects #316
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: k4leung4 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
approved
|
/assign @karan @medinatiger |
|
/uncc @kris-nova @timothysc |
|
/hold |
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
size/L
…ects By waiting for the machine objects to be either created or deleted before leaving Reconcile loop will prevent the race condition of stale cache not reporting correctly the change to machine objects.
|
/assign @mkjelland @karan @medinatiger |
|
|
||
| // stateConfirmationInterval is the amount of time between polling for the desired state. | ||
| // The polling is against a local memory cache. | ||
| var stateConfirmationInterval = 100 * time.Millisecond |
There was a problem hiding this comment.
this is against an local internal cache, it shouldnt be an issue.
i can raise the interval if it is a concern
| if errors.IsNotFound(err) { | ||
| return false, nil | ||
| } | ||
| return false, err |
There was a problem hiding this comment.
Please log all errors explicitly for debugging
There was a problem hiding this comment.
error from the retry loop goes to pollErr which is always logged.
|
lgtm |
| @@ -342,3 +332,41 @@ func getMachinesToDelete(filteredMachines []*v1alpha1.Machine, diff int) []*v1al | |||
| // see: https://github.com/kubernetes/kube-deploy/issues/625 | |||
| return filteredMachines[:diff] | |||
| } | |||
|
|
|||
| func (c *MachineSetControllerImpl) waitForMachineCreation(machineList []*v1alpha1.Machine) error { | |||
There was a problem hiding this comment.
waitForMachineCreation and Deletion probably have some common code with deployer/clusterctl. Maybe have a separate PR in the future to abstract out to put it into util.
There was a problem hiding this comment.
agree.
will clean up in future PR.
|
lgtm |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
…mplatize-k8s-version clusterctl: templatize kubernetes version
By waiting for the machine objects to be either created or deleted
before leaving Reconcile loop will prevent the race condition of stale
cache not reporting correctly the change to machine objects.
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #245
Special notes for your reviewer:
Release note:
@kubernetes/kube-deploy-reviewers