MChorfa / porter-helm3

porter-helm3
Apache License 2.0
8 stars 10 forks source link

Can't escape these very common set lines #8

Open squillace opened 4 years ago

squillace commented 4 years ago

Either I want to know how to escape these using Helm3 and porter, or if it's an issue let's fix it!

the node selectors MUST be escaped in that fashion in order to work correctly on the command line. How should they be created in YAML using the Helm3 mixin in order to work correctly?

# Create namespace for the harbor nginx ingress controller 
kubectl create namespace harbor-ingress-system

# Add the nginx helm repo
helm repo add ingress-nginx https://kubernetes.github.io/ingress-nginx

# Install nginx ingress for Harbor
helm install harbor-nginx-ingress ingress-nginx/ingress-nginx \
    --namespace harbor-ingress-system \
    --set controller.ingressClass=harbor-nginx \
    --set controller.replicaCount=2 \
    --set controller.nodeSelector."beta\.kubernetes\.io/os"=linux \
    --set defaultBackend.nodeSelector."beta\.kubernetes\.io/os"=linux
MChorfa commented 4 years ago

The issue is actually coming from helm is stripping escaping from literal key https://github.com/helm/helm/issues/4030 So patch was introduced within the version v0.1.9 where fix force the escaping at runtime : You still define your action as follow:

install:
  - helm3:
      description: "install porter-nginx"
      name: porter-nginx
      chart: stable/nginx-ingress
      version: 1.41.3
      namespace: "{{ bundle.parameters.namespace }}"
      upsert: true
      set:
        controller.scope.namespace: "{{ bundle.parameters.namespace }}"
        controller.ingressClass: "{{ bundle.parameters.namespace }}-nginx"
        defaultBackend.nodeSelector."beta\.kubernetes\.io/os": "linux"
        controller.nodeSelector."beta\.kubernetes\.io/os": "linux"
        controller.nodeSelector."kubernetes\.io/role": "main"

And at runtime it would result to :

/cnab/app/mixins/helm3/helm3-runtime install --debug
/usr/local/bin/helm3 helm3 upgrade --install porter-nginx stable/nginx-ingress --namespace porter --version 1.41.3 --set controller\.ingressClass=porter-nginx --set controller\.nodeSelector\."beta\\.kubernetes\\.io/os"=linux --set controller\.nodeSelector\."kubernetes\\.io/role"=main --set controller\.scope\.namespace=porter --set defaultBackend\.nodeSelector\."beta\\.kubernetes\\.io/os"=linux

Execution completed successfully!
NAME              CREATED         MODIFIED        LAST ACTION   LAST STATUS
nginx             5 seconds ago   5 seconds ago   install       succeeded

Will keep this open until helm introduce --set-literal option

squillace commented 4 years ago

or we could specify using --set-file per the helm issue. what do you think?

MChorfa commented 4 years ago

--set-file is some what similar to --values overrides

--set-file stringArray set values from respective files specified via the command line (can specify multiple or separate values with commas: key1=path1,key2=path2)

squillace commented 4 years ago

This in particular is a good way to think about it. I might try this as an experiment in any case. BUT: I can't thank you enough for the modification -- I look forward to using it immediately tomorrow!

https://github.com/helm/helm/issues/4030#issuecomment-440758153