helm: Add Support for Namespace Override #24445

pr0PM · 2023-03-18T13:31:12Z

Please ensure your pull request adheres to the following guidelines:

For first time contributors, read Submitting a pull request
All code is covered by unit and/or runtime tests where feasible.
All commits contain a well written commit description including a title,
description and a Fixes: #XXX line if the commit addresses a particular
GitHub issue.
If your commit description contains a Fixes: <commit-id> tag, then
please add the commit author[s] as reviewer[s] to this issue.
All commits are signed off. See the section Developer’s Certificate of Origin
Provide a title or release-note blurb suitable for the release notes.
[] Are you a user of Cilium? Please add yourself to the Users doc
Thanks for contributing!

This is helpful while deploying charts using gitOps tools like argoCD. Usually a standard among all the charts.

Patch for the helper.tpl priority class issue
document generation steps
spellcheck allowlist
make install/kubernetes

Fixes: #20511

Signed-off-by: Prateek Mishra pr0PM@pm.me

Add namespace override support in cilium charts

pr0PM · 2023-03-18T14:16:22Z

Hi @sayboras this is the same PR which broke down last time while fixing inconsistencies in my config.

sayboras · 2023-03-19T00:58:42Z

this is the same PR which broke down last time while fixing inconsistencies in my config.

Thanks for your help. There is small failure with generated doc checking, can you run the below command and submit the changes in the same commit ?

https://github.com/cilium/cilium/actions/runs/4455626054/jobs/7829681639?pr=24445

Make sure to apply your changes to cilium/values.yaml.tmpl and run 'make -C install/kubernetes cilium/values.yaml' to regenerate.

sayboras · 2023-03-20T09:51:11Z

/test

qmonnet

Looks good to me, thank you!

chancez · 2023-03-20T17:41:56Z

I can't help but feel like this needs a CI test so we can actually verify this works as expected. Do we have a good way for a contributor to add new GitHub workflows?

install/kubernetes/cilium/templates/_helpers.tpl

squeed · 2023-03-21T12:35:21Z

Hmm, what's the use-case for this? Out of curiosity, why is the Helm namespace not sufficient? I see mention of umbrella charts, but can you explain that a bit more?

Separately, are there other Helm conventions we should be implementing as well?

pr0PM · 2023-03-21T13:38:56Z

Personally I wanted to deploy cilium via argo-cd with custom values, using sync waves so that cilium is installed first.
Can't pass helm values from charts into argo-cd, so we would need to modify the argocd application spec to pass a parameter for namespace.
Would prefer to keep argocd clean as namespace override is something standard across all charts I've come across.

This is helpful while deploying charts using gitOps tools like argoCD. Usually a standard among all the charts. - Patch for the helper.tpl priority class issue - document generation steps - spellcheck allowlist - make install/kubernetes Fixes: cilium#20511 Signed-off-by: Prateek Mishra <pr0PM@pm.me>

sayboras · 2023-03-29T21:39:14Z

/test

Job 'Cilium-PR-K8s-1.25-kernel-4.19' failed:

Click to show.

Test Name

K8sDatapathServicesTest Checks E/W loadbalancing (ClusterIP, NodePort from inside cluster, etc) Tests NodePort inside cluster (kube-proxy) with IPSec and externalTrafficPolicy=Local

Failure Output

FAIL: Request from k8s1 to service http://[fd04::11]:32610 failed

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.25-kernel-4.19 so I can create one.

Job 'Cilium-PR-K8s-1.16-kernel-4.19' hit: #22578 (96.99% similarity)

pr0PM · 2023-04-05T08:53:50Z

Hi just following up, I hope nothing else is pending from my side.

squeed · 2023-04-05T09:00:47Z

I've kicked off some of the failed jobs; likely a flake but it's possible something somehow broke in the charts.

squeed · 2023-04-19T11:33:33Z

@pr0PM looks like this needs a rebase; then it should be good to go.

squeed · 2023-04-20T12:20:35Z

@pr0PM can you rebase and remove the merge commits please?

Separately, I've got a PR to add a tool for testing helm invariants -- #23881 -- this would be a good thing to add a test for.

Signed-off-by: Prateek Mishra <pr0PM@pm.me>

pr0PM · 2023-04-23T17:35:41Z

hi can you provide steps on the kind of state we need here, i tried rebase but it didn't help in this case due to normal out of date from main and the switch from master -> main ig.
can't rebase directly on github as the update with rebase button mentions some conflicts.

kaworu

Thanks for the patch @pr0PM! This PR needs to be rebased on top of main. A couple of files were removed and there is one conflict. Also git grep Release.Namespace shows some instances of .Release.Namespace that could be updated.

kaworu · 2023-04-24T08:18:01Z

install/kubernetes/cilium/values.yaml.tmpl

@@ -4,6 +4,9 @@
 # For example: 1.7, 1.8, 1.9
 # upgradeCompatibility: '1.8'

+# -- String to override release namespace for all resources


Can we add a bit more of context here? e.g. mention that it is useful for tools that are unable to pass other Helm flags than values like argo-cd.

Thanks @pr0PM! Are you going to replace more .Release.Namespace in install/kubernetes/cilium?

