DX-96480: resolve logging bug

This is a port from https://dremio.atlassian.net/browse/DX-96405 Change-Id: If0df04b715d85d22af9811838347a6175525df5d
dremio · Oct 25, 2024 · ae5cad9 · ae5cad9
1 parent 24a12d7
commit ae5cad9
Show file tree

Hide file tree

Showing 10 changed files with 186 additions and 19 deletions.
diff --git a/charts/dremio_v2/docs/setup/Writing-Logs-To-A-File.md b/charts/dremio_v2/docs/setup/Writing-Logs-To-A-File.md
@@ -1,7 +1,6 @@
 # Writing Logs to a File
 
-By default, logs are written to `stdout`. Logs can also be written to `/opt/dremio/log` by enabling `writeLogsToFile`.
-For more configuration information, see the [Values Reference](./Writing-Logs-To-A-File.md) documentation.
+By default, logs are written to `stdout`. Logs can also be written to `/opt/dremio/log` by enabling `writeLogsToFile`. For more configuration information, see the [Values Reference](./Writing-Logs-To-A-File.md) documentation.
 
 ***Important Note:*** Upgrading an existing Dremio deployment can potentially be a destructive change since enabling this feature
 requires an uninstall/install instead of a typical `helm upgrade`. Prior to enabling this feature, switch the cluster into

diff --git a/charts/dremio_v2/templates/_helpers_coordinator.tpl b/charts/dremio_v2/templates/_helpers_coordinator.tpl
@@ -19,6 +19,7 @@ Coordinator - Dremio Heap Memory allocation
 {{- sub $coordinatorMemory 2048}}
 {{- end -}}
 {{- end -}}
+
 {{/*
 Coordiantor - Dremio Direct Memory Allocation
 */}}
@@ -90,18 +91,23 @@ Coordinator - Container Extra Environment Variables
 Coordinator - Log Path
 */}}
 {{- define "dremio.coordinator.log.path" -}}
-{{- $writeLogsToFile := coalesce $.Values.coordinator.writeLogsToFile $.Values.writeLogsToFile -}}
+{{- $writeLogsToFile := include "dremio.booleanCoalesce" (list $.Values.coordinator.writeLogsToFile $.Values.writeLogsToFile nil) -}}
 {{- if $writeLogsToFile -}}
+- name: DREMIO_LOG_TO_CONSOLE
+  value: "0"
 - name: DREMIO_LOG_DIR
   value: /opt/dremio/log
+{{- else -}}
+- name: DREMIO_LOG_TO_CONSOLE
+  value: "1"
 {{- end -}}
 {{- end -}}
 
 {{/*
 Coordinator - Log Volume Mount
 */}}
 {{- define "dremio.coordinator.log.volumeMount" -}}
-{{- $writeLogsToFile := coalesce $.Values.coordinator.writeLogsToFile $.Values.writeLogsToFile -}}
+{{- $writeLogsToFile := include "dremio.booleanCoalesce" (list $.Values.coordinator.writeLogsToFile $.Values.writeLogsToFile nil) -}}
 {{- if $writeLogsToFile -}}
 - name: dremio-log-volume
   mountPath: /opt/dremio/log
@@ -113,7 +119,7 @@ Coordinator - Logs Volume Claim Template
 */}}
 {{- define "dremio.coordinator.log.volumeClaimTemplate" -}}
 {{- $coordinatorLogStorageClass := coalesce $.Values.coordinator.logStorageClass $.Values.logStorageClass -}}
-{{- $writeLogsToFile := coalesce $.Values.coordinator.writeLogsToFile $.Values.writeLogsToFile -}}
+{{- $writeLogsToFile := include "dremio.booleanCoalesce" (list $.Values.coordinator.writeLogsToFile $.Values.writeLogsToFile nil) -}}
 {{- if $writeLogsToFile -}}
 - metadata:
     name: dremio-log-volume
@@ -228,4 +234,4 @@ Coordinator - Pod Tolerations
 tolerations:
   {{- toYaml $coordinatorTolerations | nindent 2 }}
 {{- end -}}
-{{- end -}}
+{{- end -}}
diff --git a/charts/dremio_v2/templates/_helpers_executor.tpl b/charts/dremio_v2/templates/_helpers_executor.tpl
@@ -186,10 +186,15 @@ Executor - Log Path
 {{- $context := index . 0 -}}
 {{- $engineName := index . 1 -}}
 {{- $engineConfiguration := default (dict) (get (default (dict) $context.Values.executor.engineOverride) $engineName) -}}
-{{- $writeLogsToFile := coalesce $engineConfiguration.writeLogsToFile $context.Values.executor.writeLogsToFile $context.Values.writeLogsToFile -}}
+{{- $writeLogsToFile := include "dremio.booleanCoalesce" (list $engineConfiguration.writeLogsToFile $context.Values.executor.writeLogsToFile $context.Values.writeLogsToFile nil) -}}
 {{- if $writeLogsToFile -}}
