fix(helm): change how the configMap for the gateway is generated #14575

jarmd · 2024-10-22T13:46:00Z

What this PR does / why we need it:

This PR updates the helm template file for [configmap-gateway.yaml] and the default values.yaml file
When you try to change the default configuration of:

gateway:
  nginxConfig:
    file: |
      A full nginx configuration file

The helm template errors out with the following error:

Error: YAML parse error on loki/templates/gateway/configmap-gateway.yaml: error converting YAML to JSON: yaml: line 14: did not find expected comment or line break

It has been tested to do the extra configuration in several way like:

  file: |-
  file: |
  file: >-

But it all keeps errors out.

With this update we have tested the following scenarios with code snippets for the GW deployment:

Scenario 1:
The user uses the default helm values

loki:
schemaConfig:
  configs:
    - from: 2022-09-28
      store: boltdb-shipper
      object_store: azure
      schema: v11
      index:
        prefix: loki_index_
        period: 24h
      chunks:
        prefix: loki_chunk_
        period: 24h
storage:
  bucketNames:
    chunks: FIXME
    ruler: FIXME
    admin: FIXM

Scenario 2
The user has as bad practice copied the entire default values to their configuration using the old values file default config:

loki:
  schemaConfig:
    configs:
      - from: 2022-09-28
        store: boltdb-shipper
        object_store: azure
        schema: v11
        index:
          prefix: loki_index_
          period: 24h
        chunks:
          prefix: loki_chunk_
          period: 24h
  storage:
    bucketNames:
      chunks: FIXME
      ruler: FIXME
      admin: FIXME
gateway:
  nginxConfig:
    file: |
      {{- include "loki.nginxFile" . | indent 2 -}}

Scenario 3
The user has their own customized nginx configuration

loki:
  schemaConfig:
    configs:
      - from: 2022-09-28
        store: boltdb-shipper
        object_store: azure
        schema: v11
        index:
          prefix: loki_index_
          period: 24h
        chunks:
          prefix: loki_chunk_
          period: 24h
  storage:
    bucketNames:
      chunks: FIXME
      ruler: FIXME
      admin: FIXME
gateway:
  nginxConfig:
    file: |-
      worker_processes  5;  ## Default: 1
      error_log  /dev/stderr;
      pid        /tmp/nginx.pid;
      worker_rlimit_nofile 8192;

Note: The loki-distributed helm chart uses the same approach for the template of the gateway configmap:
https://github.com/grafana/helm-charts/blob/main/charts/loki-distributed/templates/gateway/configmap-gateway.yaml#L10

Which issue(s) this PR fixes:
Fixes #
N/A

Special notes for your reviewer:

Checklist

Reviewed the CONTRIBUTING.md guide (required)
Documentation added (N/A)
Tests updated (N/A)
Title matches the required conventional commits format, see here
- Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md (N/A)
If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

CLAassistant · 2024-10-22T13:46:06Z

All committers have signed the CLA.

trevorwhitney

It looks like this results in a change to the rendered configmap (see the results of the diff CI job), any idea why that's the case? On it's surface it looks like this is a backwards compatible change, so I'm fine with it as long as we can explain that difference in the rendered manifests.

jarmd · 2024-10-24T07:52:23Z

It looks like this results in a change to the rendered configmap (see the results of the diff CI job), any idea why that's the case? On it's surface it looks like this is a backwards compatible change, so I'm fine with it as long as we can explain that difference in the rendered manifests.

Hi @trevorwhitney
Due to the change to nindent from indent in the configmap-gateway.yaml the manifest changes.
To illustrate why the change to nindent from indent I will be using the the following custom file configuration:

    file: |-
      # My comment
      worker_processes  5;  ## Default: 1
      error_log  /dev/stderr;
      pid        /tmp/nginx.pid;
      worker_rlimit_nofile 8192;

Since my first line here is a comment I can illustrate what is happening since the comment is valid code in both yaml and in the configmap:

With indent 4:

data:
  nginx.conf: | # My comment
    worker_processes  5;  ## Default: 1
    error_log  /dev/stderr;
    pid        /tmp/nginx.pid;
    worker_rlimit_nofile 8192;

You can see that the comment is added to the same line as the nginx.conf: | in the ConfigMap

When using nindent 4:

data:
  nginx.conf: |
    # My comment
    worker_processes  5;  ## Default: 1
    error_log  /dev/stderr;
    pid        /tmp/nginx.pid;
    worker_rlimit_nofile 8192;

If the first line in the configuration is not a #comment this will error out when using indent 4 when using custom file configuration as above.

Error: YAML parse error on loki/templates/gateway/configmap-gateway.yaml: error converting YAML to JSON: yaml: line 14: did not find expected comment or line break

I hope this explains it well enough :)

trevorwhitney

got it, that explains it, thanks! please add a CHANGELOG entry and we're good. thanks!

jarmd · 2024-11-15T07:00:56Z

@trevorwhitney
CHANGELOG has been updated. Please tell me if I did that wrong, fist time doing that on a public repo :)

trevorwhitney · 2024-11-15T19:09:06Z

yup. looks good! will merge once it passes CI, thanks!

github-actions · 2024-11-15T23:30:16Z

Kubernetes Manifest Diff Summary

Scenario: default-single-binary-values (Added: 0, Modified: 2, Removed: 0)

Summary:

Added: 0
Modified: 2
Removed: 0

Added Files

No added files

Modified Files

loki/templates/gateway/configmap-gateway.yaml

*** /home/runner/work/loki/loki/output/base/default-single-binary-values/loki/templates/gateway/configmap-gateway.yaml	2024-11-15 23:30:10.951315751 +0000
--- /home/runner/work/loki/loki/output/pr/default-single-binary-values/loki/templates/gateway/configmap-gateway.yaml	2024-11-15 23:30:13.175324180 +0000
***************
*** 13,19 ****
 app.kubernetes.io/managed-by: Helm
 app.kubernetes.io/component: gateway
 data:
! nginx.conf: | 
 worker_processes 5; ## Default: 1
 error_log /dev/stderr;
 pid /tmp/nginx.pid;
--- 13,20 ----
 app.kubernetes.io/managed-by: Helm
 app.kubernetes.io/component: gateway
 data:
! nginx.conf: |
! 
 worker_processes 5; ## Default: 1
 error_log /dev/stderr;
 pid /tmp/nginx.pid;

loki/templates/gateway/deployment-gateway-nginx.yaml

*** /home/runner/work/loki/loki/output/base/default-single-binary-values/loki/templates/gateway/deployment-gateway-nginx.yaml	2024-11-15 23:30:10.951315751 +0000
--- /home/runner/work/loki/loki/output/pr/default-single-binary-values/loki/templates/gateway/deployment-gateway-nginx.yaml	2024-11-15 23:30:13.179324195 +0000
***************
*** 25,31 ****
 template:
 metadata:
 annotations:
! checksum/config: 0c088a5a87465a37574b79e745e7377c79e1820cb42d6818792e8a021b1e8896
 labels:
 app.kubernetes.io/name: loki
 app.kubernetes.io/instance: loki-test-chart-name
--- 25,31 ----
 template:
 metadata:
 annotations:
! checksum/config: a7ec0fd82c6a070420fd863943ef80a66b39ccc14be062ebab9d5432776e0897
 labels:
 app.kubernetes.io/name: loki
 app.kubernetes.io/instance: loki-test-chart-name

Removed Files

No removed files

Scenario: default-values (Added: 0, Modified: 2, Removed: 0)

Summary:

Added: 0
Modified: 2
Removed: 0

Added Files

No added files

Modified Files

loki/templates/gateway/configmap-gateway.yaml

*** /home/runner/work/loki/loki/output/base/default-values/loki/templates/gateway/configmap-gateway.yaml	2024-11-15 23:30:11.047316206 +0000
--- /home/runner/work/loki/loki/output/pr/default-values/loki/templates/gateway/configmap-gateway.yaml	2024-11-15 23:30:13.311324662 +0000
***************
*** 13,19 ****
 app.kubernetes.io/managed-by: Helm
 app.kubernetes.io/component: gateway
 data:
! nginx.conf: | 
 worker_processes 5; ## Default: 1
 error_log /dev/stderr;
 pid /tmp/nginx.pid;
--- 13,20 ----
 app.kubernetes.io/managed-by: Helm
 app.kubernetes.io/component: gateway
 data:
! nginx.conf: |
! 
 worker_processes 5; ## Default: 1
 error_log /dev/stderr;
 pid /tmp/nginx.pid;

loki/templates/gateway/deployment-gateway-nginx.yaml

*** /home/runner/work/loki/loki/output/base/default-values/loki/templates/gateway/deployment-gateway-nginx.yaml	2024-11-15 23:30:11.047316206 +0000
--- /home/runner/work/loki/loki/output/pr/default-values/loki/templates/gateway/deployment-gateway-nginx.yaml	2024-11-15 23:30:13.311324662 +0000
***************
*** 25,31 ****
 template:
 metadata:
 annotations:
! checksum/config: 4b91b249225cdff02f438c84af97f54b1d2ae2ecf7810f2cacfdbabbd72a2366
 labels:
 app.kubernetes.io/name: loki
 app.kubernetes.io/instance: loki-test-chart-name
--- 25,31 ----
 template:
 metadata:
 annotations:
! checksum/config: a4e1604df1a5549896a2b31d5d42524b4cfc5517b26ec3c5b2cf2a91e88624b8
 labels:
 app.kubernetes.io/name: loki
 app.kubernetes.io/instance: loki-test-chart-name

Removed Files

No removed files

Scenario: ingress-values (Added: 0, Modified: 2, Removed: 0)

Summary:

Added: 0
Modified: 2
Removed: 0

Added Files

No added files

Modified Files

loki/templates/gateway/configmap-gateway.yaml

