Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mark pod not ready feature to Lifecycle hooks #979

Merged
merged 1 commit into from
May 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apis/apps/pub/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
7 changes: 7 additions & 0 deletions apis/apps/pub/pod_readiness_gate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
14 changes: 14 additions & 0 deletions config/crd/bases/apps.kruise.io_clonesets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand Down
14 changes: 14 additions & 0 deletions config/crd/bases/apps.kruise.io_daemonsets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand Down
14 changes: 14 additions & 0 deletions config/crd/bases/apps.kruise.io_statefulsets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand Down
28 changes: 28 additions & 0 deletions config/crd/bases/apps.kruise.io_uniteddeployments.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/cloneset/cloneset_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
15 changes: 12 additions & 3 deletions pkg/controller/cloneset/sync/cloneset_scale.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/cloneset/sync/cloneset_sync_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++
}
}
Expand All @@ -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++
}
}
Expand Down Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions pkg/controller/cloneset/sync/cloneset_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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++
Expand All @@ -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
}
Expand Down
13 changes: 8 additions & 5 deletions pkg/controller/containerrecreaterequest/crr_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/daemonset/daemonset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
Loading