Skip to content

Pass DATAMOVER_IMAGE env var to kubevirt-datamover-controller deployment #2172

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

Open
shubham-pampattiwar wants to merge 1 commit into
base: oadp-dev
Choose a base branch
from
Open

Pass DATAMOVER_IMAGE env var to kubevirt-datamover-controller deployment #2172

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
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 @@ -152,6 +152,10 @@ func ensureKubevirtDatamoverRequiredSpecs(
Name: "WATCH_NAMESPACE",
Value: deploymentObject.Namespace,
},
{
Name: "DATAMOVER_IMAGE",
Copy link
Copy Markdown
Contributor

@mpryc mpryc Apr 22, 2026

Choose a reason for hiding this comment

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

Should we have KUBEVIRT_DATAMOVER_IMAGE ? to not confuse with DM ?

Copy link
Copy Markdown
Member Author

@shubham-pampattiwar shubham-pampattiwar Apr 22, 2026

Choose a reason for hiding this comment

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

The env var lives on the kubevirt-datamover-controller deployment, so DATAMOVER_IMAGE is unambiguous in that context, adding a KUBEVIRT_ prefix would be redundant. But I get your point. Happy to rename if you still prefer it though.

Value: image,
},
}

// Add log level if configured
Expand Down
54 changes: 42 additions & 12 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 @@ -566,7 +566,7 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) {
},
},
existingContainers: nil,
expectedEnvCount: 2, // WATCH_NAMESPACE, LOG_LEVEL (empty value)
expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty value)
expectError: false,
},
{
Expand All @@ -584,7 +584,7 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) {
},
},
existingContainers: []corev1.Container{{Name: "manager", Image: "old"}},
expectedEnvCount: 2, // WATCH_NAMESPACE, LOG_LEVEL (empty value)
expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty value)
expectError: false,
},
{
Expand All @@ -604,7 +604,7 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) {
},
},
existingContainers: nil,
expectedEnvCount: 2, // WATCH_NAMESPACE, LOG_LEVEL with value
expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL with value
expectError: false,
},
{
Expand All @@ -623,7 +623,7 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) {
},
},
existingContainers: nil,
expectedEnvCount: 3, // WATCH_NAMESPACE, LOG_LEVEL (empty), LOG_FORMAT
expectedEnvCount: 4, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty), LOG_FORMAT
expectError: false,
},
{
Expand All @@ -644,7 +644,7 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) {
},
},
existingContainers: nil,
expectedEnvCount: 3, // WATCH_NAMESPACE, LOG_LEVEL, LOG_FORMAT
expectedEnvCount: 4, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL, LOG_FORMAT
expectError: false,
},
{
Expand All @@ -662,7 +662,7 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) {
},
},
existingContainers: nil,
expectedEnvCount: 2, // WATCH_NAMESPACE, LOG_LEVEL (empty value)
expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty value)
expectError: false,
},
{
Expand All @@ -680,7 +680,7 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) {
},
},
existingContainers: nil,
expectedEnvCount: 2, // WATCH_NAMESPACE, LOG_LEVEL (empty)
expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty)
expectError: false,
},
{
Expand Down Expand Up @@ -779,6 +779,21 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) {
t.Error("WATCH_NAMESPACE env var not found")
}

// Verify DATAMOVER_IMAGE is always present and matches the image
hasDatamoverImage := false
for _, env := range container.Env {
if env.Name == "DATAMOVER_IMAGE" {
hasDatamoverImage = true
if env.Value != defaultKubevirtDatamoverImage {
t.Errorf("DATAMOVER_IMAGE: expected %s, got %s", defaultKubevirtDatamoverImage, env.Value)
}
break
}
}
if !hasDatamoverImage {
t.Error("DATAMOVER_IMAGE env var not found")
}

// 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 Expand Up @@ -871,7 +886,7 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) {
},
},
expectedImage: defaultKubevirtDatamoverImage,
expectedEnvCount: 2, // WATCH_NAMESPACE, LOG_LEVEL (empty)
expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty)
expectedReplicas: 1,
expectedSAName: kubevirtDatamoverObjectName,
expectError: false,
Expand All @@ -894,7 +909,7 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) {
},
},
expectedImage: "custom-registry.io/kdm:v1.0",
expectedEnvCount: 2, // WATCH_NAMESPACE, LOG_LEVEL (empty)
expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty)
expectedReplicas: 1,
expectedSAName: kubevirtDatamoverObjectName,
expectError: false,
Expand All @@ -917,7 +932,7 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) {
"RELATED_IMAGE_KUBEVIRT_DATAMOVER_CONTROLLER": "env-registry.io/kdm:v2.0",
},
expectedImage: "env-registry.io/kdm:v2.0",
expectedEnvCount: 2, // WATCH_NAMESPACE, LOG_LEVEL (empty)
expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty)
expectedReplicas: 1,
expectedSAName: kubevirtDatamoverObjectName,
expectError: false,
Expand All @@ -940,7 +955,7 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) {
},
},
expectedImage: defaultKubevirtDatamoverImage,
expectedEnvCount: 3, // WATCH_NAMESPACE, LOG_LEVEL, LOG_FORMAT
expectedEnvCount: 4, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL, LOG_FORMAT
expectedReplicas: 1,
expectedSAName: kubevirtDatamoverObjectName,
expectError: false,
Expand All @@ -960,7 +975,7 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) {
},
},
expectedImage: defaultKubevirtDatamoverImage,
expectedEnvCount: 2, // WATCH_NAMESPACE, LOG_LEVEL (empty)
expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty)
expectedReplicas: 1,
expectedSAName: kubevirtDatamoverObjectName,
expectError: false,
Expand Down Expand Up @@ -1030,6 +1045,21 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) {
t.Errorf("env var count: expected %d, got %d", tt.expectedEnvCount, len(container.Env))
}

// Verify DATAMOVER_IMAGE env var matches the container image
hasDatamoverImage := false
for _, env := range container.Env {
if env.Name == "DATAMOVER_IMAGE" {
hasDatamoverImage = true
if env.Value != tt.expectedImage {
t.Errorf("DATAMOVER_IMAGE: expected %s, got %s", tt.expectedImage, env.Value)
}
break
}
}
if !hasDatamoverImage {
t.Error("DATAMOVER_IMAGE env var not found")
}

// Verify security contexts
if deployment.Spec.Template.Spec.SecurityContext.RunAsNonRoot == nil || !*deployment.Spec.Template.Spec.SecurityContext.RunAsNonRoot {
t.Error("expected runAsNonRoot to be true")
Expand Down