Skip to content

Commit

Permalink
[CRDB-44993] pvc: support persistent volume for logs
Browse files Browse the repository at this point in the history 
This is a recommended practice in Production as it helps minimize
disk stalls on the data store volumes.
  • Loading branch information
@pritesh-lahoti
pritesh-lahoti committed Nov 28, 2024
1 parent bdc75bf commit ea2a204
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 32 deletions.
25 changes: 23 additions & 2 deletions build/templates/values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,35 @@ conf:
log:
enabled: false
# https://www.cockroachlabs.com/docs/v21.1/configure-logs
config: {}
config:
# file-defaults:
# dir: /custom/dir/path/
# dir: /cockroach/cockroach-logs
# fluent-defaults:
# format: json-fluent
# sinks:
# stderr:
# channels: [DEV]
persistentVolume:
# If enabled, then a PersistentVolumeClaim will be created and
# used to store CockroachDB's logs.
enabled: false
# CockroachDB's logs volume mount path. This gets prepended with
# `/cockroach/` in the stateful set. The `conf.log.config` should have
# `file-defaults.dir` to specify the log path and should reference the
# mounted volume.
path: cockroach-logs
size: 10Gi
# If defined, then `storageClassName: <storageClass>`.
# If set to "-", then `storageClassName: ""`, which disables dynamic
# provisioning.
# If undefined or empty (default), then no `storageClassName` spec is
# set, so the default provisioner will be chosen (gp2 on AWS, standard
# on GKE, AWS & OpenStack).
storageClass: ""
# Additional labels to apply to the created PersistentVolumeClaims.
labels: {}
# Additional annotations to apply to the created PersistentVolumeClaims.
annotations: {}

# Logs at or above this threshold to STDERR. Ignored when "log" is enabled
logtostderr: INFO
Expand Down
14 changes: 14 additions & 0 deletions cockroachdb/templates/_helpers.tpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,3 +289,17 @@ Validate that if user enabled tls, then either self-signed certificates or certi
{{- end }}
{{- end }}
{{- end }}

{{/*
Validate the log configuration.
*/}}
{{- define "cockroachdb.conf.log.validation" -}}
{{- if and (not .Values.conf.log.enabled) .Values.conf.log.persistentVolume.enabled -}}
{{ fail "Persistent volume for logs can only be enabled if logging is enabled" }}
{{- end -}}
{{- if and .Values.conf.log.persistentVolume.enabled (dig "file-defaults" "dir" "" .Values.conf.log.config) -}}
{{- if not (hasPrefix (printf "/cockroach/%s" .Values.conf.log.persistentVolume.path) (dig "file-defaults" "dir" "" .Values.conf.log.config)) }}
{{ fail "Log configuration should use the persistent volume if enabled" }}
{{- end -}}
{{- end -}}
{{- end -}}
48 changes: 46 additions & 2 deletions cockroachdb/templates/statefulset.yaml
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{{ template "cockroachdb.conf.log.validation" . }}
kind: StatefulSet
apiVersion: {{ template "cockroachdb.statefulset.apiVersion" . }}
metadata:
Expand Down Expand Up @@ -290,7 +291,11 @@ spec:
mountPath: /cockroach/log-config
readOnly: true
{{- end }}
{{ with .Values.statefulset.volumeMounts }}
{{- if .Values.conf.log.persistentVolume.enabled }}
- name: logsdir
mountPath: /cockroach/{{ .Values.conf.log.persistentVolume.path }}/
{{- end }}
{{- with .Values.statefulset.volumeMounts }}
{{ toYaml . | nindent 12 }}
{{- end }}
{{- if .Values.statefulset.customStartupProbe }}
Expand Down Expand Up @@ -393,6 +398,15 @@ spec:
secret:
secretName: {{ template "cockroachdb.fullname" . }}-log-config
{{- end }}
{{- if .Values.conf.log.enabled }}
- name: logsdir
{{- if .Values.conf.log.persistentVolume.enabled }}
persistentVolumeClaim:
claimName: logsdir
{{- else }}
emptyDir: {}
{{- end }}
{{- end }}
{{- if eq (include "cockroachdb.securityContext.versionValidation" .) "true" }}
{{- if and .Values.securityContext.enabled }}
securityContext:
Expand All @@ -404,8 +418,9 @@ spec:
runAsNonRoot: true
{{- end }}
{{- end }}
{{- if .Values.storage.persistentVolume.enabled }}
{{- if or .Values.storage.persistentVolume.enabled .Values.conf.log.persistentVolume.enabled }}
volumeClaimTemplates:
{{- if .Values.storage.persistentVolume.enabled }}
- metadata:
name: datadir
labels:
Expand All @@ -432,4 +447,33 @@ spec:
resources:
requests:
storage: {{ .Values.storage.persistentVolume.size | quote }}
{{- end }}
{{- if .Values.conf.log.persistentVolume.enabled }}
- metadata:
name: logsdir
labels:
app.kubernetes.io/name: {{ template "cockroachdb.name" . }}
app.kubernetes.io/instance: {{ .Release.Name | quote }}
{{- with .Values.conf.log.persistentVolume.labels }}
{{- toYaml . | nindent 10 }}
{{- end }}
{{- with .Values.labels }}
{{- toYaml . | nindent 10 }}
{{- end }}
{{- with .Values.conf.log.persistentVolume.annotations }}
annotations: {{- toYaml . | nindent 10 }}
{{- end }}
spec:
accessModes: ["ReadWriteOnce"]
{{- if .Values.conf.log.persistentVolume.storageClass }}
{{- if (eq "-" .Values.conf.log.persistentVolume.storageClass) }}
storageClassName: ""
{{- else }}
storageClassName: {{ .Values.conf.log.persistentVolume.storageClass | quote}}
{{- end }}
{{- end }}
resources:
requests:
storage: {{ .Values.conf.log.persistentVolume.size | quote }}
{{- end }}
{{- end }}
25 changes: 23 additions & 2 deletions cockroachdb/values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,35 @@ conf:
log:
enabled: false
# https://www.cockroachlabs.com/docs/v21.1/configure-logs
config: {}
config:
# file-defaults:
# dir: /custom/dir/path/
# dir: /cockroach/cockroach-logs
# fluent-defaults:
# format: json-fluent
# sinks:
# stderr:
# channels: [DEV]
persistentVolume:
# If enabled, then a PersistentVolumeClaim will be created and
# used to store CockroachDB's logs.
enabled: false
# CockroachDB's logs volume mount path. This gets prepended with
# `/cockroach/` in the stateful set. The `conf.log.config` should have
# `file-defaults.dir` to specify the log path and should reference the
# mounted volume.
path: cockroach-logs
size: 10Gi
# If defined, then `storageClassName: <storageClass>`.
# If set to "-", then `storageClassName: ""`, which disables dynamic
# provisioning.
# If undefined or empty (default), then no `storageClassName` spec is
# set, so the default provisioner will be chosen (gp2 on AWS, standard
# on GKE, AWS & OpenStack).
storageClass: ""
# Additional labels to apply to the created PersistentVolumeClaims.
labels: {}
# Additional annotations to apply to the created PersistentVolumeClaims.
annotations: {}

# Logs at or above this threshold to STDERR. Ignored when "log" is enabled
logtostderr: INFO
Expand Down
110 changes: 84 additions & 26 deletions tests/template/cockroachdb_helm_template_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,42 +496,42 @@ func TestSelfSignerHelmValidation(t *testing.T) {
func TestHelmLogConfigFileStatefulSet(t *testing.T) {
t.Parallel()

type expect struct {
statefulsetArgument string
logConfig string
secretExists bool
renderErr string
persistentVolumeCreated bool
}

testCases := []struct {
name string
values map[string]string
expect struct {
statefulsetArgument string
logConfig string
secretExists bool
}
expect expect
}{
{
"New logging configuration enabled",
map[string]string{"conf.log.enabled": "true"},
struct {
statefulsetArgument string
logConfig string
secretExists bool
}{
expect{
"--log-config-file=/cockroach/log-config/log-config.yaml",
"{}",
"",
true,
"",
false,
},
},
{
"New logging configuration overridden",
map[string]string{
"conf.log.enabled": "true",
"conf.log.config": "file-defaults:\ndir: /custom/dir/path/",
"conf.log.enabled": "true",
"conf.log.config.file-defaults.dir": "/cockroach/cockroach-logs",
},
struct {
statefulsetArgument string
logConfig string
secretExists bool
}{
expect{
"--log-config-file=/cockroach/log-config/log-config.yaml",
"file-defaults:\n dir: /custom/dir/path/",
"file-defaults:\n dir: /cockroach/cockroach-logs",
true,
"",
false,
},
},
{
Expand All @@ -540,14 +540,57 @@ func TestHelmLogConfigFileStatefulSet(t *testing.T) {
"conf.log.enabled": "false",
"conf.logtostderr": "INFO",
},
struct {
statefulsetArgument string
logConfig string
secretExists bool
}{
expect{
"--logtostderr=INFO",
"",
false,
"",
false,
},
},
{
"New logging configuration disabled, but persistent volume enabled",
map[string]string{
"conf.log.enabled": "false",
"conf.logtostderr": "INFO",
"conf.log.persistentVolume.enabled": "true",
},
expect{
"--logtostderr=INFO",
"",
false,
"Persistent volume for logs can only be enabled if logging is enabled",
false,
},
},
{
"New logging configuration not using persistent volume when enabled",
map[string]string{
"conf.log.enabled": "true",
"conf.log.config.file-defaults.dir": "/wrong/path",
"conf.log.persistentVolume.enabled": "true",
},
expect{
"",
"",
false,
"Log configuration should use the persistent volume if enabled",
false,
},
},
{
"New logging configuration using the persistent volume",
map[string]string{
"conf.log.enabled": "true",
"conf.log.config.file-defaults.dir": "/cockroach/cockroach-logs",
"conf.log.persistentVolume.enabled": "true",
},
expect{
"--log-config-file=/cockroach/log-config/log-config.yaml",
"file-defaults:\n dir: /cockroach/cockroach-logs",
true,
"",
true,
},
},
}
Expand All @@ -569,15 +612,30 @@ func TestHelmLogConfigFileStatefulSet(t *testing.T) {
KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName),
SetValues: testCase.values,
}
output := helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/statefulset.yaml"})

output, err := helm.RenderTemplateE(t, options, helmChartPath, releaseName, []string{"templates/statefulset.yaml"})
if err != nil {
require.ErrorContains(subT, err, testCase.expect.renderErr)
return
} else {
require.Empty(subT, testCase.expect.renderErr)
}

helm.UnmarshalK8SYaml(t, output, &statefulset)

require.Equal(subT, namespaceName, statefulset.Namespace)
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"})
if testCase.expect.persistentVolumeCreated {
// Expect 2 persistent volumes: data, logs
require.Equal(subT, 2, len(statefulset.Spec.VolumeClaimTemplates))
require.Equal(subT, "logsdir", statefulset.Spec.VolumeClaimTemplates[1].Name)
} else {
// Expect 1 persistent volume: data
require.Equal(subT, 1, len(statefulset.Spec.VolumeClaimTemplates))
}

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 {
Expand Down

0 comments on commit ea2a204

Please sign in to comment.