Conversation
There was a problem hiding this comment.
Pull request overview
Adds repo-host (pgBackRest) PVC autoscaling support to the operator, mirroring the existing pgdata autogrow behavior by having the repo-host Pod suggest a larger size via annotations and having the controller apply the resized PVC request while persisting the latest suggestion in status.
Changes:
- Add repo-host monitoring script that periodically computes and PATCHes
suggested-pgbackrest-<repo>-pvc-sizeannotations when volume usage is high (feature-gated). - Teach the PostgresCluster controller to observe suggested sizes from repo-host Pods, cap them by configured limits, apply updated PVC requests, and store the latest suggestion in
status.pgbackrest.repos[].desiredRepoVolume. - Extend repo-host RBAC and add unit + KUTTL E2E coverage for the new behavior.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/apis/upstream.pgv2.percona.com/v1beta1/pgbackrest_types.go | Adds DesiredRepoVolume to repo status to persist observed suggestions. |
| internal/pgbackrest/reconcile.go | Threads autogrow-enabled repo names into pgBackRest config container script generation. |
| internal/pgbackrest/reconcile_test.go | Unit test ensuring autogrow monitoring is only injected for eligible repos and only on repo-host. |
| internal/pgbackrest/rbac.go | Grants repo-host ServiceAccount patch on pods to allow annotation updates. |
| internal/pgbackrest/rbac_test.go | Updates RBAC test expectations for added patch verb. |
| internal/pgbackrest/config.go | Injects monitor_volume function + per-repo calls into reload loop and PATCHes pod annotations. |
| internal/pgbackrest/config_test.go | Expands shellcheck coverage for reload script with/without autogrow repos. |
| internal/naming/annotations.go | Adds helpers for suggested-size annotation naming and parsing. |
| internal/controller/postgrescluster/watches.go | Triggers reconcile when suggested repo-host volume annotations change. |
| internal/controller/postgrescluster/watches_test.go | Verifies the new watch behavior enqueues reconciliation on suggestion changes. |
| internal/controller/postgrescluster/pgbackrest.go | Observes desired repo volumes from Pods/status, applies capped PVC request changes, persists desired size in status. |
| internal/controller/postgrescluster/pgbackrest_test.go | Adds targeted unit coverage for setRepoVolumeSize and repo-host SA/mount behavior. |
| e2e-tests/tests/repo-host-autogrow/00-deploy-operator.yaml | Enables AutoGrowVolumes in E2E deployment. |
| e2e-tests/tests/repo-host-autogrow/00-assert.yaml | Asserts operator is running before test proceeds. |
| e2e-tests/tests/repo-host-autogrow/01-create-cluster.yaml | Creates cluster with repo PVC request+limit configured to exercise autogrow. |
| e2e-tests/tests/repo-host-autogrow/01-assert.yaml | Waits for cluster + repo-host StatefulSet readiness. |
| e2e-tests/tests/repo-host-autogrow/02-grow-repo-volume.yaml | Fills repo volume to exceed threshold and trigger suggestion logic. |
| e2e-tests/tests/repo-host-autogrow/02-assert.yaml | Asserts suggestion annotation appears and is above expected floor; cleans up fill file. |
| e2e-tests/tests/repo-host-autogrow/03-assert.yaml | Asserts controller persists suggestion into PostgresCluster status and PVC request is updated. |
| e2e-tests/tests/repo-host-autogrow/99-remove-cluster-gracefully.yaml | Cleans up cluster/operator and checks for panics. |
| e2e-tests/run-release.csv | Adds the new E2E test to release runs. |
| e2e-tests/run-pr.csv | Adds the new E2E test to PR runs. |
| deploy/cw-bundle.yaml | CRD bundle regeneration for new status field. |
| deploy/crd.yaml | CRD regeneration for new status field. |
| deploy/bundle.yaml | CRD bundle regeneration for new status field. |
| config/crd/bases/upstream.pgv2.percona.com_postgresclusters.yaml | Base CRD updated with new status field schema. |
| config/crd/bases/pgv2.percona.com_perconapgclusters.yaml | Base CRD updated with new status field schema. |
| build/crd/percona/generated/pgv2.percona.com_perconapgclusters.yaml | Generated CRD updated with new status field schema. |
| ) (map[string]string, error) { | ||
| desired := make(map[string]string) | ||
| if !feature.Enabled(ctx, feature.AutoGrowVolumes) { | ||
| return desired, nil |
There was a problem hiding this comment.
what happens if the flag is disabled after a resize? It looks like we return nil here and later in setRepoVolumeSize we read the size from the spec (not actual pvc size), won't that cause a downscale and possibly break reconciliation?
oksana-grishchenko
left a comment
There was a problem hiding this comment.
Agree with Mayank's concern about disabling feature gate after resizing, that is a possible real life scenario, please consider it.
Other than that LGTM
commit: eed13b0 |
DESCRIPTION
This PR adds autoscaling for pgbackrest PVCs. It follows the pgdata PVC autoscaling logic.
When the
AutoGrowVolumesfeature gate is enabled andspec.backups.pgbackrest.repos[].volume.volumeClaimSpec.resources.limits.storageis configured:pgbackrest-configcontainer checks disk usage every five seconds.suggested-pgbackrest-<repo-name>-pvc-sizeresources.requests.storageto the suggested sizestatus.pgbackrest.repos[].desiredRepoVolumeCHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability