Skip to content

feat: expose staleDataUploadThreshold in DPA KubevirtDatamoverConfig #2361

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

Merged
openshift-merge-bot merged 1 commit into from
Aug 10, 2026
+121 −0
Merged

feat: expose staleDataUploadThreshold in DPA KubevirtDatamoverConfig #2361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions api/v1alpha1/dataprotectionapplication_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,12 @@ type KubevirtDatamoverConfig struct {
// +kubebuilder:validation:Minimum=0
// +optional
MaxIncrementalBackups *int32 `json:"maxIncrementalBackups,omitempty"`

// StaleDataUploadThreshold is the duration after which a DataUpload in an
// active phase is considered stale and will no longer block younger
// DataUploads for the same VM. Default is 2h.
// +optional
StaleDataUploadThreshold *metav1.Duration `json:"staleDataUploadThreshold,omitempty"`
}

// ApplicationConfig defines the configuration for the Data Protection Application
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,12 @@ spec:
format: int32
minimum: 0
type: integer
staleDataUploadThreshold:
description: |-
StaleDataUploadThreshold is the duration after which a DataUpload in an
active phase is considered stale and will no longer block younger
DataUploads for the same VM. Default is 2h.
type: string
type: object
nodeAgent:
description: NodeAgent is needed to allow selection between kopia or restic
Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,12 @@ spec:
format: int32
minimum: 0
type: integer
staleDataUploadThreshold:
description: |-
StaleDataUploadThreshold is the duration after which a DataUpload in an
active phase is considered stale and will no longer block younger
DataUploads for the same VM. Default is 2h.
type: string
type: object
nodeAgent:
description: NodeAgent is needed to allow selection between kopia or restic
Expand Down
4 changes: 4 additions & 0 deletions internal/controller/kubevirt_datamover_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ func ensureKubevirtDatamoverRequiredSpecs(
args = append(args, fmt.Sprintf("--max-incremental-backups=%d",
*dpa.Spec.Configuration.KubevirtDatamover.MaxIncrementalBackups))
}
if dpa.Spec.Configuration.KubevirtDatamover.StaleDataUploadThreshold != nil {
args = append(args, fmt.Sprintf("--stale-dataupload-threshold=%s",
dpa.Spec.Configuration.KubevirtDatamover.StaleDataUploadThreshold.Duration.String()))
}
Comment on lines +243 to +246

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Locate the flag declaration and all operator-side references.
rg -n -C 5 --glob '*.go' \
  -e 'stale-dataupload-threshold|staleDataUploadThreshold|StaleDataUploadThreshold' .

# Locate the manager image/version source used by this operator.
rg -n -C 3 \
  --glob '*.go' --glob '*.yaml' --glob '*.yml' --glob 'go.mod' --glob 'go.sum' \
  -e 'kubevirt-datamover' .

Repository: openshift/oadp-operator

Length of output: 50379

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Inspect the exact deployment argument wiring and image resolution without
# emitting the repository-wide search results that were truncated.
sed -n '200,275p' internal/controller/kubevirt_datamover_controller.go
sed -n '350,395p' internal/controller/kubevirt_datamover_controller.go
rg -n -C 4 --glob '*.go' --glob '*.yaml' --glob '*.yml' \
  -e 'RELATED_IMAGE_KUBEVIRT_DATAMOVER_CONTROLLER|kubevirt-datamover-controller:[^[:space:]]+' .

# Check whether the manager source is vendored or referenced through a
# dependency, and query the public source tree for the flag declaration.
git ls-files | rg '(^|/)(vendor|go.mod|go.sum|Dockerfile|Makefile|Containerfile|.*kubevirt.*datamover.*)$' || true
for url in \
  'https://api.github.com/search/code?q=stale-dataupload-threshold+in:file' \
  'https://api.github.com/repos/konveyor/kubevirt-datamover/contents' \
  'https://api.github.com/repos/kubevirt/kubevirt-datamover/contents'
do
  echo "=== $url"
  curl -LfsS --max-time 20 "$url" 2>/dev/null | head -c 12000 || true
  printf '\n'
done

Repository: openshift/oadp-operator

Length of output: 14654

🌐 Web query:

kubevirt-datamover-controller "stale-dataupload-threshold"

💡 Result:

