argo-helm icon indicating copy to clipboard operation
argo-helm copied to clipboard

feat(argo-cd): Support ability to set .Values.namespaceOverride

Open andres-vara opened this issue 9 months ago • 10 comments

Resolves #2664

Checklist:

  • [x] I have bumped the chart version according to versioning
  • [x] I have updated the documentation according to documentation
  • [x] I have updated the chart changelog with all the changes that come with this pull request according to changelog.
  • [x] Any new values are backwards compatible and/or have sensible default.
  • [x] I have signed off all my commits as required by DCO.
  • [x] My build is green (troubleshooting builds).

andres-vara avatar May 07 '24 14:05 andres-vara

Hi @andres-vara

Since your PR description misses context or a related issue link: What is your motivation resp. your use case for this change?

mkilchhofer avatar May 07 '24 17:05 mkilchhofer

Hi @andres-vara

Since your PR description misses context or a related issue link: What is your motivation resp. your use case for this change?

Hi @mkilchhofer Attached issue to the context.

In my case, I don't have access to .Release.Namespace via the CD tool we use, which is why I believe it's beneficial to have an alternative method of specifying the namespace using the values file.

andres-vara avatar May 08 '24 08:05 andres-vara

@andres-vara Hi thanks for the contribution but Helm natively supports helm install --namespace <target> flag so IMHO I don't see a need to have override via values.yaml. Why is this needed?

pdrastil avatar May 13 '24 17:05 pdrastil

@pdrastil I can answer this one.

If you use a chart as a dependent chart in another, you can't use the --namespace flag to put the dependent chart in a namespace different from the 'main' chart. This is extremely frustrating.

tico24 avatar May 13 '24 17:05 tico24

@tico24 Question is why would you want to do that. You don't deploy server / repo-server / each controller into separate namespace. In my experienece if you need more than 1 namespace then you should have 2 independent deployments instead of 1 huge umbrella chart as each namespace provides isolation (and if you have Cilium / NetPolicies the namespace override will not work reasonably well anyway).

pdrastil avatar May 13 '24 18:05 pdrastil

I wouldn't want to do that. But I would want to deploy (in my case) argo workflows and another application in the same helm chart, but in different namespaces.

I can easily see a requirement to deploy argocd and another application in one helm chart but in different namespaces.

We did discuss this quite a bit in the team slack when the topic first came up. There's strong prescidence out there for doing it, even though it feels hacky (mostly because helm sucks with support for its own dependent chart deployment options).

tico24 avatar May 13 '24 18:05 tico24

@andres-vara Hi thanks for the contribution but Helm natively supports helm install --namespace <target> flag so IMHO I don't see a need to have override via values.yaml. Why is this needed?

As mentioned earlier, if your company policy requires all helm releases to be in one default namespace, '.Release.Namespace' may not be available for modification. There is no harm in having an additional option to manage the deployment namespace via values

andres-vara avatar May 13 '24 19:05 andres-vara

I think we can accept it, @tico24 found the context we already accepted some months ago:

  • #2562

mkilchhofer avatar May 13 '24 19:05 mkilchhofer

Ok thanks for sharing context and I'm fine with having extra way of managing things.

pdrastil avatar May 13 '24 20:05 pdrastil

Hi @andres-vara

I have updated the chart changelog with all the changes that come with this pull request according to changelog.

can you please update (remove old ones and add new one) changelog in Chart.yaml ? Ref: https://github.com/argoproj/argo-helm/pull/2679/files#diff-16f38cd1a4674cb682ac9f015fbc1c1ff552f024a8f791c16de0de21a1f65771R28-R34

yu-croco avatar May 14 '24 12:05 yu-croco