*** /home/runner/work/loki/loki/output/base/ingress-values/loki/templates/gateway/configmap-gateway.yaml	2024-11-15 23:30:11.143316661 +0000
--- /home/runner/work/loki/loki/output/pr/ingress-values/loki/templates/gateway/configmap-gateway.yaml	2024-11-15 23:30:13.407325002 +0000
***************
*** 13,19 ****
 app.kubernetes.io/managed-by: Helm
 app.kubernetes.io/component: gateway
 data:
! nginx.conf: | 
 worker_processes 5; ## Default: 1
 error_log /dev/stderr;
 pid /tmp/nginx.pid;
--- 13,20 ----
 app.kubernetes.io/managed-by: Helm
 app.kubernetes.io/component: gateway
 data:
! nginx.conf: |
! 
 worker_processes 5; ## Default: 1
 error_log /dev/stderr;
 pid /tmp/nginx.pid;

loki/templates/gateway/deployment-gateway-nginx.yaml

*** /home/runner/work/loki/loki/output/base/ingress-values/loki/templates/gateway/deployment-gateway-nginx.yaml	2024-11-15 23:30:11.147316680 +0000
--- /home/runner/work/loki/loki/output/pr/ingress-values/loki/templates/gateway/deployment-gateway-nginx.yaml	2024-11-15 23:30:13.411325016 +0000
***************
*** 25,31 ****
 template:
 metadata:
 annotations:
! checksum/config: 4b91b249225cdff02f438c84af97f54b1d2ae2ecf7810f2cacfdbabbd72a2366
 labels:
 app.kubernetes.io/name: loki
 app.kubernetes.io/instance: loki-test-chart-name
--- 25,31 ----
 template:
 metadata:
 annotations:
! checksum/config: a4e1604df1a5549896a2b31d5d42524b4cfc5517b26ec3c5b2cf2a91e88624b8
 labels:
 app.kubernetes.io/name: loki
 app.kubernetes.io/instance: loki-test-chart-name

Removed Files

No removed files

Scenario: legacy-monitoring-values (Added: 0, Modified: 2, Removed: 0)

Summary:

Added: 0
Modified: 2
Removed: 0

Added Files

No added files

Modified Files

loki/templates/gateway/configmap-gateway.yaml

*** /home/runner/work/loki/loki/output/base/legacy-monitoring-values/loki/templates/gateway/configmap-gateway.yaml	2024-11-15 23:30:11.247317154 +0000
--- /home/runner/work/loki/loki/output/pr/legacy-monitoring-values/loki/templates/gateway/configmap-gateway.yaml	2024-11-15 23:30:13.519325398 +0000
***************
*** 13,19 ****
 app.kubernetes.io/managed-by: Helm
 app.kubernetes.io/component: gateway
 data:
! nginx.conf: | 
 worker_processes 5; ## Default: 1
 error_log /dev/stderr;
 pid /tmp/nginx.pid;
--- 13,20 ----
 app.kubernetes.io/managed-by: Helm
 app.kubernetes.io/component: gateway
 data:
! nginx.conf: |
! 
 worker_processes 5; ## Default: 1
 error_log /dev/stderr;
 pid /tmp/nginx.pid;

loki/templates/gateway/deployment-gateway-nginx.yaml

*** /home/runner/work/loki/loki/output/base/legacy-monitoring-values/loki/templates/gateway/deployment-gateway-nginx.yaml	2024-11-15 23:30:11.247317154 +0000
--- /home/runner/work/loki/loki/output/pr/legacy-monitoring-values/loki/templates/gateway/deployment-gateway-nginx.yaml	2024-11-15 23:30:13.519325398 +0000
***************
*** 25,31 ****
 template:
 metadata:
 annotations:
! checksum/config: 4b91b249225cdff02f438c84af97f54b1d2ae2ecf7810f2cacfdbabbd72a2366
 labels:
 app.kubernetes.io/name: loki
 app.kubernetes.io/instance: loki-test-chart-name
--- 25,31 ----
 template:
 metadata:
 annotations:
! checksum/config: a4e1604df1a5549896a2b31d5d42524b4cfc5517b26ec3c5b2cf2a91e88624b8
 labels:
 app.kubernetes.io/name: loki
 app.kubernetes.io/instance: loki-test-chart-name

Removed Files

No removed files

Scenario: simple-scalable-aws-kube-irsa-values (Added: 0, Modified: 0, Removed: 0)

Summary:

Added: 0
Modified: 0
Removed: 0

Added Files

No added files

Modified Files

No modified files

Removed Files

No removed files

trevorwhitney · 2024-11-15T23:35:19Z

The helm linting failure seems to be related to the submodule in the operator folder, and submitting a PR from a fork. I've asked for some help looking into it.

fix(helm): change how the configMap for the gateway is generated

03a522a

jarmd requested a review from a team as a code owner October 22, 2024 13:46

pull-request-size bot added the size/XS label Oct 22, 2024

github-actions bot added the area/helm label Oct 22, 2024