typo in HPA annotations about library-charts

[gclawes]

Seeing this when installing thelounge with VPN enabled.

Error: Failed to render chart: exit status 1: Error: template: thelounge/templates/common.yaml:1:3: executing "thelounge/templates/common.yaml" at <include "common.all" .>: error calling include: template: thelounge/charts/common/templates/_all.tpl:15:8: executing "common.all" at <include "common.addon.vpn" .>: error calling include: template: thelounge/charts/common/templates/addons/vpn/_vpn.tpl:16:20: executing "common.addon.vpn" at <include "common.addon.vpn.configmap" .>: error calling include: template: thelounge/charts/common/templates/addons/vpn/_configmap.tpl:12:19: executing "common.addon.vpn.configmap" at <include "annotations.labels" .>: error calling include: template: no template "annotations.labels" associated with template "gotpl"

Values for thelounge (secrets omitted):

# -- environment variables.
# @default -- See below
env:
  # -- Set the container timezone
  TZ: America/New_York
  WAIT_FOR_VPN: "true"

podAnnotations:
  sidecar.istio.io/inject: "true"

securityContext:
  runAsNonRoot: true
  runAsUser: 9000
  runAsGroup: 9000

command: ["sh"]
args: [ "-c", "source /shim/vpn.sh && /usr/local/bin/docker-entrypoint.sh thelounge start"]

ingress:
  # -- Enable and configure ingress settings for the chart under this key.
  # @default -- See values.yaml
  main:
    enabled: true
    annotations:
      nginx.ingress.kubernetes.io/enable-access-log: "true"
      cert-manager.io/issuer-group: certmanager.step.sm
      cert-manager.io/issuer-kind: StepClusterIssuer
      cert-manager.io/issuer: home-arpa
    hosts:
    - host: thelounge.k8s.home.arpa
      paths:
      - path: /
        pathType: ImplementationSpecific
    tls:
    - secretName: thelounge-tls
      hosts:
        - thelounge.k8s.home.arpa

# -- Configure configMaps for the chart here.
# Additional configMaps can be added by adding a dictionary key similar to the 'config' object.
# @default -- See below
configmap:
  shim:
    # -- Enables or disables the configMap
    enabled: true
    # -- configMap data content. Helm template enabled.
    data:
      vpn.sh: |
        #!/usr/bin/env sh

        WAIT_FOR_VPN=${WAIT_FOR_VPN:-"false"}

        if
            [[ "${WAIT_FOR_VPN}" == "true" ]];
        then
            echo "Waiting for VPN to be connected..."
            while ! grep -s -q "connected" /shared/vpnstatus;
            do
                echo "VPN not connected"
                sleep 2
            done
            echo "VPN Connected, starting application..."
        fi

# -- Configure persistence settings for the chart under this key.
# @default -- See values.yaml
persistence:
  config:
    enabled: true
    type: custom
    accessMode: ReadWriteOnce
    volumeSpec:
      nfs:
        server: nfs.home.arpa
        path: /tank/kubernetes/configs/thelounge
  shim:
    enabled: true
    type: configMap
    name: thelounge-shim
  shared:
    enabled: true
    type: emptyDir
    mountPath: /shared

addons:
  netshoot:
    enabled: false
    securityContext:
      runAsUser: 0
      runAsGroup: 0
      runAsNonRoot: false
  vpn:
    enabled: true
    type: openvpn
    openvpn:
      authSecret: thelounge
    securityContext:
      runAsGroup: 0
      runAsUser: 0
    env:
      FIREWALL: 'on'
    scripts:
      # Commend out up.sh/down.sh so DNS doesn't get redirected
      up: |-
        #!/bin/bash
        #/etc/openvpn/up.sh
        echo "connected" > /shared/vpnstatus

      down: |-
        #!/bin/bash
        #/etc/openvpn/down.sh
        echo "disconnected" > /shared/vpnstatus

[onedr0p]

We saw something similar happen with the charts the create a service monitor. Since our testing can only go so far as to statically check things, it's hard to test for things in a test cluster giving the vast amount of configuration options that could exist. I am sure we will have a patch out soon to address these issues so please hold on tight.

@disconn3ct for your information, we are planning on removing HPA in the next major release, see here #157 but we can probably patch it so it works for you for now but don't expect to have it in the next major release. It might be worth creating those HPA manifests outside the helm values soon anyways.

[gclawes]

I don't think this is restricted to just the HPA. I think this occurs anywhere this is called:
annotations: {{- include "annotations.labels" . | nindent 4 }} .

Should that instead be annotations: {{- include "common.annotations" . | nindent 4 }}, based on the similar usage for labels?

Example:

library-charts/charts/stable/common/templates/addons/vpn/_configmap.tpl

Lines 9 to 12 in 3c71189

	metadata:
	name: {{ include "common.names.fullname" . }}-vpn
	labels: {{- include "common.labels" . | nindent 4 }}
	annotations: {{- include "annotations.labels" . | nindent 4 }}

[gclawes]

Looking at this, it seems that should be the case.

library-charts/charts/stable/common/templates/lib/chart/_annotations.tpl

Lines 1 to 10 in 3d66ee5

	{{/* Common annotations shared across objects */}}
	{{- define "common.annotations" -}}
	{{- with .Values.global.annotations }}
	{{- range $k, $v := . }}
	{{- $name := $k }}
	{{- $value := tpl $v $ }}
	{{ $name }}: {{ quote $value }}
	{{- end }}
	{{- end }}
	{{- end -}}

[onedr0p]

Sorry, I should have been more clear. It's not just the HPA. It affects anything using a additional resource in charts which includes HPA, serviceMonitors etc..

[gclawes]

Looks like that typo is only in 3 places:

$ grep -rI 'include "annotations.labels"' .
./charts/stable/common/templates/addons/vpn/openvpn/_secret.tpl:  annotations: {{- include "annotations.labels" . | nindent 4 }}
./charts/stable/common/templates/addons/vpn/_configmap.tpl:  annotations: {{- include "annotations.labels" . | nindent 4 }}
./charts/stable/common/templates/classes/_HorizontalPodAutoscaler.tpl:  annotations: {{- include "annotations.labels" . | nindent 4 }}

[bjw-s]

There is indeed a typo / copy-paste error :( Sorry about that. Will push a fix for that somewhere today.