Skip to content

feat(backup): add support for new velero fields #436

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

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

feat(backup): add support for new velero fields #436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c5cd27d
Initial plan
Copilot Aug 19, 2025
4c3c782
Update Velero backup fields: replace defaultVolumesToRestic with defa…
Copilot Aug 19, 2025
87fd82b
Add comprehensive Velero backup fields and improve documentation
Copilot Aug 19, 2025
cfc6008
Address feedback: remove labelSelector field, fix common labels usage…
Copilot Aug 19, 2025
86acf7c
Clean up temporary files and update gitignore
Copilot Aug 19, 2025
0d201a9
Remove specified backup fields and fix includedNamespaces multi-names…
Copilot Aug 19, 2025
eef0bfb
Remove .gitignore change for helm-unittest.tar.gz
Copilot Aug 19, 2025
c963770
Update backup.yaml
aslafy-z Aug 19, 2025
e8a17f3
Update values.yaml
aslafy-z Aug 19, 2025
d266b19
Fix backup unit tests after field removal
Copilot Aug 19, 2025
d1dd157
Update backup_test.yaml
aslafy-z Aug 19, 2025
651bd83
Update values.yaml
aslafy-z Aug 19, 2025
1e3d9da
Update backup_test.yaml
aslafy-z Aug 19, 2025
d54b159
Update values.yaml
aslafy-z Aug 19, 2025
5b3c48a
Update backup_test.yaml
aslafy-z Aug 19, 2025
e4e0aee
Update README.md
aslafy-z Aug 19, 2025
60c0eab
Update backup_test.yaml
aslafy-z Aug 19, 2025
26b88ea
Merge branch 'main' into copilot/fix-435
rasheedamir Aug 23, 2025
ecca2f6
Merge branch 'main' into copilot/fix-435
rasheedamir Aug 30, 2025
6036732
Update backup.yaml
aslafy-z Sep 13, 2025
975ac71
Add test for explicit deprecated restic integration in backup
aslafy-z Sep 13, 2025
9c2d809
Reorder backup parameters in values.yaml
aslafy-z Sep 13, 2025
c8c4e21
Refactor conditional checks for backup volumes
aslafy-z Sep 13, 2025
3473a8a
docs(README): Reorder backup options
aslafy-z Sep 13, 2025
e32911c
Merge branch 'main' into copilot/fix-435
aslafy-z Sep 13, 2025
4d6b1fb
fix: add capability guard
aslafy-z Sep 13, 2025
731cc2e
Update backup_test.yaml with capabilities section
aslafy-z Sep 13, 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
11 changes: 10 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,13 +387,22 @@ helm delete --namespace test my-application
| backup.namespace | string | `{{ .Release.Namespace }}` | Namespace for Backup. |
| backup.additionalLabels | object | `nil` | Additional labels for Backup. |
| backup.annotations | object | `nil` | Annotations for Backup. |
| backup.defaultVolumesToRestic | bool | `true` | Whether to use Restic to take snapshots of all pod volumes by default. |
| backup.defaultVolumesToFsBackup | bool | `true` | Whether to use filesystem backup to take snapshots of all pod volumes by default. |
| backup.defaultVolumesToRestic | bool | `nil` | Whether to use Restic to take snapshots of all pod volumes by default. Deprecated: Use `defaultVolumesToFsBackup` instead. |
| backup.snapshotVolumes | bool | `true` | Whether to take snapshots of persistent volumes as part of the backup. |
| backup.snapshotMoveData | bool | `nil` | Whether to move the data of the snapshot after it's taken. |
| backup.csiSnapshotTimeout | string | `nil` | Time to wait for CSI VolumeSnapshot status to turn ReadyToUse during creation (e.g., "10m"). |
| backup.datamover | string | `nil` | Data mover to be used by the backup. If empty or "velero", the built-in data mover will be used. |
| backup.itemOperationTimeout | string | `nil` | Time to wait for asynchronous BackupItemAction operations (e.g., "1h"). |
| backup.storageLocation | string | `nil` | Name of the backup storage location where the backup should be stored. |
| backup.ttl | string | `"1h0m0s"` | How long the Backup should be retained for. |
| backup.volumeSnapshotLocations | list | `nil` | Names of VolumeSnapshotLocations associated with this backup. |
| backup.includedNamespaces | tpl/list | `[ {{ include "application.namespace" $ }} ]` | List of namespaces to include objects from. |
| backup.includedResources | list | `nil` | List of resource types to include in the backup. |
| backup.excludedResources | list | `nil` | List of resource types to exclude from the backup. |
| backup.includedNamespaceScopedResources | list | `nil` | List of namespace-scoped resource types to include in the backup. |
| backup.excludedNamespaceScopedResources | list | `nil` | List of namespace-scoped resource types to exclude from the backup. |
| backup.orderedResources | object | `nil` | Specifies the backup order of resources of specific Kind. The map key is the resource name and the value is a list of object names in the order they should be backed up. |

