Attempt manual removal of CNI IP allocations on refresh #5434
Conversation
|
[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 |
openshift-ci-robot
added
the
approved
libpod/container_internal.go
Outdated
| func (c *Container) removeIPv4Allocations() error { | ||
| // Using a hardcoded path here is VERY gross, but the lack of a good | ||
| // interface into CNI means there's no non-hacky way to do this. | ||
| const cniNetworksDir = "/var/lib/cni/networks/" |
There was a problem hiding this comment.
On the fence, should there be a function like getCNINetworksDir that returns this string when on a linux distro, and then unknown or something for windows/others? Or maybe define this in containers.conf?
There was a problem hiding this comment.
Yes this should be handled in container_internal_linux.go.
mheon
force-pushed
the
force_delete_cni_netreg
branch
3 times, most recently
from
fc6bc4c to
0be7934
Compare
|
I really want to test this, but the whole system-restart thing kind of puts a kibosh on that. I don't think we can really mock the circumstances involved, either. |
|
LGTM assuming happy tests |
|
Restarted flakes |
|
@mheon Needs a rebase |
|
I'm not seeing that? |
mheon
force-pushed
the
force_delete_cni_netreg
branch
from
0be7934 to
bd98679
Compare
|
Rebased to pick up CI fixes |
|
☔ The latest upstream changes (presumably #5088) made this pull request unmergeable. Please resolve the merge conflicts. |
We previously attempted to work within CNI to do this, without success. So let's do it manually, instead. We know where the files should live, so we can remove them ourselves instead. This solves issues around sudden reboots where containers do not have time to fully tear themselves down, and leave IP address allocations which, for various reasons, are not stored in tmpfs and persist through reboot. Fixes containers#5433 Signed-off-by: Matthew Heon <[email protected]>
mheon
force-pushed
the
force_delete_cni_netreg
branch
from
bd98679 to
b695475
Compare
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
We previously attempted to work within CNI to do this, without success. So let's do it manually, instead. We know where the files should live, so we can remove them ourselves instead. This solves issues around sudden reboots where containers do not have time to fully tear themselves down, and leave IP address allocations which, for various reasons, are not stored in tmpfs and persist through reboot.
Fixes #5433