The stale-dataupload-threshold refers to a proposed configuration mechanism within the kubevirt-datamover-controller designed to identify and ignore "stuck" DataUpload (DU) resources [1]. Because Velero does not apply its standard DataUpload timeout to custom data movers, an orphaned or stalled DataUpload that remains indefinitely in an active phase (e.g., Accepted, Prepared, or InProgress) can block all subsequent backups for the same Virtual Machine [1]. To resolve this, a staleness threshold was proposed to allow the controller to recognize when a blocking DataUpload has exceeded a configurable duration—such as 30 minutes—and subsequently ignore it [1]. This ensures that younger, potentially healthy DataUploads are not permanently blocked by a stale process [1]. It is important to distinguish this from standard Velero timeouts: - The kubevirt-datamover-controller mechanism targets specific issues with per-VM serialization where one stuck DU prevents others from proceeding [1]. - Standard Velero backups use parameters like --item-operation-timeout (default 4 hours) and data-mover-prepare-timeout (default 30 minutes) to manage general DataUpload lifecycle timeouts [2]. As of the current development context, this threshold is identified as a solution to prevent indefinite backup blocking for VMs [1]. Users managing KubeVirt backups via this controller should be aware that this specific threshold is intended to provide a safety net where Velero's default mechanisms do not apply [1].

Citations:

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

repo='migtools/kubevirt-datamover-controller'
echo '=== repository metadata ==='
curl -LfsS --max-time 20 "https://api.github.com/repos/$repo" |
  jq '{full_name,default_branch,archived,updated_at}'

branch=$(curl -LfsS --max-time 20 "https://api.github.com/repos/$repo" |
  jq -r '.default_branch')
echo "=== tree: $branch ==="
curl -LfsS --max-time 20 \
  "https://api.github.com/repos/$repo/git/trees/$branch?recursive=1" |
  jq -r '.tree[] | select(.path|test("\\.(go|yaml|yml|md)$")) | .path' |
  head -200

echo '=== flag references in default-branch source ==='
curl -LfsS --max-time 20 \
  "https://api.github.com/repos/$repo/git/trees/$branch?recursive=1" |
  jq -r '.tree[] | select(.type=="blob" and (.path|test("\\.go$"))) | .url' |
  while read -r blob; do
    curl -LfsS --max-time 20 "$blob" |
      jq -r '.content // empty' | base64 -d 2>/dev/null |
      grep -n -C 5 -E 'stale-dataupload-threshold|staleDataUploadThreshold' || true
  done

echo '=== issue 30 ==='
curl -LfsS --max-time 20 "https://api.github.com/repos/$repo/issues/30" |
  jq '{title,state,body,html_url}'

Repository: openshift/oadp-operator

Length of output: 240

🌐 Web query:

site:github.com/migtools/kubevirt-datamover-controller "stale-dataupload-threshold"

💡 Result:

The term stale-dataupload-threshold refers to a proposed or implemented configuration parameter within the kubevirt-datamover-controller project [1][2]. It is used to define a duration of time after which an active DataUpload resource that appears stuck is considered stale [1]. In the context of the kubevirt-datamover-controller, the controller implements per-VM serialization of DataUploads, where newer DataUploads are requeued if an older one for the same VM is already in an active phase [1]. If a DataUpload becomes stuck in an active phase (for example, due to a controller crash or an orphaned process), it can indefinitely block all future backups for that VM because Velero's built-in DataUpload timeouts do not apply to these custom data movers [1]. To mitigate this, the stale-dataupload-threshold allows the controller to ignore such blocking DataUploads if they have remained in their current active phase longer than the configured duration [1]. When a DataUpload is identified as having exceeded this threshold, the controller can proceed to ignore or transition it to a terminal state, such as Failed, thereby preventing it from blocking subsequent backups [1]. The implementation suggested in technical discussions uses this threshold to skip DataUploads whose creation timestamp plus the defined duration is older than the current time [1].

Citations:

🌐 Web query:

site:github.com/migtools/kubevirt-datamover-controller "StaleDataUploadThreshold"

💡 Result:

StaleDataUploadThreshold is a configuration parameter (typically a duration) proposed for the migtools/kubevirt-datamover-controller [1][2]. It is intended to manage the lifecycle of DataUpload resources by identifying and handling potentially orphaned or stuck operations [1]. In the context of the KubeVirt Data Mover for Velero, the controller implements per-VM serialization of DataUploads to prevent concurrent operations on the same virtual machine [1]. If an older DataUpload remains stuck in an active phase (such as Accepted, Prepared, or InProgress) indefinitely—potentially due to a controller crash or orphaned process—subsequent younger DataUploads for that same VM would be requeued indefinitely [1]. The StaleDataUploadThreshold is proposed as a mechanism to address this by allowing the controller to ignore a blocking DataUpload if it has remained in its current active phase for longer than this specified threshold [1]. By treating such operations as "stale," the controller can bypass the blockage and potentially transition the stuck resource to a failed state, thereby preventing future backups for that VM from being blocked indefinitely [1].

