* 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>
* 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>
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>
DownloadRequest and ServerStatusRequest were the last two Velero CRDs
without printer columns, so kubectl showed only NAME and AGE for both.
DownloadRequest gains the target kind and name, its phase, and age.
ServerStatusRequest gains its phase, the reported server version, the
time the controller processed it, and age.
status.downloadURL is deliberately left out: it is a pre-signed URL that
grants access to the object, and a default list view is the wrong place
for it. status.expiration is left out because kubectl renders a date
column as time elapsed, so a future timestamp prints <invalid>.
Signed-off-by: saral <ilovegojo2580@gmail.com>
* cap the unzip of metadata download to avoid oom kill
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
* detect when EOF is retuend because of cap
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
---------
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
* refactor: use k8s.io/api well-known label constants
Several well-known Kubernetes label strings were hardcoded across the
codebase instead of using the constants already exported by
k8s.io/api/core/v1, which is an existing dependency:
"kubernetes.io/hostname" -> corev1api.LabelHostname
"kubernetes.io/os" -> corev1api.LabelOSStable
"topology.kubernetes.io/zone" -> corev1api.LabelTopologyZone
The local kube.NodeOSLabel and zoneLabel consts, which duplicated the
upstream values verbatim, are now defined in terms of the upstream
constants rather than repeating the literal. Both are kept: NodeOSLabel
is exported and referenced from four packages alongside NodeOSLinux and
NodeOSWindows, which have no upstream equivalent, and zoneLabel sits
beside the deprecated-label fallback it is compared against.
No functional change - every replacement is a constant with an identical
value.
Signed-off-by: Harshit saini <harshitsaini1188@gmail.com>
* Add changelog for #10279
Signed-off-by: Harshit saini <harshitsaini1188@gmail.com>
* Cover the selected-node path in createRestorePod
TestCreateRestorePod only exercised selectedNode == "", so the branch
that pins the restore pod to a node was never executed. Add a case with
a selected node and assert the resulting pod carries the hostname label
in its node selector.
Signed-off-by: Harshit saini <harshitsaini1188@gmail.com>
* Also use constants for the arch and deprecated zone labels
Extends the same replacement to the two remaining well-known labels
raised on the issue:
"kubernetes.io/arch" -> corev1api.LabelArchStable
"failure-domain.beta.kubernetes.io/zone" -> corev1api.LabelFailureDomainBetaZone
zoneLabelDeprecated in item_backupper.go was the last local const still
repeating a literal that upstream already exports, so the zone pair now
reads consistently against k8s.io/api. The deprecation note upstream
applies to the label itself, not the constant; Velero reads that label
deliberately as the fallback for PVs created before the topology labels
existed.
Signed-off-by: Harshit saini <harshitsaini1188@gmail.com>
---------
Signed-off-by: Harshit saini <harshitsaini1188@gmail.com>
When a Schedule has no explicit spec.skipImmediately, the reconciler
assigned &c.skipImmediately directly into the Schedule's spec pointer.
The subsequent write-through-pointer (*ptr = false) mutated the
reconciler's own shared field, silently disabling
--schedule-skip-immediately for every schedule reconciled afterward
for the life of the process.
Fix: copy the value into a fresh bool before taking its address.
Adds TestReconcileDoesNotCorruptReconcilerSkipImmediately, which
reconciles two schedules against one reconciler instance and asserts
the shared default is preserved.
Signed-off-by: Prajwal <percy38621@gmail.com>
Processed means the controller signed a URL into status.downloadURL. It
does not mean the object is present: GetDownloadURL builds the key by
convention and signs it, with no existence check, so a request whose
target never produced a file still reaches Processed and the URL 404s.
The CLI never sees this because it filters on backup and restore phase
before creating the request. Other API consumers have nothing in the
status telling them that filter is needed, and the field description
said only "Phase is the current state of the DownloadRequest".
Documentation only. The field comments are what controller-gen writes
into the CRD, so this reaches kubectl explain and generated clients
without anyone reading the Go source.
Refs #10232
Signed-off-by: saral <ilovegojo2580@gmail.com>
Shubham PampattiwarandGitHub
Xun Jiang/Bruce JiangandGitHub
R4mboandGitHub
lyndon-liandGitHub
Xun Jiang
Krishna AwasthiandGitHub
Chlins ZhangandGitHub
Lyndon-Li
Joseph Antony VaikathandGitHub
opbot_xd
harshit sainiandGitHub
V PrajwalandGitHub
Jay SawantandGitHub