Support preDelete lifecycle for Advanced DaemonSet #923
Conversation
FillZpp
force-pushed
the
support-predelete-lifecycle-hook-for-ads
branch
from
e585438 to
c1a1137
Compare
Signed-off-by: FillZpp <FillZpp.pub@gmail.com>
FillZpp
force-pushed
the
support-predelete-lifecycle-hook-for-ads
branch
from
c1a1137 to
4b8edbf
Compare
Codecov Report
@@ Coverage Diff @@
## master #923 +/- ##
==========================================
- Coverage 49.21% 49.13% -0.09%
==========================================
Files 119 119
Lines 11120 11166 +46
==========================================
+ Hits 5473 5486 +13
- Misses 4805 4833 +28
- Partials 842 847 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -255,6 +256,7 @@ func (c *realControl) updatePod(cs *appsv1alpha1.CloneSet, coreControl clonesetc | |||
| if res.InPlaceUpdate { | |||
| if res.UpdateErr == nil { | |||
| c.recorder.Eventf(cs, v1.EventTypeNormal, "SuccessfulUpdatePodInPlace", "successfully update pod %s in-place(revision %v)", pod.Name, updateRevision.Name) | |||
| clonesetutils.ResourceVersionExpectations.Expect(&metav1.ObjectMeta{UID: pod.UID, ResourceVersion: res.NewResourceVersion}) | |||
There was a problem hiding this comment.
do we still need UpdateExpectations here ?
There was a problem hiding this comment.
No, I will remove it in next PR.
| @@ -785,6 +816,9 @@ func (dsc *ReconcileDaemonSet) podsShouldBeOnNode( | |||
| // Emit an event so that it's discoverable to users. | |||
| dsc.eventRecorder.Eventf(ds, corev1.EventTypeWarning, FailedDaemonPodReason, msg) | |||
| podsToDelete = append(podsToDelete, pod.Name) | |||
| } else if isPodPreDeleting(pod) { | |||
There was a problem hiding this comment.
why we need add PreparingDelete pods into podsToDelete, if there is some logic which make the pods in the podsToDelete in the first place , is it still the case when next reconcile?
There was a problem hiding this comment.
Yeah, we should put it into podsToDelete even if it has been set to PreparingDelete in previous reconcile.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: FillZpp 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 |
Signed-off-by: FillZpp FillZpp.pub@gmail.com
Ⅰ. Describe what this PR does
Support preDelete lifecycle for Advanced DaemonSet
Ⅱ. Does this pull request fix one issue?
fixes #911