Repository navigation
Add dumb-init to service containers - #785
openshift-merge-bot[bot] merged 1 commit into
Conversation
When a pod is deleted before the service is ready to handle SIGTERM, the service can keep running until terminationGracePeriodSeconds expires, delaying pod termination. Run the service under dumb-init so SIGTERM is handled and forwarded even during early startup. Wrap Heat API, CFN API, and engine processes with dumb-init to forward SIGTERM to the service process. Add functional assertions and update KUTTL expectations for the wrapped commands. Assisted-by: Codex <codex@openai.com> Signed-off-by: Amartya Sinha <amsinha@redhat.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
|
/test heat-operator-build-deploy-kuttl |
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 02m 47s Warning: |
|
recheck |
|
/test heat-operator-build-deploy-kuttl |
|
Build succeeded (check pipeline). ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 05m 47s Warning: |
|
/test heat-operator-build-deploy-kuttl |
| { | ||
| Name: heat.ServiceName + "-" + heat.APIComponent, | ||
| Command: []string{ | ||
| "/usr/bin/dumb-init", |
There was a problem hiding this comment.
We already have that in base image right? https://github.com/openstack-k8s-operators/s2i-openstack-containers/blob/main/containers/base/Containerfile#L131
There was a problem hiding this comment.
When Command (which correspond to container's ENTRYPOINT) and Args (correspond to container's CMD) are set, both ENTRYPOINT and CMD are overriden.
See: https://kubernetes.io/docs/tasks/inject-data-application/define-command-argument-container/#:~:text=The%20command%20and%20arguments%20that%20you%20define%20in%20the%20configuration%20file%20override%20the%20default%20command%20and%20arguments%20provided%20by%20the%20container%20image.%20If%20you%20define%20args%2C%20but%20do%20not%20define%20a%20command%2C%20the%20default%20command%20is%20used%20with%20your%20new%20arguments. and the note just below the highlight.
Due to this behavior, it is overridden here.
There was a problem hiding this comment.
As I understand we don't need Command here. Setting only Args: ["/usr/sbin/httpd", "-DFOREGROUND"] keeps the image ENTRYPOINT and replaces CMD.
There was a problem hiding this comment.
Yes, that’s true. We set both Command and Args so the pod spec shows the full process invocation without needing to check the Containerfile.
The behavior should be the same either way. If you want to drop the Command part, I can do that.
There was a problem hiding this comment.
It's duplication and something that does not have to repeated in all services as base image has it. But I see it's repeated in every operator, so we can leave it here too. Is there a jira reference you can add to the PR?
There was a problem hiding this comment.
ack! updated the jira ref in the description
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: amartyasinha, rabi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
a313bd4
into
openstack-k8s-operators:main
When a pod is deleted before the service is ready to handle SIGTERM, the service can keep running until terminationGracePeriodSeconds expires, delaying pod termination. Run the service under dumb-init so SIGTERM is handled and forwarded even during early startup.
Wrap Heat API, CFN API, and engine processes with dumb-init to forward SIGTERM to the service process. Add functional assertions and update KUTTL expectations for the wrapped commands.
Assisted-by: Codex codex@openai.com
Jira: https://redhat.atlassian.net/browse/OSPRH-40307