* Add readWriteOncePod backupPVC config to enable mount-level SELinux labeling
On SELinux-enabled clusters the kubelet recursively relabels every file of
the backupPVC at mount time, which can take hours on volumes with a high
file count. Kubernetes avoids this when the volume is ReadWriteOncePod and
the CSI driver advertises SELinux mount support, by mounting with
-o context= instead.
Add an opt-in per-storage-class 'readWriteOncePod' backupPVC config option
that creates the backupPVC with the ReadWriteOncePod access mode and sets
the backup pod's SecurityContext.SELinuxChangePolicy to MountOption. It is
mutually exclusive with 'readOnly', which takes precedence.
Fixes#9873
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Add changelog file
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Do not set SELinuxChangePolicy for the backup pod
Live testing on OCP 4.22 (k8s 1.35) showed that setting
SecurityContext.SELinuxChangePolicy to MountOption makes backup pod
creation fail outright when the SELinuxMount feature gate is disabled,
which is the default on current clusters:
Pod is invalid: spec.securityContext.seLinuxChangePolicy:
Unsupported value: "MountOption": supported values: "Recursive"
The field is also unnecessary. For ReadWriteOncePod volumes the kubelet
already performs mount-level SELinux labeling via the
SELinuxMountReadWriteOncePod feature gate, which has been on by default
since k8s 1.28. Setting the backupPVC access mode to ReadWriteOncePod is
sufficient on its own, and is portable to clusters where the broader
SELinuxMount gate is still off.
Verified on-cluster that the backupPVC is mounted with
context="system_u:object_r:container_file_t:s0:c22,c28" instead of the
recursive seclabel mount used without the flag.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
---------
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
config/crd/{v1,v2alpha1}/crds/crds.go were generated files that
gzip-compressed the CRD YAML bases into committed []byte literals via
hack/crd-gen, requiring `go generate` and a dedicated CI drift check
(hack/verify-generated-crd-code.sh). This made the files large,
unreviewable in diffs, and a frequent source of merge conflicts.
Replace the generated files with config/crd/{v1,v2alpha1}/crds.go
using `//go:embed bases/*.yaml` to embed the already-committed YAML
manifests directly, decoding them the same way at init. Since Go's
go:embed can't reach outside a file's own directory tree, the crds
package now lives alongside bases/ instead of in a bases-sibling
subdirectory; import paths in pkg/install and pkg/controller were
updated accordingly.
Drop hack/crd-gen and hack/verify-generated-crd-code.sh entirely, and
trim their references from update-3generated-crd-code.sh and the
codespell skip-list. No codegen step remains, so no drift is possible.
Fixes#10328
AI-Tool-Used: Claude Code
AI-Tool-Use-Level: Category 1 (High)
AI-Code-Category: Category 1 (Production)
Signed-off-by: lubronzhan <lubron.zhan@broadcom.com>
Co-authored-by: Daniel Jiang <daniel.jiang@broadcom.com>
When CreateSnapshot fails (e.g. quota limit), the snapshot is recorded
with an empty ProviderSnapshotID. During backup deletion, velero was
calling DeleteSnapshot("") which produces unnecessary 404 API calls.
Skip the DeleteSnapshot call when ProviderSnapshotID is empty and log
a warning instead.
Fixes#9429
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Happy <yesreply@happy.engineering>
* Fix e2e kind matrix misparsing pre-release node tags
The setup-test-matrix step excluded "alpha|beta" pre-release tags but
not "rc" ones. A tag like v1.37.0-rc.1 slipped through to the awk
field-splitter, which treats "." as the only separator: splitting
"v1.37.0-rc.1" yields ["v1","37","0-rc","1"], and printing
$1"."$2"."$NF produced the bogus version "v1.37.1" - an image that
was never published, since the real tag is v1.37.0-rc.1.
Replace the two greps with a single anchored pattern that only
matches well-formed vX.Y.Z tags, so any hyphenated pre-release
suffix (rc, alpha, beta, or otherwise) is excluded before reaching
the awk step.
Fixes#10358
AI-Tool-Used: Claude Code
AI-Tool-Use-Level: Category 2 (Medium)
AI-Code-Category: Category 2 (Non-Production)
Signed-off-by: lubronzhan <lubron.zhan@broadcom.com>
* Add changelog entry for e2e matrix fix
AI-Tool-Used: Claude Code
AI-Tool-Use-Level: Category 3 (Low)
AI-Code-Category: Category 2 (Non-Production)
Signed-off-by: lubronzhan <lubron.zhan@broadcom.com>
---------
Signed-off-by: lubronzhan <lubron.zhan@broadcom.com>
getParentBackupInfo's parent-selection log lines interpolated the
parentSnapshot parameter. On the discovery branch (no explicit parent
passed by the caller) that parameter is empty by definition, so every
message about which parent was chosen, or why a run fell back to full,
printed no identifier at all -- e.g. "Using parent snapshot , start
time ...". This is the normal path for scheduled/incremental backups,
so the omission hit the common case, not an edge one.
Bind a parentID local that starts as the parameter but is overwritten
once a parent is actually resolved (explicit or discovered), and log
that instead. No behavior change.
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Backups stuck in New state have no Status.Expiration yet. The CLI
was estimating expiration from CreationTimestamp + TTL, which made
long-queued backups appear already expired.
Fixes#3555
Signed-off-by: PranjalManhgaye <manhgayepranjal@gmail.com>
* Skip signing a download URL when no artifacts can exist yet
Reported in #10232: a DownloadRequest for a backup that never ran still
reaches Processed with a signed URL, and fetching it returns 404.
The controller already has the backup, and the restore for restore
targets, in hand before it signs, so checking the phase costs no extra
call to the object store.
The check is deliberately narrow. It refuses only the pre-execution
phases, where nothing has been written for any target kind: New, Queued,
ReadyToStart and FailedValidation for backups, New and FailedValidation
for restores. InProgress onwards may hold a partial log or other
artifacts, and Deleting may still hold all of them, so those keep the
behaviour callers have today.
That matters because velero backup download has no client side phase
check of its own, unlike backup logs and restore logs. Reusing the
allowlist from pkg/cmd/cli/backup/logs.go would have changed what
backup download can fetch; this does not.
A backup with an empty phase is left alone as well, since that state is
transient and the caller can retry.
Refs #10232
Signed-off-by: saral <ilovegojo2580@gmail.com>
* Derive the phase coverage test from the generated CRDs
The previous test built a slice of phases by hand and asserted its own
length, so it passed no matter what the API did. Adding a fourteenth
backup phase would not have failed it.
This reads the status.phase enum out of the generated CRDs, via the
exported v1crds.CRDs that pkg/install already uses. The enum comes from
the same kubebuilder markers as the Go constants, so a phase added to
the API fails here until it is classified.
Verified by removing Deleting from the expectations, which now fails with
'BackupPhase "Deleting" is served by the CRD but not classified'.
Signed-off-by: saral <ilovegojo2580@gmail.com>
* Use US spelling in comments to satisfy the misspell linter
golangci-lint runs misspell, which flags behaviour as a misspelling of
behavior. Comments only, no functional change.
Signed-off-by: saral <ilovegojo2580@gmail.com>
* Set a Failed phase with a reason when the guard refuses to sign
The guard added in the previous commit left the request at New with no URL, so
the CLI polled until its own timeout and then reported that the backup storage
location may be unavailable. The BSL is fine; the backup never ran.
DownloadRequestPhase gains Failed and DownloadRequestStatus gains Message. The
controller sets both where it refuses, and the CLI stops as soon as it sees the
phase and surfaces the message instead of its generic timeout error.
Adding an enum value is additive, per the direction on the PR discussion.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: saral <ilovegojo2580@gmail.com>
---------
Signed-off-by: saral <ilovegojo2580@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit double checks the label of the VSC on the cluster before
deleting it to avoid mis-deletion.
Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com>
* Fix LoadAffinity mutation accumulating OS node selector terms
The node-agent parses the loadAffinity configuration once at startup and
keeps it in memory. GetLoadAffinityByStorageClass returned a pointer to one
of the elements of that cached list rather than a copy, so the exposers,
which append a kubernetes.io/os match expression to the returned affinity,
were mutating the shared configuration. Every DataUpload or DataDownload
appended another OS term, growing the data mover pod spec until it could
eventually exceed the object size limit.
Return a deep copy from GetLoadAffinityByStorageClass so that callers can
safely modify the result. A shallow copy is not enough because the
MatchExpressions slice header would still be shared with the source.
Fixes#10341
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Add changelog
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
---------
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Backup metadata with an empty or New phase was synced into the cluster as a pending backup, which the queue controller then ran as if it were newly requested. Hooks are dropped as well, since a synced backup never executes them.
Signed-off-by: chlins <chlins.zhang@gmail.com>
Document the new backupPVC/restorePVC secretNames and configMapNames
options (velero#9920) that copy namespace-scoped secrets/configmaps to
the Velero namespace so datamover can back up and restore encrypted CSI
volumes (e.g. ODF/ceph-csi with Vault KMS). Updates the main and v1.18
docs.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
This commit ensures the resources in the set "resourceMustHave" can only
be created in the namespace of velero deployment if it's namespace
scoped.
Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com>
* Detect block uploader cancellation through wrapped errors
Cancelling a block data mover backup was reported as a failure: the
DataUpload ended Failed with an error message and the Backup went
PartiallyFailed, for a user-requested cancel.
The cause is a sentinel equality check. block.ErrCanceled is raised in
the write loop and then wrapped twice before it reaches the provider --
once in block/uploader.go ("error backing up bdev %s") and again in
block/snapshot.go ("Failed to run uploader backup for si %v") -- so
`err == block.ErrCanceled` can never be true and the ErrorCanceled
returns are unreachable. The filesystem provider avoids this by asking
the uploader for its state (kpUploader.IsCanceled()) rather than
inspecting the error.
Use errors.Is at both the backup and restore sites.
Adds TestBlockProviderCancelThroughWrappedError, which injects the
doubly-wrapped sentinel exactly as production builds it. Note the
assertion is require.ErrorIs, not ErrorContains: provider.ErrorCanceled
and block.ErrCanceled carry identical message text, so a substring
assertion passes whether or not the sentinel was recognised -- which is
why the existing test, injecting the bare sentinel, did not catch this.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
(cherry picked from commit 9d6c5da7a893068d424b0c7896638787c636e213)
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* Add changelog for #10308
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* lint: fix misspelling (recognised -> recognized)
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
---------
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The DataUpload and DataDownload controllers now copy and delete
namespace-scoped secrets/configmaps for backup/restore PVC provisioning.
Add the corresponding kubebuilder RBAC markers (get;list;create;delete
on secrets and configmaps) and regenerate the ClusterRole.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
The copied secret/configmap label value was the owner (DataUpload/
DataDownload) name, which is derived from the Backup/Restore name and
can exceed the 63-char Kubernetes label-value limit or contain invalid
characters. That would make the copy label and the cleanup selector
diverge and orphan the copied resources.
Use string(ownerObject.UID) consistently for the label value in both
copy and cleanup (backup and restore exposers). The UID is a stable,
always-valid label value.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Mirror the backup-side fix on the restore path. The generic restore
exposer creates the intermediate restore PVC in the Velero namespace
using the target PVC's StorageClass. For encrypted volumes this fails
because ceph-csi looks up the KMS token secret in the PVC's namespace
(the Velero namespace), where it does not exist.
Add SecretNames/ConfigMapNames to the RestorePVC config. When set, the
generic restore exposer copies the named secrets/configmaps from the
target namespace to the Velero namespace before creating the restore
PVC, and cleans them up in CleanUp(). Reuses the same copy/delete
helpers and label as the backup path.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
These single-object delete helpers were introduced earlier but are no
longer called in production code: DeleteSecretsWithLabel and
DeleteConfigMapsWithLabel now delete inline with UID preconditions.
Remove the dead functions and their tests.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
- Make CopySecret/CopyConfigMap accept a generic labels map instead of
hardcoding the backup-pvc-secret label, aligning with the generic
DeleteSecretsWithLabel helper. Move the BackupPVCSecretLabel constant
from util/kube to the exposer package where it is used.
- Move the secret/configmap copy in Expose() to after
WaitVolumeSnapshotReady and before createBackupVS. That is the most
likely failure point, and nothing needs cleanup before it.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
- Fix premature deletion of shared secrets/configmaps: check owner label
in addition to data equality. Same data + different owner is now a
collision, preventing one DataUpload's CleanUp from removing resources
another DataUpload is still using.
- Copy BinaryData in CopyConfigMap and include it in the equality check,
so configmaps with binary payloads (e.g., CA bundles) are not silently
truncated.
- Add UID preconditions to DeleteSecretsWithLabel and
DeleteConfigMapsWithLabel to avoid TOCTOU races where a recreated
object with the same name could be deleted.
- Move secret/configmap copy to the beginning of Expose(), before any
intermediate objects are created, so failure doesn't require cleanup.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Add unit tests for CopyConfigMap, DeleteConfigMapIfAny, and
DeleteConfigMapsWithLabel mirroring the existing secret tests.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Move the secret and configmap copy logic from the DataUpload controller
into the CSI snapshot exposer's Expose() method. This keeps all
CSI-specific logic in the exposer and maintains symmetry with CleanUp()
which already handles the cleanup of copied resources.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Shubham PampattiwarandGitHub
Xun Jiang/Bruce JiangandGitHub
R4mboandGitHub
lyndon-liandGitHub
PranjalandGitHub
Xun Jiang
Krishna AwasthiandGitHub
Chlins ZhangandGitHub
Lyndon-Li