+- name: DREMIO_LOG_TO_CONSOLE
+  value: "0"
 - name: DREMIO_LOG_DIR
   value: /opt/dremio/log
+{{- else -}}
+- name: DREMIO_LOG_TO_CONSOLE
+  value: "1"
 {{- end -}}
 {{- end -}}
 
@@ -200,7 +205,7 @@ Executor - Log Volume Mount
 {{- $context := index . 0 -}}
 {{- $engineName := index . 1 -}}
 {{- $engineConfiguration := default (dict) (get (default (dict) $context.Values.executor.engineOverride) $engineName) -}}
-{{- $writeLogsToFile := coalesce $engineConfiguration.writeLogsToFile $context.Values.executor.writeLogsToFile $context.Values.writeLogsToFile -}}
+{{- $writeLogsToFile := include "dremio.booleanCoalesce" (list $engineConfiguration.writeLogsToFile $context.Values.executor.writeLogsToFile $context.Values.writeLogsToFile nil) -}}
 {{- if $writeLogsToFile -}}
 - name: dremio-log-volume
   mountPath: /opt/dremio/log
@@ -214,7 +219,7 @@ Executor - Logs Volume Claim Template
 {{- $context := index . 0 -}}
 {{- $engineName := index . 1 -}}
 {{- $engineConfiguration := default (dict) (get (default (dict) $context.Values.executor.engineOverride) $engineName) -}}
-{{- $writeLogsToFile := coalesce $engineConfiguration.writeLogsToFile $context.Values.executor.writeLogsToFile $context.Values.writeLogsToFile -}}
+{{- $writeLogsToFile := include "dremio.booleanCoalesce" (list $engineConfiguration.writeLogsToFile $context.Values.executor.writeLogsToFile $context.Values.writeLogsToFile nil) -}}
 {{- $volumeSize := coalesce $engineConfiguration.volumeSize $context.Values.executor.volumeSize $context.Values.volumeSize -}}
 {{- if $writeLogsToFile -}}
 - metadata:

diff --git a/charts/dremio_v2/templates/_helpers_general.tpl b/charts/dremio_v2/templates/_helpers_general.tpl
@@ -121,3 +121,20 @@ Admin - Service Account
 serviceAccount: {{ $adminServiceAccount }}
 {{- end -}}
 {{- end -}}
+
+{{/*
+This helper function is used to coalesce a list of boolean values using "trilean" logic,
+i.e., returning the first non-nil value found, even if it is false.
+If a non-nil value is found and it is true, the function returns "1"; otherwise the function returns an empty string.
+This function is suitable for use in lieu of the coalesce function, which has surprising effects
+when used with boolean values. This function should not be used with non-boolean values.
+*/}}
+{{- define "dremio.booleanCoalesce" -}}
+{{- $found := false -}}
+{{- range $value := . -}}
+{{- if and (not $found) (ne $value nil) -}}
+{{- $found = true -}}
+{{- if $value -}}1{{- end -}}
+{{- end -}}
+{{- end -}}
+{{- end -}}
diff --git a/charts/dremio_v2/templates/dremio-coordinator.yaml b/charts/dremio_v2/templates/dremio-coordinator.yaml
@@ -73,8 +73,6 @@ spec:
           value: "/opt/dremio/aws/credentials"
         - name: AWS_SHARED_CREDENTIALS_FILE
           value: "/opt/dremio/aws/credentials"
-        - name: DREMIO_LOG_TO_CONSOLE
-          value: "true"
         {{- include "dremio.coordinator.log.path" $ | nindent 8 }}
         {{- include "dremio.coordinator.extraEnvs" $ | nindent 8 }}
         command: ["/opt/dremio/bin/dremio"]

diff --git a/charts/dremio_v2/templates/dremio-executor.yaml b/charts/dremio_v2/templates/dremio-executor.yaml
@@ -78,8 +78,6 @@ spec:
           value: "/opt/dremio/aws/credentials"
         - name: AWS_SHARED_CREDENTIALS_FILE
           value: "/opt/dremio/aws/credentials"
-        - name: DREMIO_LOG_TO_CONSOLE
-          value: "true"
         {{- include "dremio.executor.log.path" (list $ $engineName) | nindent 8 }}
         {{- include "dremio.executor.extraEnvs" (list $ $engineName) | nindent 8 }}
         command: ["/opt/dremio/bin/dremio"]

diff --git a/charts/dremio_v2/templates/dremio-master.yaml b/charts/dremio_v2/templates/dremio-master.yaml
@@ -84,8 +84,6 @@ spec:
           value: "/opt/dremio/aws/credentials"
         - name: AWS_SHARED_CREDENTIALS_FILE
           value: "/opt/dremio/aws/credentials"
