fix: initContainer restart policy overridden by pod

[tony84727]

Using main branch version: f4d2d17.

Restart policy of initContainers are overriden by restart policy of the pod.

Reproduce steps:

1. Create a pod with a initContainer by podman kube play and the following manifest, podman kube play pod.yaml
pod.yaml

   ---
   apiVersion: apps/v1
   kind: Pod
   metadata:
     name: test
   spec:
     containers:
       - name: nginx
         image: 'docker.io/nginx'
     initContainers:
       - name: first
         image: docker.io/nginx

   Because the initContainer first will start a nginx daemon and will not exit, podman kube play will hang (as expected)
2. Inspect restart policy of the initContainer by podman inspect test-first --format '{{ .HostConfig.RestartPolicy.Name }}'
3. Observe the restart policy of the initContainer is not no but always instead

Does this PR introduce a user-facing change?

Init containers created by kubernetes file will not be restarted multiple times.

Todo

• add a test to check restart policy of initContainers

[openshift-ci]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[vrothberg]

@umohnani8 PTAL

Let's make sure to have similar checks for service and infra containers as well.

[umohnani8]

@tony84727 would you like to convert this into a PR and add a check for service container as well? If not, I can take over

[tony84727]

@umohnani8 I was planning to add some test, at least for initContainers. Feel free to take over, though.

[tony84727]

Added a e2e test case for init containers, converting this to an PR.

[tony84727]

This test failed in my local environment. It might means init containers added by podman create --init-ctr doesn't have a restart policy name.

[umohnani8]

Yeah, looks like restart policy is not added to an init container even when the --restart flag is set.

[tony84727]

Removed this test. So that, this PR only fix and test restart policy of init container created by podman kube play. Is it okay to handle this in another PR or issue?

[umohnani8]

Based on https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#:~:text=If%20a%20Pod's%20init%20container,the%20overall%20Pod%20as%20failed., init containers should have a restart policy of on-failure unless the pod has a restart policy never then the init container should also have a restart policy of never.

[tony84727]

Agree. so basically, the restart policy of an init container is inferred from the restart policy of the pod and users are not allowed to modify or override (i.e. through --restart)?

pod restart policy	init ctr restart policy
none(missing)	on-failure
always	on-failure
on-failure	on-failure
unless-stopped	on-failure
never	never

[edsantiago]

All this policy discussion is way over my head, but I'd just like to chime in and say that this PR fixes #18477 so I'd like to see it proceed.

[tony84727]

Maybe I can just remove the failing test and keep restart policy of init containers (created by podman kube play) as no for now? And probably open another issue to continue discuss this?

I'm wondering if there's a spec to follow, regarding to the proper behavior of various restart policies. Or we just look at kubernetes' implementation?

[umohnani8]

Agree. so basically, the restart policy of an init container is inferred from the restart policy of the pod and users are not allowed to modify or override (i.e. through --restart)?

pod restart policy init ctr restart policy
none(missing) on-failure
always on-failure
on-failure on-failure
unless-stopped on-failure
never never

Yes this looks correct. For the most part, we are following k8s conventions for restart policies. There are currently some minor differences, because we don't want to break existing podman behavior.
I am fine with setting the restart policy for init containers to never all the time and we can open another PR with follow up fixes for the on-failure part.

[TomSweeneyRedHat]

@mheon this one is saying a release note is needed, but it looks like there is one already?
LGTM otherwise, and happy green test buttons.

[rhatdan]

/approve
/lgtm
Thanks @tony84727

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, tony84727

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vrothberg]

/cherry-pick v4.5

[openshift-cherrypick-robot]

@vrothberg: #18481 failed to apply on top of branch "v4.5":

Applying: fix: initContainer restart policy overridden by pod
Using index info to reconstruct a base tree...
M	pkg/specgen/generate/container_create.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/specgen/generate/container_create.go
CONFLICT (content): Merge conflict in pkg/specgen/generate/container_create.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix: initContainer restart policy overridden by pod
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick v4.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.