diff --git a/examples/vmi-kernel-boot.yaml b/examples/vmi-kernel-boot.yaml index aa69bcf87b29..1e76b5b8be4d 100644 --- a/examples/vmi-kernel-boot.yaml +++ b/examples/vmi-kernel-boot.yaml @@ -15,8 +15,6 @@ spec: initrdPath: /boot/initramfs-virt kernelPath: /boot/vmlinuz-virt kernelArgs: console=ttyS0 - machine: - type: "" resources: requests: memory: 1Gi diff --git a/pkg/container-disk/container-disk.go b/pkg/container-disk/container-disk.go index 5c85236fab15..3ca719bcbf3e 100644 --- a/pkg/container-disk/container-disk.go +++ b/pkg/container-disk/container-disk.go @@ -214,11 +214,7 @@ func GenerateContainers(vmi *v1.VirtualMachineInstance, podVolumeName string, bi } func GenerateKernelBootContainer(vmi *v1.VirtualMachineInstance, podVolumeName string, binVolumeName string) *kubev1.Container { - if !util.IsKernelBootDefinedProperly(vmi) { - if f := vmi.Spec.Domain.Firmware; f != nil && f.KernelBoot != nil && f.KernelBoot.Container != nil && - f.KernelBoot.Container.InitrdPath == "" && f.KernelBoot.Container.KernelPath == "" { - log.Log.Object(vmi).Warning("Kernel boot container does not contain kernel path nor initrd path - ignoring kernel-boot configuration") - } + if !util.HasKernelBootContainerImage(vmi) { return nil } diff --git a/pkg/util/util.go b/pkg/util/util.go index 1afaf8feb051..cb45a84a242e 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -71,7 +71,7 @@ func ResourceNameToEnvVar(prefix string, resourceName string) string { } // Checks if kernel boot is defined in a valid way -func IsKernelBootDefinedProperly(vmi *v1.VirtualMachineInstance) bool { +func HasKernelBootContainerImage(vmi *v1.VirtualMachineInstance) bool { if vmi == nil { return false } @@ -81,9 +81,5 @@ func IsKernelBootDefinedProperly(vmi *v1.VirtualMachineInstance) bool { return false } - if c := vmiFirmware.KernelBoot.Container; (c.KernelPath == "") && (c.InitrdPath == "") { - return false - } - return true } diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/BUILD.bazel b/pkg/virt-api/webhooks/validating-webhook/admitters/BUILD.bazel index d3eb0ff56a27..f8fa7794dbd6 100644 --- a/pkg/virt-api/webhooks/validating-webhook/admitters/BUILD.bazel +++ b/pkg/virt-api/webhooks/validating-webhook/admitters/BUILD.bazel @@ -74,6 +74,7 @@ go_test( "//staging/src/kubevirt.io/client-go/generated/kubevirt/clientset/versioned/fake:go_default_library", "//staging/src/kubevirt.io/client-go/kubecli:go_default_library", "//staging/src/kubevirt.io/client-go/testutils:go_default_library", + "//tools/vms-generator/utils:go_default_library", "//vendor/github.com/golang/mock/gomock:go_default_library", "//vendor/github.com/onsi/ginkgo:go_default_library", "//vendor/github.com/onsi/ginkgo/extensions/table:go_default_library", diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter.go b/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter.go index 5735658e5b8d..5ff5a534ff55 100644 --- a/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter.go +++ b/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter.go @@ -1568,6 +1568,7 @@ func validateFirmware(field *k8sfield.Path, firmware *v1.Firmware) []metav1.Stat if firmware != nil { causes = append(causes, validateBootloader(field.Child("bootloader"), firmware.Bootloader)...) + causes = append(causes, validateKernelBoot(field.Child("kernelBoot"), firmware.KernelBoot)...) } return causes @@ -2274,3 +2275,31 @@ func validateDisks(field *k8sfield.Path, disks []v1.Disk) []metav1.StatusCause { return causes } + +// Rejects kernel boot defined with initrd/kernel path but without an image +func validateKernelBoot(field *k8sfield.Path, kernelBoot *v1.KernelBoot) (causes []metav1.StatusCause) { + if kernelBoot == nil || kernelBoot.Container == nil { + return + } + + container := kernelBoot.Container + containerField := field.Child("container").String() + + if container.Image == "" { + causes = append(causes, metav1.StatusCause{ + Type: metav1.CauseTypeFieldValueRequired, + Message: fmt.Sprintf("%s must be defined with an image", containerField), + Field: containerField, + }) + } + + if container.InitrdPath == "" && container.KernelPath == "" { + causes = append(causes, metav1.StatusCause{ + Type: metav1.CauseTypeFieldValueRequired, + Message: fmt.Sprintf("%s must be defined with at least one of the following: kernelPath, initrdPath", containerField), + Field: containerField, + }) + } + + return +} diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter_test.go b/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter_test.go index b7aadf4a61ab..c8d40c37efef 100644 --- a/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter_test.go +++ b/pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter_test.go @@ -25,6 +25,8 @@ import ( "strconv" "strings" + "kubevirt.io/kubevirt/tools/vms-generator/utils" + "kubevirt.io/kubevirt/pkg/virt-operator/resource/generate/rbac" . "github.com/onsi/ginkgo" @@ -2213,6 +2215,57 @@ var _ = Describe("Validating VMICreate Admitter", func() { Expect(causes[0].Field).To(Equal("fake.startStrategy")) Expect(causes[0].Message).To(Equal("either fake.startStrategy or fake.livenessProbe should be provided.Pausing VMI with LivenessProbe is not supported")) }) + Context("with kernel boot defined", func() { + + const ( + fakeKernelArgs = "args" + fakeImage = "image" + fakeInitrd = "initrd" + fakeKernel = "kernel" + ) + + table.DescribeTable("", func(kernelArgs, initrdPath, kernelPath, image string, defineContainerNil bool, shouldBeValid bool) { + vmi := utils.GetVMIKernelBoot() + + kb := vmi.Spec.Domain.Firmware.KernelBoot + + if defineContainerNil { + kb.Container = nil + } else { + kb.KernelArgs = kernelArgs + kb.Container.KernelPath = kernelPath + kb.Container.InitrdPath = initrdPath + kb.Container.Image = image + } + + kernelBootField := k8sfield.NewPath("spec").Child("domain").Child("firmware").Child("kernelBoot") + causes := validateKernelBoot(kernelBootField, kb) + + if shouldBeValid { + Expect(causes).To(BeEmpty()) + } else { + Expect(causes).ToNot(BeEmpty()) + } + }, + table.Entry("without kernel args and null container - should approve", + "", "", "", "", true, true), + table.Entry("with kernel args and null container - should approve", + fakeKernelArgs, "", "", "", true, true), + table.Entry("without kernel args, with container that has image & kernel & initrd defined - should approve", + "", fakeInitrd, fakeKernel, fakeImage, false, true), + table.Entry("with kernel args, with container that has image & kernel & initrd defined - should approve", + fakeKernelArgs, fakeInitrd, fakeKernel, fakeImage, false, true), + table.Entry("with kernel args, with container that has image & kernel defined - should approve", + fakeKernelArgs, "", fakeKernel, fakeImage, false, true), + table.Entry("with kernel args, with container that has image & initrd defined - should approve", + fakeKernelArgs, fakeInitrd, "", fakeImage, false, true), + table.Entry("with kernel args, with container that has only image defined - should reject", + fakeKernelArgs, "", "", fakeImage, false, false), + table.Entry("with kernel args, with container that has initrd and kernel defined but without image - should reject", + fakeKernelArgs, fakeInitrd, fakeKernel, "", false, false), + table.Entry("with kernel args, with container that has nothing defined", "", "", "", "", false, false), + ) + }) }) Context("with cpu pinning", func() { var vmi *v1.VirtualMachineInstance diff --git a/pkg/virt-controller/services/template.go b/pkg/virt-controller/services/template.go index a9150837437f..1df44fecba95 100644 --- a/pkg/virt-controller/services/template.go +++ b/pkg/virt-controller/services/template.go @@ -1212,7 +1212,7 @@ func (t *templateService) renderLaunchManifest(vmi *v1.VirtualMachineInstance, t var initContainers []k8sv1.Container - if HaveContainerDiskVolume(vmi.Spec.Volumes) || util.IsKernelBootDefinedProperly(vmi) { + if HaveContainerDiskVolume(vmi.Spec.Volumes) || util.HasKernelBootContainerImage(vmi) { initContainerVolumeMounts := []k8sv1.VolumeMount{ { diff --git a/pkg/virt-handler/container-disk/mount.go b/pkg/virt-handler/container-disk/mount.go index f0cb22157910..fd6e66b7a51b 100644 --- a/pkg/virt-handler/container-disk/mount.go +++ b/pkg/virt-handler/container-disk/mount.go @@ -375,7 +375,7 @@ func (m *mounter) MountKernelArtifacts(vmi *v1.VirtualMachineInstance, verify bo log.Log.Object(vmi).Infof("mounting kernel artifacts") - if !util.IsKernelBootDefinedProperly(vmi) { + if !util.HasKernelBootContainerImage(vmi) { log.Log.Object(vmi).Infof("kernel boot not defined - nothing to mount") return nil } @@ -485,7 +485,7 @@ func (m *mounter) MountKernelArtifacts(vmi *v1.VirtualMachineInstance, verify bo } func (m *mounter) UnmountKernelArtifacts(vmi *v1.VirtualMachineInstance) error { - if !util.IsKernelBootDefinedProperly(vmi) { + if !util.HasKernelBootContainerImage(vmi) { return nil } diff --git a/pkg/virt-launcher/virtwrap/converter/converter.go b/pkg/virt-launcher/virtwrap/converter/converter.go index 15d0abde2b6c..d17ffda5ae68 100644 --- a/pkg/virt-launcher/virtwrap/converter/converter.go +++ b/pkg/virt-launcher/virtwrap/converter/converter.go @@ -1154,7 +1154,7 @@ func Convert_v1_VirtualMachineInstance_To_api_Domain(vmi *v1.VirtualMachineInsta domain.Spec.SysInfo.System = append(domain.Spec.SysInfo.System, api.Entry{Name: "serial", Value: string(vmi.Spec.Domain.Firmware.Serial)}) } - if util.IsKernelBootDefinedProperly(vmi) { + if util.HasKernelBootContainerImage(vmi) { kb := vmi.Spec.Domain.Firmware.KernelBoot log.Log.Object(vmi).Infof("kernel boot defined for VMI. Converting to domain XML") diff --git a/pkg/virt-operator/resource/generate/components/validations_generated.go b/pkg/virt-operator/resource/generate/components/validations_generated.go index 64539cdf1410..317a58e22a87 100644 --- a/pkg/virt-operator/resource/generate/components/validations_generated.go +++ b/pkg/virt-operator/resource/generate/components/validations_generated.go @@ -4240,28 +4240,38 @@ var CRDsValidation map[string]string = map[string]string{ description: Settings to set the kernel for booting. properties: container: - description: Container defines the container that containes kernel artifacts + description: Container defines the container that containes + kernel artifacts properties: image: - description: Image that container initrd / kernel files. + description: Image that container initrd / kernel + files. type: string imagePullPolicy: - description: 'Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. Cannot be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images' + description: 'Image pull policy. One of Always, + Never, IfNotPresent. Defaults to Always if :latest + tag is specified, or IfNotPresent otherwise. Cannot + be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images' type: string imagePullSecret: - description: ImagePullSecret is the name of the Docker registry secret required to pull the image. The secret must already exist. + description: ImagePullSecret is the name of the + Docker registry secret required to pull the image. + The secret must already exist. type: string initrdPath: - description: the fully-qualified path to the ramdisk image in the host OS + description: the fully-qualified path to the ramdisk + image in the host OS type: string kernelPath: - description: The fully-qualified path to the kernel image in the host OS + description: The fully-qualified path to the kernel + image in the host OS type: string required: - image type: object kernelArgs: - description: Arguments to be passed to the kernel at boot time + description: Arguments to be passed to the kernel at + boot time type: string type: object serial: @@ -4896,7 +4906,10 @@ var CRDsValidation map[string]string = map[string]string{ type: string type: object downwardMetrics: - description: DownwardMetrics adds a very small disk to VMIs which contains a limited view of host and guest metrics. The disk content is compatible with vhostmd (https://github.com/vhostmd/vhostmd) and vm-dump-metrics. + description: DownwardMetrics adds a very small disk to VMIs + which contains a limited view of host and guest metrics. + The disk content is compatible with vhostmd (https://github.com/vhostmd/vhostmd) + and vm-dump-metrics. type: object emptyDisk: description: 'EmptyDisk represents a temporary disk which @@ -6801,22 +6814,29 @@ var CRDsValidation map[string]string = map[string]string{ description: Settings to set the kernel for booting. properties: container: - description: Container defines the container that containes kernel artifacts + description: Container defines the container that containes + kernel artifacts properties: image: description: Image that container initrd / kernel files. type: string imagePullPolicy: - description: 'Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. Cannot be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images' + description: 'Image pull policy. One of Always, Never, IfNotPresent. + Defaults to Always if :latest tag is specified, or IfNotPresent + otherwise. Cannot be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images' type: string imagePullSecret: - description: ImagePullSecret is the name of the Docker registry secret required to pull the image. The secret must already exist. + description: ImagePullSecret is the name of the Docker registry + secret required to pull the image. The secret must already + exist. type: string initrdPath: - description: the fully-qualified path to the ramdisk image in the host OS + description: the fully-qualified path to the ramdisk image + in the host OS type: string kernelPath: - description: The fully-qualified path to the kernel image in the host OS + description: The fully-qualified path to the kernel image + in the host OS type: string required: - image @@ -7421,7 +7441,10 @@ var CRDsValidation map[string]string = map[string]string{ type: string type: object downwardMetrics: - description: DownwardMetrics adds a very small disk to VMIs which contains a limited view of host and guest metrics. The disk content is compatible with vhostmd (https://github.com/vhostmd/vhostmd) and vm-dump-metrics. + description: DownwardMetrics adds a very small disk to VMIs which + contains a limited view of host and guest metrics. The disk content + is compatible with vhostmd (https://github.com/vhostmd/vhostmd) + and vm-dump-metrics. type: object emptyDisk: description: 'EmptyDisk represents a temporary disk which shares the @@ -8626,22 +8649,29 @@ var CRDsValidation map[string]string = map[string]string{ description: Settings to set the kernel for booting. properties: container: - description: Container defines the container that containes kernel artifacts + description: Container defines the container that containes + kernel artifacts properties: image: description: Image that container initrd / kernel files. type: string imagePullPolicy: - description: 'Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. Cannot be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images' + description: 'Image pull policy. One of Always, Never, IfNotPresent. + Defaults to Always if :latest tag is specified, or IfNotPresent + otherwise. Cannot be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images' type: string imagePullSecret: - description: ImagePullSecret is the name of the Docker registry secret required to pull the image. The secret must already exist. + description: ImagePullSecret is the name of the Docker registry + secret required to pull the image. The secret must already + exist. type: string initrdPath: - description: the fully-qualified path to the ramdisk image in the host OS + description: the fully-qualified path to the ramdisk image + in the host OS type: string kernelPath: - description: The fully-qualified path to the kernel image in the host OS + description: The fully-qualified path to the kernel image + in the host OS type: string required: - image @@ -10405,28 +10435,38 @@ var CRDsValidation map[string]string = map[string]string{ description: Settings to set the kernel for booting. properties: container: - description: Container defines the container that containes kernel artifacts + description: Container defines the container that containes + kernel artifacts properties: image: - description: Image that container initrd / kernel files. + description: Image that container initrd / kernel + files. type: string imagePullPolicy: - description: 'Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. Cannot be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images' + description: 'Image pull policy. One of Always, + Never, IfNotPresent. Defaults to Always if :latest + tag is specified, or IfNotPresent otherwise. Cannot + be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images' type: string imagePullSecret: - description: ImagePullSecret is the name of the Docker registry secret required to pull the image. The secret must already exist. + description: ImagePullSecret is the name of the + Docker registry secret required to pull the image. + The secret must already exist. type: string initrdPath: - description: the fully-qualified path to the ramdisk image in the host OS + description: the fully-qualified path to the ramdisk + image in the host OS type: string kernelPath: - description: The fully-qualified path to the kernel image in the host OS + description: The fully-qualified path to the kernel + image in the host OS type: string required: - image type: object kernelArgs: - description: Arguments to be passed to the kernel at boot time + description: Arguments to be passed to the kernel at + boot time type: string type: object serial: @@ -11061,7 +11101,10 @@ var CRDsValidation map[string]string = map[string]string{ type: string type: object downwardMetrics: - description: DownwardMetrics adds a very small disk to VMIs which contains a limited view of host and guest metrics. The disk content is compatible with vhostmd (https://github.com/vhostmd/vhostmd) and vm-dump-metrics. + description: DownwardMetrics adds a very small disk to VMIs + which contains a limited view of host and guest metrics. + The disk content is compatible with vhostmd (https://github.com/vhostmd/vhostmd) + and vm-dump-metrics. type: object emptyDisk: description: 'EmptyDisk represents a temporary disk which @@ -13791,31 +13834,44 @@ var CRDsValidation map[string]string = map[string]string{ type: object type: object kernelBoot: - description: Settings to set the kernel for booting. + description: Settings to set the kernel for + booting. properties: container: - description: Container defines the container that containes kernel artifacts + description: Container defines the container + that containes kernel artifacts properties: image: - description: Image that container initrd / kernel files. + description: Image that container initrd + / kernel files. type: string imagePullPolicy: - description: 'Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. Cannot be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images' + description: 'Image pull policy. One + of Always, Never, IfNotPresent. Defaults + to Always if :latest tag is specified, + or IfNotPresent otherwise. Cannot + be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images' type: string imagePullSecret: - description: ImagePullSecret is the name of the Docker registry secret required to pull the image. The secret must already exist. + description: ImagePullSecret is the + name of the Docker registry secret + required to pull the image. The secret + must already exist. type: string initrdPath: - description: the fully-qualified path to the ramdisk image in the host OS + description: the fully-qualified path + to the ramdisk image in the host OS type: string kernelPath: - description: The fully-qualified path to the kernel image in the host OS + description: The fully-qualified path + to the kernel image in the host OS type: string required: - image type: object kernelArgs: - description: Arguments to be passed to the kernel at boot time + description: Arguments to be passed to the + kernel at boot time type: string type: object serial: @@ -14508,7 +14564,11 @@ var CRDsValidation map[string]string = map[string]string{ type: string type: object downwardMetrics: - description: DownwardMetrics adds a very small disk to VMIs which contains a limited view of host and guest metrics. The disk content is compatible with vhostmd (https://github.com/vhostmd/vhostmd) and vm-dump-metrics. + description: DownwardMetrics adds a very small + disk to VMIs which contains a limited view of + host and guest metrics. The disk content is + compatible with vhostmd (https://github.com/vhostmd/vhostmd) + and vm-dump-metrics. type: object emptyDisk: description: 'EmptyDisk represents a temporary diff --git a/tests/vmi_kernel_boot_test.go b/tests/vmi_kernel_boot_test.go index 3fdc7a7e6f11..e5e4fcb9c176 100644 --- a/tests/vmi_kernel_boot_test.go +++ b/tests/vmi_kernel_boot_test.go @@ -43,8 +43,30 @@ var _ = Describe("VMI with external kernel boot", func() { It("ensure successful boot", func() { vmi := utils.GetVMIKernelBoot() obj, err := virtClient.VirtualMachineInstance(tests.NamespaceTestDefault).Create(vmi) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) tests.WaitForSuccessfulVMIStart(obj) }) }) + + Context("with illegal definition ensure rejection of", func() { + + It("VMI defined without an image", func() { + vmi := utils.GetVMIKernelBoot() + kernelBoot := vmi.Spec.Domain.Firmware.KernelBoot + kernelBoot.Container.Image = "" + _, err := virtClient.VirtualMachineInstance(tests.NamespaceTestDefault).Create(vmi) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("denied the request: spec.domain.firmware.kernelBoot.container must be defined with an image")) + }) + + It("VMI defined with image but without initrd & kernel paths", func() { + vmi := utils.GetVMIKernelBoot() + kernelBoot := vmi.Spec.Domain.Firmware.KernelBoot + kernelBoot.Container.KernelPath = "" + kernelBoot.Container.InitrdPath = "" + _, err := virtClient.VirtualMachineInstance(tests.NamespaceTestDefault).Create(vmi) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("denied the request: spec.domain.firmware.kernelBoot.container must be defined with at least one of the following: kernelPath, initrdPath")) + }) + }) })