diff --git a/internal/cleanup/cleanup_test.go b/internal/cleanup/cleanup_test.go index db9a04f..e51e669 100644 --- a/internal/cleanup/cleanup_test.go +++ b/internal/cleanup/cleanup_test.go @@ -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 @@ -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", }, @@ -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 @@ -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", }, diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 9e51e71..8db791a 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -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, @@ -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, diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go index 4cde149..3c6a1f2 100644 --- a/internal/testutil/testutil.go +++ b/internal/testutil/testutil.go @@ -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", diff --git a/internal/testutil/testutil_test.go b/internal/testutil/testutil_test.go index 5ee8791..e8ec2c0 100644 --- a/internal/testutil/testutil_test.go +++ b/internal/testutil/testutil_test.go @@ -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")) }) }) }) diff --git a/internal/vm/vm.go b/internal/vm/vm.go index 159c375..7c1c1ac 100644 --- a/internal/vm/vm.go +++ b/internal/vm/vm.go @@ -25,10 +25,11 @@ 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. @@ -36,8 +37,7 @@ 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 @@ -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 @@ -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{ diff --git a/internal/vm/vm_test.go b/internal/vm/vm_test.go index 0b4f673..88fc077 100644 --- a/internal/vm/vm_test.go +++ b/internal/vm/vm_test.go @@ -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", @@ -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() { @@ -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()) @@ -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() { @@ -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) @@ -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", + ), ) }) }) @@ -279,13 +270,13 @@ 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 @@ -293,12 +284,12 @@ var _ = Describe("CreateVM", func() { 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")) @@ -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() @@ -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()