* Report a measured zero incremental instead of erasing it
A CBT incremental with an exactly zero delta -- nothing changed since
the parent -- was reported identically to a backup that moved the whole
device. `velero backup describe --details` printed only
"Moved data Size (bytes): 3221225472" with no incremental line, and
status.incrementalBytes was absent, for a run that transferred nothing.
The best possible CBT outcome displayed as the worst, and was
indistinguishable from a genuine full, a whole-device fallback, or a
backup predating incremental accounting.
The zero was being erased twice. Besides the API status fields,
datapath.BackupResult also carried omitempty, and that struct crosses a
JSON boundary from the data mover pod to the controller (see
micro_service_watcher.go), so the value was destroyed before the
controller could persist it. Every uploader always reports a figure
there, so 0 internally always means "transferred nothing" -- dropping
omitempty is sufficient and correct for that hop.
The API fields move to *int64 rather than just dropping omitempty. The
field shipped in v1.18.0-v1.18.2, so backups exist whose stored volume
info has no incrementalSize at all; with a plain int64 those unmarshal
to 0 and would render "Incremental data Size (bytes): 0", a false claim
of a perfect incremental on a run that never measured one. nil means not
measured, a pointer to 0 means measured zero. Both fields already carry
+optional, so the generated CRD schema is unchanged and no regeneration
is required.
Display gates relax from > 0 to != nil in all three places, including
volumesByPod.Add, whose signature takes *int64 now; the restore describer
passes nil, which is correct since restores measure no incremental.
Verified live: the same zero-delta scenario that reported <none> now
reports 0 and renders "Incremental data Size (bytes): 0", while an older
backup described with the new client still correctly prints no
incremental line at all.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
(cherry picked from commit 6c7aa9d588f6d5eab134d4ce19c92b838f45557c)
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* gofmt: fix import ordering in backup_test.go
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* Regenerate CRDs for IncrementalBytes pointer type
make update-crd was missed in the original commit. Regenerated with
the pinned controller-gen v0.16.5 to avoid unrelated version-annotation
churn across other CRDs.
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* Add changelog for #10309
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* Address review: make IncrementalBytes a pointer to preserve backward compat
Per Lyndon-Li's review on #10309: dropping omitempty on the plain int64
field breaks compatibility with a data mover from release-1.17 or
earlier that predates IncrementalBytes and never writes the key -- the
new controller would unmarshal a zero value ("nothing transferred")
instead of recognizing the field is simply absent ("not measured").
Switch to *int64 with omitempty restored:
- an old mover's omitted key unmarshals to nil ("not measured")
- a current mover's genuine zero still serializes the key, unmarshaling
to a non-nil pointer to 0 ("measured zero")
- nonzero values work exactly as before
- an old controller can still unmarshal a numeric value from a new mover
pkg/controller/data_upload_controller.go and pod_volume_backup_controller.go
assign the wire-struct field directly to their already-*int64,omitempty
CRD status field instead of re-wrapping it with ptr.To, since both are
now the same pointer type.
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
* Fix CI: update marshal-fail test assertions for IncrementalBytes pointer
Both backup_micro_service_test.go files hardcoded the %v-formatted
zero-value BackupResult struct in an error-message assertion. Now that
IncrementalBytes is *int64, its zero value prints as <nil> instead of 0.
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>
* 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>
* Support set-based filter label selectors
Use matchLabels/matchExpressions in fine-grained filters.
Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
* omit the details of resource policy for cli
The reason to not resolve and display CLI is because it may go
out of sync, we want to avoid display it to mislead users.
We may consider to cpature those information and display it
in later release.
Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
---------
Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
Co-authored-by: Scott Seago <sseago@redhat.com>
Introduces a concurrent-safe sync.Map cache to the backup Request struct
to memoize GetNamespaceFilter results. This avoids re-evaluating glob
patterns for every item, significantly improving backup performance while
preserving the original exact-match precedence logic.
Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
Change errors.Cause to errors.Is, because github.com/cockroachdb/errors
New() function create a error with error stack with depth 1, but
github.com/pkg/errors's New() function create error with no depth.
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
fix fallbacks in backup filter policies,
- refactor the code to start with global policies, and apply
override if exists, and document the behaviour inline according
to the design
- Ensure that unlisted cluster-scoped kinds properly fall back
to global label selectors
- unlisted namespace-scoped kinds are explicitly skipped when
evaluating policy label selectors
add test coverage for GetNamespaceFilter glob matching and ordering
Added a unit test to verify that GetNamespaceFilter correctly matches
compiled glob patterns against namespace strings, and properly honors
first-match semantics when a namespace matches multiple patterns.
Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
implement backup logic to support backups with
NamespacedFilterPolicies and ClusterScopedFilterPolicy
Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
Run backup post hooks inside ItemBlock synchronously as the ItemBlocks are handled asynchronously
Fixes#8516
Signed-off-by: Wenkai Yin(尹文开) <yinw@vmware.com>
Check the PVB status via podvolume Backupper rather than calling API server to avoid API server issue
Fixes#8587
Signed-off-by: Wenkai Yin(尹文开) <yinw@vmware.com>
handleSkippedPVHasRetainPolicy
According to comment, calling executePVAction aims to reset PV's
claimRef, but the reset logic was moved into resetVolumeBindingInfo
since release-1.4.
Signed-off-by: Xun Jiang <blackpigletbruce@gmail.com>
Krishna AwasthiandGitHub
harshit sainiandGitHub
Xun Jiang
0xLeo258
Lyndon-Li
Matthieu MOREL
Wenkai Yin(尹文开)
lyndon-liandGitHub
Xun Jiang
Daniel Jiang
clonefetchandGitHub
Xun Jiang