Skip to content

Design for global volume policies#9901

Merged
reasonerjt merged 1 commit into
velero-io:mainfrom
reasonerjt:bak-vol-policy-design
Jun 17, 2026
Merged

Design for global volume policies#9901
reasonerjt merged 1 commit into
velero-io:mainfrom
reasonerjt:bak-vol-policy-design

Conversation

@reasonerjt

@reasonerjt reasonerjt commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Add the design for global volume policies to address the requirement in #9858

@netlify

netlify Bot commented Jun 10, 2026

Copy link
Copy Markdown

Deploy Preview for velero canceled.

Name Link
🔨 Latest commit 8a31544
🔍 Latest deploy log https://app.netlify.com/projects/velero/deploys/6a30f86e0affb600084ca2fe

@github-actions github-actions Bot added the Area/Design Design Documents label Jun 10, 2026
@github-actions github-actions Bot requested review from blackpiglet and sseago June 10, 2026 11:55
@reasonerjt reasonerjt requested review from Lyndon-Li and removed request for blackpiglet June 10, 2026 11:56
@reasonerjt reasonerjt marked this pull request as draft June 10, 2026 11:56
@reasonerjt reasonerjt added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Jun 10, 2026
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comment thread design/global-backup-volume-policies.md Outdated
blackpiglet
blackpiglet previously approved these changes Jun 11, 2026
@reasonerjt reasonerjt force-pushed the bak-vol-policy-design branch 2 times, most recently from 81781b0 to 98c16cb Compare June 11, 2026 09:54
@reasonerjt reasonerjt marked this pull request as ready for review June 11, 2026 09:56
@github-actions github-actions Bot requested a review from kaovilai June 11, 2026 09:56
@reasonerjt reasonerjt force-pushed the bak-vol-policy-design branch from 98c16cb to 0a67b2b Compare June 11, 2026 09:57
chlins
chlins previously approved these changes Jun 12, 2026

@chlins chlins left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

kaovilai
kaovilai previously approved these changes Jun 15, 2026

@kaovilai kaovilai left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

would be nice if you clarify what happens when configmap configured is missing. warn/error/no-error.


## Design

A new Velero server flag, `--global-backup-volume-policies-configmap`, accepts the name of a ConfigMap that lives in the Velero install namespace. The ConfigMap has the exact same format as an existing per-backup resource policies ConfigMap (a single data key holding a `ResourcePolicies` YAML document).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The design should specify the behavior when --global-backup-volume-policies-configmap is set but the referenced ConfigMap doesn't exist, both at server startup and at backup time (in case it's deleted after startup). Should the backup fail with a validation error? Or warn and proceed without global policies?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure I'll update it to add more details.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added a "Validation" section.


The merge combines two `ResourcePolicies` documents: the global policy (`G`) and the backup-level policy (`B`). The guiding principle is that the global policy provides a baseline, and the backup-level policy is layered with it.

- **`volumePolicies`**: `volumePolicies` is an ordered list where the *first* matching policy wins (per the existing `Policies.match` logic). The merged list is the concatenation of the backup-level policies followed by the global policies:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The issue description says the global policy should be merged "if the VolumePolicy is absent," which reads like a fallback-only behavior. This design proposes always merging (backup-level first, then global). The Alternatives section acknowledges the fallback approach but rejects it. Always-merge means existing backups that already have volume policies will start picking up global rules they didn't have before, which could be surprising.

@reasonerjt reasonerjt Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this is the intention, b/c when the admin configures the global volume policy for velero he wanna influence the behavior of all backups, unless over-written by the user. I clarified it in the "Compatibility" section.

Comment thread design/global-backup-volume-policies.md
Comment thread design/global-backup-volume-policies.md

## Design

A new Velero server flag, `--global-backup-volume-policies-configmap`, accepts the name of a ConfigMap that lives in the Velero install namespace. The ConfigMap has the exact same format as an existing per-backup resource policies ConfigMap (a single data key holding a `ResourcePolicies` YAML document).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor: the flag name --global-backup-volume-policies-configmap is quite long. Since it's a server flag (already global scope) and the value is always a ConfigMap name, something shorter like --default-volume-policy or --global-volume-policy might be friendlier. Not a blocker, just a usability thought.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are currently two issues around global settings for backup and restore. Do you think we need to align the naming around these two flag names?

Two issues related to "global" settings.
#9858: global default for Backup volume policy.
#9719: global default for Restore resource mutation policy.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

see also pr #9921

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@shubham-pampattiwar Thanks for the comment.

I chose to use xxx-volume-policies-configmap to be consistent with the parameter --resource-policies-configmap of velero backup create.

I think the backup is also needed in the parameter, b/c it's a policy for backups, and there could be policy for restores.

@reasonerjt reasonerjt dismissed stale reviews from kaovilai and chlins via 95dae20 June 16, 2026 07:09
@reasonerjt reasonerjt force-pushed the bak-vol-policy-design branch from 0a67b2b to 95dae20 Compare June 16, 2026 07:09
Add the design for global volume policies to address the requirement in
 velero-io#9858

Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com>
@reasonerjt reasonerjt force-pushed the bak-vol-policy-design branch from 95dae20 to 8a31544 Compare June 16, 2026 07:17
@reasonerjt reasonerjt merged commit faf24ae into velero-io:main Jun 17, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area/Design Design Documents kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants