New logging configuration (#113)

* Do not add logtostderr if omitted

* Add log configuration

* Bump chart version

* Add log config as secret, use log-config-file

* Remove templated yaml from repo, add values examples

* Fix template extension

* Add gitignore for goland, tests for new configuration api: enable, override, disable + use --logtostderr

* Enable Parallel() tests back

Author: sergeyshaykhullin

.gitignore

@@ -0,0 +1 @@
+.idea

cockroachdb/Chart.yaml

@@ -1,7 +1,7 @@
 apiVersion: v1
 name: cockroachdb
 home: https://www.cockroachlabs.com
-version: 6.1.1
+version: 6.1.2
 appVersion: 21.1.7
 description: CockroachDB is a scalable, survivable, strongly-consistent SQL database.
 icon: https://raw.githubusercontent.com/cockroachdb/cockroach/master/docs/media/cockroach_db.png

cockroachdb/templated.yaml

@@ -0,0 +1,19 @@
+{{- if .Values.conf.log.enabled }}
+kind: Secret
+apiVersion: v1
+metadata:
+  name: {{ template "cockroachdb.fullname" . }}-log-config
+  namespace: {{ .Release.Namespace | quote }}
+  labels:
+    helm.sh/chart: {{ template "cockroachdb.chart" . }}
+    app.kubernetes.io/name: {{ template "cockroachdb.name" . }}
+    app.kubernetes.io/instance: {{ .Release.Name | quote }}
+    app.kubernetes.io/managed-by: {{ .Release.Service | quote }}
+  {{- with .Values.labels }}
+    {{- toYaml . | nindent 4 }}
+  {{- end }}
+type: Opaque
+stringData:
+  log-config.yaml: |
+    {{- toYaml .Values.conf.log.config | nindent 4 }}
+{{- end }}

cockroachdb/templates/secret.logconfig.yaml

@@ -184,7 +184,6 @@ spec:
             {{- end }}
             {{- end }}
               --advertise-host=$(hostname).${STATEFULSET_FQDN}
-              --logtostderr={{ .Values.conf.logtostderr }}
             {{- if .Values.tls.enabled }}
               --certs-dir=/cockroach/cockroach-certs/
             {{- else }}
@@ -212,6 +211,11 @@ spec:
             {{- if .Values.conf.store.enabled }}
               --store={{ template "cockroachdb.conf.store" . }}
             {{- end }}
+            {{- if .Values.conf.log.enabled }}
+              --log-config-file=/cockroach/log-config/log-config.yaml
+            {{- else }}
+              --logtostderr={{ .Values.conf.logtostderr }}
+            {{- end }}
             {{- range .Values.statefulset.args }}
               {{ . }}
             {{- end }}
@@ -248,6 +252,11 @@ spec:
               mountPath: {{ printf "/etc/cockroach/secrets/%s" . | quote }}
               readOnly: true
           {{- end }}
+          {{- if .Values.conf.log.enabled }}
+            - name: log-config
+              mountPath: /cockroach/log-config
+              readOnly: true
+          {{- end }}
           livenessProbe:
           {{- if .Values.statefulset.customLivenessProbe }}
             {{ toYaml .Values.statefulset.customLivenessProbe | nindent 12 }}
@@ -325,6 +334,11 @@ spec:
           secret:
             secretName: {{ . | quote }}
       {{- end }}
+      {{- if .Values.conf.log.enabled }}
+        - name: log-config
+          secret:
+            secretName: {{ template "cockroachdb.fullname" . }}-log-config
+      {{- end }}
 {{- if .Values.storage.persistentVolume.enabled }}
   volumeClaimTemplates:
     - metadata:

cockroachdb/templates/statefulset.yaml

@@ -59,7 +59,20 @@ conf:
   # new Pods.
   join: []
 
-  # Logs at or above this threshold to STDERR.
+  # New logging configuration.
+  log:
+    enabled: false
+    # https://www.cockroachlabs.com/docs/v21.1/configure-logs
+    config: {}
+      # file-defaults:
+      #   dir: /custom/dir/path/
+      # fluent-defaults:
+      #   format: json-fluent
+      # sinks:
+      #   stderr:
+      #     channels: [DEV]
+
+  # Logs at or above this threshold to STDERR. Ignored when "log" is enabled
   logtostderr: INFO
 
   # Maximum storage capacity available to store temporary disk-based data for

cockroachdb/values.yaml

@@ -500,3 +500,99 @@ func TestSelfSignerHelmValidation(t *testing.T) {
 		})
 	}
 }
+
+// TestHelmLogConfigFileStatefulSet contains the tests around the new logging configuration
+func TestHelmLogConfigFileStatefulSet(t *testing.T) {
+	t.Parallel()
+
+	var statefulset appsv1.StatefulSet
+	var secret corev1.Secret
+	// Path to the helm chart we will test
+	helmChartPath, err := filepath.Abs("../../cockroachdb")
+	require.NoError(t, err)
+
+	testCases := []struct {
+		name   string
+		values map[string]string
+		expect struct {
+			statefulsetArgument string
+			logConfig           string
+			secretExists        bool
+		}
+	}{
+		{
+			"New logging configuration enabled",
+			map[string]string{"conf.log.enabled": "true"},
+			struct {
+				statefulsetArgument string
+				logConfig           string
+				secretExists        bool
+			} {
+				"--log-config-file=/cockroach/log-config/log-config.yaml",
+				"{}",
+				true,
+			},
+		},
+		{
+			"New logging configuration overridden",
+			map[string]string{
+				"conf.log.enabled": "true",
+				"conf.log.config": "file-defaults:\ndir: /custom/dir/path/",
+			},
+			struct {
+				statefulsetArgument string
+				logConfig           string
+				secretExists        bool
+			} {
+				"--log-config-file=/cockroach/log-config/log-config.yaml",
+				"file-defaults:\n  dir: /custom/dir/path/",
+				true,
+			},
+		},
+		{
+			"New logging configuration disabled",
+			map[string]string{
+				"conf.log.enabled": "false",
+				"conf.logtostderr": "INFO",
+			},
+			struct {
+				statefulsetArgument string
+				logConfig           string
+				secretExists        bool
+			} {
+				"--logtostderr=INFO",
+				"",
+				false,
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		// Here, we capture the range variable and force it into the scope of this block. If we don't do this, when the
+		// subtest switches contexts (because of t.Parallel), the testCase value will have been updated by the for loop
+		// and will be the next testCase!
+		testCase := testCase
+		t.Run(testCase.name, func(subT *testing.T) {
+			subT.Parallel()
+
+			// Now we try rendering the template, but verify we get an error
+			options := &helm.Options{
+				KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName),
+				SetValues:      testCase.values,
+			}
+			output := helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/statefulset.yaml"})
+
+			helm.UnmarshalK8SYaml(t, output, &statefulset)
+			require.Contains(t, statefulset.Spec.Template.Spec.Containers[0].Args[2], testCase.expect.statefulsetArgument)
+
+			output, err = helm.RenderTemplateE(t, options, helmChartPath, releaseName, []string{"templates/secret.logconfig.yaml"})
+
+			require.Equal(subT, testCase.expect.secretExists, err == nil)
+
+			if testCase.expect.secretExists {
+				helm.UnmarshalK8SYaml(t, output, &secret)
+				require.Contains(subT, secret.StringData["log-config.yaml"], testCase.expect.logConfig)
+			}
+		})
+	}
+}