Skip to content
This repository has been archived by the owner on Aug 22, 2022. It is now read-only.

typo in HPA annotations #158

Closed
disconn3ct opened this issue May 1, 2022 · 7 comments · Fixed by #159
Closed

typo in HPA annotations #158

disconn3ct opened this issue May 1, 2022 · 7 comments · Fixed by #159
Labels
bug Something isn't working

Comments

@disconn3ct
Copy link

Details

Helm chart name and version: Common 4.4.0

What steps did you take and what happened: HPA is enabled. CICD accepted a minor revision of a leaf chart (searx) and failed with a templating error: Helm upgrade failed: template: searx/templates/common.yaml:89:3: executing "searx/templates/common.yaml" at <include "common.all" .>: error calling include: template: searx/charts/common/templates/_all.tpl:49:5: executing "common.all" at <include "common.classes.hpa" .>: error calling include: template: searx/charts/common/templates/classes/_HorizontalPodAutoscaler.tpl:15:19: executing "common.classes.hpa" at <include "annotations.labels" .>: error calling include: template: no template "annotations.labels" associated with template "gotpl"

Relevant Helm values: autoscaling.enabled: true

What did you expect to happen: minor upgrade

Anything else you would like to add: Looks like a simple typo. The other files are using common.annotations but this line is annotations.labels

Additional Information:

@disconn3ct disconn3ct added the bug Something isn't working label May 1, 2022
@gclawes
Copy link

gclawes commented May 1, 2022

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
Copy link
Member

onedr0p commented May 1, 2022

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
Copy link

gclawes commented May 1, 2022

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:

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

@gclawes
Copy link

gclawes commented May 1, 2022

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

{{/* 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
Copy link
Member

onedr0p commented May 2, 2022

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
Copy link

gclawes commented May 2, 2022

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
Copy link
Contributor

bjw-s commented May 2, 2022

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants