From 5ae418bfae69e113a5fb83390b666f1026b9493a Mon Sep 17 00:00:00 2001 From: Kyle Schochenmaier Date: Tue, 29 Jun 2021 20:28:39 -0500 Subject: [PATCH] Add ability to set log level and enable json logging for all consul-k8s commands (#980) * Adds templating for passing global values for log-level and log-json flags to all consul-k8s commands. --- CHANGELOG.md | 3 ++ templates/connect-inject-deployment.yaml | 5 ++- templates/controller-deployment.yaml | 3 +- templates/create-federation-secret-job.yaml | 2 ++ templates/ingress-gateways-deployment.yaml | 12 ++++--- templates/mesh-gateway-deployment.yaml | 4 +++ templates/server-acl-init-cleanup-job.yaml | 2 ++ templates/server-acl-init-job.yaml | 2 ++ templates/sync-catalog-deployment.yaml | 5 ++- .../terminating-gateways-deployment.yaml | 2 ++ templates/tls-init-job.yaml | 2 ++ .../webhook-cert-manager-deployment.yaml | 2 ++ test/unit/connect-inject-deployment.bats | 4 +-- test/unit/controller-deployment.bats | 31 +++++++++++++++++++ test/unit/ingress-gateways-deployment.bats | 4 +++ test/unit/mesh-gateway-deployment.bats | 12 +++++++ test/unit/server-acl-init-cleanup-job.bats | 4 +-- test/unit/sync-catalog-deployment.bats | 31 +++++++++++++++++++ values.yaml | 23 +++++++++++--- 19 files changed, 133 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 686c5624e..e39be70b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## Unreleased +IMPROVEMENTS: +* Allow setting global.logLevel and global.logJSON and propogate this to all consul-k8s commands. [[GH-980](https://github.com/hashicorp/consul-helm/pull/980)] + ## 0.32.1 (June 29, 2021) BUG FIXES: diff --git a/templates/connect-inject-deployment.yaml b/templates/connect-inject-deployment.yaml index 1c0ae7006..9c3572877 100644 --- a/templates/connect-inject-deployment.yaml +++ b/templates/connect-inject-deployment.yaml @@ -83,6 +83,8 @@ spec: CONSUL_FULLNAME="{{template "consul.fullname" . }}" consul-k8s inject-connect \ + -log-level={{ default .Values.global.logLevel .Values.connectInject.logLevel }} \ + -log-json={{ .Values.global.logJSON }} \ -default-inject={{ .Values.connectInject.default }} \ -consul-image="{{ default .Values.global.image .Values.connectInject.imageConsul }}" \ -envoy-image="{{ .Values.global.imageEnvoy }}" \ @@ -106,9 +108,6 @@ spec: {{- if .Values.global.openshift.enabled }} -enable-openshift \ {{- end }} - {{- if .Values.connectInject.logLevel }} - -log-level={{ .Values.connectInject.logLevel }} \ - {{- end }} {{- if (or (and (ne (.Values.connectInject.metrics.defaultEnabled | toString) "-") .Values.connectInject.metrics.defaultEnabled) (and (eq (.Values.connectInject.metrics.defaultEnabled | toString) "-") .Values.global.metrics.enabled)) }} -default-enable-metrics=true \ {{- else }} diff --git a/templates/controller-deployment.yaml b/templates/controller-deployment.yaml index 73c2aafb8..50d5d1328 100644 --- a/templates/controller-deployment.yaml +++ b/templates/controller-deployment.yaml @@ -59,10 +59,11 @@ spec: - "-ec" - | consul-k8s controller \ + -log-level={{ default .Values.global.logLevel .Values.controller.logLevel }} \ + -log-json={{ .Values.global.logJSON }} \ -webhook-tls-cert-dir=/tmp/controller-webhook/certs \ -datacenter={{ .Values.global.datacenter }} \ -enable-leader-election \ - -log-level={{ .Values.controller.logLevel | quote }} \ {{- if .Values.global.enableConsulNamespaces }} -enable-namespaces=true \ {{- if .Values.connectInject.consulNamespaces.consulDestinationNamespace }} diff --git a/templates/create-federation-secret-job.yaml b/templates/create-federation-secret-job.yaml index 5b0af8fa5..0745a44e8 100644 --- a/templates/create-federation-secret-job.yaml +++ b/templates/create-federation-secret-job.yaml @@ -116,6 +116,8 @@ spec: - "-ec" - | consul-k8s create-federation-secret \ + -log-level={{ .Values.global.logLevel }} \ + -log-json={{ .Values.global.logJSON }} \ {{- if (and .Values.global.gossipEncryption.secretName .Values.global.gossipEncryption.secretKey) }} -gossip-key-file=/consul/gossip/gossip.key \ {{- end }} diff --git a/templates/ingress-gateways-deployment.yaml b/templates/ingress-gateways-deployment.yaml index c3658cc4a..116d62a8e 100644 --- a/templates/ingress-gateways-deployment.yaml +++ b/templates/ingress-gateways-deployment.yaml @@ -166,6 +166,8 @@ spec: WAN_ADDR="${HOST_IP}" {{- else if (or (eq $serviceType "ClusterIP") (eq $serviceType "LoadBalancer")) }} consul-k8s service-address \ + -log-level={{ $root.Values.global.logLevel }} \ + -log-json={{ $root.Values.global.logJSON }} \ -k8s-namespace={{ $root.Release.Namespace }} \ -name={{ template "consul.fullname" $root }}-{{ .name }} \ -output-file=/tmp/address.txt @@ -369,10 +371,10 @@ spec: - "-ec" - | /consul-bin/consul services deregister \ - {{- if $root.Values.global.enableConsulNamespaces }} - -namespace={{ default $defaults.consulNamespace .consulNamespace }} \ - {{- end }} - -id="${POD_NAME}" + {{- if $root.Values.global.enableConsulNamespaces }} + -namespace={{ default $defaults.consulNamespace .consulNamespace }} \ + {{- end }} + -id="${POD_NAME}" # consul-sidecar ensures the ingress gateway is always registered with # the local Consul agent, even if it loses the initial registration. @@ -419,6 +421,8 @@ spec: command: - consul-k8s - consul-sidecar + - -log-level={{ $root.Values.global.logLevel }} + - -log-json={{ $root.Values.global.logJSON }} - -service-config=/consul/service/service.hcl - -consul-binary=/consul-bin/consul {{- if $root.Values.global.acls.manageSystemACLs }} diff --git a/templates/mesh-gateway-deployment.yaml b/templates/mesh-gateway-deployment.yaml index e81658198..e4032231e 100644 --- a/templates/mesh-gateway-deployment.yaml +++ b/templates/mesh-gateway-deployment.yaml @@ -147,6 +147,8 @@ spec: WAN_ADDR="${NODE_NAME}" {{- else if and (eq $source "Service") (or (eq $serviceType "ClusterIP") (eq $serviceType "LoadBalancer")) }} consul-k8s service-address \ + -log-level={{ .Values.global.logLevel }} \ + -log-json={{ .Values.global.logJSON }} \ -k8s-namespace={{ .Release.Namespace }} \ -name={{ template "consul.fullname" . }}-mesh-gateway \ -output-file=/tmp/address.txt @@ -365,6 +367,8 @@ spec: command: - consul-k8s - consul-sidecar + - -log-level={{ .Values.global.logLevel }} + - -log-json={{ .Values.global.logJSON }} - -service-config=/consul/service/service.hcl - -consul-binary=/consul-bin/consul {{- if .Values.global.acls.manageSystemACLs }} diff --git a/templates/server-acl-init-cleanup-job.yaml b/templates/server-acl-init-cleanup-job.yaml index 4f13e4412..c5c901f25 100644 --- a/templates/server-acl-init-cleanup-job.yaml +++ b/templates/server-acl-init-cleanup-job.yaml @@ -50,6 +50,8 @@ spec: - consul-k8s args: - delete-completed-job + - -log-level={{ .Values.global.logLevel }} + - -log-json={{ .Values.global.logJSON }} - -k8s-namespace={{ .Release.Namespace }} - {{ template "consul.fullname" . }}-server-acl-init resources: diff --git a/templates/server-acl-init-job.yaml b/templates/server-acl-init-job.yaml index 8a9e6479e..3ac5c4676 100644 --- a/templates/server-acl-init-job.yaml +++ b/templates/server-acl-init-job.yaml @@ -98,6 +98,8 @@ spec: CONSUL_FULLNAME="{{template "consul.fullname" . }}" consul-k8s server-acl-init \ + -log-level={{ .Values.global.logLevel }} \ + -log-json={{ .Values.global.logJSON }} \ -resource-prefix=${CONSUL_FULLNAME} \ -k8s-namespace={{ .Release.Namespace }} \ diff --git a/templates/sync-catalog-deployment.yaml b/templates/sync-catalog-deployment.yaml index 1e8fbd20d..43792e2d8 100644 --- a/templates/sync-catalog-deployment.yaml +++ b/templates/sync-catalog-deployment.yaml @@ -113,6 +113,8 @@ spec: - "-ec" - | consul-k8s sync-catalog \ + -log-level={{ default .Values.global.logLevel .Values.syncCatalog.logLevel }} \ + -log-json={{ .Values.global.logJSON }} \ -k8s-default-sync={{ .Values.syncCatalog.default }} \ {{- if (not .Values.syncCatalog.toConsul) }} -to-consul=false \ @@ -143,9 +145,6 @@ spec: {{- if .Values.syncCatalog.consulWriteInterval }} -consul-write-interval={{ .Values.syncCatalog.consulWriteInterval }} \ {{- end }} - {{- if .Values.syncCatalog.logLevel }} - -log-level={{ .Values.syncCatalog.logLevel }} \ - {{- end }} {{- if .Values.syncCatalog.k8sTag }} -consul-k8s-tag={{ .Values.syncCatalog.k8sTag }} \ {{- end }} diff --git a/templates/terminating-gateways-deployment.yaml b/templates/terminating-gateways-deployment.yaml index f58205a87..1a685730f 100644 --- a/templates/terminating-gateways-deployment.yaml +++ b/templates/terminating-gateways-deployment.yaml @@ -367,6 +367,8 @@ spec: command: - consul-k8s - consul-sidecar + - -log-level={{ $root.Values.global.logLevel }} + - -log-json={{ $root.Values.global.logJSON }} - -service-config=/consul/service/service.hcl - -consul-binary=/consul-bin/consul {{- if $root.Values.global.acls.manageSystemACLs }} diff --git a/templates/tls-init-job.yaml b/templates/tls-init-job.yaml index ebd0e5f41..c402cb1f6 100644 --- a/templates/tls-init-job.yaml +++ b/templates/tls-init-job.yaml @@ -61,6 +61,8 @@ spec: # and use * at the start of the dns name when setting -additional-dnsname. set -o noglob consul-k8s tls-init \ + -log-level={{ .Values.global.logLevel }} \ + -log-json={{ .Values.global.logJSON }} \ -domain={{ .Values.global.domain }} \ -days=730 \ -name-prefix={{ template "consul.fullname" . }} \ diff --git a/templates/webhook-cert-manager-deployment.yaml b/templates/webhook-cert-manager-deployment.yaml index 5a44aa124..66bcdae0a 100644 --- a/templates/webhook-cert-manager-deployment.yaml +++ b/templates/webhook-cert-manager-deployment.yaml @@ -37,6 +37,8 @@ spec: - "-ec" - | consul-k8s webhook-cert-manager \ + -log-level={{ .Values.global.logLevel }} \ + -log-json={{ .Values.global.logJSON }} \ -config-file=/bootstrap/config/webhook-config.json \ -deployment-name={{ template "consul.fullname" . }}-webhook-cert-manager \ -deployment-namespace={{ .Release.Namespace }} diff --git a/test/unit/connect-inject-deployment.bats b/test/unit/connect-inject-deployment.bats index 0c3a93cf6..2b6195090 100755 --- a/test/unit/connect-inject-deployment.bats +++ b/test/unit/connect-inject-deployment.bats @@ -1346,7 +1346,7 @@ EOF #-------------------------------------------------------------------- # logLevel -@test "connectInject/Deployment: logLevel info by default" { +@test "connectInject/Deployment: logLevel info by default from global" { cd `chart_dir` local cmd=$(helm template \ -s templates/connect-inject-deployment.yaml \ @@ -1359,7 +1359,7 @@ EOF [ "${actual}" = "true" ] } -@test "connectInject/Deployment: logLevel can be set" { +@test "connectInject/Deployment: logLevel can be overridden" { cd `chart_dir` local cmd=$(helm template \ -s templates/connect-inject-deployment.yaml \ diff --git a/test/unit/controller-deployment.bats b/test/unit/controller-deployment.bats index 13c7960d5..c7c649de3 100644 --- a/test/unit/controller-deployment.bats +++ b/test/unit/controller-deployment.bats @@ -481,3 +481,34 @@ load _helpers yq '[.spec.template.spec.containers[0].env[].name] | any(contains("CONSUL_HTTP_TOKEN"))' | tee /dev/stderr) [ "${actual}" = "true" ] } + +#-------------------------------------------------------------------- +# logLevel + +@test "controller/Deployment: logLevel info by default from global" { + cd `chart_dir` + local cmd=$(helm template \ + -s templates/controller-deployment.yaml \ + --set 'controller.enabled=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) + + local actual=$(echo "$cmd" | + yq 'any(contains("-log-level=info"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "controller/Deployment: logLevel can be overridden" { + cd `chart_dir` + local cmd=$(helm template \ + -s templates/controller-deployment.yaml \ + --set 'controller.enabled=true' \ + --set 'controller.logLevel=error' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) + + local actual=$(echo "$cmd" | + yq 'any(contains("-log-level=error"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + diff --git a/test/unit/ingress-gateways-deployment.bats b/test/unit/ingress-gateways-deployment.bats index eb2c123b0..0ccd5e653 100644 --- a/test/unit/ingress-gateways-deployment.bats +++ b/test/unit/ingress-gateways-deployment.bats @@ -1173,6 +1173,8 @@ key2: value2' \ yq -s -r '.[0].spec.template.spec.initContainers | map(select(.name == "service-init"))[0] | .command[2]' | tee /dev/stderr) exp='consul-k8s service-address \ + -log-level=info \ + -log-json=false \ -k8s-namespace=default \ -name=RELEASE-NAME-consul-ingress-gateway \ -output-file=/tmp/address.txt @@ -1239,6 +1241,8 @@ EOF -token-sink-file=/consul/service/acl-token consul-k8s service-address \ + -log-level=info \ + -log-json=false \ -k8s-namespace=default \ -name=RELEASE-NAME-consul-ingress-gateway \ -output-file=/tmp/address.txt diff --git a/test/unit/mesh-gateway-deployment.bats b/test/unit/mesh-gateway-deployment.bats index 70001f02e..fdae83388 100755 --- a/test/unit/mesh-gateway-deployment.bats +++ b/test/unit/mesh-gateway-deployment.bats @@ -816,6 +816,8 @@ key2: value2' \ yq -r '.spec.template.spec.initContainers | map(select(.name == "service-init"))[0] | .command[2]' | tee /dev/stderr) exp='consul-k8s service-address \ + -log-level=info \ + -log-json=false \ -k8s-namespace=default \ -name=RELEASE-NAME-consul-mesh-gateway \ -output-file=/tmp/address.txt @@ -871,6 +873,8 @@ EOF -token-sink-file=/consul/service/acl-token consul-k8s service-address \ + -log-level=info \ + -log-json=false \ -k8s-namespace=default \ -name=RELEASE-NAME-consul-mesh-gateway \ -output-file=/tmp/address.txt @@ -923,6 +927,8 @@ EOF yq -r '.spec.template.spec.initContainers | map(select(.name == "service-init"))[0] | .command[2]' | tee /dev/stderr) exp='consul-k8s service-address \ + -log-level=info \ + -log-json=false \ -k8s-namespace=default \ -name=RELEASE-NAME-consul-mesh-gateway \ -output-file=/tmp/address.txt @@ -1200,6 +1206,8 @@ EOF yq -r '.spec.template.spec.initContainers | map(select(.name == "service-init"))[0] | .command[2]' | tee /dev/stderr) exp='consul-k8s service-address \ + -log-level=info \ + -log-json=false \ -k8s-namespace=default \ -name=RELEASE-NAME-consul-mesh-gateway \ -output-file=/tmp/address.txt @@ -1317,6 +1325,8 @@ EOF yq -r '.spec.template.spec.initContainers | map(select(.name == "service-init"))[0] | .command[2]' | tee /dev/stderr) exp='consul-k8s service-address \ + -log-level=info \ + -log-json=false \ -k8s-namespace=default \ -name=RELEASE-NAME-consul-mesh-gateway \ -output-file=/tmp/address.txt @@ -1367,6 +1377,8 @@ EOF yq -r '.spec.template.spec.initContainers | map(select(.name == "service-init"))[0] | .command[2]' | tee /dev/stderr) exp='consul-k8s service-address \ + -log-level=info \ + -log-json=false \ -k8s-namespace=default \ -name=RELEASE-NAME-consul-mesh-gateway \ -output-file=/tmp/address.txt diff --git a/test/unit/server-acl-init-cleanup-job.bats b/test/unit/server-acl-init-cleanup-job.bats index 0e61a5149..8b885ba03 100644 --- a/test/unit/server-acl-init-cleanup-job.bats +++ b/test/unit/server-acl-init-cleanup-job.bats @@ -54,8 +54,8 @@ load _helpers -s templates/server-acl-init-cleanup-job.yaml \ --set 'global.acls.manageSystemACLs=true' \ . | tee /dev/stderr | - yq -c '.spec.template.spec.containers[0].args' | tee /dev/stderr) - [ "${actual}" = '["delete-completed-job","-k8s-namespace=default","RELEASE-NAME-consul-server-acl-init"]' ] + yq -c '.spec.template.spec.containers[0].args' | tee /dev/stderr) + [ "${actual}" = '["delete-completed-job","-log-level=info","-log-json=false","-k8s-namespace=default","RELEASE-NAME-consul-server-acl-init"]' ] } @test "serverACLInitCleanup/Job: enabled with externalServers.enabled=true and global.acls.manageSystemACLs=true, but server.enabled set to false" { diff --git a/test/unit/sync-catalog-deployment.bats b/test/unit/sync-catalog-deployment.bats index c87aeeb47..79da757a4 100755 --- a/test/unit/sync-catalog-deployment.bats +++ b/test/unit/sync-catalog-deployment.bats @@ -947,3 +947,34 @@ load _helpers [ "${actual}" = "bar" ] } + +#-------------------------------------------------------------------- +# logLevel + +@test "syncCatalog/Deployment: logLevel info by default from global" { + cd `chart_dir` + local cmd=$(helm template \ + -s templates/sync-catalog-deployment.yaml \ + --set 'syncCatalog.enabled=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) + + local actual=$(echo "$cmd" | + yq 'any(contains("-log-level=info"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "syncCatalog/Deployment: logLevel can be overridden" { + cd `chart_dir` + local cmd=$(helm template \ + -s templates/sync-catalog-deployment.yaml \ + --set 'syncCatalog.enabled=true' \ + --set 'syncCatalog.logLevel=debug' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) + + local actual=$(echo "$cmd" | + yq 'any(contains("-log-level=debug"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + diff --git a/values.yaml b/values.yaml index efe8f9dbe..3d9bfa8a0 100644 --- a/values.yaml +++ b/values.yaml @@ -9,6 +9,16 @@ global: # setting `server.enabled` to true. enabled: true + # The default log level to apply to all components which do not otherwise override this setting. + # It is recommended to generally not set this below "info" unless actively debugging due to logging verbosity. + # One of "debug", "info", "warn", or "error". + # @type: string + logLevel: "info" + + # Enable all component logs to be output in JSON format. + # @type: boolean + logJSON: false + # Set the prefix used for all resources in the Helm chart. If not set, # the prefix will be `-consul`. # @type: string @@ -1330,8 +1340,9 @@ syncCatalog: memory: "50Mi" cpu: "50m" - # Log verbosity level. One of "trace", "debug", "info", "warn", or "error". - logLevel: info + # Override global log verbosity level. One of "debug", "info", "warn", or "error". + # @type: string + logLevel: "" # Override the default interval to perform syncing operations creating Consul services. # @type: string @@ -1435,8 +1446,9 @@ connectInject: # @type: string imageConsul: null - # Log verbosity level. One of "debug", "info", "warn", or "error". - logLevel: info + # Override global log verbosity level. One of "debug", "info", "warn", or "error". + # @type: string + logLevel: "" serviceAccount: # This value defines additional annotations for the injector service account. This should be formatted as a @@ -1635,7 +1647,8 @@ controller: replicas: 1 # Log verbosity level. One of "debug", "info", "warn", or "error". - logLevel: info + # @type: string + logLevel: "" serviceAccount: # This value defines additional annotations for the controller service account. This should be formatted as a