Citations:

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Use read-only Git transport and raw file retrieval because the GitHub API
# rate limit rejected the metadata request.
for remote in \
  https://github.com/migtools/kubevirt-datamover-controller.git \
  https://github.com/konveyor/kubevirt-datamover-controller.git
do
  echo "=== $remote refs ==="
  git ls-remote --heads --tags "$remote" 2>&1 | head -80 || true
done

for path in \
  cmd/main.go \
  main.go \
  controller/main.go \
  internal/controller/main.go \
  pkg/controller/main.go
do
  for branch in main master; do
    url="https://raw.githubusercontent.com/migtools/kubevirt-datamover-controller/$branch/$path"
    body=$(curl -LfsS --max-time 20 "$url" 2>/dev/null || true)
    if [ -n "$body" ]; then
      echo "=== $url ==="
      printf '%s\n' "$body" | grep -n -C 8 \
        -E 'stale-dataupload-threshold|staleDataUploadThreshold|flag\.Duration|2h' || true
    fi
  done
done

Repository: openshift/oadp-operator

Length of output: 2335

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

repo='https://github.com/migtools/kubevirt-datamover-controller.git'
echo '=== default branch and tags ==='
git ls-remote --symref "$repo" HEAD
git ls-remote --tags "$repo" | tail -40

branch=$(git ls-remote --symref "$repo" HEAD | awk '$1=="ref:" {sub("refs/heads/","",$2); print $2}')
[ -n "$branch" ] || branch=oadp-dev
echo "=== source files in $branch ==="
curl -LfsS --max-time 30 \
  "https://codeload.github.com/migtools/kubevirt-datamover-controller/tar.gz/refs/heads/$branch" |
  tar -tzf - | grep -E '\.(go|yaml|yml|md)$' | head -250

echo "=== flag and default references in $branch ==="
curl -LfsS --max-time 30 \
  "https://codeload.github.com/migtools/kubevirt-datamover-controller/tar.gz/refs/heads/$branch" |
  tar -xzf - --to-stdout --wildcards '*/go.mod' '*/main.go' '*/cmd/*.go' '*/internal/**/*.go' '*/pkg/**/*.go' 2>/dev/null |
  grep -n -C 8 -E \
    'stale-dataupload-threshold|staleDataUploadThreshold|flag\.Duration|2h' || true

Repository: openshift/oadp-operator

Length of output: 5167

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

repo='https://github.com/migtools/kubevirt-datamover-controller.git'
branch='oadp-dev'

echo '=== manager entry point ==='
curl -LfsS --max-time 30 \
  "https://codeload.github.com/migtools/kubevirt-datamover-controller/tar.gz/refs/heads/$branch" |
  tar -xzf - --to-stdout "kubevirt-datamover-controller-$branch/cmd/main.go" |
  cat -n

echo '=== all Go-source references on the default branch ==='
curl -LfsS --max-time 30 \
  "https://codeload.github.com/migtools/kubevirt-datamover-controller/tar.gz/refs/heads/$branch" |
  tar -xzf - --to-stdout --wildcards \
    "kubevirt-datamover-controller-$branch/*.go" \
    "kubevirt-datamover-controller-$branch/**/*.go" 2>/dev/null |
  grep -n -C 5 -E \
    'stale-dataupload-threshold|staleDataUploadThreshold|flag\.Duration|2h' || true

echo '=== flag references across all remote branches ==='
git ls-remote --heads "$repo" |
  awk '{sub("refs/heads/","",$2); print $2}' |
  while IFS= read -r branch; do
    encoded_branch=$(printf '%s' "$branch" | sed 's#/#%2F#g')
    matches=$(
      curl -LfsS --max-time 30 \
        "https://codeload.github.com/migtools/kubevirt-datamover-controller/tar.gz/refs/heads/$encoded_branch" |
      tar -xzf - --to-stdout --wildcards \
        "kubevirt-datamover-controller-$branch/*.go" \
        "kubevirt-datamover-controller-$branch/**/*.go" 2>/dev/null |
      grep -n -E 'stale-dataupload-threshold|staleDataUploadThreshold|flag\.Duration|2h' || true
    )
    if [ -n "$matches" ]; then
      echo "=== branch: $branch ==="
      printf '%s\n' "$matches"
    fi
  done

Repository: openshift/oadp-operator

Length of output: 13396

Do not pass an unsupported manager flag.

The kubevirt-datamover-controller manager does not declare --stale-dataupload-threshold or a 2h default. When configured, this argument causes manager startup to fail with an unknown-flag error. Add the flag and default to the manager first, or remove this operator argument.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/controller/kubevirt_datamover_controller.go` around lines 243 - 246,
The controller currently appends the unsupported --stale-dataupload-threshold
argument in the DPA argument-building flow; remove this append block unless the
kubevirt-datamover-controller manager is updated to declare the flag and its 2h
default first. Ensure configured StaleDataUploadThreshold values no longer
produce an unknown manager flag.

}

// Build container spec
Expand Down
94 changes: 94 additions & 0 deletions internal/controller/kubevirt_datamover_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"strings"
"testing"
"time"

"github.com/go-logr/logr"
"github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -748,6 +749,70 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) {
expectedEnvCount: 3,
expectError: false,
},
{
name: "Should include --stale-dataupload-threshold arg when configured",
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "test-dpa",
Namespace: "test-namespace",
ResourceVersion: "12345",
},
Spec: oadpv1alpha1.DataProtectionApplicationSpec{
Configuration: &oadpv1alpha1.ApplicationConfig{
Velero: &oadpv1alpha1.VeleroConfig{},
KubevirtDatamover: &oadpv1alpha1.KubevirtDatamoverConfig{
StaleDataUploadThreshold: &metav1.Duration{Duration: 1 * time.Hour},
},
},
},
},
existingContainers: nil,
expectedEnvCount: 3,
expectError: false,
},
{
name: "Should not include --stale-dataupload-threshold arg when not configured",
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "test-dpa",
Namespace: "test-namespace",
ResourceVersion: "12345",
},
Spec: oadpv1alpha1.DataProtectionApplicationSpec{
Configuration: &oadpv1alpha1.ApplicationConfig{
Velero: &oadpv1alpha1.VeleroConfig{},
},
},
},
existingContainers: nil,
expectedEnvCount: 3,
expectError: false,
},
{
name: "Should update --stale-dataupload-threshold arg on existing container",
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "test-dpa",
Namespace: "test-namespace",
ResourceVersion: "12345",
},
Spec: oadpv1alpha1.DataProtectionApplicationSpec{
Configuration: &oadpv1alpha1.ApplicationConfig{
Velero: &oadpv1alpha1.VeleroConfig{},
KubevirtDatamover: &oadpv1alpha1.KubevirtDatamoverConfig{
StaleDataUploadThreshold: &metav1.Duration{Duration: 3 * time.Hour},
},
},
},
},
existingContainers: []corev1.Container{{
Name: "manager",
Image: "old",
Args: []string{"--leader-elect", "--stale-dataupload-threshold=1h0m0s", "--old-arg"},
}},
expectedEnvCount: 3,
expectError: false,
},
{
name: "Should error when manager container not found",
dpa: &oadpv1alpha1.DataProtectionApplication{
Expand Down Expand Up @@ -888,6 +953,35 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) {
}
}

// Verify --stale-dataupload-threshold arg
if tt.dpa.Spec.Configuration != nil && tt.dpa.Spec.Configuration.KubevirtDatamover != nil &&
tt.dpa.Spec.Configuration.KubevirtDatamover.StaleDataUploadThreshold != nil {
expectedArg := fmt.Sprintf("--stale-dataupload-threshold=%s",
tt.dpa.Spec.Configuration.KubevirtDatamover.StaleDataUploadThreshold.Duration.String())
hasArg := false
staleArgCount := 0
for _, arg := range container.Args {
if strings.HasPrefix(arg, "--stale-dataupload-threshold=") {
staleArgCount++
}
if arg == expectedArg {
hasArg = true
}
}
if !hasArg {
t.Errorf("expected arg %s in container args %v", expectedArg, container.Args)
}
if staleArgCount != 1 {
t.Errorf("expected exactly one --stale-dataupload-threshold arg, got %d in %v", staleArgCount, container.Args)
}
} else {
for _, arg := range container.Args {
if strings.Contains(arg, "--stale-dataupload-threshold") {
t.Errorf("unexpected --stale-dataupload-threshold arg found: %s", arg)
}
}
}

// Verify security contexts (only checked for new deployments)
// Note: The function only sets security contexts when creating new containers,
// not when updating existing ones (static fields are not changed)
Expand Down