Merge pull request #10757 from RamLavi/release-1.1_add-full-pcpu-only…

…-support

[release 1.1] isolateEmulatorThread: Add full-pcpu-only support

pkg/virt-api/webhooks/mutating-webhook/mutators/vmi-mutator.go

@@ -97,6 +97,18 @@ func (mutator *VMIsMutator) Mutate(ar *admissionv1.AdmissionReview) *admissionv1
 			addNodeSelector(newVMI, v1.SEVESLabel)
 		}
 
+		if newVMI.Spec.Domain.CPU.IsolateEmulatorThread {
+			_, emulatorThreadCompleteToEvenParityAnnotationExists := mutator.ClusterConfig.GetConfigFromKubeVirtCR().Annotations[v1.EmulatorThreadCompleteToEvenParity]
+			if emulatorThreadCompleteToEvenParityAnnotationExists &&
+				mutator.ClusterConfig.AlignCPUsEnabled() {
+				log.Log.V(4).Infof("Copy %s annotation from Kubevirt CR", v1.EmulatorThreadCompleteToEvenParity)
+				if newVMI.Annotations == nil {
+					newVMI.Annotations = map[string]string{}
+				}
+				newVMI.Annotations[v1.EmulatorThreadCompleteToEvenParity] = ""
+			}
+		}
+
 		// Add foreground finalizer
 		newVMI.Finalizers = append(newVMI.Finalizers, v1.VirtualMachineInstanceFinalizer)
 

pkg/virt-api/webhooks/mutating-webhook/mutators/vmi-mutator_test.go

@@ -530,6 +530,54 @@ var _ = Describe("VirtualMachineInstance Mutator", func() {
 		Expect(vmiSpec.Domain.Resources.Requests.Memory()).To(Equal(vmi.Spec.Domain.Resources.Requests.Memory()))
 	})
 
