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

Use cilium as kube-proxy replacement #38

Merged
merged 1 commit into from
Mar 27, 2023
Merged

Use cilium as kube-proxy replacement #38

merged 1 commit into from
Mar 27, 2023

Conversation

kopiczko
Copy link
Contributor

@kopiczko kopiczko commented Mar 22, 2023 •
edited
Loading

@kopiczko kopiczko self-assigned this Mar 22, 2023
@kopiczko kopiczko mentioned this pull request Mar 22, 2023
Closed
@kopiczko
Copy link
Contributor Author

Tested with gravel MC.

@kopiczko kopiczko marked this pull request as ready for review March 24, 2023 20:01
@kopiczko kopiczko requested a review from a team as a code owner March 24, 2023 20:01
erkanerol
erkanerol reviewed Mar 27, 2023
View reviewed changes
helm/cluster-vsphere/templates/cilium-helmrelease.yaml
@@ -32,6 +32,9 @@ spec:
values:
ipam:
mode: kubernetes
k8sServiceHost: api.{{ include "resource.default.name" $ }}.{{ .Values.baseDomain }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this change mean we have to create DNS records to have a working cilium app? Can't we replace this with an IP somehow? I know IP is assigned later but we can set it like RDE_ID in CAPVCD.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it means we require DNS and we do that in all existing providers so far. I wouldn't like to deviate unless we have a general agreement.

Copy link
Contributor

Choose a reason for hiding this comment

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

This means that this chart will not work without having dns-operator in the MC. Not a big issue for us but it can be an issue for external users if there are such people :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I took a note to update README with a note about DNS. If you want we can discuss this during the kaas sync as well.

@kopiczko kopiczko merged commit c9827fd into main Mar 27, 2023
@kopiczko kopiczko deleted the cilium-kube-proxy-replacement branch March 27, 2023 18:19
@kopiczko kopiczko mentioned this pull request Mar 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erkanerol erkanerol erkanerol approved these changes

Assignees

@kopiczko kopiczko

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kopiczko @erkanerol