## Naming convention for ConfigMap, Secret, SealedSecret and ExternalSecret

Expand Down
72 changes: 55 additions & 17 deletions application/templates/backup.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,38 +1,76 @@
{{- if (.Values.backup).enabled }}
{{- if and .Values.backup .Values.backup.enabled (.Capabilities.APIVersions.Has "velero.io/v1") -}}
apiVersion: velero.io/v1
kind: Backup
metadata:
name: {{ printf "%s-backup" .Values.applicationName | trunc 63 | quote }}
namespace: {{ .Values.backup.namespace | default ( include "application.namespace" . ) | quote }}
labels:
{{- include "application.labels" $ | nindent 4 }}
{{- if .Values.backup.additionalLabels }}
{{ toYaml .Values.backup.additionalLabels | indent 4 }}
{{- end }}
{{- if .Values.backup.annotations }}
{{- include "application.labels" $ | nindent 4 }}
{{- if .Values.backup.additionalLabels }}
{{- toYaml .Values.backup.additionalLabels | nindent 4 }}
{{- end }}
{{- if .Values.backup.annotations }}
annotations:
{{ toYaml .Values.backup.annotations | indent 4 }}
{{- end }}
{{- toYaml .Values.backup.annotations | nindent 4 }}
{{- end }}
spec:
labelSelector:
matchLabels:
app.kubernetes.io/part-of: {{ include "application.name" . }}
{{- include "application.selectorLabels" . | nindent 6 }}
{{- if not (kindIs "invalid" .Values.backup.includedNamespaces) }}
includedNamespaces:
{{- range $namespace := .Values.backup.includedNamespaces }}
- {{ include "application.tplvalues.render" ( dict "value" $namespace "context" $ ) }}
{{- end }}
{{- end }}
defaultVolumesToRestic: {{ .Values.backup.defaultVolumesToRestic }}
Copy link
Collaborator

@d3adb5 d3adb5 Aug 30, 2025

Choose a reason for hiding this comment

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

Whether Velero acknowledges it or not, this is a breaking change for us, since it would inhibit current chart users from continuing to use Velero 1.9 and below. defaultVolumesToFsBackup did not exist prior to 1.10, and this should've been a major bump for this CRD's API version, in my opinion.

Regardless, we are able to support this without creating too much trouble for ourselves. Here's a diff built on top of ecca2f6:

diff --git a/application/templates/backup.yaml b/application/templates/backup.yaml
index a8831e5..0c2853d 100644
--- a/application/templates/backup.yaml
+++ b/application/templates/backup.yaml
@@ -23,7 +23,11 @@ spec:
   - {{ include "application.tplvalues.render" ( dict "value" $namespace "context" $ ) }}
   {{- end }}
 {{- end }}
+  {{- if not (kindIs "invalid" .Values.backup.defaultVolumesToRestic) }}
+  defaultVolumesToRestic: {{  .Values.backup.defaultVolumesToRestic }}
+  {{- else if not (kindIs "invalid" .Values.backup.defaultVolumesToFsBackup) }}
   defaultVolumesToFsBackup: {{  .Values.backup.defaultVolumesToFsBackup }}
+  {{- end }}
   snapshotVolumes: {{ .Values.backup.snapshotVolumes }}
 {{- if not (kindIs "invalid" .Values.backup.snapshotMoveData) }}
   snapshotMoveData: {{ .Values.backup.snapshotMoveData }}
diff --git a/application/tests/backup_test.yaml b/application/tests/backup_test.yaml
index 1a79430..fcfb1c3 100644
--- a/application/tests/backup_test.yaml
+++ b/application/tests/backup_test.yaml
@@ -66,6 +66,20 @@ tests:
           path: spec.ttl
           value: "1h0m0s"
 
+  - it: uses restic integration is explicitly enabled
+    set:
+      backup:
+        enabled: true
+        namespace: my-namespace
+        storageLocation: my-storage-location
+        defaultVolumesToRestic: true
+    asserts:
+      - notExists:
+          path: spec.defaultVolumesToFsBackup
+      - equal:
+          path: spec.defaultVolumesToRestic
+          value: true
+
   - it: includes correct includedNamespaces by default
     set:
       namespaceOverride: base-namespace

