Use of native SidecarContainers (init container with `restartPolicy: Always`) results in pod status getting stuck on `Init` about argo-rollouts

[martynd]

@phyzical Im not sure actually as the use cases we have get the restart always init containers added at pod creation via mutation webhooks (istio and gcsfuse csi)

It may be possible to manually edit the live rollout object to have the correct restart policy (similar to other cases where argo doesnt want to change things due to managed fields) but I suspect you will get a validation error there too

I recall from [https://github.com//issues/3130](this bug) that the deployment part of the rollout spec is validated against the k8s libraries deployment validation which would be fine since its a valid 1.28+ spec deployment spec except that rollouts is built against the 1.26 k8s apis

That said, it looks like the 1.7 branch is built against 1.29 as of this commit 2dc71e3 which should at least get it to validate

[phyzical]

We will also need to add logic to allow the spec to support redinessProbe and startupProbe for initContainers with the restartPolicy set to Always

 spec.template.spec.initContainers[1].readinessProbe: Forbidden: may not be set for init containers

side question,

@martynd are you able to confirm that the rollout spec works with restartPolicy when i try to add this i find that the live manifest never adds its but i do see it in the desired manifest?

[phyzical]

I gave "release-1.7" tag a go, didn't help sadl.

turns out i missed part of the error from before its actually is invalid: spec.template.spec.initContainers[1].readinessProbe: Forbidden: may not be set for init containers without restartPolicy=Always

So i wondered if the restart policy for normal containers propagated, and it turns out it doesnt for that either.

that makes me think this is by design and couuuld explain the side effects your experiencing ? @martynd

Ill have another sniff at the source to see if theres any comments as to why it does this

edit: hmmm but i see this ref

argo-rollouts/pkg/apiclient/rollout/rollout.swagger.json

Line 3300 in 2e7cf69

"title": "RestartPolicy defines the restart behavior of individual containers in a pod.\nThis field may only be set for init containers, and the only allowed value is \"Always\".\nFor non-init containers or when this field is not specified,\nthe restart behavior is defined by the Pod's restart policy and the container type.\nSetting the RestartPolicy as \"Always\" for the init container will have the following effect:\nthis init container will be continually restarted on\nexit until all regular containers have terminated. Once all regular\ncontainers have completed, all init containers with restartPolicy \"Always\"\nwill be shut down. This lifecycle differs from normal init containers and\nis often referred to as a \"sidecar\" container. Although this init\ncontainer still starts in the init container sequence, it does not wait\nfor the container to complete before proceeding to the next init\ncontainer. Instead, the next init container starts immediately after this\ninit container is started, or after any startupProbe has successfully\ncompleted.\n+featureGate=SidecarContainers\n+optional"

Ah actually, the helm's crds still havn't been updated so pining the containing to 1.7 doesn't help the crd side ofc

After manually applying the updated crd all works as expected thanks 🦆