-
Notifications
You must be signed in to change notification settings - Fork 127
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
feat: cilium add-mode support #312
Conversation
when cni management by kilo is disable, we can use existing cluster's cni setup thanks to add-on mode https://kilo.squat.ai/docs/introduction#add-on-mode
@RouxAntoine thanks so much for putting this PR together 😍 |
You welcome, yes it's seem to be running, with cilium as replacement of kube-proxy so like comment i have manually bind kubeconfig. Only one peer this is my pod configuration :
|
moreover to build image I use manual tools because makefile seem to not working with my local docker configuration (I use lima https://github.com/lima-vm/lima) :
commit RouxAntoine@0ef31ca#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R157-R160 |
In same time i change this on custom branch (not include here) :
on commit RouxAntoine@0ef31ca#diff-9d1920ed195f23a0a31a5d736410a029c56e65f207971525797d1b900302bfceR88 Indeed i have some time drift between my local kgctl and kg server |
ack, I am hoping to move away entirely from using this |
Interesting, I did not understand the purpose of this condition. Test are flaky is it normal ? |
No, it's not normal. I haven't seen a flaky test in almost a year. In any case, the e2e tests passed now :) Could you also add an example manifest for Kilo with cilium? If not, we can certainly translate your HCL to YAML later. |
Thank you @RouxAntoine I'll take another closer look during lunch :) |
4c63474
to
8e14ae5
Compare
@squat I rewrite the pull request by error three day ago sorry. No more change to had on this pull request ? Do you think this could be merge for one of the next release delivery ? |
yes, absolutely want this in the next release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay!
This looks good overall, I just have a few nits and one more important consideration for concurrency safety
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @RouxAntoine. Very cool 🎉
In a follow up, if like to add an e2e test to assert that this keeps working 👍
when cni management by kilo is disable, we can use existing cluster's cni setup thanks to add-on mode
https://kilo.squat.ai/docs/introduction#add-on-mode
this is a really basic implementation based on comment #81 (comment)