add lifecycle mark pod not ready feature

Signed-off-by: mingzhou.swx <[email protected]>
openkruise · May 20, 2022 · 8fa6413 · 8fa6413
1 parent cf95157
commit 8fa6413
Show file tree

Hide file tree

Showing 17 changed files with 335 additions and 89 deletions.
diff --git a/apis/apps/pub/lifecycle.go b/apis/apps/pub/lifecycle.go
@@ -40,4 +40,9 @@ type Lifecycle struct {
 type LifecycleHook struct {
 	LabelsHandler     map[string]string `json:"labelsHandler,omitempty"`
 	FinalizersHandler []string          `json:"finalizersHandler,omitempty"`
+	// MarkPodNotReady = true means:
+	// - Pod will be set to 'NotReady' at preparingDelete/preparingUpdate state.
+	// - Pod will be restored to 'Ready' at Updated state if it was set to 'NotReady' at preparingUpdate state.
+	// Default to false.
+	MarkPodNotReady bool `json:"markPodNotReady,omitempty"`
 }
diff --git a/apis/apps/pub/pod_readiness_gate.go b/apis/apps/pub/pod_readiness_gate.go
@@ -19,5 +19,12 @@ package pub
 import v1 "k8s.io/api/core/v1"
 
 const (
+	// KruisePodReadyConditionType can support multiple writers, such as:
+	// - ContainerRecreateRequest;
+	// - Workload controller, including CloneSet, Advanced StatefulSet, Advanced Daemonset.
+	//
+	// If its corresponding condition status was set to "False" by multiple writers,
+	// the condition status will be considered as "True" only when all these writers
+	// set it to "True".
 	KruisePodReadyConditionType v1.PodConditionType = "KruisePodReady"
 )
diff --git a/config/crd/bases/apps.kruise.io_clonesets.yaml b/config/crd/bases/apps.kruise.io_clonesets.yaml
@@ -97,6 +97,13 @@ spec:
                         additionalProperties:
                           type: string
                         type: object
+                      markPodNotReady:
+                        description: 'MarkPodNotReady = true means: - Pod will be
+                          set to ''NotReady'' at preparingDelete/preparingUpdate state.
+                          - Pod will be restored to ''Ready'' at Updated state if
+                          it was set to ''NotReady'' at preparingUpdate state. Default
+                          to false.'
+                        type: boolean
                     type: object
                   preDelete:
                     description: PreDelete is the hook before Pod to be deleted.
@@ -109,6 +116,13 @@ spec:
                         additionalProperties:
                           type: string
                         type: object
+                      markPodNotReady:
+                        description: 'MarkPodNotReady = true means: - Pod will be
+                          set to ''NotReady'' at preparingDelete/preparingUpdate state.
+                          - Pod will be restored to ''Ready'' at Updated state if
+                          it was set to ''NotReady'' at preparingUpdate state. Default
+                          to false.'
+                        type: boolean
                     type: object
                 type: object
               minReadySeconds:

diff --git a/config/crd/bases/apps.kruise.io_daemonsets.yaml b/config/crd/bases/apps.kruise.io_daemonsets.yaml
@@ -93,6 +93,13 @@ spec:
                         additionalProperties:
                           type: string
                         type: object
+                      markPodNotReady:
+                        description: 'MarkPodNotReady = true means: - Pod will be
+                          set to ''NotReady'' at preparingDelete/preparingUpdate state.
+                          - Pod will be restored to ''Ready'' at Updated state if
+                          it was set to ''NotReady'' at preparingUpdate state. Default
+                          to false.'
+                        type: boolean
                     type: object
                   preDelete:
                     description: PreDelete is the hook before Pod to be deleted.
@@ -105,6 +112,13 @@ spec:
                         additionalProperties:
                           type: string
                         type: object
+                      markPodNotReady:
+                        description: 'MarkPodNotReady = true means: - Pod will be
+                          set to ''NotReady'' at preparingDelete/preparingUpdate state.
+                          - Pod will be restored to ''Ready'' at Updated state if
+                          it was set to ''NotReady'' at preparingUpdate state. Default
+                          to false.'
+                        type: boolean
                     type: object
                 type: object
               minReadySeconds:

diff --git a/config/crd/bases/apps.kruise.io_statefulsets.yaml b/config/crd/bases/apps.kruise.io_statefulsets.yaml
@@ -514,6 +514,13 @@ spec:
                         additionalProperties:
                           type: string
                         type: object
+                      markPodNotReady:
+                        description: 'MarkPodNotReady = true means: - Pod will be
+                          set to ''NotReady'' at preparingDelete/preparingUpdate state.
+                          - Pod will be restored to ''Ready'' at Updated state if
+                          it was set to ''NotReady'' at preparingUpdate state. Default
+                          to false.'
+                        type: boolean
                     type: object
                   preDelete:
                     description: PreDelete is the hook before Pod to be deleted.
@@ -526,6 +533,13 @@ spec:
                         additionalProperties:
                           type: string
                         type: object
+                      markPodNotReady:
+                        description: 'MarkPodNotReady = true means: - Pod will be
+                          set to ''NotReady'' at preparingDelete/preparingUpdate state.
+                          - Pod will be restored to ''Ready'' at Updated state if
+                          it was set to ''NotReady'' at preparingUpdate state. Default
+                          to false.'
+                        type: boolean
                     type: object
                 type: object
               persistentVolumeClaimRetentionPolicy:

diff --git a/config/crd/bases/apps.kruise.io_uniteddeployments.yaml b/config/crd/bases/apps.kruise.io_uniteddeployments.yaml
@@ -145,6 +145,13 @@ spec:
                                     additionalProperties:
                                       type: string
                                     type: object
+                                  markPodNotReady:
+                                    description: 'MarkPodNotReady = true means: -
+                                      Pod will be set to ''NotReady'' at preparingDelete/preparingUpdate
+                                      state. - Pod will be restored to ''Ready'' at
+                                      Updated state if it was set to ''NotReady''
+                                      at preparingUpdate state. Default to false.'
+                                    type: boolean
                                 type: object
                               preDelete:
                                 description: PreDelete is the hook before Pod to be
@@ -158,6 +165,13 @@ spec:
                                     additionalProperties:
                                       type: string
                                     type: object
+                                  markPodNotReady:
+                                    description: 'MarkPodNotReady = true means: -
+                                      Pod will be set to ''NotReady'' at preparingDelete/preparingUpdate
+                                      state. - Pod will be restored to ''Ready'' at
+                                      Updated state if it was set to ''NotReady''
+                                      at preparingUpdate state. Default to false.'
+                                    type: boolean
                                 type: object
                             type: object
                           persistentVolumeClaimRetentionPolicy:
@@ -546,6 +560,13 @@ spec:
                                     additionalProperties:
                                       type: string
                                     type: object
+                                  markPodNotReady:
+                                    description: 'MarkPodNotReady = true means: -
+                                      Pod will be set to ''NotReady'' at preparingDelete/preparingUpdate
+                                      state. - Pod will be restored to ''Ready'' at
+                                      Updated state if it was set to ''NotReady''
+                                      at preparingUpdate state. Default to false.'
+                                    type: boolean
                                 type: object
                               preDelete:
                                 description: PreDelete is the hook before Pod to be
@@ -559,6 +580,13 @@ spec:
                                     additionalProperties:
                                       type: string
                                     type: object
+                                  markPodNotReady:
+                                    description: 'MarkPodNotReady = true means: -
+                                      Pod will be set to ''NotReady'' at preparingDelete/preparingUpdate
+                                      state. - Pod will be restored to ''Ready'' at
+                                      Updated state if it was set to ''NotReady''
+                                      at preparingUpdate state. Default to false.'
+                                    type: boolean
                                 type: object
                             type: object
                           minReadySeconds:

diff --git a/pkg/controller/cloneset/cloneset_status.go b/pkg/controller/cloneset/cloneset_status.go
@@ -21,6 +21,7 @@ import (
 
 	appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1"
 	clonesetcore "github.com/openkruise/kruise/pkg/controller/cloneset/core"
+	"github.com/openkruise/kruise/pkg/controller/cloneset/sync"
 	clonesetutils "github.com/openkruise/kruise/pkg/controller/cloneset/utils"
 	"github.com/openkruise/kruise/pkg/util"
 	v1 "k8s.io/api/core/v1"
@@ -88,7 +89,7 @@ func (r *realStatusUpdater) calculateStatus(cs *appsv1alpha1.CloneSet, newStatus
 		if coreControl.IsPodUpdateReady(pod, 0) {
 			newStatus.ReadyReplicas++
 		}
-		if coreControl.IsPodUpdateReady(pod, cs.Spec.MinReadySeconds) {
+		if sync.IsPodAvailable(coreControl, pod, cs.Spec.MinReadySeconds) {
 			newStatus.AvailableReplicas++
 		}
 		if clonesetutils.EqualToRevisionHash("", pod, newStatus.UpdateRevision) {

diff --git a/pkg/controller/cloneset/sync/cloneset_scale.go b/pkg/controller/cloneset/sync/cloneset_scale.go
@@ -165,6 +165,14 @@ func (r *realControl) Scale(
 }
 
 func (r *realControl) managePreparingDelete(cs *appsv1alpha1.CloneSet, pods, podsInPreDelete []*v1.Pod, numToDelete int) (bool, error) {
+	//  We do not allow regret once the pod enter PreparingDelete state if MarkPodNotReady is set.
+	// Actually, there is a bug cased by this transformation from PreparingDelete to Normal,
+	// i.e., Lifecycle Updated Hook may be lost if the pod was transformed from Updating state
+	// to PreparingDelete.
+	if lifecycle.IsLifecycleMarkPodNotReady(cs.Spec.Lifecycle) {
+		return false, nil
+	}
+
 	diff := int(*cs.Spec.Replicas) - len(pods) + numToDelete
 	var modified bool
 	for _, pod := range podsInPreDelete {
@@ -177,7 +185,7 @@ func (r *realControl) managePreparingDelete(cs *appsv1alpha1.CloneSet, pods, pod
 
 		klog.V(3).Infof("CloneSet %s patch pod %s lifecycle from PreparingDelete to Normal",
 			clonesetutils.GetControllerKey(cs), pod.Name)
-		if updated, gotPod, err := r.lifecycleControl.UpdatePodLifecycle(pod, appspub.LifecycleStateNormal); err != nil {
+		if updated, gotPod, err := r.lifecycleControl.UpdatePodLifecycle(pod, appspub.LifecycleStateNormal, false); err != nil {
 			return modified, err
 		} else if updated {
 			modified = true
@@ -270,7 +278,8 @@ func (r *realControl) deletePods(cs *appsv1alpha1.CloneSet, podsToDelete []*v1.P
 	var modified bool
 	for _, pod := range podsToDelete {
 		if cs.Spec.Lifecycle != nil && lifecycle.IsPodHooked(cs.Spec.Lifecycle.PreDelete, pod) {
-			if updated, gotPod, err := r.lifecycleControl.UpdatePodLifecycle(pod, appspub.LifecycleStatePreparingDelete); err != nil {
+			markPodNotReady := cs.Spec.Lifecycle.PreDelete.MarkPodNotReady
+			if updated, gotPod, err := r.lifecycleControl.UpdatePodLifecycle(pod, appspub.LifecycleStatePreparingDelete, markPodNotReady); err != nil {
 				return false, err
 			} else if updated {
 				klog.V(3).Infof("CloneSet %s scaling update pod %s lifecycle to PreparingDelete",
@@ -383,7 +392,7 @@ func (r *realControl) choosePodsToDelete(cs *appsv1alpha1.CloneSet, totalDiff in
 				Pods:   pods,
 				Ranker: ranker,
 				AvailableFunc: func(pod *v1.Pod) bool {
-					return isPodAvailable(coreControl, pod, cs.Spec.MinReadySeconds)
+					return IsPodAvailable(coreControl, pod, cs.Spec.MinReadySeconds)
 				},
 			})
 		} else if diff > len(pods) {

diff --git a/pkg/controller/cloneset/sync/cloneset_sync_utils.go b/pkg/controller/cloneset/sync/cloneset_sync_utils.go
@@ -122,7 +122,7 @@ func calculateDiffsWithExpectation(cs *appsv1alpha1.CloneSet, pods []*v1.Pod, cu
 
 				if isSpecifiedDelete(cs, p) {
 					toDeleteNewRevisionCount++
-				} else if !isPodAvailable(coreControl, p, cs.Spec.MinReadySeconds) {
+				} else if !IsPodAvailable(coreControl, p, cs.Spec.MinReadySeconds) {
 					unavailableNewRevisionCount++
 				}
 			}
@@ -138,7 +138,7 @@ func calculateDiffsWithExpectation(cs *appsv1alpha1.CloneSet, pods []*v1.Pod, cu
 
 				if isSpecifiedDelete(cs, p) {
 					toDeleteOldRevisionCount++
-				} else if !isPodAvailable(coreControl, p, cs.Spec.MinReadySeconds) {
+				} else if !IsPodAvailable(coreControl, p, cs.Spec.MinReadySeconds) {
 					unavailableOldRevisionCount++
 				}
 			}
@@ -235,10 +235,10 @@ func isSpecifiedDelete(cs *appsv1alpha1.CloneSet, pod *v1.Pod) bool {
 }
 
 func isPodReady(coreControl clonesetcore.Control, pod *v1.Pod) bool {
-	return isPodAvailable(coreControl, pod, 0)
+	return IsPodAvailable(coreControl, pod, 0)
 }
 
-func isPodAvailable(coreControl clonesetcore.Control, pod *v1.Pod, minReadySeconds int32) bool {
+func IsPodAvailable(coreControl clonesetcore.Control, pod *v1.Pod, minReadySeconds int32) bool {
 	state := lifecycle.GetPodLifecycleState(pod)
 	if state != "" && state != appspub.LifecycleStateNormal {
 		return false

diff --git a/pkg/controller/cloneset/sync/cloneset_update.go b/pkg/controller/cloneset/sync/cloneset_update.go
@@ -194,7 +194,11 @@ func (c *realControl) refreshPodState(cs *appsv1alpha1.CloneSet, coreControl clo
 	}
 
 	if state != "" {
-		if updated, gotPod, err := c.lifecycleControl.UpdatePodLifecycle(pod, state); err != nil {
+		var markPodNotReady bool
+		if cs.Spec.Lifecycle != nil && cs.Spec.Lifecycle.InPlaceUpdate != nil {
+			markPodNotReady = cs.Spec.Lifecycle.InPlaceUpdate.MarkPodNotReady
+		}
+		if updated, gotPod, err := c.lifecycleControl.UpdatePodLifecycle(pod, state, markPodNotReady); err != nil {
 			return false, 0, err
 		} else if updated {
 			clonesetutils.ResourceVersionExpectations.Expect(gotPod)
@@ -245,7 +249,8 @@ func (c *realControl) updatePod(cs *appsv1alpha1.CloneSet, coreControl clonesetc
 				var updated bool
 				var gotPod *v1.Pod
 				if cs.Spec.Lifecycle != nil && lifecycle.IsPodHooked(cs.Spec.Lifecycle.InPlaceUpdate, pod) {
-					if updated, gotPod, err = c.lifecycleControl.UpdatePodLifecycle(pod, appspub.LifecycleStatePreparingUpdate); err == nil && updated {
+					markPodNotReady := cs.Spec.Lifecycle.InPlaceUpdate.MarkPodNotReady
+					if updated, gotPod, err = c.lifecycleControl.UpdatePodLifecycle(pod, appspub.LifecycleStatePreparingUpdate, markPodNotReady); err == nil && updated {
 						clonesetutils.ResourceVersionExpectations.Expect(gotPod)
 						klog.V(3).Infof("CloneSet %s update pod %s lifecycle to PreparingUpdate",
 							clonesetutils.GetControllerKey(cs), pod.Name)
@@ -344,7 +349,7 @@ func limitUpdateIndexes(coreControl clonesetcore.Control, minReadySeconds int32,
 
 	var unavailableCount, targetRevisionUnavailableCount, canUpdateCount int
 	for _, p := range pods {
-		if !isPodAvailable(coreControl, p, minReadySeconds) {
+		if !IsPodAvailable(coreControl, p, minReadySeconds) {
 			unavailableCount++
 			if clonesetutils.EqualToRevisionHash("", p, targetRevisionHash) {
 				targetRevisionUnavailableCount++
@@ -359,7 +364,7 @@ func limitUpdateIndexes(coreControl clonesetcore.Control, minReadySeconds int32,
 
 		// Make sure unavailable pods in all revisions should not be more than maxUnavailable.
 		// Note that update an old pod that already be unavailable will not increase the unavailable number.
-		if isPodAvailable(coreControl, pods[i], minReadySeconds) {
+		if IsPodAvailable(coreControl, pods[i], minReadySeconds) {
 			if unavailableCount >= diffRes.updateMaxUnavailable {
 				break
 			}

diff --git a/pkg/controller/containerrecreaterequest/crr_controller.go b/pkg/controller/containerrecreaterequest/crr_controller.go
@@ -70,9 +70,11 @@ func Add(mgr manager.Manager) error {
 
 // newReconciler returns a new reconcile.Reconciler
 func newReconciler(mgr manager.Manager) *ReconcileContainerRecreateRequest {
+	cli := util.NewClientFromManager(mgr, "containerrecreaterequest-controller")
 	return &ReconcileContainerRecreateRequest{
-		Client: util.NewClientFromManager(mgr, "containerrecreaterequest-controller"),
-		clock:  clock.RealClock{},
+		Client:              cli,
+		clock:               clock.RealClock{},
+		podReadinessControl: utilpodreadiness.New(cli),
 	}
 }
 
@@ -104,7 +106,8 @@ var _ reconcile.Reconciler = &ReconcileContainerRecreateRequest{}
 // ReconcileContainerRecreateRequest reconciles a ContainerRecreateRequest object
 type ReconcileContainerRecreateRequest struct {
 	client.Client
-	clock clock.Clock
+	clock               clock.Clock
+	podReadinessControl utilpodreadiness.Interface
 }
 
 // +kubebuilder:rbac:groups=apps.kruise.io,resources=containerrecreaterequests,verbs=get;list;watch;create;update;patch;delete
@@ -273,7 +276,7 @@ func (r *ReconcileContainerRecreateRequest) acquirePodNotReady(crr *appsv1alpha1
 			}
 		}
 
-		err := utilpodreadiness.AddNotReadyKey(r.Client, pod, getReadinessMessage(crr))
+		err := r.podReadinessControl.AddNotReadyKey(pod, getReadinessMessage(crr))
 		if err != nil {
 			return fmt.Errorf("add Pod not ready error: %v", err)
 		}
@@ -287,7 +290,7 @@ func (r *ReconcileContainerRecreateRequest) acquirePodNotReady(crr *appsv1alpha1
 
 func (r *ReconcileContainerRecreateRequest) releasePodNotReady(crr *appsv1alpha1.ContainerRecreateRequest, pod *v1.Pod) error {
 	if pod != nil && pod.DeletionTimestamp == nil && utilpodreadiness.ContainsReadinessGate(pod) {
-		err := utilpodreadiness.RemoveNotReadyKey(r.Client, pod, getReadinessMessage(crr))
+		err := r.podReadinessControl.RemoveNotReadyKey(pod, getReadinessMessage(crr))
 		if err != nil {
 			return fmt.Errorf("remove Pod not ready error: %v", err)
 		}

diff --git a/pkg/controller/daemonset/daemonset_controller.go b/pkg/controller/daemonset/daemonset_controller.go
@@ -755,7 +755,8 @@ func (dsc *ReconcileDaemonSet) syncWithPreparingDelete(ds *appsv1alpha1.DaemonSe
 			podsCanDelete = append(podsCanDelete, podName)
 			continue
 		}
-		if updated, gotPod, err := dsc.lifecycleControl.UpdatePodLifecycle(pod, appspub.LifecycleStatePreparingDelete); err != nil {
+		markPodNotReady := ds.Spec.Lifecycle.PreDelete.MarkPodNotReady
+		if updated, gotPod, err := dsc.lifecycleControl.UpdatePodLifecycle(pod, appspub.LifecycleStatePreparingDelete, markPodNotReady); err != nil {
 			return nil, err
 		} else if updated {
 			klog.V(3).Infof("DaemonSet %s/%s has marked Pod %s as PreparingDelete", ds.Namespace, ds.Name, podName)
@@ -989,7 +990,6 @@ func (dsc *ReconcileDaemonSet) refreshUpdateStates(ds *appsv1alpha1.DaemonSet) e
 
 	opts := &inplaceupdate.UpdateOptions{}
 	opts = inplaceupdate.SetOptionsDefaults(opts)
-
 	for _, pod := range pods {
 		if dsc.inplaceControl == nil {
 			continue