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.

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

Migrate L2 announcements and LB-IPAM to cilium #505

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

aaronmondal
Copy link
Contributor

@aaronmondal aaronmondal commented Dec 18, 2023

No need to use MetalLB since Cilium can do this as well.


This change is Reviewable

No need to use MetalLB since Cilium can do this as well.
Copy link
Contributor Author

@aaronmondal aaronmondal left a comment

Choose a reason for hiding this comment

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

+@MarcusSorealheis +@adam-singer +@allada

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @adam-singer, @allada, and @MarcusSorealheis)

Copy link
Collaborator

@MarcusSorealheis MarcusSorealheis left a comment

Choose a reason for hiding this comment

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

LGTM. I'm totally fine with this one as it is only a deployment example.

deployment-examples/kubernetes/00_infra.sh Show resolved Hide resolved
deployment-examples/kubernetes/00_infra.sh Show resolved Hide resolved
Copy link
Contributor Author

@aaronmondal aaronmondal left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @adam-singer, @allada, and @MarcusSorealheis)


deployment-examples/kubernetes/00_infra.sh line 104 at r1 (raw file):

Previously, MarcusSorealheis (Marcus Eagan) wrote…

This is less stable not more stable, right? I think it's fine, but we need to ensure that neither our CI our future releases are impeded by an Alpha announcement policy.

From what I've tested it amounts to the same stability as in "just works", but the cilium variant is much faster to set up (e.g. in CI) since we don't need to fetch/deploy another helm chart.


deployment-examples/kubernetes/00_infra.sh line 117 at r1 (raw file):

Previously, MarcusSorealheis (Marcus Eagan) wrote…

Replacing MetalLb with Cilium is obviously an improvement.

Done.

@aaronmondal aaronmondal merged commit df6f5b9 into TraceMachina:main Dec 18, 2023
20 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants