Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up when stopping pods #16057

Merged
merged 1 commit into from
Oct 6, 2022
Merged

Conversation

mheon
Copy link
Member

@mheon mheon commented Oct 5, 2022

We have a test to verify that init containers in pods are deleted when the --init-ctr=once option is specified. The test creates two containers, one of them an init container, starts the pod, stops the pod, and restarts the pod, checking for the presence of a file created by the init container during the second start. We're seeing a race where the file still exists, which I'm fairly certain comes down to the SHM mount not being cleaned up after the pod is stopped.

Fortunately, we already have code to do this - just flip the bool that controls cleanup from false to true.

[NO NEW TESTS NEEDED] Fixes a difficult to reproduce race condition.

Fixes #16046

NONE

We have a test to verify that init containers in pods are
deleted when the `--init-ctr=once` option is specified. The test
creates two containers, one of them an init container, starts the
pod, stops the pod, and restarts the pod, checking for the
presence of a file created by the init container during the
second start. We're seeing a race where the file still exists,
which I'm fairly certain comes down to the SHM mount not being
cleaned up after the pod is stopped.

Fortunately, we already have code to do this - just flip the bool
that controls cleanup from false to true.

[NO NEW TESTS NEEDED] Fixes a difficult to reproduce race
condition.

Fixes containers#16046

Signed-off-by: Matthew Heon <[email protected]>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 5, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 5, 2022
@edsantiago
Copy link
Member

LGTM and tests are green (after some 0x3c flakes). I won't merge because I don't know what, if any, unintended consequences this might have. @baude, the original false dates to your PR #5625 way back in March 2020. How's your memory? 😜

@rhatdan
Copy link
Member

rhatdan commented Oct 6, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 6, 2022
@rhatdan
Copy link
Member

rhatdan commented Oct 6, 2022

I am 99% sure Brent just went with the default.

@openshift-merge-robot openshift-merge-robot merged commit d33a315 into containers:main Oct 6, 2022
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

create --init-ctr once: once again, seems to be incorrectly running
4 participants