feat: Allow the Helm release namespace to be overridden

[sathieu]

for multi-namespace deployments in combined charts

[dnskr]

What is the use case for the changes?

What do you think about avoiding introducing minio-operator.namespace template and using the following code instead?

  namespace: {{ .Values.namespaceOverride | default .Release.Namespace }}

[sathieu]

Thanks @dnskr for this review.

What is the use case for the changes?

I use chart dependencies to deploy the operator and multiple tenants.

# Chart.yaml
apiVersion: v2
name: "minio-operator"
version: 0.0.1
dependencies:
  - name: operator
    repository: https://operator.min.io/
    version: 5.0.9
  - alias: tenant1 
    name: tenant
    repository: https://operator.min.io/
    version: 5.0.9
  - alias: tenant2
    name: tenant
    repository: https://operator.min.io/
    version: 5.0.9

# values.yaml
tenant1:
  namespaceOverride: minio-tenant1
tenant2:
  namespaceOverride: minio-tenant2

What do you think about avoiding introducing minio-operator.namespace template and using the following code instead?

  namespace: {{ .Values.namespaceOverride | default .Release.Namespace }}

Done.

[jiuker]

It looks like you're just using it for testing, and while that's harmless, I don't think it's necessary or best practice for helm. @pjuarezd @shtripat @harshavardhana ^^

[sathieu]

@jiuker We use this for production, not test. We are currently using MinIO Helm chart (i.e. not the operator), with it's limitations (including very limited pool configuration). We want to move to the operator which seems to be the recommended method.

We only have one MinIO cluster (per Kubernetes cluster), i.e. one tenant in the MinIO operator's terminology. This is good practice to use a separate namespace for the operator and each of it's tenants.

The namespaceOverride pattern is very common, as it the Helm dependencies pattern (for example here).

We are using ArgoCD to deploy the Helm charts, and for us MinIO operator and it's tenant is only one ArgoCD application. We can move to one application for the operator and one application for each tenant, if this MR is not accepted.

[jiuker]

Yeah. Great idea. @harshavardhana @shtripat @pjuarezd cc

@sathieu And could you define as a template like promethrous do that.

[shtripat]

y one ArgoCD application.

Yeah. Great idea. @harshavardhana @shtripat @pjuarezd cc

@sathieu And could you define as a template like promethrous do that.

@jiuker In my opinion operator is meant to manage multiple tenants. So one operator managing multiple tenants in one k8s cluster is fine. Any specific reason we want to have different operator deployments in different namespaces in same k8s cluster and they in turn maintaining their own tenants?

[jiuker]

y one ArgoCD application.

Yeah. Great idea. @harshavardhana @shtripat @pjuarezd cc
@sathieu And could you define as a template like promethrous do that.

@jiuker In my opinion operator is meant to manage multiple tenants. So one operator managing multiple tenants in one k8s cluster is fine. Any specific reason we want to have different operator deployments in different namespaces in same k8s cluster and they in turn maintaining their own tenants?

Check the diff files. This pr only want to change the tenant. Not the operator. @shtripat

[shtripat]

y one ArgoCD application.

Yeah. Great idea. @harshavardhana @shtripat @pjuarezd cc
@sathieu And could you define as a template like promethrous do that.

@jiuker In my opinion operator is meant to manage multiple tenants. So one operator managing multiple tenants in one k8s cluster is fine. Any specific reason we want to have different operator deployments in different namespaces in same k8s cluster and they in turn maintaining their own tenants?

Check the diff files. This pr only want to change the tenant. Not the operator. @shtripat

Oh ok. Then it should be fine.

[dnskr]

We are using ArgoCD to deploy the Helm charts, and for us MinIO operator and it's tenant is only one ArgoCD application. We can move to one application for the operator and one application for each tenant, if this MR is not accepted.

@sathieu I’m sure it’s better solution, especially because you already use ArgoCD. Also consider App Of Apps Pattern, which provides better management capabilities compare to chart of charts approach.

[sathieu]

We are using ArgoCD to deploy the Helm charts, and for us MinIO operator and it's tenant is only one ArgoCD application. We can move to one application for the operator and one application for each tenant, if this MR is not accepted.

@sathieu I’m sure it’s better solution, especially because you already use ArgoCD. Also consider App Of Apps Pattern, which provides better management capabilities compare to chart of charts approach.

We'll go this path. But it can still contribute to this PR if there is value for you.

@jiuker

@sathieu And could you define as a template like promethrous do that.

The template was in the initial commit. See 5f14e77. I can rollback to this if you agree.

[jiuker]

We are using ArgoCD to deploy the Helm charts, and for us MinIO operator and it's tenant is only one ArgoCD application. We can move to one application for the operator and one application for each tenant, if this MR is not accepted.

@sathieu I’m sure it’s better solution, especially because you already use ArgoCD. Also consider App Of Apps Pattern, which provides better management capabilities compare to chart of charts approach.

We'll go this path. But it can still contribute to this PR if there is value for you.

@jiuker

@sathieu And could you define as a template like promethrous do that.

The template was in the initial commit. See 5f14e77. I can rollback to this if you agree.

Yeah. It looks better.

[sathieu]

@jiuker rolled back to 5f14e77 and rebased.

@dnskr We moved to another ArgoCD app. So we don't need this PR anymore.

[shtripat]

lgtm

[dnskr]

@dnskr We moved to another ArgoCD app. So we don't need this PR anymore.

Great! I'm using ArgoCD and App of Apps approach as well. It allows to deploy and upgrade applications in a specific order, which is not possible for chart with subcharts.

---

I found that specifying namespace in Helm charts is quite complex question without super clear answer from my point of view.
There is an issue in the helm repository with a long discussion:

• Best Practices/Docs Question - Specify Namespace in Chart? #5465

As I understand from the first and the closing answers, namespace should not be explicitly specified in Helm chart templates. All special cases like custom GitOps flows and Spinnaker behavior can be easily mitigated if they still exist.

[jiuker]

@harshavardhana Any advices for this?

[sathieu]

abandoning this change. We now install two argocd apps.