-        - name: DREMIO_LOG_TO_CONSOLE
-          value: "true"
         {{- include "dremio.coordinator.log.path" $ | nindent 8 }}
         {{- include "dremio.coordinator.extraEnvs" $ | nindent 8 }}
         command: ["/opt/dremio/bin/dremio"]

diff --git a/charts/dremio_v2/tests/dremio-coordinator_test.yaml b/charts/dremio_v2/tests/dremio-coordinator_test.yaml
@@ -82,3 +82,73 @@ tests:
           content:
             name: TEST_DREMIO
             value: test_value
+  - it: should write logs to console by default
+    asserts:
+      - notContains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_TO_CONSOLE
+            value: "0"
+      - notContains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_DIR
+            value: /opt/dremio/log
+  - it: should write logs to file when global writeLogsToFile is true
+    set:
+      writeLogsToFile: true
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_TO_CONSOLE
+            value: "0"
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_DIR
+            value: /opt/dremio/log
+  - it: should not write logs to file when global writeLogsToFile is false
+    set:
+      writeLogsToFile: false
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_TO_CONSOLE
+            value: "1"
+      - notContains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_DIR
+  - it: should not write logs to file when coordinator writeLogsToFile is false
+    set:
+      writeLogsToFile: true
+      coordinator:
+        writeLogsToFile: false
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_TO_CONSOLE
+            value: "1"
+      - notContains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_DIR
+  - it: should write logs to file when coordinator writeLogsToFile is true
+    set:
+      writeLogsToFile: false
+      coordinator:
+        writeLogsToFile: true
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_DIR
+            value: /opt/dremio/log
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_TO_CONSOLE
+            value: "0"
diff --git a/charts/dremio_v2/tests/dremio-executor_test.yaml b/charts/dremio_v2/tests/dremio-executor_test.yaml
@@ -157,3 +157,73 @@ tests:
           content:
             name: TEST_DREMIO
             value: test_value
+  - it: should write logs to console by default
+    asserts:
+      - notContains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_TO_CONSOLE
+            value: "0"
+      - notContains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_DIR
+            value: /opt/dremio/log
+  - it: should write logs to file when global writeLogsToFile is true
+    set:
+      writeLogsToFile: true
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_TO_CONSOLE
+            value: "0"
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_DIR
+            value: /opt/dremio/log
+  - it: should not write logs to file when global writeLogsToFile is false
+    set:
+      writeLogsToFile: false
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_TO_CONSOLE
+            value: "1"
+      - notContains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_DIR
+  - it: should not write logs to file when executor writeLogsToFile is false
+    set:
+      writeLogsToFile: true
+      executor:
+        writeLogsToFile: false
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_TO_CONSOLE
+            value: "1"
+      - notContains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_DIR
+  - it: should write logs to file when executor writeLogsToFile is true
+    set:
+      writeLogsToFile: false
+      executor:
+        writeLogsToFile: true
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_DIR
+            value: /opt/dremio/log
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: DREMIO_LOG_TO_CONSOLE
+            value: "0"
diff --git a/charts/dremio_v2/values.yaml b/charts/dremio_v2/values.yaml
@@ -85,7 +85,9 @@ coordinator:
   # Uncomment this value to use a different storage class for the coordinator logs.
   #logStorageClass:
 
-  # Uncomment this value to write logs to a file on disk.
+  # Whether to write coordinator logs to a file on disk. Disabled by default.
+  # When enabled, log messages are logged to the console and written to a persistent volume mounted at /opt/dremio/log.
+  # When disabled, log messages are logged to the console only.
   #writeLogsToFile: true
 
   # These values, when defined, override the provided shared annotations, labels, node selectors, or tolerations.
@@ -198,10 +200,12 @@ executor:
   # Uncomment this value to use a different storage class for executors.
   #storageClass:
 
-  # Uncomment this value to use a different storage class for the coordinator logs.
+  # Uncomment this value to use a different storage class for the executor logs.
   #logStorageClass:
 
-  # Uncomment this value to write logs to a file on disk.
+  # Whether to write executor logs to a file on disk. Disabled by default.
+  # When enabled, log messages are logged to the console and written to a persistent volume mounted at /opt/dremio/log.
+  # When disabled, log messages are logged to the console only.
   #writeLogsToFile: true
 
   # Dremio C3
@@ -513,7 +517,9 @@ extraVolumeMounts: []
 # Array to add extra environment variables to Dremio's pods
 extraEnvs: []
 
-# Uncomment this value to write logs to a file on disk for all Dremio resources.
+# Whether to write logs to a file on disk for all Dremio resources. Disabled by default.
+# When enabled, log messages are logged to the console and written to a persistent volume mounted at /opt/dremio/log.
+# When disabled, log messages are logged to the console only.
 #writeLogsToFile: true
 
 # Dremio Service