Skip to content

Commit 7b1a4fa

Browse files
author
chenpeicheng
committed
the pod lifecycle can be transformed from PreparingNormal to Normal only if ContainerReady is true(#1485)
1 parent 30a660b commit 7b1a4fa

5 files changed

Lines changed: 70 additions & 4 deletions

File tree

‎apis/apps/pub/lifecycle.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ const (
2121
LifecycleTimestampKey = "lifecycle.apps.kruise.io/timestamp"
2222

2323
// LifecycleStatePreparingNormal means the Pod is created but unavailable.
24-
// It will translate to Normal state if Lifecycle.PreNormal is hooked.
24+
// It will translate to Normal state if Lifecycle.PreNormal is hooked and pod condition ContainerReady is true.
2525
LifecycleStatePreparingNormal LifecycleStateType = "PreparingNormal"
2626
// LifecycleStateNormal is a necessary condition for Pod to be available.
2727
LifecycleStateNormal LifecycleStateType = "Normal"

‎pkg/controller/cloneset/sync/cloneset_update.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,13 @@ func (c *realControl) refreshPodState(cs *appsv1alpha1.CloneSet, coreControl clo
169169
var state appspub.LifecycleStateType
170170
switch lifecycle.GetPodLifecycleState(pod) {
171171
case appspub.LifecycleStatePreparingNormal:
172-
if cs.Spec.Lifecycle == nil ||
173-
cs.Spec.Lifecycle.PreNormal == nil ||
174-
lifecycle.IsPodAllHooked(cs.Spec.Lifecycle.PreNormal, pod) {
172+
// 1.the pod lifecycle can be transformed from PreparingNormal to Normal only if ContainerReady is true:
173+
// because: https://github.com/openkruise/kruise/issues/1485
174+
// 2.the semantic of this hook can be a bit confusing, but let's leave it as it is:
175+
// PreNormalHook: only if all hooks are hooked (or no hook), the pod can be transformed to Normal
176+
// PreDeleteHook: if any hook is hooked, we can NOT delete the pod.
177+
// InPlaceUpdateHook: if any hook is hooked, we can NOT inplaceUpdate the pod.
178+
if util.IsRunningAndContainerReady(pod) && lifecycle.IsPreNormalHookNilOrAllHooked(cs.Spec.Lifecycle, pod) {
175179
state = appspub.LifecycleStateNormal
176180
}
177181
case appspub.LifecycleStatePreparingUpdate:

‎pkg/controller/cloneset/sync/cloneset_update_test.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,7 @@ func TestUpdate(t *testing.T) {
592592
Spec: v1.PodSpec{ReadinessGates: []v1.PodReadinessGate{{ConditionType: appspub.InPlaceUpdateReady}}},
593593
Status: v1.PodStatus{Phase: v1.PodRunning, Conditions: []v1.PodCondition{
594594
{Type: v1.PodReady, Status: v1.ConditionFalse},
595+
{Type: v1.ContainersReady, Status: v1.ConditionTrue},
595596
{Type: appspub.InPlaceUpdateReady, Status: v1.ConditionTrue},
596597
}},
597598
},
@@ -606,6 +607,7 @@ func TestUpdate(t *testing.T) {
606607
Spec: v1.PodSpec{ReadinessGates: []v1.PodReadinessGate{{ConditionType: appspub.InPlaceUpdateReady}}},
607608
Status: v1.PodStatus{Phase: v1.PodRunning, Conditions: []v1.PodCondition{
608609
{Type: v1.PodReady, Status: v1.ConditionFalse},
610+
{Type: v1.ContainersReady, Status: v1.ConditionTrue},
609611
{Type: appspub.InPlaceUpdateReady, Status: v1.ConditionTrue},
610612
}},
611613
},
@@ -669,6 +671,7 @@ func TestUpdate(t *testing.T) {
669671
Spec: v1.PodSpec{ReadinessGates: []v1.PodReadinessGate{{ConditionType: appspub.InPlaceUpdateReady}}},
670672
Status: v1.PodStatus{Phase: v1.PodRunning, Conditions: []v1.PodCondition{
671673
{Type: v1.PodReady, Status: v1.ConditionFalse},
674+
{Type: v1.ContainersReady, Status: v1.ConditionTrue},
672675
{Type: appspub.InPlaceUpdateReady, Status: v1.ConditionTrue},
673676
}},
674677
},
@@ -684,6 +687,7 @@ func TestUpdate(t *testing.T) {
684687
Spec: v1.PodSpec{ReadinessGates: []v1.PodReadinessGate{{ConditionType: appspub.InPlaceUpdateReady}}},
685688
Status: v1.PodStatus{Phase: v1.PodRunning, Conditions: []v1.PodCondition{
686689
{Type: v1.PodReady, Status: v1.ConditionFalse},
690+
{Type: v1.ContainersReady, Status: v1.ConditionTrue},
687691
{Type: appspub.InPlaceUpdateReady, Status: v1.ConditionTrue},
688692
}},
689693
},
@@ -843,6 +847,7 @@ func TestUpdate(t *testing.T) {
843847
Phase: v1.PodRunning,
844848
Conditions: []v1.PodCondition{
845849
{Type: v1.PodReady, Status: v1.ConditionTrue},
850+
{Type: v1.ContainersReady, Status: v1.ConditionTrue},
846851
{Type: appspub.InPlaceUpdateReady, Status: v1.ConditionTrue, LastTransitionTime: now},
847852
},
848853
ContainerStatuses: []v1.ContainerStatus{{Name: "c1", ImageID: "image-id-xyz"}},
@@ -867,6 +872,7 @@ func TestUpdate(t *testing.T) {
867872
Phase: v1.PodRunning,
868873
Conditions: []v1.PodCondition{
869874
{Type: v1.PodReady, Status: v1.ConditionTrue},
875+
{Type: v1.ContainersReady, Status: v1.ConditionTrue},
870876
{Type: appspub.InPlaceUpdateReady, Status: v1.ConditionTrue, LastTransitionTime: now},
871877
},
872878
ContainerStatuses: []v1.ContainerStatus{{Name: "c1", ImageID: "image-id-xyz"}},
@@ -945,6 +951,41 @@ func TestUpdate(t *testing.T) {
945951
},
946952
},
947953
},
954+
{
955+
name: "create: preparingNormal->Normal without hook, pod not ready",
956+
cs: &appsv1alpha1.CloneSet{Spec: appsv1alpha1.CloneSetSpec{Replicas: getInt32Pointer(1)}},
957+
updateRevision: &apps.ControllerRevision{ObjectMeta: metav1.ObjectMeta{Name: "rev_new"}},
958+
pods: []*v1.Pod{
959+
{
960+
ObjectMeta: metav1.ObjectMeta{Name: "pod-0", Labels: map[string]string{
961+
apps.ControllerRevisionHashLabelKey: "rev_new",
962+
apps.DefaultDeploymentUniqueLabelKey: "rev_new",
963+
appspub.LifecycleStateKey: string(appspub.LifecycleStatePreparingNormal),
964+
}},
965+
Spec: v1.PodSpec{ReadinessGates: []v1.PodReadinessGate{{ConditionType: appspub.InPlaceUpdateReady}}},
966+
Status: v1.PodStatus{Phase: v1.PodRunning, Conditions: []v1.PodCondition{
967+
{Type: v1.PodReady, Status: v1.ConditionFalse},
968+
{Type: v1.ContainersReady, Status: v1.ConditionFalse},
969+
{Type: appspub.InPlaceUpdateReady, Status: v1.ConditionTrue},
970+
}},
971+
},
972+
},
973+
expectedPods: []*v1.Pod{
974+
{
975+
ObjectMeta: metav1.ObjectMeta{Name: "pod-0", Labels: map[string]string{
976+
apps.ControllerRevisionHashLabelKey: "rev_new",
977+
apps.DefaultDeploymentUniqueLabelKey: "rev_new",
978+
appspub.LifecycleStateKey: string(appspub.LifecycleStatePreparingNormal),
979+
}},
980+
Spec: v1.PodSpec{ReadinessGates: []v1.PodReadinessGate{{ConditionType: appspub.InPlaceUpdateReady}}},
981+
Status: v1.PodStatus{Phase: v1.PodRunning, Conditions: []v1.PodCondition{
982+
{Type: v1.PodReady, Status: v1.ConditionFalse},
983+
{Type: v1.ContainersReady, Status: v1.ConditionFalse},
984+
{Type: appspub.InPlaceUpdateReady, Status: v1.ConditionTrue},
985+
}},
986+
},
987+
},
988+
},
948989
}
949990

950991
inplaceupdate.Clock = clock.NewFakeClock(now.Time)

‎pkg/util/lifecycle/lifecycle_utils.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,23 @@ func IsPodAllHooked(hook *appspub.LifecycleHook, pod *v1.Pod) bool {
237237
return true
238238
}
239239

240+
// IsPreNormalHookNilOrAllHooked
241+
// pod lifecycle can be transformed to Normal only if no hook or all hooks are hooked
242+
func IsPreNormalHookNilOrAllHooked(hooks *appspub.Lifecycle, pod *v1.Pod) bool {
243+
// nothing changed, but avoiding excessive complexity in if conditions
244+
return hooks == nil ||
245+
hooks.PreNormal == nil ||
246+
IsPodAllHooked(hooks.PreNormal, pod)
247+
}
248+
249+
// IsInPlaceUpdateHookNilOrAllHooked
250+
// after updating, we need to wait for all InPlaceUpdateHook hooks to be hooked back
251+
func IsInPlaceUpdateHookNilOrAllHooked(hooks *appspub.Lifecycle, pod *v1.Pod) bool {
252+
return hooks == nil ||
253+
hooks.InPlaceUpdate == nil ||
254+
IsPodAllHooked(hooks.InPlaceUpdate, pod)
255+
}
256+
240257
func getReadinessMessage(key string) podreadiness.Message {
241258
return podreadiness.Message{UserAgent: "Lifecycle", Key: key}
242259
}

‎pkg/util/pods.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,10 @@ func IsRunningAndReady(pod *v1.Pod) bool {
204204
return pod.Status.Phase == v1.PodRunning && podutil.IsPodReady(pod) && pod.DeletionTimestamp.IsZero()
205205
}
206206

207+
func IsRunningAndContainerReady(pod *v1.Pod) bool {
208+
return pod.Status.Phase == v1.PodRunning && podutil.IsContainersReadyConditionTrue(pod.Status) && pod.DeletionTimestamp.IsZero()
209+
}
210+
207211
func GetPodContainerImageIDs(pod *v1.Pod) map[string]string {
208212
cImageIDs := make(map[string]string, len(pod.Status.ContainerStatuses))
209213
for i := range pod.Status.ContainerStatuses {

0 commit comments

Comments
 (0)