Skip to content
New issue

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

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

Already on GitHub? # to your account

Bridge Plugin's del order prevents dhcp from releasing any leases #791

Closed
EmilyShepherd opened this issue Dec 4, 2022 · 1 comment
Closed
Labels

Comments

@EmilyShepherd
Copy link
Contributor

As a result of #666 / 7aa07ef the order of tasks in cmdDel for the bridge plugin was updated such that the bridged interface is removed before DEL is called on the ipam provider.

This causes an issue for the DHCP Ipam plugin, as it requires the link to still exist so that it can communicate with the DHCP server to release the pod's IP address.

Example logs of the failure ("no such device" is caused by a failure when calling dhcp4client.NewPacketSock on the cleaned up link):

2022/12/04 18:37:19 37060c8f178b85cdfd9a5524dbd88b630cae422c33e5c3bbb59aac6c398658ad/default/eth0: releasing lease
2022/12/04 18:37:19 37060c8f178b85cdfd9a5524dbd88b630cae422c33e5c3bbb59aac6c398658ad/default/eth0: failed to release DHCP lease: no such device

Although I understand the logic in #666, I think it would be best to revert 7aa07ef for the following reasons:

  1. All of the other main plugins in the project (host-device, macvlan, etc) call ipam.ExecDel before their own cleanup - is there a reason the bridge plugin in particular needs different behaviour?
  2. As pointed out in the original issue, issues cleaning up / removing the link itself are rare / impossible.
  3. As a result of the current behaviour, the DHCP server always ends up filling up with loads of unreleased leases when pods shutdown.

cc @gojoy @squeed @mars1024 @dcbw as you reviewed / discussed / authored the original issue / PR so may have better context.

EmilyShepherd added a commit to EmilyShepherd/kiOS that referenced this issue Dec 10, 2022
[This change][upstream] originally caused the bridge to clean up the
interface before calling the DHCP plugin to clean up.

Unfortuneately, this caused an error as DHCP requires the interface to
still exist so it can release the IP address to the DHCP server.

This patch reverts it as it is required in kios. Please see [the
upstream issue][issue] for further discussion on this.

[upstream]: containernetworking/plugins#702
[issue]: containernetworking/plugins#791
@github-actions github-actions bot added the Stale label Feb 3, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 10, 2023
@EmilyShepherd
Copy link
Contributor Author

@squeed can we reopen this one?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant