diff --git a/pkg/controller/job/backoff_utils.go b/pkg/controller/job/backoff_utils.go index ecd86dc3113..3bef68117b8 100644 --- a/pkg/controller/job/backoff_utils.go +++ b/pkg/controller/job/backoff_utils.go @@ -22,9 +22,12 @@ import ( "time" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" apipod "k8s.io/kubernetes/pkg/api/v1/pod" + "k8s.io/kubernetes/pkg/features" "k8s.io/utils/clock" "k8s.io/utils/ptr" ) @@ -183,12 +186,32 @@ func getFinishedTime(p *v1.Pod) time.Time { } func getFinishTimeFromContainers(p *v1.Pod) *time.Time { - var finishTime *time.Time - for _, containerState := range p.Status.ContainerStatuses { - if containerState.State.Terminated == nil { - return nil + finishTime := latestFinishTime(nil, p.Status.ContainerStatuses, nil) + if utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) { + // We need to check InitContainerStatuses here also, + // because with the sidecar (restartable init) containers, + // sidecar containers will always finish later than regular containers. + names := sets.New[string]() + for _, c := range p.Spec.InitContainers { + if c.RestartPolicy != nil && *c.RestartPolicy == v1.ContainerRestartPolicyAlways { + names.Insert(c.Name) + } } - if containerState.State.Terminated.FinishedAt.Time.IsZero() { + finishTime = latestFinishTime(finishTime, p.Status.InitContainerStatuses, func(status v1.ContainerStatus) bool { + return names.Has(status.Name) + }) + } + return finishTime +} + +func latestFinishTime(prevFinishTime *time.Time, cs []v1.ContainerStatus, check func(status v1.ContainerStatus) bool) *time.Time { + var finishTime = prevFinishTime + for _, containerState := range cs { + if check != nil && !check(containerState) { + continue + } + if containerState.State.Terminated == nil || + containerState.State.Terminated.FinishedAt.Time.IsZero() { return nil } if finishTime == nil || finishTime.Before(containerState.State.Terminated.FinishedAt.Time) { diff --git a/pkg/controller/job/backoff_utils_test.go b/pkg/controller/job/backoff_utils_test.go index e300982c0b8..677d8251cb8 100644 --- a/pkg/controller/job/backoff_utils_test.go +++ b/pkg/controller/job/backoff_utils_test.go @@ -23,7 +23,10 @@ import ( "github.com/google/go-cmp/cmp" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/klog/v2/ktesting" + "k8s.io/kubernetes/pkg/features" clocktesting "k8s.io/utils/clock/testing" "k8s.io/utils/ptr" ) @@ -200,8 +203,10 @@ func TestNewBackoffRecord(t *testing.T) { } func TestGetFinishedTime(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SidecarContainers, true) defaultTestTime := time.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC) defaultTestTimeMinus30s := defaultTestTime.Add(-30 * time.Second) + containerRestartPolicyAlways := v1.ContainerRestartPolicyAlways testCases := map[string]struct { pod v1.Pod wantFinishTime time.Time @@ -355,6 +360,39 @@ func TestGetFinishedTime(t *testing.T) { }, wantFinishTime: defaultTestTime, }, + // In this case, init container is stopped after the regular containers. + // This is because with the sidecar (restartable init) containers, + // sidecar containers will always finish later than regular containers. + "Pod with sidecar container and all containers terminated": { + pod: v1.Pod{ + Spec: v1.PodSpec{ + InitContainers: []v1.Container{ + { + Name: "sidecar", + RestartPolicy: &containerRestartPolicyAlways, + }, + }, + }, + Status: v1.PodStatus{ + ContainerStatuses: []v1.ContainerStatus{ + { + State: v1.ContainerState{ + Terminated: &v1.ContainerStateTerminated{FinishedAt: metav1.NewTime(defaultTestTime.Add(-1 * time.Second))}, + }, + }, + }, + InitContainerStatuses: []v1.ContainerStatus{ + { + Name: "sidecar", + State: v1.ContainerState{ + Terminated: &v1.ContainerStateTerminated{FinishedAt: metav1.NewTime(defaultTestTime)}, + }, + }, + }, + }, + }, + wantFinishTime: defaultTestTime, + }, } for name, tc := range testCases {