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

fix aardvark-dns netns check #956

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Apr 3, 2024

Right now there is a race condition where we return errors even in cases where they should be ignored. When we send SIGHUP to aardvark on teardown it might exit when all containers are removed. This means the check afterwards might read the netns path at a weird time while the process is being removed from the kernel structures. I assumed the only error can be ENOENT but I was wrong, in CI we also see EACCES and in my reproducer I also saw ESRCH. Given the check is a nice to have do ignore all errors there.

Fixes containers/podman#22103

Right now there is a race condition where we return errors even in
cases where they should be ignored. When we send SIGHUP to aardvark on
teardown it might exit when all containers are removed. This means the
check afterwards might read the netns path at a weird time while the
process is being removed from the kernel structures. I assumed the only
error can be ENOENT but I was wrong, in CI we also see EACCES and in my
reproducer I also saw ESRCH. Given the check is a nice to have do ignore
all errors there.

Fixes containers/podman#22103

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

openshift-ci bot commented Apr 3, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 3, 2024

LGTM

@baude
Copy link
Member

baude commented Apr 10, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Apr 10, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit e18defc into containers:main Apr 10, 2024
26 checks passed
@Luap99 Luap99 deleted the aardvark-ns branch April 10, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants