Helm file values-istio-demo.yaml is wrong

[israel-hdez]

Bug description

The install/kubernetes/helm/istio/values-istio-demo.yaml available in the released package defines the "global" key twice. There is one definition at the top:

global:
  proxy:
    accessLogFile: "/dev/stdout"
    resources:
      requests:
        cpu: 10m
        memory: 40Mi

  disablePolicyChecks: false

and one at the bottom:

# @include <values-istio-demo-common.yaml>
#
global:
  controlPlaneSecurityEnabled: false

  mtls:
    # Default setting for service-to-service mtls. Can be set explicitly using
    # destination rules or service annotations.
    enabled: false

When rendering with:

helm template install/kubernetes/helm/istio --name istio --namespace istio-system -f install/kubernetes/helm/istio/values-istio-demo.yaml

definitions are not merged, but the second one completely overwrites the first one. So, the rendered template is not suitable for small envs.

Affected product area (please put an X in all that apply)

[ ] Configuration Infrastructure
[ ] Docs
[x] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

Expected behavior
Perhaps, the values-istio-demo.yaml file provided in the released package should define global only once, with merged values.

Steps to reproduce the bug
It's all in the bug description :)

FYI: I discovered this because I was trying to setup a multi-cluster env using instructions in https://istio.io/docs/setup/kubernetes/install/multicluster/gateways/. But I wanted a demo profile plus multi-cluster. So, I was trying:

helm template install/kubernetes/helm/istio --name istio --namespace istio-system \
    -f install/kubernetes/helm/istio/values-istio-demo.yaml \
    -f install/kubernetes/helm/istio/example-values/values-istio-multicluster-gateways.yaml

which was generating a yaml not suitable for small envs (thus pods not starting because insufficient cpu).

Version (include the output of istioctl version --remote and kubectl version)
Istio: 1.2.4
Helm: 2.14.3

How was Istio installed?
N/A

Environment where bug was observed (cloud vendor, OS, etc)
minikube

[howardjohn]

The globals appearing twice is intended. It not merging is not. However, I am not seeing it not merge..

$ helm template . | grep PILOT_TRACE_SAMPLING -A 1
          - name: PILOT_TRACE_SAMPLING
            value: "1"
$ helm template . -f values-istio-demo.yaml| grep PILOT_TRACE_SAMPLING -A 1
          - name: PILOT_TRACE_SAMPLING
            value: "100"

Using helm 2.14 as well

[israel-hdez]

@howardjohn I think the better way to test is to generate two YAMLs. One with the provided values-istio-demo.yaml and, then, patching it.

So, save this patch file in the chart directory which should unify the duplicated globals:

--- values-istio-demo.yaml	2019-07-31 19:06:02.000000000 -0500
+++ values-istio-demo.yaml	2019-08-21 15:25:07.299659314 -0500
@@ -11,6 +11,12 @@
         memory: 40Mi
 
   disablePolicyChecks: false
+  controlPlaneSecurityEnabled: false
+
+  mtls:
+    # Default setting for service-to-service mtls. Can be set explicitly using
+    # destination rules or service annotations.
+    enabled: false
 
 sidecarInjectorWebhook:
   enabled: true
@@ -73,16 +79,4 @@
       requests:
         cpu: 10m
         memory: 40Mi
-# This is used to generate istio.yaml for minimal, demo mode.
-# It is shipped with the release, used for bookinfo or quick installation of istio.
-# Includes components used in the demo, defaults to alpha3 rules.
-
-# @include <values-istio-demo-common.yaml>
-#
-global:
-  controlPlaneSecurityEnabled: false
 
-  mtls:
-    # Default setting for service-to-service mtls. Can be set explicitly using
-    # destination rules or service annotations.
-    enabled: false

Then run these commands:

helm template . -f values-istio-demo.yaml > demo.yaml # Generate demo.yaml with non-patched values file
patch < values-istio-demo.yaml.patch # Patch the values file
helm template . -f values-istio-demo.yaml > demo_patched.yaml # Generate demo_patched.yaml with patched values file
diff -u  demo.yaml demo_patched.yaml  | less # See the differences

Diff is quite large, so I'm not pasting it here.

[howardjohn]

Hmm interesting.. It seems like it works in some places and doesn't work in other places, that is weird. Definitely something that needs to be fixed