Skip to content

Commit e9eff74

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#128516 from gjkim42/group-container-statuses-by-its-sandboxid
Ensure that the pod has proper phase upon re-initialization
2 parents caf03a8 + 657ccc3 commit e9eff74

10 files changed

+291
-45
lines changed

‎pkg/kubelet/container/helpers.go

+25
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,8 @@ func MakePortMappings(container *v1.Container) (ports []PortMapping) {
396396

397397
// HasAnyRegularContainerStarted returns true if any regular container has
398398
// started, which indicates all init containers have been initialized.
399+
// Deprecated: This function is not accurate when its pod sandbox is recreated.
400+
// Use HasAnyActiveRegularContainerStarted instead.
399401
func HasAnyRegularContainerStarted(spec *v1.PodSpec, statuses []v1.ContainerStatus) bool {
400402
if len(statuses) == 0 {
401403
return false
@@ -417,3 +419,26 @@ func HasAnyRegularContainerStarted(spec *v1.PodSpec, statuses []v1.ContainerStat
417419

418420
return false
419421
}
422+
423+
// HasAnyActiveRegularContainerStarted returns true if any regular container of
424+
// the current pod sandbox has started, which indicates all init containers
425+
// have been initialized.
426+
func HasAnyActiveRegularContainerStarted(spec *v1.PodSpec, podStatus *PodStatus) bool {
427+
if podStatus == nil {
428+
return false
429+
}
430+
431+
containerNames := sets.New[string]()
432+
for _, c := range spec.Containers {
433+
containerNames.Insert(c.Name)
434+
}
435+
436+
for _, status := range podStatus.ActiveContainerStatuses {
437+
if !containerNames.Has(status.Name) {
438+
continue
439+
}
440+
return true
441+
}
442+
443+
return false
444+
}

‎pkg/kubelet/container/helpers_test.go

+140
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
v1 "k8s.io/api/core/v1"
2828
"k8s.io/apimachinery/pkg/api/resource"
2929
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
3031
)
3132

3233
func TestEnvVarsToMap(t *testing.T) {
@@ -989,3 +990,142 @@ func TestHashContainerWithoutResources(t *testing.T) {
989990
})
990991
}
991992
}
993+
994+
func TestHasAnyActiveRegularContainerStarted(t *testing.T) {
995+
testCases := []struct {
996+
desc string
997+
spec *v1.PodSpec
998+
podStatus *PodStatus
999+
expected bool
1000+
}{
1001+
{
1002+
desc: "pod has no active container",
1003+
spec: &v1.PodSpec{
1004+
InitContainers: []v1.Container{
1005+
{
1006+
Name: "init",
1007+
},
1008+
},
1009+
Containers: []v1.Container{
1010+
{
1011+
Name: "regular",
1012+
},
1013+
},
1014+
},
1015+
podStatus: &PodStatus{
1016+
SandboxStatuses: []*runtimeapi.PodSandboxStatus{
1017+
{
1018+
Id: "old",
1019+
State: runtimeapi.PodSandboxState_SANDBOX_NOTREADY,
1020+
},
1021+
},
1022+
},
1023+
expected: false,
1024+
},
1025+
{
1026+
desc: "pod is initializing",
1027+
spec: &v1.PodSpec{
1028+
InitContainers: []v1.Container{
1029+
{
1030+
Name: "init",
1031+
},
1032+
},
1033+
Containers: []v1.Container{
1034+
{
1035+
Name: "regular",
1036+
},
1037+
},
1038+
},
1039+
podStatus: &PodStatus{
1040+
SandboxStatuses: []*runtimeapi.PodSandboxStatus{
1041+
{
1042+
Id: "current",
1043+
State: runtimeapi.PodSandboxState_SANDBOX_READY,
1044+
},
1045+
},
1046+
ActiveContainerStatuses: []*Status{
1047+
{
1048+
Name: "init",
1049+
State: ContainerStateRunning,
1050+
},
1051+
},
1052+
},
1053+
expected: false,
1054+
},
1055+
{
1056+
desc: "pod has initialized",
1057+
spec: &v1.PodSpec{
1058+
InitContainers: []v1.Container{
1059+
{
1060+
Name: "init",
1061+
},
1062+
},
1063+
Containers: []v1.Container{
1064+
{
1065+
Name: "regular",
1066+
},
1067+
},
1068+
},
1069+
podStatus: &PodStatus{
1070+
SandboxStatuses: []*runtimeapi.PodSandboxStatus{
1071+
{
1072+
Id: "current",
1073+
State: runtimeapi.PodSandboxState_SANDBOX_READY,
1074+
},
1075+
},
1076+
ActiveContainerStatuses: []*Status{
1077+
{
1078+
Name: "init",
1079+
State: ContainerStateExited,
1080+
},
1081+
{
1082+
Name: "regular",
1083+
State: ContainerStateRunning,
1084+
},
1085+
},
1086+
},
1087+
expected: true,
1088+
},
1089+
{
1090+
desc: "pod is re-initializing after the sandbox recreation",
1091+
spec: &v1.PodSpec{
1092+
InitContainers: []v1.Container{
1093+
{
1094+
Name: "init",
1095+
},
1096+
},
1097+
Containers: []v1.Container{
1098+
{
1099+
Name: "regular",
1100+
},
1101+
},
1102+
},
1103+
podStatus: &PodStatus{
1104+
SandboxStatuses: []*runtimeapi.PodSandboxStatus{
1105+
{
1106+
Id: "current",
1107+
State: runtimeapi.PodSandboxState_SANDBOX_READY,
1108+
},
1109+
{
1110+
Id: "old",
1111+
State: runtimeapi.PodSandboxState_SANDBOX_NOTREADY,
1112+
},
1113+
},
1114+
ActiveContainerStatuses: []*Status{
1115+
{
1116+
Name: "init",
1117+
State: ContainerStateRunning,
1118+
},
1119+
},
1120+
},
1121+
expected: false,
1122+
},
1123+
}
1124+
1125+
for _, tc := range testCases {
1126+
t.Run(tc.desc, func(t *testing.T) {
1127+
actual := HasAnyActiveRegularContainerStarted(tc.spec, tc.podStatus)
1128+
assert.Equal(t, tc.expected, actual)
1129+
})
1130+
}
1131+
}

