Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions charts/cluster/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ refer to the [CloudNativePG Documentation](https://cloudnative-pg.io/documentat
| cluster.monitoring.podMonitor.relabelings | list | `[]` | The list of relabelings for the PodMonitor. Applied to samples before scraping. |
| cluster.monitoring.prometheusRule.enabled | bool | `true` | Whether to enable the PrometheusRule automated alerts |
| cluster.monitoring.prometheusRule.excludeRules | list | `[]` | Exclude specified rules |
| cluster.plugins | list | `[]` | The plugins configuration, containing any plugin to be loaded with the corresponding configuration |
| cluster.postgresGID | int | `-1` | The GID of the postgres user inside the image, defaults to 26 |
| cluster.postgresUID | int | `-1` | The UID of the postgres user inside the image, defaults to 26 |
| cluster.postgresql.ldap | object | `{}` | PostgreSQL LDAP configuration (see https://cloudnative-pg.io/documentation/current/postgresql_conf/#ldap-configuration) |
Expand Down
2 changes: 2 additions & 0 deletions charts/cluster/templates/_backup.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
{{- if .Values.backups.enabled }}
backup:
target: "prefer-standby"
{{ if (eq (include "cluster.useBarmanCloudPlugin" .) "false") }}
retentionPolicy: {{ .Values.backups.retentionPolicy }}
barmanObjectStore:
wal:
Expand All @@ -19,5 +20,6 @@ backup:

{{- $d := dict "chartFullname" (include "cluster.fullname" .) "scope" .Values.backups "secretPrefix" "backup" }}
{{- include "cluster.barmanObjectStoreConfig" $d | nindent 2 }}
{{- end}}
{{- end }}
{{- end }}
16 changes: 16 additions & 0 deletions charts/cluster/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,19 @@ Postgres GID
{{- 26 -}}
{{- end -}}
{{- end -}}


{{/*
Check if barman-cloud plugin exists and is enabled
*/}}
{{- define "cluster.useBarmanCloudPlugin" -}}
{{- $hasPlugin := false }}
{{- if .Values.cluster.plugins }}
{{- range .Values.cluster.plugins }}
{{- if and (eq .name "barman-cloud.cloudnative-pg.io") .enabled }}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the CRD enabled is set to true by default, thus this may be changed to something like this:

Suggested change
{{- if and (eq .name "barman-cloud.cloudnative-pg.io") .enabled }}
{{- if and (eq .name "barman-cloud.cloudnative-pg.io") (ne .enabled false) }}

Furthermore I guess this should also check if isWALArchiver is true (but I am not sure):

Suggested change
{{- if and (eq .name "barman-cloud.cloudnative-pg.io") .enabled }}
{{- if and (eq .name "barman-cloud.cloudnative-pg.io") (ne .enabled false) .isWALArchiver }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending whether the first or no suggestion gets applied, the suggestion in #680 (comment) needs to get adapted (twice; in both nested template definitions).

{{- $hasPlugin = true }}
{{- end }}
{{- end }}
{{- end }}
{{- $hasPlugin }}
{{- end }}
Comment on lines +149 to +162
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the suggestion with the ObjectStore name will be accepted the following could be helpful:

Suggested change
{{/*
Check if barman-cloud plugin exists and is enabled
*/}}
{{- define "cluster.useBarmanCloudPlugin" -}}
{{- $hasPlugin := false }}
{{- if .Values.cluster.plugins }}
{{- range .Values.cluster.plugins }}
{{- if and (eq .name "barman-cloud.cloudnative-pg.io") .enabled }}
{{- $hasPlugin = true }}
{{- end }}
{{- end }}
{{- end }}
{{- $hasPlugin }}
{{- end }}
{{/*
Patch the plugins list to contain auto generated information and return it formatted as JSON
*/}}
{{- define "cluster.patchPlugins" -}}
{{- $plugins := list }}
{{- range default list (deepCopy .Values.cluster.plugins) }}
{{- if and (eq .name "barman-cloud.cloudnative-pg.io") (ne .enabled false) .isWALArchiver }}
{{- $_ := set . "parameters" (merge (default dict .parameters) (dict "barmanObjectName" (printf "%s-object-store" (include "cluster.fullname" .)))) }}
{{- end }}
{{- $plugins = append $plugins . }}
{{- end }}
{{- toJson $plugins }}
{{- end }}
{{/*
Check if barman-cloud plugin exists, is enabled, is WAL archiver, and return the plugin parameters (or null) formatted as JSON
*/}}
{{- define "cluster.findBarmanCloudPlugin" -}}
{{- range include "cluster.patchPlugins" . | fromJsonArray }}
{{- if and (eq .name "barman-cloud.cloudnative-pg.io") (ne .enabled false) .isWALArchiver }}
{{- toJson .parameters }}
{{- break }}
{{- end }}
{{- end }}
{{- end }}

Usage would then be as follows:

{{ if include "cluster.findBarmanCloudPlugin" . | fromJson }}

or:

{{ include "cluster.findBarmanCloudPlugin" . | fromJson | .barmanObjectName | quote }}

4 changes: 4 additions & 0 deletions charts/cluster/templates/cluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ spec:
name: {{ . }}
{{ end }}
enablePDB: {{ .Values.cluster.enablePDB }}
{{- with .Values.cluster.plugins }}
plugins:
{{- toYaml . | nindent 4}}
{{- end }}
Comment on lines +70 to +73
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the suggestion with the ObjectStore name and the corresponding change within the _helpers.tpl will be accepted this must be changed to:

Suggested change
{{- with .Values.cluster.plugins }}
plugins:
{{- toYaml . | nindent 4}}
{{- end }}
plugins: {{ include "cluster.patchPlugins" . }}

postgresql:
{{- if or (eq .Values.type "timescaledb") (not (empty .Values.cluster.postgresql.shared_preload_libraries)) }}
shared_preload_libraries:
Expand Down
22 changes: 22 additions & 0 deletions charts/cluster/templates/objectStore.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{{ if and (eq (include "cluster.useBarmanCloudPlugin" .) "true") .Values.backups.enabled }}
apiVersion: barmancloud.cnpg.io/v1
kind: ObjectStore
metadata:
name: {{ include "cluster.fullname" $ }}-object-store
Comment thread
pha91 marked this conversation as resolved.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is currently "hardcoded" but must also be specified within the plugin declaration. This creates the problem, that the user must know/predict the name to be used and if that ever changes (either by changing what cluster.fullname renders, the suffix/prefix, the "full" name), one must adapt their values even though they should not need to. Within the _helpers.tpl and cluster.yaml are suggestions which would fix this and set a default barmanObjectName plugin parameter if non is given. If this change is accepted, the name should be change to:

Suggested change
name: {{ include "cluster.fullname" $ }}-object-store
name: {{ include "cluster.findBarmanCloudPlugin" . | fromJson | .barmanObjectName | quote }}

Possibly save the output of include "cluster.findBarmanCloudPlugin" . | fromJson in a variable so that it must not be computed twice (for the name and the if in line one).

spec:
configuration:
wal:
compression: {{ .Values.backups.wal.compression }}
{{- if .Values.backups.wal.encryption }}
encryption: {{ .Values.backups.wal.encryption }}
{{- end }}
maxParallel: {{ .Values.backups.wal.maxParallel }}
data:
compression: {{ .Values.backups.data.compression }}
{{- if .Values.backups.data.encryption }}
encryption: {{ .Values.backups.data.encryption }}
{{- end }}
jobs: {{ .Values.backups.data.jobs }}
{{- $d := dict "chartFullname" (include "cluster.fullname" .) "scope" .Values.backups "secretPrefix" "backup" -}}
{{- include "cluster.barmanObjectStoreConfig" $d | indent 2 }}
{{- end }}
8 changes: 7 additions & 1 deletion charts/cluster/templates/scheduled-backups.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,15 @@ metadata:
spec:
immediate: true
schedule: {{ .schedule | quote }}
method: {{ .method }}
backupOwnerReference: {{ .backupOwnerReference }}
cluster:
name: {{ include "cluster.fullname" $context }}
{{- if (eq (include "cluster.useBarmanCloudPlugin" $context ) "true") }}
pluginConfiguration:
name: barman-cloud.cloudnative-pg.io
method: plugin
{{ else }}
method: {{ .method }}
{{- end }}
Comment on lines +17 to +23
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this should be "hardcoded" in case the barman cloud plugin is enabled. In case it is possible to still use volumeSnapshot as method even though the barman cloud plugin is enabled (I do not know if this is possible) the following may be better:

Suggested change
{{- if (eq (include "cluster.useBarmanCloudPlugin" $context ) "true") }}
pluginConfiguration:
name: barman-cloud.cloudnative-pg.io
method: plugin
{{ else }}
method: {{ .method }}
{{- end }}
{{- $barmanCloudPluginParameters := include "cluster.findBarmanCloudPlugin" . | fromJson }}
{{- if or (eq .method "plugin") (and (eq .method "") (or .pluginConfiguration $barmanCloudPluginParameters)) }}
method: plugin
{{- $pluginConfiguration := default dict (deepCopy .pluginConfiguration) }}
{{- if $barmanCloudPluginParameters }}
{{- $_ := set $pluginConfiguration "name" (default "barman-cloud.cloudnative-pg.io" $pluginConfiguration.name) }}
{{- end }}
pluginConfiguration: {{ toJson $pluginConfiguration }}
{{ else }}
method: {{ default "barmanObjectStore" .method | quote }}
{{- end }}

{{ end -}}
{{ end }}
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,13 @@ spec:
memory: 256Mi
limits:
cpu: 100m
memory: 256Mi
memory: 256Mi
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trailing spaces after memory: 256Mi

plugins:
- name: cnpg-i-plugin-example.my-org.io
enabled: true
parameters:
key1: value1
key2: value2
priorityClassName: mega-high
primaryUpdateStrategy: supervised
primaryUpdateMethod: restart
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ cluster:
inRoles:
- pg_monitor
- pg_signal_backend
plugins:
- name: cnpg-i-plugin-example.my-org.io
enabled: true
parameters:
key1: value1
key2: value2
postgresql:
ldap:
server: 'openldap.default.svc.cluster.local'
Expand Down
21 changes: 21 additions & 0 deletions charts/cluster/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,27 @@
}
}
},
"plugins": {
"type": "array",
"items": {
"type": "object",
"properties": {
"name": {
"type": "string"
},
"enabled": {
"type": "boolean"
},
"isWALArchiver": {
"type": "boolean"
},
"parameters": {
"type": "object"
}
},
"required": ["name"]
}
},
"postgresGID": {
"type": "integer"
},
Expand Down
7 changes: 7 additions & 0 deletions charts/cluster/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,13 @@ cluster:
# - name: custom-queries-secret
# key: custom-queries

plugins: []
# - name: cnpg-i-plugin-example.my-org.io
# enabled: true
# parameters:
# key1: value1
# key2: value2

postgresql:
# -- PostgreSQL configuration options (postgresql.conf)
parameters: {}
Expand Down