If we do move forward with this, the question then becomes: do we keep the current behavior and allow users to explicitly set defaultVolumesToFsBackup, or do we default to Velero 1.10+ behavior and allow users to explicitly set the old behavior? Is this another major bump or no?

What do you think, @rasheedamir and @aslafy-z? Personally, I think we should default to 1.10+ behavior and don't think a major bump is really necessary.

Copy link
Collaborator

@aslafy-z aslafy-z Sep 13, 2025

Choose a reason for hiding this comment

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

Yeah, this was definitely not the goal. This comment-based workflow is not ideal.

I've integrated your patch to re-add support for defaultVolumesToRestic. The 1.10+ behavior is now the default. I don't think a major bump is required, but I wouldn't vote against.

{{- if not (kindIs "invalid" .Values.backup.defaultVolumesToRestic) }}
defaultVolumesToRestic: {{ .Values.backup.defaultVolumesToRestic }}
{{- else if not (kindIs "invalid" .Values.backup.defaultVolumesToFsBackup) }}
defaultVolumesToFsBackup: {{ .Values.backup.defaultVolumesToFsBackup }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.snapshotVolumes) }}
snapshotVolumes: {{ .Values.backup.snapshotVolumes }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.snapshotMoveData) }}
snapshotMoveData: {{ .Values.backup.snapshotMoveData }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.csiSnapshotTimeout) }}
csiSnapshotTimeout: {{ .Values.backup.csiSnapshotTimeout | quote }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.datamover) }}
datamover: {{ .Values.backup.datamover | quote }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.itemOperationTimeout) }}
itemOperationTimeout: {{ .Values.backup.itemOperationTimeout | quote }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.storageLocation) }}
storageLocation: {{ .Values.backup.storageLocation | quote }}
ttl: {{ .Values.backup.ttl }}
{{- if .Values.backup.includedResources }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.ttl) }}
ttl: {{ .Values.backup.ttl | quote }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.volumeSnapshotLocations) }}
volumeSnapshotLocations:
{{- toYaml .Values.backup.volumeSnapshotLocations | nindent 4 }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.includedResources) }}
includedResources:
{{ toYaml .Values.backup.includedResources | indent 4 }}
{{- end -}}
{{- if .Values.backup.excludedResources }}
{{- toYaml .Values.backup.includedResources | nindent 4 }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.excludedResources) }}
excludedResources:
{{ toYaml .Values.backup.excludedResources | indent 4 }}
{{- end -}}
{{- toYaml .Values.backup.excludedResources | nindent 4 }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.includedNamespaceScopedResources) }}
includedNamespaceScopedResources:
{{- toYaml .Values.backup.includedNamespaceScopedResources | nindent 4 }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.excludedNamespaceScopedResources) }}
excludedNamespaceScopedResources:
{{- toYaml .Values.backup.excludedNamespaceScopedResources | nindent 4 }}
{{- end }}
{{- if not (kindIs "invalid" .Values.backup.orderedResources) }}
orderedResources:
{{- toYaml .Values.backup.orderedResources | nindent 4 }}
{{- end }}
{{- end }}
139 changes: 138 additions & 1 deletion application/tests/backup_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ suite: Backup
templates:
- backup.yaml

capabilities:
apiVersions:
- velero.io/v1
- velero.io/v1/Backup

tests:
- it: does not yield Backup resource if backup.enabled is false
set:
Expand Down Expand Up @@ -54,8 +59,10 @@ tests:
namespace: my-namespace
storageLocation: my-storage-location
asserts:
- equal:
- notExists:
path: spec.defaultVolumesToRestic
- equal:
path: spec.defaultVolumesToFsBackup
value: true
- equal:
path: spec.snapshotVolumes
Expand All @@ -64,6 +71,20 @@ tests:
path: spec.ttl
value: "1h0m0s"

- it: uses restic integration is explicitly enabled
set:
backup:
enabled: true
namespace: my-namespace
storageLocation: my-storage-location
defaultVolumesToRestic: true
asserts:
- notExists:
path: spec.defaultVolumesToFsBackup
- equal:
path: spec.defaultVolumesToRestic
value: true

- it: includes correct includedNamespaces by default
set:
namespaceOverride: base-namespace
Expand Down Expand Up @@ -115,3 +136,119 @@ tests:
value:
- secrets
- configmaps

- it: includes snapshotMoveData when defined
set:
backup:
enabled: true
snapshotMoveData: true
asserts:
- equal:
path: spec.snapshotMoveData
value: true

- it: does not include snapshotMoveData when not defined
set:
backup:
enabled: true
asserts:
- notExists:
path: spec.snapshotMoveData

- it: uses application selector labels
set:
backup:
enabled: true
applicationName: my-app
asserts:
- equal:
path: spec.labelSelector.matchLabels
value:
app.kubernetes.io/name: my-app

- it: includes csiSnapshotTimeout when defined
set:
backup:
enabled: true
csiSnapshotTimeout: "10m"
asserts:
- equal:
path: spec.csiSnapshotTimeout
value: "10m"

- it: includes datamover when defined
set:
backup:
enabled: true
datamover: "kopia"
asserts:
- equal:
path: spec.datamover
value: "kopia"

- it: includes itemOperationTimeout when defined
set:
backup:
enabled: true
itemOperationTimeout: "1h"
asserts:
- equal:
path: spec.itemOperationTimeout
value: "1h"

- it: includes volumeSnapshotLocations when defined
set:
backup:
enabled: true
volumeSnapshotLocations:
- default
- gp2
asserts:
- equal:
path: spec.volumeSnapshotLocations
value:
- default
- gp2

- it: includes includedNamespaceScopedResources when defined
set:
backup:
enabled: true
includedNamespaceScopedResources:
- deployments
- services
asserts:
- equal:
path: spec.includedNamespaceScopedResources
value:
- deployments
- services

- it: includes excludedNamespaceScopedResources when defined
set:
backup:
enabled: true
excludedNamespaceScopedResources:
- secrets
- configmaps
asserts:
- equal:
path: spec.excludedNamespaceScopedResources
value:
- secrets
- configmaps

- it: includes orderedResources when defined
set:
backup:
enabled: true
orderedResources:
deployments:
- my-deployment-1
- my-deployment-2
asserts:
- equal:
path: spec.orderedResources.deployments
value:
- my-deployment-1
- my-deployment-2
42 changes: 41 additions & 1 deletion application/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1236,18 +1236,38 @@ backup:
# @section -- Backup Parameters
annotations:
# key: value
# -- (bool) Whether to use filesystem backup to take snapshots of all pod volumes by default.
# @section -- Backup Parameters
defaultVolumesToFsBackup: true
# -- (bool) Whether to use Restic to take snapshots of all pod volumes by default.
# Deprecated: Use `defaultVolumesToFsBackup` instead.
# @section -- Backup Parameters
defaultVolumesToRestic: true
defaultVolumesToRestic:
# -- (bool) Whether to take snapshots of persistent volumes as part of the backup.
# @section -- Backup Parameters
snapshotVolumes: true
# -- (bool) Whether to move the data of the snapshot after it's taken.
# @section -- Backup Parameters
snapshotMoveData:
# -- (string) Time to wait for CSI VolumeSnapshot status to turn ReadyToUse during creation (e.g., "10m").
# @section -- Backup Parameters
csiSnapshotTimeout:
# -- (string) Data mover to be used by the backup. If empty or "velero", the built-in data mover will be used.
# @section -- Backup Parameters
datamover:
# -- (string) Time to wait for asynchronous BackupItemAction operations (e.g., "1h").
# @section -- Backup Parameters
itemOperationTimeout:
# -- (string) Name of the backup storage location where the backup should be stored.
# @section -- Backup Parameters
storageLocation:
# -- (string) How long the Backup should be retained for.
# @section -- Backup Parameters
ttl: "1h0m0s"
# -- (list) Names of VolumeSnapshotLocations associated with this backup.
# @section -- Backup Parameters
volumeSnapshotLocations:
# - my-volume-snapshot-location
# -- (tpl/list) List of namespaces to include objects from.
# @default -- `[ {{ include "application.namespace" $ }} ]`
# @section -- Backup Parameters
Expand All @@ -1256,9 +1276,29 @@ backup:
# -- (list) List of resource types to include in the backup.
# @section -- Backup Parameters
includedResources:
# - deployments
# - services
# -- (list) List of resource types to exclude from the backup.
# @section -- Backup Parameters
excludedResources:
# - secrets
# - configmaps
# -- (list) List of namespace-scoped resource types to include in the backup.
# @section -- Backup Parameters
includedNamespaceScopedResources:
# - deployments
# - services
# -- (list) List of namespace-scoped resource types to exclude from the backup.
# @section -- Backup Parameters
excludedNamespaceScopedResources:
# - secrets
# - configmaps
# -- (object) Specifies the backup order of resources of specific Kind. The map key is the resource name and the value is a list of object names in the order they should be backed up.
# @section -- Backup Parameters
orderedResources:
# deployments:
# - my-deployment-1
# - my-deployment-2

# -- ([list or object] of [tpl/object or tpl/string]) Extra K8s manifests to deploy.
# Can be of type list or object. If object, keys are ignored and only values are used. The used values can be defined as object or string and are passed through tpl to render.
Expand Down
Loading