Remove namespace setting from helm chart

[galovics]

When deploying the helm chart, the namespace is hardcoded in the values.yaml:

deploymentNamespace : native-ingress-controller-system

Looking into usage, I can see the deployment.yaml template is creating the namespace itself and deploying the resources into the namespace.

This chart shouldn't be responsible of:

1. creating any namespace
2. deploying the resources into a namespace that's different from what you use for the helm chart deployment

Consider the following Terraform script (same for helm CLI):

resource "helm_release" "oci-native-ingress-controller" {
  name            = "oci-native-ingress-controller"
  chart           = "oci-native-ingress-controller"
  namespace       = "cluster-tools"
  wait            = true
  cleanup_on_fail = true
  atomic          = true

  set {
    name  = "compartment_id"
    value = var.compartment_id
  }

  set {
    name  = "subnet_id"
    value = var.load_balancer_subnet_id
  }

  set {
    name  = "cluster_id"
    value = var.cluster_id
  }
}

Obviously the expectation is that the ingress-controller will be deployed in the cluster-tools namespace but it's not since the namespace definition is hardcoded as mentioned above.

This is misleading and definitely not the responsibility of the chart.

I suggest to entirely remove any namespace specifics.

[piyush-tiwari]

Hi @galovics, thanks for reaching out.

You can simply set the pre-existing namespace for deploymentNamespace in values.yaml for this. Looking at your TF example, I believe this can achieved by adding the snippet

set {
    name  = "deploymentNamespace"
    value = <Pre-existing Namespace>
  }

The latest chart released with v1.3.7 checks if the namespace already exists and doesn't create it if that's so. Have a look at the code here - https://github.com/oracle/oci-native-ingress-controller/blob/main/helm/oci-native-ingress-controller/templates/deployment.yaml#L5-L13

I am closing this issue as it's already dealt with, feel free to reopen in case you think that's inappropriate.

[galovics]

@piyush-tiwari that's true I can set it, but it's a matter of responsibilities. I just don't get why the chart handles namespace creation and deploying into that specific namespace. Why not follow the best practices in the industry and make the helm chart free from any pre-defined namespaces?