Skip to content

Commit

Permalink
Merge pull request kubernetes#127204 from SergeyKanzhelev/automated-c…
Browse files Browse the repository at this point in the history
…herry-pick-of-#127162-upstream-release-1.29

Automated cherry pick of kubernetes#127162: Avoid SidecarContainers code path for non-sidecar pods
  • Loading branch information
k8s-ci-robot authored Sep 9, 2024
2 parents cd195a0 + 13e0727 commit 9921687
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 12 deletions.
2 changes: 1 addition & 1 deletion pkg/kubelet/kuberuntime/kuberuntime_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ func (m *kubeGenericRuntimeManager) killContainersWithSyncResult(ctx context.Con
wg.Add(len(runningPod.Containers))
var termOrdering *terminationOrdering
// we only care about container termination ordering if the sidecars feature is enabled
if utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
if utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) && types.HasRestartableInitContainer(pod) {
var runningContainerNames []string
for _, container := range runningPod.Containers {
runningContainerNames = append(runningContainerNames, container.Name)
Expand Down
12 changes: 7 additions & 5 deletions pkg/kubelet/kuberuntime/kuberuntime_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,8 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod *
ContainersToKill: make(map[kubecontainer.ContainerID]containerToKillInfo),
}

handleRestartableInitContainers := utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) && types.HasRestartableInitContainer(pod)

// If we need to (re-)create the pod sandbox, everything will need to be
// killed and recreated, and init containers should be purged.
if createPodSandbox {
Expand Down Expand Up @@ -873,7 +875,7 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod *
// is done and there is no container to start.
if len(containersToStart) == 0 {
hasInitialized := false
if !utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
if !handleRestartableInitContainers {
_, _, hasInitialized = findNextInitContainerToRun(pod, podStatus)
} else {
// If there is any regular container, it means all init containers have
Expand All @@ -891,7 +893,7 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod *
// state.
if len(pod.Spec.InitContainers) != 0 {
// Pod has init containers, return the first one.
if !utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
if !handleRestartableInitContainers {
changes.NextInitContainerToStart = &pod.Spec.InitContainers[0]
} else {
changes.InitContainersToStart = []int{0}
Expand All @@ -914,7 +916,7 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod *
}

// Check initialization progress.
if !utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
if !handleRestartableInitContainers {
initLastStatus, next, done := findNextInitContainerToRun(pod, podStatus)
if !done {
if next != nil {
Expand Down Expand Up @@ -1041,7 +1043,7 @@ func (m *kubeGenericRuntimeManager) computePodActions(ctx context.Context, pod *

if keepCount == 0 && len(changes.ContainersToStart) == 0 {
changes.KillPod = true
if utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
if handleRestartableInitContainers {
// To prevent the restartable init containers to keep pod alive, we should
// not restart them.
changes.InitContainersToStart = nil
Expand Down Expand Up @@ -1285,7 +1287,7 @@ func (m *kubeGenericRuntimeManager) SyncPod(ctx context.Context, pod *v1.Pod, po
start(ctx, "ephemeral container", metrics.EphemeralContainer, ephemeralContainerStartSpec(&pod.Spec.EphemeralContainers[idx]))
}

if !utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
if !types.HasRestartableInitContainer(pod) {
// Step 6: start the init container.
if container := podContainerChanges.NextInitContainerToStart; container != nil {
// Start the next init container.
Expand Down
33 changes: 27 additions & 6 deletions pkg/kubelet/kuberuntime/kuberuntime_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
containertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
proberesults "k8s.io/kubernetes/pkg/kubelet/prober/results"
kubelettypes "k8s.io/kubernetes/pkg/kubelet/types"
)

var (
Expand Down Expand Up @@ -1427,6 +1428,20 @@ func testComputePodActionsWithInitContainers(t *testing.T, sidecarContainersEnab
ContainersToKill: getKillMapWithInitContainers(basePod, baseStatus, []int{}),
},
},
"an init container is in the created state due to an unknown error when starting container; restart it": {
mutatePodFn: func(pod *v1.Pod) { pod.Spec.RestartPolicy = v1.RestartPolicyAlways },
mutateStatusFn: func(status *kubecontainer.PodStatus) {
status.ContainerStatuses[2].State = kubecontainer.ContainerStateCreated
},
actions: podActions{
KillPod: false,
SandboxID: baseStatus.SandboxStatuses[0].Id,
NextInitContainerToStart: &basePod.Spec.InitContainers[2],
InitContainersToStart: []int{2},
ContainersToStart: []int{},
ContainersToKill: getKillMapWithInitContainers(basePod, baseStatus, []int{}),
},
},
} {
pod, status := makeBasePodAndStatusWithInitContainers()
if test.mutatePodFn != nil {
Expand All @@ -1437,12 +1452,15 @@ func testComputePodActionsWithInitContainers(t *testing.T, sidecarContainersEnab
}
ctx := context.Background()
actions := m.computePodActions(ctx, pod, status)
if !sidecarContainersEnabled {
// If sidecar containers are disabled, we should not see any
handleRestartableInitContainers := sidecarContainersEnabled && kubelettypes.HasRestartableInitContainer(pod)
if !handleRestartableInitContainers {
// If sidecar containers are disabled or the pod does not have any
// restartable init container, we should not see any
// InitContainersToStart in the actions.
test.actions.InitContainersToStart = nil
} else {
// If sidecar containers are enabled, we should not see any
// If sidecar containers are enabled and the pod has any
// restartable init container, we should not see any
// NextInitContainerToStart in the actions.
test.actions.NextInitContainerToStart = nil
}
Expand Down Expand Up @@ -2040,12 +2058,15 @@ func testComputePodActionsWithInitAndEphemeralContainers(t *testing.T, sidecarCo
}
ctx := context.Background()
actions := m.computePodActions(ctx, pod, status)
if !sidecarContainersEnabled {
// If sidecar containers are disabled, we should not see any
handleRestartableInitContainers := sidecarContainersEnabled && kubelettypes.HasRestartableInitContainer(pod)
if !handleRestartableInitContainers {
// If sidecar containers are disabled or the pod does not have any
// restartable init container, we should not see any
// InitContainersToStart in the actions.
test.actions.InitContainersToStart = nil
} else {
// If sidecar containers are enabled, we should not see any
// If sidecar containers are enabled and the pod has any
// restartable init container, we should not see any
// NextInitContainerToStart in the actions.
test.actions.NextInitContainerToStart = nil
}
Expand Down
11 changes: 11 additions & 0 deletions pkg/kubelet/types/pod_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,14 @@ func IsRestartableInitContainer(initContainer *v1.Container) bool {

return *initContainer.RestartPolicy == v1.ContainerRestartPolicyAlways
}

// HasRestartableInitContainer returns true if the pod has any restartable init
// container
func HasRestartableInitContainer(pod *v1.Pod) bool {
for _, container := range pod.Spec.InitContainers {
if IsRestartableInitContainer(&container) {
return true
}
}
return false
}

0 comments on commit 9921687

Please sign in to comment.