Skip to content

Conversation

f1ko
Copy link
Member

@f1ko f1ko commented Nov 28, 2023

pkg/l2announcer: ensure leases are only created for services that are being announced

Previously leases were created for all services in the cluster regardless of them having an announceable IP or not.

This change ensures that these services are skipped and only services with an external and/or LB IP - depending on the policy - will have a lease. Fixes: #28752

l2announcer: Leases are only created for services that are being announced.

@f1ko f1ko requested a review from a team as a code owner November 28, 2023 14:18
@f1ko f1ko requested a review from youngnick November 28, 2023 14:18
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 28, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Nov 28, 2023
@youngnick youngnick requested review from a team and aspsk and removed request for a team November 29, 2023 07:10
@dylandreimerink dylandreimerink added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Nov 29, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 29, 2023
@maintainer-s-little-helper
Copy link

Commit 4f3c0e6 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Nov 29, 2023
@f1ko f1ko force-pushed the fix/l2-announcement-lease branch from 4f3c0e6 to 26d1c28 Compare November 29, 2023 13:57
@maintainer-s-little-helper
Copy link

Commit 4f3c0e6 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@f1ko f1ko requested a review from youngnick November 29, 2023 14:02
@f1ko f1ko force-pushed the fix/l2-announcement-lease branch from 26d1c28 to 528e994 Compare November 29, 2023 14:54
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Nov 29, 2023
Copy link
Contributor

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

Thanks for that, this LGTM.

…nced

Previously leases were created for all services in the cluster regardless of them having an announceable IP or not.

This change ensures that these services are skipped and only services with an external and/or LB IP - depending on the policy - will have a lease. Fixes: cilium#28752

Signed-off-by: Filip Nikolic <oss.filipn@gmail.com>
@f1ko f1ko force-pushed the fix/l2-announcement-lease branch from 528e994 to b93f3e1 Compare November 30, 2023 12:15
@aanm aanm requested a review from dylandreimerink December 1, 2023 10:17
@aanm
Copy link
Member

aanm commented Dec 1, 2023

/test

@aanm aanm enabled auto-merge December 1, 2023 10:17
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 1, 2023
@aanm aanm added this pull request to the merge queue Dec 1, 2023
Merged via the queue into cilium:main with commit 00d4836 Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

L2 announcements creates and maintains leases for services without announce-able IPs
5 participants