-
Notifications
You must be signed in to change notification settings - Fork 199
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
pkg/netns: add 60s timeout #2239
Conversation
Removing one level of nesting with a early return. Signed-off-by: Paul Holzinger <[email protected]>
So this is not so simple as one thinks, apparently there are cases where it is impossible to remove the file but umount() worked fine... We fixed one issue that ran into this[1] but there seems to be another[2] problem, unknown cause yet. Regardless of the real fix for issue[2] add a timeout to not hang/loop forever. If we were not able to remove the file after 60s give up and print an error. Leaking these files is not great as the netns references stay around but it will not prevent containers from running. It will only start leaking resources. [1] https://issues.redhat.com/browse/RHEL-59620 [2] containers/podman#24487 Signed-off-by: Paul Holzinger <[email protected]>
[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 |
@mheon @baude @TomSweeneyRedHat PTAL, I would like to get this in before the release cut here to at least avoid the infinitive loop |
LGTM I never saw the |
LGTM |
@rhatdan I'd to dig the |
/lgtm |
Yeah the range over int is new and nice and was added for go 1.22, but if you want to see the actual crazy stuff in go 1.23 added a new iterator syntax https://pkg.go.dev/iter#hdr-Iterators so no you can loop over anything that implements an iterator |
pkg/netns: do not loop forever
So this is not so simple as one thinks, apparently there are cases where
it is impossible to remove the file but umount() worked fine...
We fixed one issue that ran into this[1] but there seems to be
another[2] problem, unknown cause yet.
Regardless of the real fix for issue[2] add a timeout to not hang/loop
forever. If we were not able to remove the file after 60s give up and
print an error. Leaking these files is not great as the netns references
stay around but it will not prevent containers from running. It will
only start leaking resources.
[1] https://issues.redhat.com/browse/RHEL-59620
[2] containers/podman#24487