From 207918fd623585f4c2e2d7bf9a90965943ec3d5a Mon Sep 17 00:00:00 2001 From: kubevirt-bot Date: Wed, 16 Mar 2022 19:25:23 +0100 Subject: [PATCH] Ignore sriovLiveMigration FG on SNO (#1820) The default value of sriovLiveMigration is true but the value is meaningless on SNO cluster, let's simply ignore it: the CRD defaulting mechanism on the APIServer is not so flexible to allow to specify a default based on the cluster topology. On the other side, validating it with the webhook is overkilling for users that simply expect to have the simplest possible CR always accepted. Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=2062227 Signed-off-by: Simone Tiraboschi Co-authored-by: Simone Tiraboschi --- deploy/crds/hco00.crd.yaml | 1 + .../1.6.0/manifests/hco00.crd.yaml | 1 + .../1.5.0/manifests/hco00.crd.yaml | 1 + .../1.6.0/manifests/hco00.crd.yaml | 1 + docs/api.md | 2 +- docs/cluster-configuration.md | 2 +- pkg/apis/hco/v1beta1/hyperconverged_types.go | 2 +- pkg/controller/operands/kubevirt.go | 2 +- pkg/controller/operands/kubevirt_test.go | 114 +++++++++++++++--- 9 files changed, 107 insertions(+), 19 deletions(-) diff --git a/deploy/crds/hco00.crd.yaml b/deploy/crds/hco00.crd.yaml index 75f5527039..bcdb788eec 100644 --- a/deploy/crds/hco00.crd.yaml +++ b/deploy/crds/hco00.crd.yaml @@ -869,6 +869,7 @@ spec: sriovLiveMigration: default: true description: Allow migrating a virtual machine with SRIOV interfaces. + Ignored on single node clusters. type: boolean withHostPassthroughCPU: default: false diff --git a/deploy/index-image/community-kubevirt-hyperconverged/1.6.0/manifests/hco00.crd.yaml b/deploy/index-image/community-kubevirt-hyperconverged/1.6.0/manifests/hco00.crd.yaml index 75f5527039..bcdb788eec 100644 --- a/deploy/index-image/community-kubevirt-hyperconverged/1.6.0/manifests/hco00.crd.yaml +++ b/deploy/index-image/community-kubevirt-hyperconverged/1.6.0/manifests/hco00.crd.yaml @@ -869,6 +869,7 @@ spec: sriovLiveMigration: default: true description: Allow migrating a virtual machine with SRIOV interfaces. + Ignored on single node clusters. type: boolean withHostPassthroughCPU: default: false diff --git a/deploy/olm-catalog/community-kubevirt-hyperconverged/1.5.0/manifests/hco00.crd.yaml b/deploy/olm-catalog/community-kubevirt-hyperconverged/1.5.0/manifests/hco00.crd.yaml index 79b93fedd2..e620c8fc64 100644 --- a/deploy/olm-catalog/community-kubevirt-hyperconverged/1.5.0/manifests/hco00.crd.yaml +++ b/deploy/olm-catalog/community-kubevirt-hyperconverged/1.5.0/manifests/hco00.crd.yaml @@ -130,6 +130,7 @@ spec: sriovLiveMigration: default: true description: Allow migrating a virtual machine with SRIOV interfaces. + Ignored on single node clusters. type: boolean withHostPassthroughCPU: default: false diff --git a/deploy/olm-catalog/community-kubevirt-hyperconverged/1.6.0/manifests/hco00.crd.yaml b/deploy/olm-catalog/community-kubevirt-hyperconverged/1.6.0/manifests/hco00.crd.yaml index 75f5527039..bcdb788eec 100644 --- a/deploy/olm-catalog/community-kubevirt-hyperconverged/1.6.0/manifests/hco00.crd.yaml +++ b/deploy/olm-catalog/community-kubevirt-hyperconverged/1.6.0/manifests/hco00.crd.yaml @@ -869,6 +869,7 @@ spec: sriovLiveMigration: default: true description: Allow migrating a virtual machine with SRIOV interfaces. + Ignored on single node clusters. type: boolean withHostPassthroughCPU: default: false diff --git a/docs/api.md b/docs/api.md index 241fa03a97..50e4765a8a 100644 --- a/docs/api.md +++ b/docs/api.md @@ -89,7 +89,7 @@ HyperConvergedFeatureGates is a set of optional feature gates to enable or disab | Field | Description | Scheme | Default | Required | | ----- | ----------- | ------ | -------- |-------- | | withHostPassthroughCPU | Allow migrating a virtual machine with CPU host-passthrough mode. This should be enabled only when the Cluster is homogeneous from CPU HW perspective doc here | bool | false | true | -| sriovLiveMigration | Allow migrating a virtual machine with SRIOV interfaces. | bool | true | true | +| sriovLiveMigration | Allow migrating a virtual machine with SRIOV interfaces. Ignored on single node clusters. | bool | true | true | | enableCommonBootImageImport | Opt-in to automatic delivery/updates of the common data import cron templates. There are two sources for the data import cron templates: hard coded list of common templates, and custom templates that can be added to the dataImportCronTemplates field. This feature gates only control the common templates. It is possible to use custom templates by adding them to the dataImportCronTemplates field. | bool | true | true | [Back to TOC](#table-of-contents) diff --git a/docs/cluster-configuration.md b/docs/cluster-configuration.md index f0dab23080..ef55927b81 100644 --- a/docs/cluster-configuration.md +++ b/docs/cluster-configuration.md @@ -143,7 +143,7 @@ Additional information: [LibvirtXMLCPUModel](https://wiki.openstack.org/wiki/Lib Set the `sriovLiveMigration` feature gate in order to allow migrating a virtual machine with SRIOV interfaces. When enabled virt-launcher pods of virtual machines with SRIOV interfaces run with CAP_SYS_RESOURCE capability. This may -degrade virt-launcher security. +degrade virt-launcher security. `sriovLiveMigration` is ignored on single node clusters. **Default**: `true` diff --git a/pkg/apis/hco/v1beta1/hyperconverged_types.go b/pkg/apis/hco/v1beta1/hyperconverged_types.go index 0d12e2e6d2..6121c256fa 100644 --- a/pkg/apis/hco/v1beta1/hyperconverged_types.go +++ b/pkg/apis/hco/v1beta1/hyperconverged_types.go @@ -225,7 +225,7 @@ type HyperConvergedFeatureGates struct { // +kubebuilder:default=false WithHostPassthroughCPU bool `json:"withHostPassthroughCPU"` - // Allow migrating a virtual machine with SRIOV interfaces. + // Allow migrating a virtual machine with SRIOV interfaces. Ignored on single node clusters. // +optional // +kubebuilder:default=true SRIOVLiveMigration bool `json:"sriovLiveMigration"` diff --git a/pkg/controller/operands/kubevirt.go b/pkg/controller/operands/kubevirt.go index b71c1895b9..f7109fa644 100644 --- a/pkg/controller/operands/kubevirt.go +++ b/pkg/controller/operands/kubevirt.go @@ -517,7 +517,7 @@ func getFeatureGateChecks(featureGates *hcov1beta1.HyperConvergedFeatureGates) [ fgs = append(fgs, kvWithHostPassthroughCPU) } - if featureGates.SRIOVLiveMigration { + if featureGates.SRIOVLiveMigration && hcoutil.GetClusterInfo().IsInfrastructureHighlyAvailable() { fgs = append(fgs, kvSRIOVLiveMigration) } diff --git a/pkg/controller/operands/kubevirt_test.go b/pkg/controller/operands/kubevirt_test.go index 676fed80e1..680ce760c0 100644 --- a/pkg/controller/operands/kubevirt_test.go +++ b/pkg/controller/operands/kubevirt_test.go @@ -30,6 +30,7 @@ import ( var _ = Describe("KubeVirt Operand", func() { var ( basicNumFgOnOpenshift = len(hardCodeKvFgs) + len(sspConditionKvFgs) + deltaFGNotSNO = 1 ) Context("KubeVirt Priority Classes", func() { @@ -1214,6 +1215,19 @@ Version: 1.2.3`) }) Context("Feature Gates", func() { + + getClusterInfo := hcoutil.GetClusterInfo + + BeforeEach(func() { + hcoutil.GetClusterInfo = func() hcoutil.ClusterInfo { + return &commonTestUtils.ClusterInfoMock{} + } + }) + + AfterEach(func() { + hcoutil.GetClusterInfo = getClusterInfo + }) + Context("test feature gates in NewKubeVirt", func() { It("should add the WithHostPassthroughCPU feature gate if it's set in HyperConverged CR", func() { // one enabled, one disabled and one missing @@ -1271,6 +1285,42 @@ Version: 1.2.3`) }) }) + Context("should ignore SRIOVLiveMigration on SNO ", func() { + BeforeEach(func() { + hcoutil.GetClusterInfo = func() hcoutil.ClusterInfo { + return &commonTestUtils.ClusterInfoSNOMock{} + } + }) + + It("should not add the SRIOVLiveMigration feature gate if it's set in HyperConverged on SNO", func() { + // one enabled, one disabled and one missing + hco.Spec.FeatureGates = hcov1beta1.HyperConvergedFeatureGates{ + SRIOVLiveMigration: true, + } + + existingResource, err := NewKubeVirt(hco) + Expect(err).ToNot(HaveOccurred()) + By("KV CR should contain the HotplugVolumes feature gate", func() { + Expect(existingResource.Spec.Configuration.DeveloperConfiguration).NotTo(BeNil()) + Expect(existingResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).ToNot(ContainElement(kvSRIOVLiveMigration)) + }) + }) + + It("should not add the SRIOVLiveMigration feature gate if it's disabled in HyperConverged CR on SNO", func() { + // one enabled, one disabled and one missing + hco.Spec.FeatureGates = hcov1beta1.HyperConvergedFeatureGates{ + SRIOVLiveMigration: false, + } + + existingResource, err := NewKubeVirt(hco) + Expect(err).ToNot(HaveOccurred()) + By("KV CR should contain the HotplugVolumes feature gate", func() { + Expect(existingResource.Spec.Configuration.DeveloperConfiguration).NotTo(BeNil()) + Expect(existingResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).ToNot(ContainElement("SRIOVLiveMigration")) + }) + }) + }) + It("should not add the feature gates if FeatureGates field is empty", func() { mandatoryKvFeatureGates = getMandatoryKvFeatureGates(false) hco.Spec.FeatureGates = hcov1beta1.HyperConvergedFeatureGates{} @@ -1280,13 +1330,26 @@ Version: 1.2.3`) Expect(existingResource.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil()) fgList := getKvFeatureGateList(&hco.Spec.FeatureGates) - Expect(fgList).To(HaveLen(basicNumFgOnOpenshift)) + Expect(fgList).To(HaveLen(basicNumFgOnOpenshift + deltaFGNotSNO)) Expect(fgList).Should(ContainElements(hardCodeKvFgs)) Expect(fgList).Should(ContainElements(sspConditionKvFgs)) }) }) Context("test feature gates in KV handler", func() { + + getClusterInfo := hcoutil.GetClusterInfo + + BeforeEach(func() { + hcoutil.GetClusterInfo = func() hcoutil.ClusterInfo { + return &commonTestUtils.ClusterInfoMock{} + } + }) + + AfterEach(func() { + hcoutil.GetClusterInfo = getClusterInfo + }) + It("should add feature gates if they are set to true", func() { existingResource, err := NewKubeVirt(hco) Expect(err).ToNot(HaveOccurred()) @@ -1346,7 +1409,7 @@ Version: 1.2.3`) mandatoryKvFeatureGates = getMandatoryKvFeatureGates(false) Expect(foundResource.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil()) fgList := getKvFeatureGateList(&hco.Spec.FeatureGates) - Expect(fgList).To(HaveLen(basicNumFgOnOpenshift)) + Expect(fgList).To(HaveLen(basicNumFgOnOpenshift + deltaFGNotSNO)) Expect(fgList).Should(ContainElements(hardCodeKvFgs)) Expect(fgList).Should(ContainElements(sspConditionKvFgs)) }) @@ -1377,7 +1440,7 @@ Version: 1.2.3`) mandatoryKvFeatureGates = getMandatoryKvFeatureGates(false) Expect(foundResource.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil()) fgList := getKvFeatureGateList(&hco.Spec.FeatureGates) - Expect(fgList).To(HaveLen(basicNumFgOnOpenshift)) + Expect(fgList).To(HaveLen(basicNumFgOnOpenshift + deltaFGNotSNO)) Expect(fgList).Should(ContainElements(hardCodeKvFgs)) Expect(fgList).Should(ContainElements(sspConditionKvFgs)) }) @@ -1385,7 +1448,7 @@ Version: 1.2.3`) It("should keep FG if already exist", func() { mandatoryKvFeatureGates = getMandatoryKvFeatureGates(true) - fgs := append(hardCodeKvFgs, kvWithHostPassthroughCPU, kvSRIOVLiveMigration) + fgs := append(hardCodeKvFgs, kvWithHostPassthroughCPU, kvSRIOVLiveMigration, kvLiveMigrationGate) existingResource, err := NewKubeVirt(hco) Expect(err).ToNot(HaveOccurred()) existingResource.Spec.Configuration.DeveloperConfiguration.FeatureGates = fgs @@ -1398,7 +1461,7 @@ Version: 1.2.3`) By("Make sure the existing KV is with the the expected FGs", func() { Expect(existingResource.Spec.Configuration.DeveloperConfiguration).NotTo(BeNil()) Expect(existingResource.Spec.Configuration.DeveloperConfiguration.FeatureGates). - To(ContainElements(kvWithHostPassthroughCPU, kvSRIOVLiveMigration)) + To(ContainElements(kvLiveMigrationGate, kvWithHostPassthroughCPU, kvSRIOVLiveMigration)) }) cl := commonTestUtils.InitClient([]runtime.Object{hco, existingResource}) @@ -1419,6 +1482,8 @@ Version: 1.2.3`) Expect(foundResource.Spec.Configuration.DeveloperConfiguration).NotTo(BeNil()) Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates). To(ContainElements(kvWithHostPassthroughCPU, kvSRIOVLiveMigration)) + + Expect(res.Updated).To(BeFalse()) }) It("should remove FG if it disabled in HC CR", func() { @@ -1456,7 +1521,7 @@ Version: 1.2.3`) ).To(BeNil()) Expect(foundResource.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil()) - Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(basicNumFgOnOpenshift)) + Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(basicNumFgOnOpenshift + deltaFGNotSNO)) Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElements(hardCodeKvFgs)) Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElements(sspConditionKvFgs)) }) @@ -1493,7 +1558,7 @@ Version: 1.2.3`) ).To(BeNil()) Expect(foundResource.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil()) - Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(basicNumFgOnOpenshift)) + Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(basicNumFgOnOpenshift + deltaFGNotSNO)) Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElements(hardCodeKvFgs)) Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElements(sspConditionKvFgs)) }) @@ -1530,7 +1595,7 @@ Version: 1.2.3`) ).To(BeNil()) Expect(foundResource.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil()) - Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(len(hardCodeKvFgs))) + Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(len(hardCodeKvFgs) + deltaFGNotSNO)) Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElements(hardCodeKvFgs)) }) }) @@ -1539,6 +1604,12 @@ Version: 1.2.3`) getClusterInfo := hcoutil.GetClusterInfo + BeforeEach(func() { + hcoutil.GetClusterInfo = func() hcoutil.ClusterInfo { + return &commonTestUtils.ClusterInfoMock{} + } + }) + AfterEach(func() { hcoutil.GetClusterInfo = getClusterInfo }) @@ -1555,37 +1626,37 @@ Version: 1.2.3`) Entry("When not using kvm-emulation and FG is empty", false, &hcov1beta1.HyperConvergedFeatureGates{}, - basicNumFgOnOpenshift, + basicNumFgOnOpenshift+deltaFGNotSNO, [][]string{hardCodeKvFgs, sspConditionKvFgs}, ), Entry("When using kvm-emulation and FG is empty", true, &hcov1beta1.HyperConvergedFeatureGates{}, - len(hardCodeKvFgs), + len(hardCodeKvFgs)+deltaFGNotSNO, [][]string{hardCodeKvFgs}, ), Entry("When not using kvm-emulation and all FGs are disabled", false, &hcov1beta1.HyperConvergedFeatureGates{SRIOVLiveMigration: false, WithHostPassthroughCPU: false}, - basicNumFgOnOpenshift, + basicNumFgOnOpenshift+deltaFGNotSNO, [][]string{hardCodeKvFgs, sspConditionKvFgs}, ), Entry("When using kvm-emulation all FGs are disabled", true, &hcov1beta1.HyperConvergedFeatureGates{SRIOVLiveMigration: false, WithHostPassthroughCPU: false}, - len(hardCodeKvFgs), + len(hardCodeKvFgs)+deltaFGNotSNO, [][]string{hardCodeKvFgs}, ), Entry("When not using kvm-emulation and all FGs are enabled", false, &hcov1beta1.HyperConvergedFeatureGates{SRIOVLiveMigration: true, WithHostPassthroughCPU: true}, - basicNumFgOnOpenshift+2, + basicNumFgOnOpenshift+deltaFGNotSNO+2, [][]string{hardCodeKvFgs, sspConditionKvFgs, {kvWithHostPassthroughCPU}}, ), Entry("When using kvm-emulation all FGs are enabled", true, &hcov1beta1.HyperConvergedFeatureGates{SRIOVLiveMigration: true, WithHostPassthroughCPU: true}, - len(hardCodeKvFgs)+2, + len(hardCodeKvFgs)+deltaFGNotSNO+2, [][]string{hardCodeKvFgs, {kvWithHostPassthroughCPU}}, )) @@ -2397,6 +2468,19 @@ Version: 1.2.3`) }) Context("jsonpath Annotation", func() { + + getClusterInfo := hcoutil.GetClusterInfo + + BeforeEach(func() { + hcoutil.GetClusterInfo = func() hcoutil.ClusterInfo { + return &commonTestUtils.ClusterInfoMock{} + } + }) + + AfterEach(func() { + hcoutil.GetClusterInfo = getClusterInfo + }) + mandatoryKvFeatureGates = getMandatoryKvFeatureGates(false) It("Should create KV object with changes from the annotation", func() { @@ -2597,7 +2681,7 @@ Version: 1.2.3`) ).ToNot(HaveOccurred()) Expect(kv.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil()) - Expect(kv.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(len(mandatoryKvFeatureGates) + 1)) + Expect(kv.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(len(mandatoryKvFeatureGates) + deltaFGNotSNO + 1)) Expect(kv.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElements(hardCodeKvFgs)) Expect(kv.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElement(kvSRIOVGate)) Expect(kv.Spec.Configuration.CPURequest).To(BeNil())