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.

Sign up for GitHub

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

Jump to bottom

bgpv1: ExternalIP advertisement with BGP Control Plane #31245

Merged
merged 1 commit into from Mar 13, 2024
Merged

bgpv1: ExternalIP advertisement with BGP Control Plane #31245

merged 1 commit into from Mar 13, 2024

Conversation

chaunceyjiang
Copy link
Member

Fixes: #29990

Add support for ExternalIP service advertisement with BGP Control Plane

@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 Mar 8, 2024
@chaunceyjiang
Copy link
Member Author

/test

@chaunceyjiang chaunceyjiang marked this pull request as ready for review March 8, 2024 12:51
@chaunceyjiang chaunceyjiang requested review from a team as code owners March 8, 2024 12:51
@chaunceyjiang
Copy link
Member Author

/cc @YutaroHayakawa PTAL.

rastislavs
rastislavs approved these changes Mar 11, 2024
View reviewed changes
Copy link
Contributor

@rastislavs rastislavs left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@rastislavs rastislavs added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Mar 11, 2024
@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 Mar 11, 2024
YutaroHayakawa
YutaroHayakawa requested changes Mar 11, 2024
View reviewed changes
Copy link
Member

@YutaroHayakawa YutaroHayakawa left a comment

Choose a reason for hiding this comment

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

Very nice work, thanks! I forgot to mention this in ClusterIP PR, but you need to bump the patch version here when you change the CRD schema.

https://github.com/cilium/cilium/blob/main/pkg/k8s/apis/cilium.io/register.go#L18

@chaunceyjiang
Copy link
Member Author

Very nice work, thanks! I forgot to mention this in ClusterIP PR, but you need to bump the patch version here when you change the CRD schema.

So should I change it to 1.29.3? @YutaroHayakawa

@chaunceyjiang
bgpv1: ExternalIP advertisement with BGP Control Plane
ebb863c 
Fixes: cilium#29990

Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
YutaroHayakawa
YutaroHayakawa approved these changes Mar 11, 2024
View reviewed changes
Copy link
Member

@YutaroHayakawa YutaroHayakawa left a comment

Choose a reason for hiding this comment

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

Now looks good! Thanks! Please don't forget to write a document together with ClusterIP's one.

#30963 (comment)

Same caveats applied for ExternalIP + KPR.

@chaunceyjiang
Copy link
Member Author

Please don't forget to write a document together with ClusterIP's one.
Same caveats applied for ExternalIP + KPR.

Ok. I'm currently writing documentation for these two features.

@chaunceyjiang
Copy link
Member Author

/test

@chaunceyjiang chaunceyjiang mentioned this pull request Mar 12, 2024
Merged
@chaunceyjiang
Copy link
Member Author

CC @youngnick PTAL.

@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 Mar 13, 2024
@YutaroHayakawa YutaroHayakawa added this pull request to the merge queue Mar 13, 2024
Merged via the queue into cilium:main with commit 54c01e2 Mar 13, 2024
62 checks passed
@chaunceyjiang chaunceyjiang deleted the bgp_extip_advert branch March 13, 2024 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YutaroHayakawa YutaroHayakawa YutaroHayakawa approved these changes

@youngnick youngnick youngnick approved these changes

@rastislavs rastislavs rastislavs approved these changes

Assignees
No one assigned
Labels
area/bgp ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CFP: BGP Control Plane - add ExternalIPs support
4 participants
@chaunceyjiang @YutaroHayakawa @youngnick @rastislavs