+	DescribeTable("should not copy the EmulatorThreadCompleteToEvenParity annotation to the VMI",
+		func(featureGate string, annotations map[string]string, isolateEmulatorThread bool) {
+			if featureGate != "" || annotations != nil {
+				testutils.UpdateFakeKubeVirtClusterConfig(kvInformer, &v1.KubeVirt{
+					ObjectMeta: k8smetav1.ObjectMeta{
+						Annotations: annotations,
+					},
+					Spec: v1.KubeVirtSpec{
+						Configuration: v1.KubeVirtConfiguration{
+							DeveloperConfiguration: &v1.DeveloperConfiguration{
+								FeatureGates: []string{featureGate},
+							},
+						},
+					},
+				})
+			}
+			vmi.Spec.Domain.CPU = &v1.CPU{IsolateEmulatorThread: isolateEmulatorThread}
+
+			vmiMeta, _, _ := getMetaSpecStatusFromAdmit(vmi.Spec.Architecture)
+			_, exist := vmiMeta.Annotations[v1.EmulatorThreadCompleteToEvenParity]
+			Expect(exist).To(BeFalse())
+		},
+		Entry("when the AlignCPUs featureGate is disabled", "", map[string]string{v1.EmulatorThreadCompleteToEvenParity: ""}, true),
+		Entry("when the EmulatorThreadCompleteToEvenParity annotation is not set on the kubevirt CR", virtconfig.AlignCPUsGate, nil, true),
+		Entry("when isolateEmulatorThread is disabled on the VMI spec", virtconfig.AlignCPUsGate, map[string]string{v1.EmulatorThreadCompleteToEvenParity: ""}, false),
+	)
+
+	It("should copy the EmulatorThreadCompleteToEvenParity annotation to the VMI", func() {
+		testutils.UpdateFakeKubeVirtClusterConfig(kvInformer, &v1.KubeVirt{
+			ObjectMeta: k8smetav1.ObjectMeta{
+				Annotations: map[string]string{v1.EmulatorThreadCompleteToEvenParity: ""},
+			},
+			Spec: v1.KubeVirtSpec{
+				Configuration: v1.KubeVirtConfiguration{
+					DeveloperConfiguration: &v1.DeveloperConfiguration{
+						FeatureGates: []string{virtconfig.AlignCPUsGate},
+					},
+				},
+			},
+		})
+
+		vmi.Spec.Domain.CPU = &v1.CPU{IsolateEmulatorThread: true}
+
+		vmiMeta, _, _ := getMetaSpecStatusFromAdmit(vmi.Spec.Architecture)
+		_, exist := vmiMeta.Annotations[v1.EmulatorThreadCompleteToEvenParity]
+		Expect(exist).To(BeTrue())
+	})
+
 	It("should convert CPU requests to sockets", func() {
 		vmi.Spec.Domain.CPU = &v1.CPU{Model: "EPYC"}
 		vmi.Spec.Domain.Resources.Requests = k8sv1.ResourceList{

pkg/virt-config/feature-gates.go

@@ -82,6 +82,8 @@ const (
 	//
 	// CommonInstancetypesDeploymentGate enables the deployment of common-instancetypes by virt-operator
 	CommonInstancetypesDeploymentGate = "CommonInstancetypesDeploymentGate"
+	// AlignCPUsGate allows emulator thread to assign two extra CPUs if needed to complete even parity.
+	AlignCPUsGate = "AlignCPUs"
 )
 
 var deprecatedFeatureGates = [...]string{
@@ -261,3 +263,7 @@ func (config *ClusterConfig) AutoResourceLimitsEnabled() bool {
 func (config *ClusterConfig) CommonInstancetypesDeploymentEnabled() bool {
 	return config.isFeatureGateEnabled(CommonInstancetypesDeploymentGate)
 }
+
+func (config *ClusterConfig) AlignCPUsEnabled() bool {
+	return config.isFeatureGateEnabled(AlignCPUsGate)
+}

pkg/virt-controller/services/renderresources.go

@@ -199,7 +199,7 @@ func WithAutoMemoryLimits(namespace string, namespaceStore cache.Store) Resource
 	}
 }
 
-func WithCPUPinning(cpu *v1.CPU) ResourceRendererOption {
+func WithCPUPinning(cpu *v1.CPU, annotations map[string]string) ResourceRendererOption {
 	return func(renderer *ResourceRenderer) {
 		vcpus := hardware.GetNumberOfVCPUs(cpu)
 		if vcpus != 0 {
@@ -212,14 +212,22 @@ func WithCPUPinning(cpu *v1.CPU) ResourceRendererOption {
 			}
 		}
 
-		// allocate 1 more pcpu if IsolateEmulatorThread request
+		// allocate pcpus for emulatorThread if IsolateEmulatorThread is requested
 		if cpu.IsolateEmulatorThread {
-			emulatorThreadCPU := resource.NewQuantity(1, resource.BinarySI)
+			emulatorThreadCPUs := resource.NewQuantity(1, resource.BinarySI)
+
 			limits := renderer.calculatedLimits[k8sv1.ResourceCPU]
-			limits.Add(*emulatorThreadCPU)
+			_, emulatorThreadCompleteToEvenParityAnnotationExists := annotations[v1.EmulatorThreadCompleteToEvenParity]
+			if emulatorThreadCompleteToEvenParityAnnotationExists &&
+				limits.Value()%2 == 0 {
+				emulatorThreadCPUs = resource.NewQuantity(2, resource.BinarySI)
+			}
+
+			limits.Add(*emulatorThreadCPUs)
 			renderer.vmLimits[k8sv1.ResourceCPU] = limits
+
 			if cpuRequest, ok := renderer.vmRequests[k8sv1.ResourceCPU]; ok {
-				cpuRequest.Add(*emulatorThreadCPU)
+				cpuRequest.Add(*emulatorThreadCPUs)
 				renderer.vmRequests[k8sv1.ResourceCPU] = cpuRequest
 			}
 		}

pkg/virt-controller/services/renderresources_test.go

@@ -175,17 +175,17 @@ var _ = Describe("Resource pod spec renderer", func() {
 		userSpecifiedCPU := kubev1.ResourceList{kubev1.ResourceCPU: userCPURequest}
 
 		It("the user requested CPU configs are *not* overriden", func() {
-			rr = NewResourceRenderer(nil, userSpecifiedCPU, WithCPUPinning(&v1.CPU{Cores: 5}))
+			rr = NewResourceRenderer(nil, userSpecifiedCPU, WithCPUPinning(&v1.CPU{Cores: 5}, map[string]string{}))
 			Expect(rr.Requests()).To(HaveKeyWithValue(kubev1.ResourceCPU, userCPURequest))
 		})
 
 		It("carries over the CPU limits as requests when no CPUs are requested", func() {
-			rr = NewResourceRenderer(userSpecifiedCPU, nil, WithCPUPinning(&v1.CPU{}))
+			rr = NewResourceRenderer(userSpecifiedCPU, nil, WithCPUPinning(&v1.CPU{}, map[string]string{}))
 			Expect(rr.Requests()).To(HaveKeyWithValue(kubev1.ResourceCPU, userCPURequest))
 		})
 
 		It("carries over the CPU requests as limits when no CPUs are requested", func() {
-			rr = NewResourceRenderer(nil, userSpecifiedCPU, WithCPUPinning(&v1.CPU{}))
+			rr = NewResourceRenderer(nil, userSpecifiedCPU, WithCPUPinning(&v1.CPU{}, map[string]string{}))
 			Expect(rr.Requests()).To(HaveKeyWithValue(kubev1.ResourceCPU, userCPURequest))
 		})
 
@@ -195,41 +195,47 @@ var _ = Describe("Resource pod spec renderer", func() {
 				kubev1.ResourceCPU:    userCPURequest,
 				kubev1.ResourceMemory: memoryRequest,
 			}
-			rr = NewResourceRenderer(nil, userSpecifiedCPU, WithCPUPinning(&v1.CPU{Cores: 5}))
+			rr = NewResourceRenderer(nil, userSpecifiedCPU, WithCPUPinning(&v1.CPU{Cores: 5}, map[string]string{}))
 			Expect(rr.Requests()).To(HaveKeyWithValue(kubev1.ResourceCPU, resource.MustParse("200m")))
 			Expect(rr.Limits()).To(HaveKeyWithValue(kubev1.ResourceMemory, memoryRequest))
 		})
 
 		When("an isolated emulator thread is requested", func() {
-			cpuIsolatedEmulatorThreadOverhead := resource.MustParse("1000m")
 			userSpecifiedCPURequest := kubev1.ResourceList{kubev1.ResourceCPU: userCPURequest}
 
-			DescribeTable("requires an additional 1000m CPU, and an additional CPU is added to the limits", func(defineUserSpecifiedCPULimit bool) {
-
-				var userSpecifiedCPULimit kubev1.ResourceList
-
-				if defineUserSpecifiedCPULimit {
-					userSpecifiedCPULimit = kubev1.ResourceList{kubev1.ResourceCPU: userCPURequest}
-				}
-				rr = NewResourceRenderer(
-					userSpecifiedCPULimit,
-					userSpecifiedCPURequest,
-					WithCPUPinning(&v1.CPU{
-						Cores:                 5,
-						IsolateEmulatorThread: true,
-					}),
-				)
-				Expect(rr.Limits()).To(HaveKeyWithValue(
-					kubev1.ResourceCPU,
-					*resource.NewQuantity(6, resource.BinarySI),
-				))
-				Expect(rr.Requests()).To(HaveKeyWithValue(
-					kubev1.ResourceCPU,
-					addResources(userCPURequest, cpuIsolatedEmulatorThreadOverhead),
-				))
-			},
-				Entry("only CPU requests set by the user", false),
-				Entry("request and limits set by the user", true),
+			DescribeTable("requires additional EmulatorThread CPUs overhead, and additional CPUs added to the limits",
+				func(vmiAnnotations map[string]string, defineUserSpecifiedCPULimit bool, cores uint32, expectedCPUOverhead string) {
+					cpuIsolatedEmulatorThreadOverhead := resource.MustParse(expectedCPUOverhead)
+					var userSpecifiedCPULimit kubev1.ResourceList
+
+					if defineUserSpecifiedCPULimit {
+						userSpecifiedCPULimit = kubev1.ResourceList{kubev1.ResourceCPU: userCPURequest}
+					}
+
+					rr = NewResourceRenderer(
+						userSpecifiedCPULimit,
+						userSpecifiedCPURequest,
+						WithCPUPinning(&v1.CPU{
+							Cores:                 cores,
+							IsolateEmulatorThread: true,
+						},
+							vmiAnnotations),
+					)
+					Expect(rr.Limits()).To(HaveKeyWithValue(
+						kubev1.ResourceCPU,
+						*resource.NewQuantity(cpuIsolatedEmulatorThreadOverhead.Value()+int64(cores), resource.BinarySI),
+					))
+					Expect(rr.Requests()).To(HaveKeyWithValue(
+						kubev1.ResourceCPU,
+						addResources(userCPURequest, cpuIsolatedEmulatorThreadOverhead),
+					))
+				},
+				Entry("EmulatorThreadCompleteToEvenParity mode is disabled, only CPU requests set by the user", map[string]string{}, false, uint32(5), "1000m"),
+				Entry("EmulatorThreadCompleteToEvenParity mode is disabled, request and limits set by the user", map[string]string{}, true, uint32(5), "1000m"),
+				Entry("EmulatorThreadCompleteToEvenParity mode is enabled, only CPU requests set by the user, odd amount of cores is requested", map[string]string{v1.EmulatorThreadCompleteToEvenParity: ""}, false, uint32(5), "1000m"),
+				Entry("EmulatorThreadCompleteToEvenParity mode is enabled, only CPU requests set by the user, even amount of cores is requested", map[string]string{v1.EmulatorThreadCompleteToEvenParity: ""}, false, uint32(6), "2000m"),
+				Entry("EmulatorThreadCompleteToEvenParity mode is enabled, request and limits set by the user, odd amount of cores is requested", map[string]string{v1.EmulatorThreadCompleteToEvenParity: ""}, true, uint32(5), "1000m"),
+				Entry("EmulatorThreadCompleteToEvenParity mode is enabled, request and limits set by the user, even amount of cores is requested", map[string]string{v1.EmulatorThreadCompleteToEvenParity: ""}, true, uint32(6), "2000m"),
 			)
 		})
 	})

pkg/virt-controller/services/template.go

@@ -1395,7 +1395,7 @@ func (t *templateService) VMIResourcePredicates(vmi *v1.VirtualMachineInstance,
 	return VMIResourcePredicates{
 		vmi: vmi,
 		resourceRules: []VMIResourceRule{
-			NewVMIResourceRule(doesVMIRequireDedicatedCPU, WithCPUPinning(vmi.Spec.Domain.CPU)),
+			NewVMIResourceRule(doesVMIRequireDedicatedCPU, WithCPUPinning(vmi.Spec.Domain.CPU, vmi.Annotations)),
 			NewVMIResourceRule(not(doesVMIRequireDedicatedCPU), WithoutDedicatedCPU(vmi.Spec.Domain.CPU, t.clusterConfig.GetCPUAllocationRatio(), withCPULimits)),
 			NewVMIResourceRule(util.HasHugePages, WithHugePages(vmi.Spec.Domain.Memory, memoryOverhead)),
 			NewVMIResourceRule(not(util.HasHugePages), WithMemoryOverhead(vmi.Spec.Domain.Resources, memoryOverhead)),

pkg/virt-controller/services/template_test.go

@@ -1690,21 +1690,25 @@ var _ = Describe("Template", func() {
 					}, true),
 			)
 
-			It("should allocate 1 more cpu when isolateEmulatorThread requested", func() {
+			DescribeTable("when isolateEmulatorThread requested", func(
+				annotations map[string]string, requestedCores uint32, expectedCPULimits string) {
 				config, kvInformer, svc = configFactory(defaultArch)
+
 				vmi := v1.VirtualMachineInstance{
 					ObjectMeta: metav1.ObjectMeta{
-						Name:      "testvmi",
-						Namespace: "default",
-						UID:       "1234",
+						Name:        "testvmi",
+						Namespace:   "default",
+						UID:         "1234",
+						Annotations: annotations,
 					},
 					Spec: v1.VirtualMachineInstanceSpec{
 						Domain: v1.DomainSpec{
 							Devices: v1.Devices{
 								DisableHotplug: true,
 							},
 							CPU: &v1.CPU{
-								Cores:                 2,
+								Cores:                 requestedCores,
+								Threads:               1,
 								DedicatedCPUPlacement: true,
 								IsolateEmulatorThread: true,
 							},
@@ -1714,9 +1718,14 @@ var _ = Describe("Template", func() {
 
 				pod, err := svc.RenderLaunchManifest(&vmi)
 				Expect(err).ToNot(HaveOccurred())
-				cpu := resource.MustParse("3")
+				cpu := resource.MustParse(expectedCPULimits)
 				Expect(pod.Spec.Containers[0].Resources.Limits.Cpu().Cmp(cpu)).To(BeZero())
-			})
+			},
+				Entry("no annotation added, odd  CPUs requested, should allocate one extra emulator CPU", map[string]string{}, uint32(3), "4"),
+				Entry("no annotation added, even CPUs requested, should allocate one extra emulator CPU", map[string]string{}, uint32(2), "3"),
+				Entry("EmulatorThreadCompleteToEvenParity annotation added, odd CPUs requested, should allocate one extra emulator CPU", map[string]string{v1.EmulatorThreadCompleteToEvenParity: ""}, uint32(3), "4"),
+				Entry("EmulatorThreadCompleteToEvenParity annotation added, even CPUs requested, should allocate two extra emulator CPU (to align SMT scheduling)", map[string]string{v1.EmulatorThreadCompleteToEvenParity: ""}, uint32(4), "6"),
+			)
 			It("should add node affinity to pod", func() {
 				config, kvInformer, svc = configFactory(defaultArch)
 				nodeAffinity := k8sv1.NodeAffinity{}

pkg/virt-handler/vm.go

@@ -2946,14 +2946,14 @@ func (d *VirtualMachineController) configureHousekeepingCgroup(vmi *v1.VirtualMa
 		return nil
 	}
 
-	hkcpu, err := strconv.Atoi(domain.Spec.CPUTune.EmulatorPin.CPUSet)
+	hkcpus, err := hardware.ParseCPUSetLine(domain.Spec.CPUTune.EmulatorPin.CPUSet, 100)
 	if err != nil {
 		return err
 	}
 
-	log.Log.V(3).Object(vmi).Infof("housekeeping cpu: %v", hkcpu)
+	log.Log.V(3).Object(vmi).Infof("housekeeping cpu: %v", hkcpus)
 
-	err = cgroupManager.SetCpuSet("housekeeping", []int{hkcpu})
+	err = cgroupManager.SetCpuSet("housekeeping", hkcpus)
 	if err != nil {
 		return err
 	}

pkg/virt-launcher/virtwrap/converter/BUILD.bazel

@@ -62,6 +62,7 @@ go_test(
         "//pkg/handler-launcher-com/cmd/v1:go_default_library",
         "//pkg/pointer:go_default_library",
         "//pkg/testutils:go_default_library",
+        "//pkg/util/hardware:go_default_library",
         "//pkg/virt-api/webhooks:go_default_library",
         "//pkg/virt-controller/services:go_default_library",
         "//pkg/virt-launcher/virtwrap/api:go_default_library",