From 81bd0447c385dd1b3c5d007f1811510e789ef63d Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Thu, 6 Aug 2026 13:51:20 -0700 Subject: [PATCH] feat: expose staleDataUploadThreshold in DPA KubevirtDatamoverConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add staleDataUploadThreshold field to KubevirtDatamoverConfig so the OADP operator passes --stale-dataupload-threshold to the kubevirt-datamover-controller deployment. This allows users to configure how long a stuck DataUpload can block future backups for the same VM before being skipped. Companion to migtools/kubevirt-datamover-controller#172. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../dataprotectionapplication_types.go | 6 ++ api/v1alpha1/zz_generated.deepcopy.go | 5 + ...enshift.io_dataprotectionapplications.yaml | 6 ++ ...enshift.io_dataprotectionapplications.yaml | 6 ++ .../kubevirt_datamover_controller.go | 4 + .../kubevirt_datamover_controller_test.go | 94 +++++++++++++++++++ 6 files changed, 121 insertions(+) diff --git a/api/v1alpha1/dataprotectionapplication_types.go b/api/v1alpha1/dataprotectionapplication_types.go index f26f5d51926..44671a2c04b 100644 --- a/api/v1alpha1/dataprotectionapplication_types.go +++ b/api/v1alpha1/dataprotectionapplication_types.go @@ -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 diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index aa1db83c2a0..6e29d168d19 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -745,6 +745,11 @@ func (in *KubevirtDatamoverConfig) DeepCopyInto(out *KubevirtDatamoverConfig) { *out = new(int32) **out = **in } + if in.StaleDataUploadThreshold != nil { + in, out := &in.StaleDataUploadThreshold, &out.StaleDataUploadThreshold + *out = new(v1.Duration) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubevirtDatamoverConfig. diff --git a/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml b/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml index a88ff5ad91b..c1a36217729 100644 --- a/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml +++ b/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml @@ -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 diff --git a/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml b/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml index b820f033624..55cb4db0d61 100644 --- a/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml +++ b/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml @@ -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 diff --git a/internal/controller/kubevirt_datamover_controller.go b/internal/controller/kubevirt_datamover_controller.go index 1bdce1416c0..b6dcf55c010 100644 --- a/internal/controller/kubevirt_datamover_controller.go +++ b/internal/controller/kubevirt_datamover_controller.go @@ -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())) + } } // Build container spec diff --git a/internal/controller/kubevirt_datamover_controller_test.go b/internal/controller/kubevirt_datamover_controller_test.go index 4c94f820e81..fce021ddf77 100644 --- a/internal/controller/kubevirt_datamover_controller_test.go +++ b/internal/controller/kubevirt_datamover_controller_test.go @@ -6,6 +6,7 @@ import ( "os" "strings" "testing" + "time" "github.com/go-logr/logr" "github.com/onsi/ginkgo/v2" @@ -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{ @@ -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)