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

rootless netns: recover from invalid netns #18024

Merged
merged 1 commit into from
Apr 4, 2023

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Apr 3, 2023

I made a change in c/common[1] to prevent duplicates in netns names. This now causes problem in podman[2] where the rootless netns will no longer work after the netns got invalid but the underlying path still exists. AFAICT this happens when the podman pause process got killed and we are now in a different user namespace.

While I do not know what causes this, this commit should make it at least possible to recover from this situation automatically as it used to be before[1].

the problem with that is that containers started before it will not be able to talk to contianers started after this. A restart of the previous container will fix it but this was also the case before.

[1] containers/common#1381
[2] #17903 (comment)

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added release-note-none approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 3, 2023
@edsantiago
Copy link
Member

Doesn't this need the no-new-tests bypass?

@Luap99
Copy link
Member Author

Luap99 commented Apr 3, 2023

Doesn't this need the no-new-tests bypass?

Yeah I will never remember this.

libpod/networking_linux.go Outdated Show resolved Hide resolved
I made a change in c/common[1] to prevent duplicates in netns names.
This now causes problem in podman[2] where the rootless netns will no
longer work after the netns got invalid but the underlying path still
exists. AFAICT this happens when the podman pause process got killed and
we are now in a different user namespace.

While I do not know what causes this, this commit should make it at
least possible to recover from this situation automatically as it used
to be before[1].

the problem with that is that containers started before it will not be
able to talk to contianers started after this. A restart of the previous
container will fix it but this was also the case before.

[NO NEW TESTS NEEDED]

[1] containers/common#1381
[2] containers#17903 (comment)

Signed-off-by: Paul Holzinger <[email protected]>
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 4, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, Luap99

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

@mheon
Copy link
Member

mheon commented Apr 4, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 4, 2023
@openshift-merge-robot openshift-merge-robot merged commit ac1d297 into containers:main Apr 4, 2023
@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 3, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 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.

7 participants