‎pkg/kubelet/container/runtime.go

+2
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,8 @@ type PodStatus struct {
321321
IPs []string
322322
// Status of containers in the pod.
323323
ContainerStatuses []*Status
324+
// Statuses of containers of the active sandbox in the pod.
325+
ActiveContainerStatuses []*Status
324326
// Status of the pod sandbox.
325327
// Only for kuberuntime now, other runtime may keep it nil.
326328
SandboxStatuses []*runtimeapi.PodSandboxStatus

‎pkg/kubelet/kubelet_pods.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -1576,7 +1576,7 @@ func (kl *Kubelet) GetKubeletContainerLogs(ctx context.Context, podFullName, con
15761576
}
15771577

15781578
// getPhase returns the phase of a pod given its container info.
1579-
func getPhase(pod *v1.Pod, info []v1.ContainerStatus, podIsTerminal bool) v1.PodPhase {
1579+
func getPhase(pod *v1.Pod, info []v1.ContainerStatus, podIsTerminal, podHasInitialized bool) v1.PodPhase {
15801580
spec := pod.Spec
15811581
pendingRestartableInitContainers := 0
15821582
pendingRegularInitContainers := 0
@@ -1695,7 +1695,7 @@ func getPhase(pod *v1.Pod, info []v1.ContainerStatus, podIsTerminal bool) v1.Pod
16951695
// This is needed to handle the case where the pod has been initialized but
16961696
// the restartable init containers are restarting and the pod should not be
16971697
// placed back into v1.PodPending since the regular containers have run.
1698-
!kubecontainer.HasAnyRegularContainerStarted(&spec, info)):
1698+
!podHasInitialized):
16991699
fallthrough
17001700
case waiting > 0:
17011701
klog.V(5).InfoS("Pod waiting > 0, pending")
@@ -1768,7 +1768,7 @@ func (kl *Kubelet) generateAPIPodStatus(pod *v1.Pod, podStatus *kubecontainer.Po
17681768
s := kl.convertStatusToAPIStatus(pod, podStatus, oldPodStatus)
17691769
// calculate the next phase and preserve reason
17701770
allStatus := append(append([]v1.ContainerStatus{}, s.ContainerStatuses...), s.InitContainerStatuses...)
1771-
s.Phase = getPhase(pod, allStatus, podIsTerminal)
1771+
s.Phase = getPhase(pod, allStatus, podIsTerminal, kubecontainer.HasAnyActiveRegularContainerStarted(&pod.Spec, podStatus))
17721772
klog.V(4).InfoS("Got phase for pod", "pod", klog.KObj(pod), "oldPhase", oldPodStatus.Phase, "phase", s.Phase)
17731773

17741774
// Perform a three-way merge between the statuses from the status manager,

0 commit comments

Comments
 (0)