diff --git a/pkg/virt-api/api.go b/pkg/virt-api/api.go index 94676bce16ef..d9e0f8228ea8 100644 --- a/pkg/virt-api/api.go +++ b/pkg/virt-api/api.go @@ -1013,11 +1013,13 @@ func (app *virtAPIApp) Run() { go webhookInformers.VMIInformer.Run(stopChan) go webhookInformers.VMIPresetInformer.Run(stopChan) go webhookInformers.NamespaceLimitsInformer.Run(stopChan) + go webhookInformers.ConfigMapInformer.Run(stopChan) cache.WaitForCacheSync(stopChan, webhookInformers.VMIInformer.HasSynced, webhookInformers.VMIPresetInformer.HasSynced, - webhookInformers.NamespaceLimitsInformer.HasSynced) + webhookInformers.NamespaceLimitsInformer.HasSynced, + webhookInformers.ConfigMapInformer.HasSynced) // Verify/create webhook endpoint. err = app.createWebhook() diff --git a/pkg/virt-api/webhooks/mutating-webhook/BUILD.bazel b/pkg/virt-api/webhooks/mutating-webhook/BUILD.bazel index c4538a19649e..8a2017213bfe 100644 --- a/pkg/virt-api/webhooks/mutating-webhook/BUILD.bazel +++ b/pkg/virt-api/webhooks/mutating-webhook/BUILD.bazel @@ -12,6 +12,7 @@ go_library( deps = [ "//pkg/api/v1:go_default_library", "//pkg/log:go_default_library", + "//pkg/util:go_default_library", "//pkg/virt-api/webhooks:go_default_library", "//vendor/k8s.io/api/admission/v1beta1:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", diff --git a/pkg/virt-api/webhooks/mutating-webhook/mutating-webhook.go b/pkg/virt-api/webhooks/mutating-webhook/mutating-webhook.go index c9894caa5b6e..e34fb9db827c 100644 --- a/pkg/virt-api/webhooks/mutating-webhook/mutating-webhook.go +++ b/pkg/virt-api/webhooks/mutating-webhook/mutating-webhook.go @@ -105,6 +105,8 @@ func mutateVMIs(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse { }, } } + // Apply default cpu model + setDefaultCPUModel(&vmi, informers.ConfigMapInformer.GetStore()) // Apply namespace limits applyNamespaceLimitRangeValues(&vmi, informers.NamespaceLimitsInformer) diff --git a/pkg/virt-api/webhooks/mutating-webhook/mutating-webhook_test.go b/pkg/virt-api/webhooks/mutating-webhook/mutating-webhook_test.go index ff707c414b37..5c823fd5f31f 100644 --- a/pkg/virt-api/webhooks/mutating-webhook/mutating-webhook_test.go +++ b/pkg/virt-api/webhooks/mutating-webhook/mutating-webhook_test.go @@ -123,11 +123,12 @@ var _ = Describe("Mutating Webhook", func() { } namespaceLimitInformer, _ = testutils.NewFakeInformerFor(&k8sv1.LimitRange{}) namespaceLimitInformer.GetIndexer().Add(namespaceLimit) - + configMapInformer, _ := testutils.NewFakeInformerFor(&k8sv1.ConfigMap{}) webhooks.SetInformers( &webhooks.Informers{ VMIPresetInformer: presetInformer, NamespaceLimitsInformer: namespaceLimitInformer, + ConfigMapInformer: configMapInformer, }, ) }) diff --git a/pkg/virt-api/webhooks/mutating-webhook/preset.go b/pkg/virt-api/webhooks/mutating-webhook/preset.go index f4c5506f4a9c..9623efb6ad0f 100644 --- a/pkg/virt-api/webhooks/mutating-webhook/preset.go +++ b/pkg/virt-api/webhooks/mutating-webhook/preset.go @@ -31,9 +31,14 @@ import ( kubev1 "kubevirt.io/kubevirt/pkg/api/v1" "kubevirt.io/kubevirt/pkg/log" + "kubevirt.io/kubevirt/pkg/util" ) -const exclusionMarking = "virtualmachineinstancepresets.admission.kubevirt.io/exclude" +const ( + exclusionMarking = "virtualmachineinstancepresets.admission.kubevirt.io/exclude" + configMapName = "kubevirt-config" + defaultCPUModelKey = "default-cpu-model" +) // listPresets returns all VirtualMachinePresets by namespace func listPresets(vmiPresetInformer cache.SharedIndexInformer, namespace string) ([]kubev1.VirtualMachineInstancePreset, error) { @@ -275,3 +280,25 @@ func isVMIExcluded(vmi *kubev1.VirtualMachineInstance) bool { } return false } + +//setDefaultCPUModel sets default cpu model from config if vmi doesn't have cpu model +func setDefaultCPUModel(vmi *kubev1.VirtualMachineInstance, configMapStore cache.Store) { + //if vmi doesn't have cpu topology or cpu model set + if vmi.Spec.Domain.CPU == nil || vmi.Spec.Domain.CPU.Model == "" { + namespace, err := util.GetNamespace() + if err != nil { + return + } + // if default cluster cpu model is defined + if obj, exists, err := configMapStore.GetByKey(namespace + "/" + configMapName); err == nil && exists { + if obj.(*k8sv1.ConfigMap).Data[defaultCPUModelKey] != "" { + // create cpu topology struct + if vmi.Spec.Domain.CPU == nil { + vmi.Spec.Domain.CPU = &kubev1.CPU{} + } + //set is as vmi cpu model + vmi.Spec.Domain.CPU.Model = obj.(*k8sv1.ConfigMap).Data[defaultCPUModelKey] + } + } + } +} diff --git a/pkg/virt-api/webhooks/mutating-webhook/preset_test.go b/pkg/virt-api/webhooks/mutating-webhook/preset_test.go index 3699dcbc24b2..616c33c5f059 100644 --- a/pkg/virt-api/webhooks/mutating-webhook/preset_test.go +++ b/pkg/virt-api/webhooks/mutating-webhook/preset_test.go @@ -27,6 +27,7 @@ import ( k8sv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" k8smetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/cache" @@ -518,6 +519,72 @@ var _ = Describe("Mutating Webhook Presets", func() { }) }) + Context("Apply default cpu model", func() { + var vmi v1.VirtualMachineInstance + var configMapIndexer cache.Indexer + var defaultCPUModel = "Haswell" + var cfgMap k8sv1.ConfigMap + + BeforeEach(func() { + configMapIndexer = cache.NewIndexer(cache.DeletionHandlingMetaNamespaceKeyFunc, nil) + vmi = v1.VirtualMachineInstance{Spec: v1.VirtualMachineInstanceSpec{Domain: v1.DomainSpec{}}} + }) + + It("Should set default cpu model when vmi doesn't have it", func() { + cfgMap = k8sv1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "kubevirt", + Name: "kubevirt-config", + }, + Data: map[string]string{ + defaultCPUModelKey: defaultCPUModel, + }, + } + configMapIndexer.Add(&cfgMap) + setDefaultCPUModel(&vmi, configMapIndexer) + + Expect(vmi.Spec.Domain.CPU).ToNot(BeNil()) + Expect(vmi.Spec.Domain.CPU.Model).To(Equal(defaultCPUModel)) + }) + + It("Should not set default cpu model when vmi does have it", func() { + cfgMap = k8sv1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "kubevirt", + Name: "kubevirt-config", + }, + Data: map[string]string{ + defaultCPUModelKey: defaultCPUModel, + }, + } + configMapIndexer.Add(&cfgMap) + + vmCPUModel := "EPYC" + vmi.Spec.Domain.CPU = &v1.CPU{ + Model: vmCPUModel, + } + setDefaultCPUModel(&vmi, configMapIndexer) + + Expect(vmi.Spec.Domain.CPU).ToNot(BeNil()) + Expect(vmi.Spec.Domain.CPU.Model).To(Equal(vmCPUModel)) + }) + + It("Should has empty cpu model when cpu model is not set", func() { + cfgMap = k8sv1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "kubevirt", + Name: "kubevirt-config", + }, + Data: map[string]string{}, + } + configMapIndexer.Add(&cfgMap) + vmi.Spec.Domain.CPU = &v1.CPU{} + setDefaultCPUModel(&vmi, configMapIndexer) + Expect(vmi.Spec.Domain.CPU).ToNot(BeNil()) + Expect(vmi.Spec.Domain.CPU.Model).To(BeEmpty()) + }) + }) + Context("Apply Presets", func() { var vmi v1.VirtualMachineInstance var preset *v1.VirtualMachineInstancePreset diff --git a/pkg/virt-api/webhooks/utils.go b/pkg/virt-api/webhooks/utils.go index 653000fce2b6..74e0ea2b6240 100644 --- a/pkg/virt-api/webhooks/utils.go +++ b/pkg/virt-api/webhooks/utils.go @@ -80,6 +80,7 @@ type Informers struct { VMIPresetInformer cache.SharedIndexInformer NamespaceLimitsInformer cache.SharedIndexInformer VMIInformer cache.SharedIndexInformer + ConfigMapInformer cache.SharedIndexInformer } func GetInformers() *Informers { @@ -110,6 +111,7 @@ func newInformers() *Informers { VMIInformer: kubeInformerFactory.VMI(), VMIPresetInformer: kubeInformerFactory.VirtualMachinePreset(), NamespaceLimitsInformer: kubeInformerFactory.LimitRanges(), + ConfigMapInformer: kubeInformerFactory.ConfigMap(), } } diff --git a/tests/vmi_lifecycle_test.go b/tests/vmi_lifecycle_test.go index 5e63a8d15d6d..4a97b8492e35 100644 --- a/tests/vmi_lifecycle_test.go +++ b/tests/vmi_lifecycle_test.go @@ -49,6 +49,8 @@ import ( "kubevirt.io/kubevirt/tests" ) +const kubevirtConfig = "kubevirt-config" + func newCirrosVMI() *v1.VirtualMachineInstance { return tests.NewRandomVMIWithEphemeralDiskAndUserdata(tests.ContainerDiskFor(tests.ContainerDiskCirros), "#!/bin/bash\necho 'hello'\n") } @@ -632,6 +634,91 @@ var _ = Describe("[rfe_id:273][crit:high][vendor:cnv-qe@redhat.com][level:compon }) + Context("with default cpu model", func() { + var cfgMap *k8sv1.ConfigMap + var originalData map[string]string + var options metav1.GetOptions + var defaultCPUModelKey = "default-cpu-model" + var defaultCPUModel = "Conroe" + var vmiCPUModel = "SandyBridge" + + //store old kubevirt-config + BeforeEach(func() { + cfgMap, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Get(kubevirtConfig, options) + Expect(err).ToNot(HaveOccurred()) + originalData = cfgMap.Data + }) + + //replace new kubevirt-config with old config + AfterEach(func() { + cfgMap, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Get(kubevirtConfig, options) + Expect(err).ToNot(HaveOccurred()) + cfgMap.Data = originalData + _, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Update(cfgMap) + Expect(err).ToNot(HaveOccurred()) + time.Sleep(5 * time.Second) + }) + + It("should set default cpu model when vmi doesn't have it set", func() { + cfgMap, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Get(kubevirtConfig, options) + Expect(err).ToNot(HaveOccurred(), "Expect config map to be loaded without error") + + cfgMap.Data[defaultCPUModelKey] = defaultCPUModel + _, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Update(cfgMap) + Expect(err).ToNot(HaveOccurred(), "Expect config map to be updated without error") + + time.Sleep(5 * time.Second) + + vmi := tests.NewRandomVMIWithEphemeralDiskAndUserdata(tests.ContainerDiskFor(tests.ContainerDiskCirros), "#!/bin/bash\necho 'hello'\n") + + _, err = virtClient.VirtualMachineInstance(vmi.Namespace).Create(vmi) + Expect(err).ToNot(HaveOccurred(), "Should create VMI") + tests.WaitForSuccessfulVMIStart(vmi) + + curVMI, err := virtClient.VirtualMachineInstance(vmi.Namespace).Get(vmi.Name, &metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred(), "Should get VMI") + Expect(curVMI.Spec.Domain.CPU.Model).To(Equal("Conroe"), "Expected default CPU model") + + }) + + It("should not set default cpu model when vmi has it set", func() { + cfgMap, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Get(kubevirtConfig, options) + Expect(err).ToNot(HaveOccurred(), "Expect config map to be loaded without error") + + cfgMap.Data[defaultCPUModelKey] = defaultCPUModel + + _, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Update(cfgMap) + Expect(err).ToNot(HaveOccurred(), "Expect config map to be updated without error") + + time.Sleep(5 * time.Second) + + vmi := tests.NewRandomVMIWithEphemeralDiskAndUserdata(tests.ContainerDiskFor(tests.ContainerDiskCirros), "#!/bin/bash\necho 'hello'\n") + vmi.Spec.Domain.CPU = &v1.CPU{ + Model: vmiCPUModel, + } + _, err = virtClient.VirtualMachineInstance(vmi.Namespace).Create(vmi) + Expect(err).ToNot(HaveOccurred(), "Should create VMI") + tests.WaitForSuccessfulVMIStart(vmi) + + curVMI, err := virtClient.VirtualMachineInstance(vmi.Namespace).Get(vmi.Name, &metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred(), "Should get VMI") + Expect(curVMI.Spec.Domain.CPU.Model).To(Equal(vmiCPUModel), "Expected vmi CPU model") + + }) + + It("should not set cpu model when vmi does not have it set and default cpu model is not set", func() { + vmi := tests.NewRandomVMIWithEphemeralDiskAndUserdata(tests.ContainerDiskFor(tests.ContainerDiskCirros), "#!/bin/bash\necho 'hello'\n") + _, err = virtClient.VirtualMachineInstance(vmi.Namespace).Create(vmi) + Expect(err).ToNot(HaveOccurred(), "Should create VMI") + + tests.WaitForSuccessfulVMIStart(vmi) + + curVMI, err := virtClient.VirtualMachineInstance(vmi.Namespace).Get(vmi.Name, &metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred(), "Should get VMI") + Expect(curVMI.Spec.Domain.CPU).To(BeNil(), "Expected CPU to be nil") + }) + }) + Context("with node feature discovery", func() { var options metav1.GetOptions @@ -648,20 +735,17 @@ var _ = Describe("[rfe_id:273][crit:high][vendor:cnv-qe@redhat.com][level:compon originalLabels = node.GetObjectMeta().GetLabels() options = metav1.GetOptions{} - cfgMap, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Get("kubevirt-config", options) + cfgMap, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Get(kubevirtConfig, options) Expect(err).ToNot(HaveOccurred()) originalFeatureGates = cfgMap.Data[virtconfig.FeatureGatesKey] cfgMap.Data[virtconfig.FeatureGatesKey] = virtconfig.CPUNodeDiscoveryGate _, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Update(cfgMap) Expect(err).ToNot(HaveOccurred()) - - //FIXME improve the detection if virt-controller already received the config-map change - time.Sleep(time.Millisecond * 500) - + time.Sleep(5 * time.Second) }) AfterEach(func() { - cfgMap, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Get("kubevirt-config", options) + cfgMap, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Get(kubevirtConfig, options) Expect(err).ToNot(HaveOccurred()) cfgMap.Data[virtconfig.FeatureGatesKey] = originalFeatureGates _, err = virtClient.CoreV1().ConfigMaps(namespaceKubevirt).Update(cfgMap) @@ -673,8 +757,7 @@ var _ = Describe("[rfe_id:273][crit:high][vendor:cnv-qe@redhat.com][level:compon _, err = virtClient.CoreV1().Nodes().Update(n) Expect(err).ToNot(HaveOccurred()) - //FIXME improve the detection if virt-controller already received the config-map change - time.Sleep(time.Millisecond * 500) + time.Sleep(5 * time.Second) }) It("[test_id:1639]the vmi with cpu.model matching a nfd label on a node should be scheduled", func() { @@ -1339,7 +1422,7 @@ var _ = Describe("[rfe_id:273][crit:high][vendor:cnv-qe@redhat.com][level:compon func shouldUseEmulation(virtClient kubecli.KubevirtClient) bool { useEmulation := false options := metav1.GetOptions{} - cfgMap, err := virtClient.CoreV1().ConfigMaps(tests.KubeVirtInstallNamespace).Get("kubevirt-config", options) + cfgMap, err := virtClient.CoreV1().ConfigMaps(tests.KubeVirtInstallNamespace).Get(kubevirtConfig, options) if err == nil { val, ok := cfgMap.Data["debug.useEmulation"] useEmulation = ok && (val == "true")