Skip to content

Commit

Permalink
Merge pull request kubernetes#128676 from vivzbansal/sidecar-3
Browse files Browse the repository at this point in the history
Refactor: Move IsRestartableInitContainer to common utility package
  • Loading branch information
k8s-ci-robot authored Nov 8, 2024
2 parents e1e92bc + cf8ee42 commit 210f129
Show file tree
Hide file tree
Showing 17 changed files with 62 additions and 61 deletions.
10 changes: 10 additions & 0 deletions pkg/api/pod/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -1227,6 +1227,16 @@ func hasInvalidLabelValueInAffinitySelector(spec *api.PodSpec) bool {
return false
}

// IsRestartableInitContainer returns true if the container has ContainerRestartPolicyAlways.
// This function is not checking if the container passed to it is indeed an init container.
// It is just checking if the container restart policy has been set to always.
func IsRestartableInitContainer(initContainer *api.Container) bool {
if initContainer == nil || initContainer.RestartPolicy == nil {
return false
}
return *initContainer.RestartPolicy == api.ContainerRestartPolicyAlways
}

func MarkPodProposedForResize(oldPod, newPod *api.Pod) {
if len(newPod.Spec.Containers) != len(oldPod.Spec.Containers) {
// Update is invalid: ignore changes and let validation handle it
Expand Down
10 changes: 10 additions & 0 deletions pkg/api/v1/pod/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,3 +406,13 @@ func UpdatePodCondition(status *v1.PodStatus, condition *v1.PodCondition) bool {
// Return true if one of the fields have changed.
return !isEqual
}

// IsRestartableInitContainer returns true if the container has ContainerRestartPolicyAlways.
// This function is not checking if the container passed to it is indeed an init container.
// It is just checking if the container restart policy has been set to always.
func IsRestartableInitContainer(initContainer *v1.Container) bool {
if initContainer == nil || initContainer.RestartPolicy == nil {
return false
}
return *initContainer.RestartPolicy == v1.ContainerRestartPolicyAlways
}
6 changes: 3 additions & 3 deletions pkg/kubelet/apis/podresources/server_v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import (

v1 "k8s.io/api/core/v1"
utilfeature "k8s.io/apiserver/pkg/util/feature"
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
kubefeatures "k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/kubelet/metrics"
"k8s.io/kubernetes/pkg/kubelet/types"

podresourcesv1 "k8s.io/kubelet/pkg/apis/podresources/v1"
)
Expand Down Expand Up @@ -70,7 +70,7 @@ func (p *v1PodResourcesServer) List(ctx context.Context, req *podresourcesv1.Lis
pRes.Containers = make([]*podresourcesv1.ContainerResources, 0, len(pod.Spec.InitContainers)+len(pod.Spec.Containers))

for _, container := range pod.Spec.InitContainers {
if !types.IsRestartableInitContainer(&container) {
if !podutil.IsRestartableInitContainer(&container) {
continue
}

Expand Down Expand Up @@ -130,7 +130,7 @@ func (p *v1PodResourcesServer) Get(ctx context.Context, req *podresourcesv1.GetP
podResources.Containers = make([]*podresourcesv1.ContainerResources, 0, len(pod.Spec.InitContainers)+len(pod.Spec.Containers))

for _, container := range pod.Spec.InitContainers {
if !types.IsRestartableInitContainer(&container) {
if !podutil.IsRestartableInitContainer(&container) {
continue
}

Expand Down
5 changes: 2 additions & 3 deletions pkg/kubelet/cm/cpumanager/policy_static.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager"
"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/bitmask"
"k8s.io/kubernetes/pkg/kubelet/metrics"
"k8s.io/kubernetes/pkg/kubelet/types"
"k8s.io/utils/cpuset"
)

Expand Down Expand Up @@ -298,7 +297,7 @@ func (p *staticPolicy) updateCPUsToReuse(pod *v1.Pod, container *v1.Container, c
// If so, add its cpuset to the cpuset of reusable CPUs for any new allocations.
for _, initContainer := range pod.Spec.InitContainers {
if container.Name == initContainer.Name {
if types.IsRestartableInitContainer(&initContainer) {
if podutil.IsRestartableInitContainer(&initContainer) {
// If the container is a restartable init container, we should not
// reuse its cpuset, as a restartable init container can run with
// regular containers.
Expand Down Expand Up @@ -489,7 +488,7 @@ func (p *staticPolicy) podGuaranteedCPUs(pod *v1.Pod) int {
requestedCPU := p.guaranteedCPUs(pod, &container)
// See https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/753-sidecar-containers#resources-calculation-for-scheduling-and-pod-admission
// for the detail.
if types.IsRestartableInitContainer(&container) {
if podutil.IsRestartableInitContainer(&container) {
requestedByRestartableInitContainers += requestedCPU
} else if requestedByRestartableInitContainers+requestedCPU > requestedByInitContainers {
requestedByInitContainers = requestedByRestartableInitContainers + requestedCPU
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/cm/devicemanager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"k8s.io/apiserver/pkg/server/healthz"
utilfeature "k8s.io/apiserver/pkg/util/feature"
pluginapi "k8s.io/kubelet/pkg/apis/deviceplugin/v1beta1"
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/kubelet/checkpointmanager"
"k8s.io/kubernetes/pkg/kubelet/checkpointmanager/errors"
Expand All @@ -49,7 +50,6 @@ import (
"k8s.io/kubernetes/pkg/kubelet/lifecycle"
"k8s.io/kubernetes/pkg/kubelet/metrics"
"k8s.io/kubernetes/pkg/kubelet/pluginmanager/cache"
"k8s.io/kubernetes/pkg/kubelet/types"
schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework"
)

Expand Down Expand Up @@ -384,7 +384,7 @@ func (m *ManagerImpl) Allocate(pod *v1.Pod, container *v1.Container) error {
if err := m.allocateContainerResources(pod, container, m.devicesToReuse[string(pod.UID)]); err != nil {
return err
}
if !types.IsRestartableInitContainer(&initContainer) {
if !podutil.IsRestartableInitContainer(&initContainer) {
m.podDevices.addContainerAllocatedResources(string(pod.UID), container.Name, m.devicesToReuse[string(pod.UID)])
} else {
// If the init container is restartable, we need to keep the
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/cm/memorymanager/memory_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
"k8s.io/klog/v2"
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
corev1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
"k8s.io/kubernetes/pkg/kubelet/cm/containermap"
"k8s.io/kubernetes/pkg/kubelet/cm/memorymanager/state"
"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager"
"k8s.io/kubernetes/pkg/kubelet/config"
"k8s.io/kubernetes/pkg/kubelet/status"
"k8s.io/kubernetes/pkg/kubelet/types"
)

// memoryManagerStateFileName is the file name where memory manager stores its state
Expand Down Expand Up @@ -228,7 +228,7 @@ func (m *manager) AddContainer(pod *v1.Pod, container *v1.Container, containerID
// Since a restartable init container remains running for the full
// duration of the pod's lifecycle, we should not remove it from the
// memory manager state.
if types.IsRestartableInitContainer(&initContainer) {
if podutil.IsRestartableInitContainer(&initContainer) {
continue
}

Expand Down
7 changes: 3 additions & 4 deletions pkg/kubelet/cm/memorymanager/policy_static.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager"
"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/bitmask"
"k8s.io/kubernetes/pkg/kubelet/metrics"
"k8s.io/kubernetes/pkg/kubelet/types"
)

const policyTypeStatic policyType = "Static"
Expand Down Expand Up @@ -353,7 +352,7 @@ func getPodRequestedResources(pod *v1.Pod) (map[v1.ResourceName]uint64, error) {

// See https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/753-sidecar-containers#resources-calculation-for-scheduling-and-pod-admission
// for the detail.
if types.IsRestartableInitContainer(&ctr) {
if podutil.IsRestartableInitContainer(&ctr) {
reqRsrcsByRestartableInitCtrs[rsrcName] += qty
} else if reqRsrcsByRestartableInitCtrs[rsrcName]+qty > reqRsrcsByInitCtrs[rsrcName] {
reqRsrcsByInitCtrs[rsrcName] = reqRsrcsByRestartableInitCtrs[rsrcName] + qty
Expand Down Expand Up @@ -969,7 +968,7 @@ func (p *staticPolicy) updateInitContainersMemoryBlocks(s state.State, pod *v1.P
break
}

if types.IsRestartableInitContainer(&initContainer) {
if podutil.IsRestartableInitContainer(&initContainer) {
// we should not reuse the resource from any restartable init
// container
continue
Expand Down Expand Up @@ -1011,7 +1010,7 @@ func (p *staticPolicy) updateInitContainersMemoryBlocks(s state.State, pod *v1.P
func isRegularInitContainer(pod *v1.Pod, container *v1.Container) bool {
for _, initContainer := range pod.Spec.InitContainers {
if initContainer.Name == container.Name {
return !types.IsRestartableInitContainer(&initContainer)
return !podutil.IsRestartableInitContainer(&initContainer)
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/kubelet_pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -1585,7 +1585,7 @@ func getPhase(pod *v1.Pod, info []v1.ContainerStatus, podIsTerminal bool) v1.Pod

// regular init containers
for _, container := range spec.InitContainers {
if kubetypes.IsRestartableInitContainer(&container) {
if podutil.IsRestartableInitContainer(&container) {
// Skip the restartable init containers here to handle them separately as
// they are slightly different from the init containers in terms of the
// pod phase.
Expand Down Expand Up @@ -1628,7 +1628,7 @@ func getPhase(pod *v1.Pod, info []v1.ContainerStatus, podIsTerminal bool) v1.Pod
if utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
// restartable init containers
for _, container := range spec.InitContainers {
if !kubetypes.IsRestartableInitContainer(&container) {
if !podutil.IsRestartableInitContainer(&container) {
// Skip the regular init containers, as they have been handled above.
continue
}
Expand Down
13 changes: 7 additions & 6 deletions pkg/kubelet/kuberuntime/kuberuntime_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
remote "k8s.io/cri-client/pkg"
kubelettypes "k8s.io/kubelet/pkg/types"
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
"k8s.io/kubernetes/pkg/features"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
"k8s.io/kubernetes/pkg/kubelet/events"
Expand Down Expand Up @@ -1069,13 +1070,13 @@ func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, pod
// If the container is previously initialized but its status is not
// found, it means its last status is removed for some reason.
// Restart it if it is a restartable init container.
if isPreviouslyInitialized && types.IsRestartableInitContainer(container) {
if isPreviouslyInitialized && podutil.IsRestartableInitContainer(container) {
changes.InitContainersToStart = append(changes.InitContainersToStart, i)
}
continue
}

if isPreviouslyInitialized && !types.IsRestartableInitContainer(container) {
if isPreviouslyInitialized && !podutil.IsRestartableInitContainer(container) {
// after initialization, only restartable init containers need to be kept
// running
continue
Expand All @@ -1091,11 +1092,11 @@ func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, pod
changes.InitContainersToStart = append(changes.InitContainersToStart, i)

case kubecontainer.ContainerStateRunning:
if !types.IsRestartableInitContainer(container) {
if !podutil.IsRestartableInitContainer(container) {
break
}

if types.IsRestartableInitContainer(container) {
if podutil.IsRestartableInitContainer(container) {
if container.StartupProbe != nil {
startup, found := m.startupManager.Get(status.ID)
if !found {
Expand Down Expand Up @@ -1166,7 +1167,7 @@ func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, pod
// If the init container failed and the restart policy is Never, the pod is terminal.
// Otherwise, restart the init container.
case kubecontainer.ContainerStateExited:
if types.IsRestartableInitContainer(container) {
if podutil.IsRestartableInitContainer(container) {
changes.InitContainersToStart = append(changes.InitContainersToStart, i)
} else { // init container
if isInitContainerFailed(status) {
Expand All @@ -1189,7 +1190,7 @@ func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, pod
}

default: // kubecontainer.ContainerStatusUnknown or other unknown states
if types.IsRestartableInitContainer(container) {
if podutil.IsRestartableInitContainer(container) {
// If the restartable init container is in unknown state, restart it.
changes.ContainersToKill[status.ID] = containerToKillInfo{
name: container.Name,
Expand Down
3 changes: 2 additions & 1 deletion pkg/kubelet/kuberuntime/kuberuntime_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import (
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"

"k8s.io/kubernetes/pkg/api/legacyscheme"
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
"k8s.io/kubernetes/pkg/credentialprovider"
"k8s.io/kubernetes/pkg/credentialprovider/plugin"
"k8s.io/kubernetes/pkg/features"
Expand Down Expand Up @@ -1327,7 +1328,7 @@ func (m *kubeGenericRuntimeManager) SyncPod(ctx context.Context, pod *v1.Pod, po
container := &pod.Spec.InitContainers[idx]
// Start the next init container.
if err := start(ctx, "init container", metrics.InitContainer, containerStartSpec(container)); err != nil {
if types.IsRestartableInitContainer(container) {
if podutil.IsRestartableInitContainer(container) {
klog.V(4).InfoS("Failed to start the restartable init container for the pod, skipping", "initContainerName", container.Name, "pod", klog.KObj(pod))
continue
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/kuberuntime/kuberuntime_termination_order.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (

v1 "k8s.io/api/core/v1"

"k8s.io/kubernetes/pkg/kubelet/types"
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
)

// terminationOrdering is used to enforce a termination ordering for sidecar containers. It sets up
Expand Down Expand Up @@ -80,7 +80,7 @@ func newTerminationOrdering(pod *v1.Pod, runningContainerNames []string) *termin
close(channel)
}

if types.IsRestartableInitContainer(&ic) {
if podutil.IsRestartableInitContainer(&ic) {
// sidecars need to wait for all main containers to exit
to.prereqs[ic.Name] = append(to.prereqs[ic.Name], mainContainerChannels...)

Expand Down
3 changes: 2 additions & 1 deletion pkg/kubelet/lifecycle/predicate.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/component-helpers/scheduling/corev1"
"k8s.io/klog/v2"
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/kubelet/types"
Expand Down Expand Up @@ -141,7 +142,7 @@ func (w *predicateAdmitHandler) Admit(attrs *PodAdmitAttributes) PodAdmitResult
// TODO: Remove this after the SidecarContainers feature gate graduates to GA.
if !utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
for _, c := range admitPod.Spec.InitContainers {
if types.IsRestartableInitContainer(&c) {
if podutil.IsRestartableInitContainer(&c) {
message := fmt.Sprintf("Init container %q may not have a non-default restartPolicy", c.Name)
klog.InfoS("Failed to admit pod", "pod", klog.KObj(admitPod), "message", message)
return PodAdmitResult{
Expand Down
6 changes: 3 additions & 3 deletions pkg/kubelet/prober/prober_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ import (
"k8s.io/client-go/tools/record"
"k8s.io/component-base/metrics"
"k8s.io/klog/v2"
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
"k8s.io/kubernetes/pkg/kubelet/prober/results"
"k8s.io/kubernetes/pkg/kubelet/status"
kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
kubeutil "k8s.io/kubernetes/pkg/kubelet/util"
"k8s.io/utils/clock"
)
Expand Down Expand Up @@ -171,7 +171,7 @@ func (t probeType) String() string {
func getRestartableInitContainers(pod *v1.Pod) []v1.Container {
var restartableInitContainers []v1.Container
for _, c := range pod.Spec.InitContainers {
if kubetypes.IsRestartableInitContainer(&c) {
if podutil.IsRestartableInitContainer(&c) {
restartableInitContainers = append(restartableInitContainers, c)
}
}
Expand Down Expand Up @@ -325,7 +325,7 @@ func (m *manager) UpdatePodStatus(pod *v1.Pod, podStatus *v1.PodStatus) {
klog.V(4).InfoS("Mismatch between pod spec and status, likely programmer error", "pod", klog.KObj(pod), "containerName", c.Name)
continue
}
if !kubetypes.IsRestartableInitContainer(&initContainer) {
if !podutil.IsRestartableInitContainer(&initContainer) {
if c.State.Terminated != nil && c.State.Terminated.ExitCode == 0 {
podStatus.InitContainerStatuses[i].Ready = true
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/kubelet/status/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
runtimeutil "k8s.io/kubernetes/pkg/kubelet/kuberuntime/util"
kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
)

const (
Expand Down Expand Up @@ -57,7 +56,7 @@ func GenerateContainersReadyCondition(spec *v1.PodSpec, containerStatuses []v1.C
unreadyContainers := []string{}

for _, container := range spec.InitContainers {
if !kubetypes.IsRestartableInitContainer(&container) {
if !podutil.IsRestartableInitContainer(&container) {
continue
}

Expand Down Expand Up @@ -159,7 +158,7 @@ func GeneratePodReadyCondition(spec *v1.PodSpec, conditions []v1.PodCondition, c
}

func isInitContainerInitialized(initContainer *v1.Container, containerStatus *v1.ContainerStatus) bool {
if kubetypes.IsRestartableInitContainer(initContainer) {
if podutil.IsRestartableInitContainer(initContainer) {
if containerStatus.Started == nil || !*containerStatus.Started {
return false
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/status/status_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ func hasPodInitialized(pod *v1.Pod) bool {
}

containerStatus := pod.Status.InitContainerStatuses[l-1]
if kubetypes.IsRestartableInitContainer(&container) {
if podutil.IsRestartableInitContainer(&container) {
if containerStatus.State.Running != nil &&
containerStatus.Started != nil && *containerStatus.Started {
return true
Expand Down Expand Up @@ -616,7 +616,7 @@ func checkContainerStateTransition(oldStatuses, newStatuses *v1.PodStatus, podSp
return fmt.Errorf("found mismatch between pod spec and status, container: %v", oldStatus.Name)
}
// Skip any restartable init container as it always is allowed to restart
if kubetypes.IsRestartableInitContainer(&initContainer) {
if podutil.IsRestartableInitContainer(&initContainer) {
continue
}
// Skip any container that wasn't terminated
Expand Down
Loading

0 comments on commit 210f129

Please sign in to comment.