feat: expose showAdditionalCommandArgs and listAdditionalCommandArgs#924
Open
davidalmgren95 wants to merge 1 commit into
Open
feat: expose showAdditionalCommandArgs and listAdditionalCommandArgs#924davidalmgren95 wants to merge 1 commit into
davidalmgren95 wants to merge 1 commit into
Conversation
An ObjectStore lets users tack extra command-line flags onto four of the six barman-cloud-* invocations the plugin shells out to: barman-cloud-backup (data.additionalCommandArgs), -wal-archive (wal.archiveAdditionalCommandArgs), -wal-restore (wal.restoreAdditionalCommandArgs), and -restore (data.restoreAdditionalCommandArgs, PR cloudnative-pg#914). The remaining two -- barman-cloud-backup-show (post-write verification) and barman-cloud-backup-list (retention pruning) -- had no equivalent, which is the gap reported in cloudnative-pg#712. On strictly-vhost S3-compatible endpoints users need `--addressing-style=virtual` on every cloud command, and currently those two reject the user-provided args, marking otherwise-successful backups as failed and silently disabling retention pruning. The library-side change adds the two new fields and helpers (sister PR in cloudnative-pg/barman-cloud) and threads them through GetBackupList / GetBackupByName. This plugin commit just exposes them via the CRD and documents the new shape. ## Sister PR (must merge first) The CRD field schema is generated from the BarmanObjectStoreConfiguration Go type in cloudnative-pg/barman-cloud. The sister PR there adds: - DataBackupConfiguration.ShowAdditionalCommandArgs []string - DataBackupConfiguration.ListAdditionalCommandArgs []string - AppendShowAdditionalCommandArgs / AppendListAdditionalCommandArgs helpers Once that PR merges and a barman-cloud release is cut, controller-gen here picks up the new fields automatically. Until then a developer running \`go test ./...\` against this branch needs a local replace directive in go.mod pointing at the barman-cloud branch (omitted from this commit). Closes cloudnative-pg#712 Signed-off-by: David Almgren <dalmgren@coreweave.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
An
ObjectStorealready lets users tack extra command-line flags onto four of the sixbarman-cloud-*invocations the plugin shells out to:barman-cloud-backupdata.additionalCommandArgsbarman-cloud-wal-archivewal.archiveAdditionalCommandArgsbarman-cloud-wal-restorewal.restoreAdditionalCommandArgsbarman-cloud-restoredata.restoreAdditionalCommandArgs(#914)The remaining two —
barman-cloud-backup-show(post-write verification) andbarman-cloud-backup-list(retention pruning) — had no equivalent, which is the gap reported in #712.This is observable in practice on S3-compatible endpoints that strictly require virtual-hosted-style addressing for LIST. On such backends:
barman-cloud-backupaccepts user-provided--addressing-style=virtualvia the existingdata.additionalCommandArgs✅ — base backup completes and lands in the bucketbarman-cloud-backup-showruns immediately afterwards with no way to thread the flag through, sends path-style by default, getsPathStyleRequestNotAllowed, exits with status 4phase: failedbarman-cloud-backup-list) is similarly broken — old backups silently never get expiredSo user-visible Backup CR status is wrong and retention is silently disabled, even though the actual base-backup data is intact in the bucket. The Backup CRs from such clusters look like:
```
NAME AGE METHOD PHASE ERROR
my-cluster-db-backup-20260524000000 2d11h plugin failed rpc error: code = Unknown desc = exit status 4
my-cluster-db-backup-20260525000000 35h plugin failed rpc error: code = Unknown desc = exit status 4
```
with plugin logs showing:
```
{level:info, msg:"Starting barman-cloud-backup", options:[..., "--addressing-style=virtual", ...]}
{level:info, msg:"Completed barman-cloud-backup", ...}
{level:error, logger:"barman", msg:"Can't extract backup id", command:"barman-cloud-backup-show",
options:["--format","json","--endpoint-url",...,"--cloud-provider","aws-s3","s3://bucket/path/","server-name","backup-..."],
stderr:"ERROR: Barman cloud backup show exception: An error occurred (PathStyleRequestNotAllowed)\n"}
```
Note the
--addressing-style=virtualis present on the first invocation but missing on the second.Changes
spec.configuration.data:showAdditionalCommandArgs(passed tobarman-cloud-backup-show)listAdditionalCommandArgs(passed tobarman-cloud-backup-list)config/crd/bases/barmancloud.cnpg.io_objectstores.yamlandmanifest.yamlupdated to include the two new array fieldsweb/docs/misc.mdupdated to list all four args fields, with a note on the cross-cutting--addressing-style=virtualuse caseNo Go code changes are needed in this repo — the actual plumbing lives in the sister PR against cloudnative-pg/barman-cloud#249, which adds the matching Go fields,
Append*helpers, and the call-site changes insideGetBackupByNameandGetBackupList. Once that PR merges and abarman-cloudrelease is cut, bumpinggo.modhere will pick up the new behavior automatically.Sister PR (must merge first)
cloudnative-pg/barman-cloud#249
Closes #712