Skip to content

fix: remove dead inline userdata branch in BuildVMSpec #195

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
mrhillsman merged 1 commit into from
Jun 13, 2026
Merged

fix: remove dead inline userdata branch in BuildVMSpec #195

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
52 changes: 26 additions & 26 deletions internal/cleanup/cleanup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ var _ = Describe("PreviewCleanup", func() {
maps.Copy(l, el)
}
v, err := vm.BuildVMSpec(vm.VMSpecOpts{
Name: name,
Namespace: namespace,
ContainerDiskImage: "test-image",
CloudInitUserdata: "#cloud-config\n",
CPUCores: 1,
Memory: "1Gi",
Labels: l,
Name: name,
Namespace: namespace,
ContainerDiskImage: "test-image",
CloudInitSecretName: "test-cloudinit",
CPUCores: 1,
Memory: "1Gi",
Labels: l,
})
Expect(err).NotTo(HaveOccurred())
return v
Expand Down Expand Up @@ -168,12 +168,12 @@ var _ = Describe("PreviewCleanup", func() {
It("should not count unmanaged resources", func() {
managedVM := newManagedVM("managed-vm")
unmanagedVM, err := vm.BuildVMSpec(vm.VMSpecOpts{
Name: "unmanaged-vm",
Namespace: namespace,
ContainerDiskImage: "test-image",
CloudInitUserdata: "#cloud-config\n",
CPUCores: 1,
Memory: "1Gi",
Name: "unmanaged-vm",
Namespace: namespace,
ContainerDiskImage: "test-image",
CloudInitSecretName: "test-cloudinit",
CPUCores: 1,
Memory: "1Gi",
Labels: map[string]string{
constants.LabelManagedBy: "other-tool",
},
Expand Down Expand Up @@ -329,13 +329,13 @@ var _ = Describe("CleanupAll", func() {

newManagedVM := func(name string) *kubevirtv1.VirtualMachine {
v, err := vm.BuildVMSpec(vm.VMSpecOpts{
Name: name,
Namespace: namespace,
ContainerDiskImage: "test-image",
CloudInitUserdata: "#cloud-config\n",
CPUCores: 1,
Memory: "1Gi",
Labels: labels,
Name: name,
Namespace: namespace,
ContainerDiskImage: "test-image",
CloudInitSecretName: "test-cloudinit",
CPUCores: 1,
Memory: "1Gi",
Labels: labels,
})
Expect(err).NotTo(HaveOccurred())
return v
Expand Down Expand Up @@ -745,12 +745,12 @@ var _ = Describe("CleanupAll", func() {
// Create a managed VM and an unmanaged VM
managedVM := newManagedVM("managed-vm")
unmanagedVM, err := vm.BuildVMSpec(vm.VMSpecOpts{
Name: "unmanaged-vm",
Namespace: namespace,
ContainerDiskImage: "test-image",
CloudInitUserdata: "#cloud-config\n",
CPUCores: 1,
Memory: "1Gi",
Name: "unmanaged-vm",
Namespace: namespace,
ContainerDiskImage: "test-image",
CloudInitSecretName: "test-cloudinit",
CPUCores: 1,
Memory: "1Gi",
Labels: map[string]string{
constants.LabelManagedBy: "other-tool",
},
Expand Down
2 changes: 0 additions & 2 deletions internal/orchestrator/orchestrator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,6 @@ func (ro *RunOrchestrator) createVMs(
Name: p.VMSpec.Name,
Namespace: p.VMSpec.Namespace,
ContainerDiskImage: p.VMSpec.ContainerDiskImage,
CloudInitUserdata: p.VMSpec.CloudInitUserdata,
CloudInitSecretName: p.VMSpec.CloudInitSecretName,
CPUCores: p.VMSpec.CPUCores,
Memory: p.VMSpec.Memory,
Expand Down Expand Up @@ -764,7 +763,6 @@ func (ro *RunOrchestrator) printDryRun(
Name: p.VMSpec.Name,
Namespace: p.VMSpec.Namespace,
ContainerDiskImage: p.VMSpec.ContainerDiskImage,
CloudInitUserdata: p.VMSpec.CloudInitUserdata,
CloudInitSecretName: p.VMSpec.CloudInitSecretName,
CPUCores: p.VMSpec.CPUCores,
Memory: p.VMSpec.Memory,
Expand Down
12 changes: 6 additions & 6 deletions internal/testutil/testutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@ func CleanupNamespace(ctx context.Context, c client.Client, namespace string) {
// vm.CreateVM(). Modify the returned options as needed for specific tests.
func DefaultVMOpts(name, namespace string) vm.VMSpecOpts {
return vm.VMSpecOpts{
Name: name,
Namespace: namespace,
ContainerDiskImage: constants.DefaultContainerDiskImage,
CloudInitUserdata: "#cloud-config\n",
CPUCores: 1,
Memory: "512Mi",
Name: name,
Namespace: namespace,
ContainerDiskImage: constants.DefaultContainerDiskImage,
CloudInitSecretName: "test-cloudinit",
CPUCores: 1,
Memory: "512Mi",
Labels: map[string]string{
constants.LabelManagedBy: constants.ManagedByValue,
constants.LabelAppName: "virtwork",
Expand Down
4 changes: 2 additions & 2 deletions internal/testutil/testutil_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ var _ = Describe("DefaultVMOpts", func() {
Expect(opts.Labels).To(HaveKeyWithValue(constants.LabelComponent, "test"))
})

It("should have a simple cloud-init userdata", func() {
It("should have a cloud-init secret name", func() {
opts := testutil.DefaultVMOpts(vmName, vmNamespace)
Expect(opts.CloudInitUserdata).To(Equal("#cloud-config\n"))
Expect(opts.CloudInitSecretName).To(Equal("test-cloudinit"))
})
})
})
41 changes: 16 additions & 25 deletions internal/vm/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@ const defaultMaxRetries = 5
var baseRetryBackoff = time.Second

var (
ErrInvalidName = errors.New("invalid name: must not be empty")
ErrInvalidNamespace = errors.New("invalid namespace: must not be empty")
ErrInvalidCPUCores = errors.New("invalid cpuCores: must be a positive integer")
ErrInvalidContainerDiskImage = errors.New("invalid containerDiskImage: must not be empty")
ErrInvalidName = errors.New("invalid name: must not be empty")
ErrInvalidNamespace = errors.New("invalid namespace: must not be empty")
ErrInvalidCPUCores = errors.New("invalid cpuCores: must be a positive integer")
ErrInvalidContainerDiskImage = errors.New("invalid containerDiskImage: must not be empty")
ErrInvalidCloudInitSecretName = errors.New("invalid cloudInitSecretName: must not be empty")
)

// VMSpecOpts contains all parameters needed to construct a VirtualMachine spec.
type VMSpecOpts struct {
Name string
Namespace string
ContainerDiskImage string
CloudInitUserdata string
CloudInitSecretName string // When set, use UserDataSecretRef instead of inline
CloudInitSecretName string
CPUCores int
Memory string
Labels map[string]string
Expand All @@ -62,6 +62,9 @@ func BuildVMSpec(opts VMSpecOpts) (*kubevirtv1.VirtualMachine, error) {
if opts.ContainerDiskImage == "" {
return nil, ErrInvalidContainerDiskImage
}
if opts.CloudInitSecretName == "" {
return nil, ErrInvalidCloudInitSecretName
}

runStrategy := kubevirtv1.RunStrategyAlways

Expand All @@ -85,27 +88,15 @@ func BuildVMSpec(opts VMSpecOpts) (*kubevirtv1.VirtualMachine, error) {
}
disks = append(disks, opts.ExtraDisks...)

var cloudInitVolume kubevirtv1.Volume
if opts.CloudInitSecretName != "" {
cloudInitVolume = kubevirtv1.Volume{
Name: constants.DiskNameCloudInit,
VolumeSource: kubevirtv1.VolumeSource{
CloudInitNoCloud: &kubevirtv1.CloudInitNoCloudSource{
UserDataSecretRef: &corev1.LocalObjectReference{
Name: opts.CloudInitSecretName,
},
cloudInitVolume := kubevirtv1.Volume{
Name: constants.DiskNameCloudInit,
VolumeSource: kubevirtv1.VolumeSource{
CloudInitNoCloud: &kubevirtv1.CloudInitNoCloudSource{
UserDataSecretRef: &corev1.LocalObjectReference{
Name: opts.CloudInitSecretName,
},
},
}
} else {
cloudInitVolume = kubevirtv1.Volume{
Name: constants.DiskNameCloudInit,
VolumeSource: kubevirtv1.VolumeSource{
CloudInitNoCloud: &kubevirtv1.CloudInitNoCloudSource{
UserData: opts.CloudInitUserdata,
},
},
}
},
}

volumes := []kubevirtv1.Volume{
Expand Down
125 changes: 58 additions & 67 deletions internal/vm/vm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ var _ = Describe("BuildVMSpec", func() {

BeforeEach(func() {
opts = vm.VMSpecOpts{
Name: "test-vm",
Namespace: "test-ns",
ContainerDiskImage: "quay.io/containerdisks/fedora:41",
CloudInitUserdata: "#cloud-config\n",
CPUCores: 2,
Memory: "2Gi",
Name: "test-vm",
Namespace: "test-ns",
ContainerDiskImage: "quay.io/containerdisks/fedora:41",
CloudInitSecretName: "test-vm-cloudinit",
CPUCores: 2,
Memory: "2Gi",
Labels: map[string]string{
"app.kubernetes.io/managed-by": "virtwork",
"app.kubernetes.io/name": "cpu",
Expand Down Expand Up @@ -82,7 +82,8 @@ var _ = Describe("BuildVMSpec", func() {
}
Expect(cloudInit).NotTo(BeNil())
Expect(cloudInit.CloudInitNoCloud).NotTo(BeNil())
Expect(cloudInit.CloudInitNoCloud.UserData).To(Equal("#cloud-config\n"))
Expect(cloudInit.CloudInitNoCloud.UserDataSecretRef).NotTo(BeNil())
Expect(cloudInit.CloudInitNoCloud.UserDataSecretRef.Name).To(Equal("test-vm-cloudinit"))
})

It("should set labels", func() {
Expand Down Expand Up @@ -120,8 +121,7 @@ var _ = Describe("BuildVMSpec", func() {
Expect(networks[0].Pod).NotTo(BeNil())
})

It("should use UserDataSecretRef when CloudInitSecretName is set", func() {
opts.CloudInitSecretName = "my-vm-cloudinit"
It("should use UserDataSecretRef from CloudInitSecretName", func() {
var err error
result, err = vm.BuildVMSpec(opts)
Expect(err).NotTo(HaveOccurred())
Expand All @@ -137,28 +137,14 @@ var _ = Describe("BuildVMSpec", func() {
Expect(cloudInit).NotTo(BeNil())
Expect(cloudInit.CloudInitNoCloud).NotTo(BeNil())
Expect(cloudInit.CloudInitNoCloud.UserDataSecretRef).NotTo(BeNil())
Expect(cloudInit.CloudInitNoCloud.UserDataSecretRef.Name).To(Equal("my-vm-cloudinit"))
Expect(cloudInit.CloudInitNoCloud.UserDataSecretRef.Name).To(Equal("test-vm-cloudinit"))
Expect(cloudInit.CloudInitNoCloud.UserData).To(BeEmpty())
})

It("should use inline UserData when CloudInitSecretName is empty", func() {
It("should return error when CloudInitSecretName is empty", func() {
opts.CloudInitSecretName = ""
var err error
result, err = vm.BuildVMSpec(opts)
Expect(err).NotTo(HaveOccurred())

volumes := result.Spec.Template.Spec.Volumes
var cloudInit *kubevirtv1.Volume
for i := range volumes {
if volumes[i].Name == "cloudinitdisk" {
cloudInit = &volumes[i]
break
}
}
Expect(cloudInit).NotTo(BeNil())
Expect(cloudInit.CloudInitNoCloud).NotTo(BeNil())
Expect(cloudInit.CloudInitNoCloud.UserData).To(Equal("#cloud-config\n"))
Expect(cloudInit.CloudInitNoCloud.UserDataSecretRef).To(BeNil())
_, err := vm.BuildVMSpec(opts)
Expect(err).To(MatchError(vm.ErrInvalidCloudInitSecretName))
})

It("should include extra disks when provided", func() {
Expand Down Expand Up @@ -209,13 +195,13 @@ var _ = Describe("BuildVMSpec", func() {
"should reject invalid inputs",
func(mutate func(*vm.VMSpecOpts), substr string) {
good := vm.VMSpecOpts{
Name: "test-vm",
Namespace: "test-ns",
ContainerDiskImage: "quay.io/containerdisks/fedora:41",
CloudInitUserdata: "#cloud-config\n",
CPUCores: 2,
Memory: "2Gi",
Labels: map[string]string{"test": "true"},
Name: "test-vm",
Namespace: "test-ns",
ContainerDiskImage: "quay.io/containerdisks/fedora:41",
CloudInitSecretName: "test-cloudinit",
CPUCores: 2,
Memory: "2Gi",
Labels: map[string]string{"test": "true"},
}
mutate(&good)
_, err := vm.BuildVMSpec(good)
Expand All @@ -231,6 +217,11 @@ var _ = Describe("BuildVMSpec", func() {
func(o *vm.VMSpecOpts) { o.ContainerDiskImage = "" },
"containerDiskImage",
),
Entry(
"empty cloud init secret name",
func(o *vm.VMSpecOpts) { o.CloudInitSecretName = "" },
"cloudInitSecretName",
),
)
})
})
Expand Down Expand Up @@ -279,26 +270,26 @@ var _ = Describe("CreateVM", func() {

newTestVM := func(name string) *kubevirtv1.VirtualMachine {
v, err := vm.BuildVMSpec(vm.VMSpecOpts{
Name: name,
Namespace: "default",
ContainerDiskImage: "test-image",
CloudInitUserdata: "#cloud-config\n",
CPUCores: 1,
Memory: "1Gi",
Labels: map[string]string{"test": "true"},
Name: name,
Namespace: "default",
ContainerDiskImage: "test-image",
CloudInitSecretName: "test-cloudinit",
CPUCores: 1,
Memory: "1Gi",
Labels: map[string]string{"test": "true"},
})
Expect(err).NotTo(HaveOccurred())
return v
}

It("should return error for invalid memory", func() {
_, err := vm.BuildVMSpec(vm.VMSpecOpts{
Name: "bad-mem",
Namespace: "default",
ContainerDiskImage: "test-image",
CloudInitUserdata: "#cloud-config\n",
CPUCores: 1,
Memory: "not-a-quantity",
Name: "bad-mem",
Namespace: "default",
ContainerDiskImage: "test-image",
CloudInitSecretName: "test-cloudinit",
CPUCores: 1,
Memory: "not-a-quantity",
})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("invalid memory"))
Expand Down Expand Up @@ -398,12 +389,12 @@ var _ = Describe("DeleteVM", func() {

It("should delete VM successfully", func() {
testVM, err := vm.BuildVMSpec(vm.VMSpecOpts{
Name: "delete-me",
Namespace: "default",
ContainerDiskImage: "test-image",
CloudInitUserdata: "#cloud-config\n",
CPUCores: 1,
Memory: "1Gi",
Name: "delete-me",
Namespace: "default",
ContainerDiskImage: "test-image",
CloudInitSecretName: "test-cloudinit",
CPUCores: 1,
Memory: "1Gi",
})
Expect(err).NotTo(HaveOccurred())
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(testVM).Build()
Expand Down Expand Up @@ -436,23 +427,23 @@ var _ = Describe("ListVMs", func() {

It("should list VMs by labels", func() {
vm1, err := vm.BuildVMSpec(vm.VMSpecOpts{
Name: "vm-1",
Namespace: "default",
ContainerDiskImage: "test-image",
CloudInitUserdata: "#cloud-config\n",
CPUCores: 1,
Memory: "1Gi",
Labels: map[string]string{"app.kubernetes.io/managed-by": "virtwork"},
Name: "vm-1",
Namespace: "default",
ContainerDiskImage: "test-image",
CloudInitSecretName: "test-cloudinit",
CPUCores: 1,
Memory: "1Gi",
Labels: map[string]string{"app.kubernetes.io/managed-by": "virtwork"},
})
Expect(err).NotTo(HaveOccurred())
vm2, err := vm.BuildVMSpec(vm.VMSpecOpts{
Name: "vm-2",
Namespace: "default",
ContainerDiskImage: "test-image",
CloudInitUserdata: "#cloud-config\n",
CPUCores: 1,
Memory: "1Gi",
Labels: map[string]string{"app.kubernetes.io/managed-by": "other"},
Name: "vm-2",
Namespace: "default",
ContainerDiskImage: "test-image",
CloudInitSecretName: "test-cloudinit",
CPUCores: 1,
Memory: "1Gi",
Labels: map[string]string{"app.kubernetes.io/managed-by": "other"},
})
Expect(err).NotTo(HaveOccurred())
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vm1, vm2).Build()
Expand Down
Loading