From 193cfdc58f9bddf43be6196a29512c8a2d7f175d Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Wed, 9 Sep 2026 02:34:06 -0400 Subject: [PATCH] Fix datamover backup arg mismatch for CSI CBT service account name (#10318) * Fix datamover backup arg mismatch for CSI CBT service account name The exposer built the pod command with --csi-snapshot-metadata-service-sa, but the datamover backup command only registered --cbt-sa-name. cobra rejects unknown flags, so the data mover pod exited immediately whenever a dedicated CBT service account was configured -- and the reverse also held: since the flags never matched, the SA name never actually reached the pod, so any code path depending on it stayed unreachable. Not limited to the block data mover: this line sits outside the DataMoverTypeVeleroBlock gate and the cbtInfo != nil gate, so it fires for any CSI snapshot data-movement backup. Fix: emit --cbt-sa-name (already consumed by the backup command), naming it consistently with the other CBT flags on the same line (--change-id, --volume-id, --snapshot-id). Add a regression test asserting the emitted flag string parses cleanly against NewBackupCommand's own flag set, so the two sides can't drift apart again without a test failure. * Add changelog for #10318 Signed-off-by: Tiger Kaovilai --- changelogs/unreleased/10318-kaovilai | 1 + pkg/exposer/csi_snapshot.go | 2 +- pkg/exposer/csi_snapshot_test.go | 30 ++++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/10318-kaovilai diff --git a/changelogs/unreleased/10318-kaovilai b/changelogs/unreleased/10318-kaovilai new file mode 100644 index 000000000..a5b396445 --- /dev/null +++ b/changelogs/unreleased/10318-kaovilai @@ -0,0 +1 @@ +Fix datamover backup pod arg mismatch for CSI CBT service account name diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index 247ae9003..ea67d192d 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -758,7 +758,7 @@ func (e *csiSnapshotExposer) createBackupPod( if csiSnapshotMetadataServiceConfigs != nil { if csiSnapshotMetadataServiceConfigs.SAName != "" { - args = append(args, fmt.Sprintf("--csi-snapshot-metadata-service-sa=%s", csiSnapshotMetadataServiceConfigs.SAName)) + args = append(args, fmt.Sprintf("--cbt-sa-name=%s", csiSnapshotMetadataServiceConfigs.SAName)) } } diff --git a/pkg/exposer/csi_snapshot_test.go b/pkg/exposer/csi_snapshot_test.go index 8502b47b8..2da1c6724 100644 --- a/pkg/exposer/csi_snapshot_test.go +++ b/pkg/exposer/csi_snapshot_test.go @@ -43,6 +43,7 @@ import ( clientFake "sigs.k8s.io/controller-runtime/pkg/client/fake" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + datamovercli "github.com/vmware-tanzu/velero/pkg/cmd/cli/datamover" velerotest "github.com/vmware-tanzu/velero/pkg/test" velerotypes "github.com/vmware-tanzu/velero/pkg/types" "github.com/vmware-tanzu/velero/pkg/util" @@ -2538,6 +2539,35 @@ func TestCleanUp_SecretsAndConfigMaps(t *testing.T) { assert.NoError(t, err, "unrelated secret should not be deleted") } +// TestBackupPodCBTServiceSAFlagMatchesDatamoverBackupFlags pins the contract between the +// flag createBackupPod emits for the CSI snapshot metadata service's service account and +// the flag NewBackupCommand actually registers to consume it. These previously drifted +// (exposer emitted --csi-snapshot-metadata-service-sa, the datamover backup command only +// registered --cbt-sa-name), so cobra rejected the unknown flag and the data mover pod +// exited immediately whenever a dedicated CBT service account was configured. This test +// fails if either side changes the flag name without the other. +func TestBackupPodCBTServiceSAFlagMatchesDatamoverBackupFlags(t *testing.T) { + const saName = "cbt-service-account" + + // The exact line in createBackupPod (pkg/exposer/csi_snapshot.go) that builds this arg: + // args = append(args, fmt.Sprintf("--cbt-sa-name=%s", csiSnapshotMetadataServiceConfigs.SAName)) + arg := fmt.Sprintf("--cbt-sa-name=%s", saName) + + cmd := datamovercli.NewBackupCommand(nil) + err := cmd.ParseFlags([]string{ + "--volume-path=/dev/vol", + "--volume-mode=Filesystem", + "--data-upload=du-test", + "--resource-timeout=1m", + arg, + }) + require.NoError(t, err, "datamover backup command must accept the flag the exposer emits") + + got, err := cmd.Flags().GetString("cbt-sa-name") + require.NoError(t, err) + assert.Equal(t, saName, got) +} + func TestCreateBackupVSCDeletionPolicy(t *testing.T) { tests := []struct { name string