nathanjsweet · 2023-04-25T18:17:07Z

Uh oh. It looks like you created a rebase commit instead of rebasing. You might just want to create a new branch off of main and cherry-pick your commit on top of it.

joestringer · 2023-05-05T16:33:54Z

I've marked the PR as draft while you address the feedback on the PR, feel free to click the "Ready for review" button at the bottom of the page to re-request review when you've addressed that. Thanks for your contributions!

github-actions · 2023-06-05T02:04:37Z

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

github-actions · 2023-06-20T01:56:12Z

This pull request has not seen any activity since it was marked stale.
Closing.

pr0PM requested review from a team as code owners March 18, 2023 13:31

pr0PM requested a review from squeed March 18, 2023 13:31

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 18, 2023

pr0PM requested review from chancez and qmonnet March 18, 2023 13:31

github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Mar 18, 2023

sayboras added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Mar 19, 2023

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 19, 2023

sayboras changed the title ~~Add Support for Namespace Override~~ helm: Add Support for Namespace Override Mar 19, 2023

pr0PM force-pushed the helm-ns-override branch from 38f6b29 to 0424dd6 Compare March 19, 2023 02:24

qmonnet approved these changes Mar 20, 2023

View reviewed changes

squeed reviewed Mar 21, 2023

View reviewed changes

install/kubernetes/cilium/templates/_helpers.tpl Outdated Show resolved Hide resolved

pr0PM force-pushed the helm-ns-override branch from 0424dd6 to e777a0a Compare March 21, 2023 13:43

pr0PM requested review from squeed and removed request for chancez March 27, 2023 05:43

julianwiedmann requested a review from chancez March 28, 2023 10:03

chancez approved these changes Mar 28, 2023

View reviewed changes

squeed approved these changes Mar 29, 2023

View reviewed changes

maintainer-s-little-helper bot mentioned this pull request Mar 29, 2023

CI: K8sDatapathConfig Host firewall With VXLAN #22578

Closed

pr0PM requested a review from squeed April 20, 2023 12:19

Merge branch 'master and main' into helm-ns-override

3c19de8

Signed-off-by: Prateek Mishra <pr0PM@pm.me>

pr0PM force-pushed the helm-ns-override branch from a12b4e1 to 3c19de8 Compare April 23, 2023 17:31

pr0PM requested review from a team as code owners April 23, 2023 17:31

pr0PM requested review from viktor-kurchenko, nathanjsweet and kaworu April 23, 2023 17:31

kaworu requested changes Apr 24, 2023

View reviewed changes

kaworu added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Apr 28, 2023

joestringer marked this pull request as draft May 5, 2023 16:33

github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jun 5, 2023

github-actions bot closed this Jun 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

helm: Add Support for Namespace Override #24445

helm: Add Support for Namespace Override #24445

pr0PM commented Mar 18, 2023

pr0PM commented Mar 18, 2023

sayboras commented Mar 19, 2023

sayboras commented Mar 20, 2023

qmonnet left a comment

chancez commented Mar 20, 2023 •

edited

Loading

squeed commented Mar 21, 2023

pr0PM commented Mar 21, 2023

sayboras commented Mar 29, 2023 •

edited by maintainer-s-little-helper bot

Loading

Test Name

Failure Output

pr0PM commented Apr 5, 2023

squeed commented Apr 5, 2023

squeed commented Apr 19, 2023

squeed commented Apr 20, 2023

pr0PM commented Apr 23, 2023

kaworu left a comment •

edited

Loading

kaworu Apr 24, 2023

viktor-kurchenko May 1, 2023

nathanjsweet commented Apr 25, 2023

joestringer commented May 5, 2023

github-actions bot commented Jun 5, 2023

github-actions bot commented Jun 20, 2023

helm: Add Support for Namespace Override #24445

helm: Add Support for Namespace Override #24445

Conversation

pr0PM commented Mar 18, 2023

pr0PM commented Mar 18, 2023

sayboras commented Mar 19, 2023

sayboras commented Mar 20, 2023

qmonnet left a comment

Choose a reason for hiding this comment

chancez commented Mar 20, 2023 • edited Loading

squeed commented Mar 21, 2023

pr0PM commented Mar 21, 2023

sayboras commented Mar 29, 2023 • edited by maintainer-s-little-helper bot Loading

Test Name

Failure Output

pr0PM commented Apr 5, 2023

squeed commented Apr 5, 2023

squeed commented Apr 19, 2023

squeed commented Apr 20, 2023

pr0PM commented Apr 23, 2023

kaworu left a comment • edited Loading

Choose a reason for hiding this comment

kaworu Apr 24, 2023

Choose a reason for hiding this comment

viktor-kurchenko May 1, 2023

Choose a reason for hiding this comment

nathanjsweet commented Apr 25, 2023

joestringer commented May 5, 2023

github-actions bot commented Jun 5, 2023

github-actions bot commented Jun 20, 2023

chancez commented Mar 20, 2023 •

edited

Loading

sayboras commented Mar 29, 2023 •

edited by maintainer-s-little-helper bot

Loading

kaworu left a comment •

edited

Loading