-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Add helm support for custom_cni deployment #10529
Conversation
Welcome @kukacz! |
Hi @kukacz. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Hi @kukacz, thanks for the PR! Could you add a test using this? I think we could have a file /ok-to-test |
Thanks for the great docs! 🙏 It looks good to me but I think you need to rebase your branch to make the CI happy 🤔. I should be able to trigger the test you added afterwards. |
Signed-off-by: Lukáš Kubín <[email protected]>
Signed-off-by: Lukáš Kubín <[email protected]>
Signed-off-by: Lukáš Kubín <[email protected]>
d510a7c
to
83c8f5f
Compare
Thanks @MrFreezeex! Of course, rebased now, basic CI tests passed now. Would you kindly trigger the |
Hmmm it seems that the pipeline for stuck 🤔, could you rebase again or amend last commit/push force to trigger it again. And yes correct I will be able to trigger your test manually. |
Signed-off-by: Lukáš Kubín <[email protected]>
83c8f5f
to
1a94f6e
Compare
@MrFreezeex Sure, rebased from upstream master again (no changes) and force-pushed amended commit. Thanks! |
Ok so the pipeline are not reported in github but actually launched, I triggered your new job but it seems to fail unfortunately :(: https://gitlab.com/kargo-ci/kubernetes-sigs-kubespray/-/jobs/5529779199 |
Signed-off-by: Lukáš Kubín <[email protected]>
So now Cilium is crashing unfortunately: https://gitlab.com/kargo-ci/kubernetes-sigs-kubespray/-/jobs/5533156698 It shows this in the logs:
BTW adding this: https://github.com/kubernetes-sigs/kubespray/blob/master/tests/files/custom_cni/values.yaml#L11 is probably a good idea but it don't seems to be the reason of cilium failing though... |
Hi @MrFreezeex, it seems that there might be a mount directory ownership issue. Hard to confirm it without access to the tested system. Also, it might be specific to the Debian image or even something else I'm not experiencing in my deployments which are actually using Ubuntu 22.04. And I had no issues deploying there with default chart values. So now I can try changing the test packet to use Ubuntu image instead Debian, or perhaps apply some force with non-default Possibly related: #10499 and cilium/cilium#23838. |
Thanks for the investigation 🙏. So something have probably changed between Cilium 1.13 and 1.14 as the other version of custom_cni Cilium install 1.13.0 fine. The second issue suggested that |
Signed-off-by: Lukáš Kubín <[email protected]>
Signed-off-by: Lukáš Kubín <[email protected]>
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 for the great work! Here is working gitlab CI job: https://gitlab.com/kargo-ci/kubernetes-sigs-kubespray/-/jobs/5542894180 :D
/lgtm
/assign @chadswen |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chadswen, kukacz, MrFreezeex The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* Add helm support for custom_cni deployment * Linting correction * Ansible linting correction * Add test packet with values Signed-off-by: Lukáš Kubín <[email protected]> * Add custom_cni configuration file with comments Signed-off-by: Lukáš Kubín <[email protected]> * Default values cleanup Signed-off-by: Lukáš Kubín <[email protected]> * Add details to custom_cni configuration file Signed-off-by: Lukáš Kubín <[email protected]> * Set correct yaml type of helm values Signed-off-by: Lukáš Kubín <[email protected]> * Set CNI filesystem ownership to root Signed-off-by: Lukáš Kubín <[email protected]> * Update cilium example parameter name Signed-off-by: Lukáš Kubín <[email protected]> --------- Signed-off-by: Lukáš Kubín <[email protected]>
What type of PR is this?
/kind feature
What this PR does / why we need it:
Add option to deploy custom_cni type of network backend using helm chart.
There is already manifest based method of custom network backend deployment implemented. Some backend providers (eg. Cilium) offer helm charts of their applications and prefer those for deployment and following upgrades. This change adds such exclusive option in addition to the existing manifest one.
Which issue(s) this PR fixes:
N/A
Special notes for your reviewer:
Does this PR introduce a user-facing change?: