Skip to content

Upgrade PGO to 5.7.0; Add option to disable backups #191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pantierra merged 3 commits into from
Apr 3, 2025
Merged

Upgrade PGO to 5.7.0; Add option to disable backups #191

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44f5ff0
Upgrade PGO to 5.7.0; Add option to disable backups
sunu Mar 24, 2025
bca3622
disable backups by default
sunu Apr 3, 2025
aeb1f43
update PGO version in the docs
sunu Apr 3, 2025
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
6 changes: 4 additions & 2 deletions helm-chart/postgrescluster/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ name: postgrescluster
description: A Helm chart for eoapi database dep for k8s
type: application
# The version below should match the version on the PostgresCluster CRD
version: 5.5.3
appVersion: 5.5.3
# from CrunchyData's Postgres Operator
# https://access.crunchydata.com/documentation/postgres-operator/latest/releases
version: 5.7.0
appVersion: 5.7.0
force: true
2 changes: 2 additions & 0 deletions helm-chart/postgrescluster/templates/postgres.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ spec:
memory: {{ default "" .Values.instanceMemory | quote }}
{{- end }}
{{- end }}
{{- if .Values.backupsEnabled }}
backups:
pgbackrest:
{{- if .Values.imagePgBackRest }}
Expand Down Expand Up @@ -142,6 +143,7 @@ spec:
requests:
storage: {{ default "1Gi" .Values.backupsSize | quote }}
{{- end }}
{{- end }}
{{- if or .Values.pgBouncerReplicas .Values.pgBouncerConfig }}
proxy:
pgBouncer:
Expand Down
7 changes: 7 additions & 0 deletions helm-chart/postgrescluster/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,13 @@ postgresVersion: 16
# Backups / pgBackRest Settings #
#################################

# backupsEnabled sets whether pgBackRest backups are enabled.
# This is enabled by default.
# Disabling backups is only supported on CrunchyData Postgres Operator v5.7.0 and above.
# If enabled, please carefully review the pgBackRest config below.
# Check out CrunchyData's Postgres Operator docs for more info:
# https://access.crunchydata.com/documentation/postgres-operator/v5/tutorials/backups-disaster-recovery/backups
backupsEnabled: true
Copy link
Contributor

@ividito ividito Mar 24, 2025

Choose a reason for hiding this comment

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

Should this default to false until we can recommend a more robust default configuration for pgBackrest?

Copy link
Member Author

@sunu sunu Mar 25, 2025

Choose a reason for hiding this comment

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

@ividito Yeah, that was my first instinct too. But if the default is false, we could break existing setups where someone upgrades eoapi without also updating PGO. The bigger risk is that users with a working backup config might upgrade to the latest chart and miss this new flag -- suddenly their working backup mechanism will be silently ignored.

Copy link
Contributor

@pantierra pantierra Apr 1, 2025

Choose a reason for hiding this comment

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

I think at the moment we can still be a bit aggressive on introducing "breaking changes". I would put is as false and make sure in the next release notes we mention this new flag.

Copy link
Member Author

@sunu sunu Apr 3, 2025

Choose a reason for hiding this comment

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

Alright, happy to make the change if we think breaking changes are ok for now.

Copy link
Member Author

@sunu sunu Apr 3, 2025

Choose a reason for hiding this comment

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

@ividito @pantierra - updated the PR. Can I get another round of review before we merge please?

# backupsSize sets the storage size of the backups to a volume in Kubernetes.
# can be overridden by "pgBackRestConfig", if set. Defaults to the value below.
# backupsSize: 1Gi
Expand Down