feat: Set default resources for dedicated CPU placement

smartxworks · Nov 22, 2022 · 6bb864a · 6bb864a
1 parent 2a1724d
commit 6bb864a
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 25 deletions.
diff --git a/deploy/crd/virt.virtink.smartx.com_virtualmachines.yaml b/deploy/crd/virt.virtink.smartx.com_virtualmachines.yaml
@@ -948,8 +948,6 @@ spec:
                         pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
                         x-kubernetes-int-or-string: true
                     type: object
-                required:
-                - memory
                 type: object
               livenessProbe:
                 description: Probe describes a health check to be performed against

diff --git a/docs/dedicated_cpu_placement.md b/docs/dedicated_cpu_placement.md
@@ -10,7 +10,7 @@ Kubernetes CPU manager is a mechanism that affects the scheduling of workloads.
 
 Setting `spec.instance.cpu.dedicatedCPUPlacement` to `true` in a VM spec will indicate the desire to allocate dedicated CPU resource to the VM.
 
-Expressing the desired amount of VM's vCPUs must be done by setting both the guest topology in `spec.instance.cpu` (`sockets` and `coresPerSocket`) and corresponding number of vCPUs (counted as `sockets * coresPerSocket`) in `spec.resources.[requests/limits].cpu`.
+Expressing the desired amount of VM's vCPUs must be done by setting the guest topology in `spec.instance.cpu` (`sockets` and `coresPerSocket`).
 
 Example:
 
@@ -23,13 +23,4 @@ spec:
       sockets: 2
       coresPerSocket: 1
       dedicatedCPUPlacement: true
-    memory:
-      size: 2Gi
-  resources:
-    requests:
-      cpu: 2
-      memory: 2.2Gi
-    limits:
-      cpu: 2
-      memory: 2.2Gi
 ```
diff --git a/pkg/apis/virt/v1alpha1/types.go b/pkg/apis/virt/v1alpha1/types.go
@@ -53,7 +53,7 @@ const (
 
 type Instance struct {
 	CPU         CPU          `json:"cpu,omitempty"`
-	Memory      Memory       `json:"memory"`
+	Memory      Memory       `json:"memory,omitempty"`
 	Kernel      *Kernel      `json:"kernel,omitempty"`
 	Disks       []Disk       `json:"disks,omitempty"`
 	FileSystems []FileSystem `json:"fileSystems,omitempty"`

diff --git a/pkg/controller/vm_webhook.go b/pkg/controller/vm_webhook.go
@@ -86,6 +86,41 @@ func MutateVM(ctx context.Context, vm *virtv1alpha1.VirtualMachine, oldVM *virtv
 		}
 	}
 
+	if vm.Spec.Instance.CPU.DedicatedCPUPlacement {
+		memSize := resource.MustParse(memoryOverhead)
+		if !vm.Spec.Instance.Memory.Size.IsZero() {
+			if vm.Spec.Instance.Memory.Hugepages == nil {
+				memSize.Add(vm.Spec.Instance.Memory.Size)
+			}
+		}
+		rsList := map[corev1.ResourceName]resource.Quantity{
+			corev1.ResourceCPU:    *resource.NewQuantity(int64(vm.Spec.Instance.CPU.CoresPerSocket*vm.Spec.Instance.CPU.Sockets), resource.DecimalSI),
+			corev1.ResourceMemory: memSize,
+		}
+
+		if vm.Spec.Resources.Requests == nil {
+			vm.Spec.Resources.Requests = rsList
+		} else {
+			if vm.Spec.Resources.Requests.Cpu().IsZero() {
+				vm.Spec.Resources.Requests[corev1.ResourceCPU] = rsList[corev1.ResourceCPU]
+			}
+			if vm.Spec.Resources.Requests.Memory().IsZero() {
+				vm.Spec.Resources.Requests[corev1.ResourceMemory] = rsList[corev1.ResourceMemory]
+			}
+		}
+
+		if vm.Spec.Resources.Limits == nil {
+			vm.Spec.Resources.Limits = rsList
+		} else {
+			if vm.Spec.Resources.Limits.Cpu().IsZero() {
+				vm.Spec.Resources.Limits[corev1.ResourceCPU] = rsList[corev1.ResourceCPU]
+			}
+			if vm.Spec.Resources.Limits.Memory().IsZero() {
+				vm.Spec.Resources.Limits[corev1.ResourceMemory] = rsList[corev1.ResourceMemory]
+			}
+		}
+	}
+
 	if vm.Spec.Instance.Memory.Hugepages != nil {
 		hugepagesSize := fmt.Sprintf("hugepages-%s", vm.Spec.Instance.Memory.Hugepages.PageSize)
 
@@ -213,10 +248,9 @@ func ValidateVMSpec(ctx context.Context, spec *virtv1alpha1.VirtualMachineSpec,
 		}
 
 		memoryRequestField := fieldPath.Child("resources.requests").Child(string(corev1.ResourceMemory))
-		// TODO: add overhead without hugepages
-		memRequired := spec.Instance.Memory.Size.DeepCopy()
-		if spec.Instance.Memory.Hugepages != nil {
-			memRequired = resource.MustParse(memoryOverhead)
+		memRequired := resource.MustParse(memoryOverhead)
+		if spec.Instance.Memory.Hugepages == nil {
+			memRequired.Add(spec.Instance.Memory.Size)
 		}
 		if spec.Resources.Requests.Memory().IsZero() {
 			errs = append(errs, field.Required(memoryRequestField, ""))

diff --git a/pkg/controller/vm_webhook_test.go b/pkg/controller/vm_webhook_test.go
@@ -72,11 +72,11 @@ func TestValidateVM(t *testing.T) {
 			vm.Spec.Resources = corev1.ResourceRequirements{
 				Requests: map[corev1.ResourceName]resource.Quantity{
 					corev1.ResourceCPU:    resource.MustParse("0"),
-					corev1.ResourceMemory: resource.MustParse("1Gi"),
+					corev1.ResourceMemory: resource.MustParse("1280Mi"),
 				},
 				Limits: map[corev1.ResourceName]resource.Quantity{
 					corev1.ResourceCPU:    resource.MustParse("1"),
-					corev1.ResourceMemory: resource.MustParse("1Gi"),
+					corev1.ResourceMemory: resource.MustParse("1280Mi"),
 				},
 			}
 			return vm
@@ -89,11 +89,11 @@ func TestValidateVM(t *testing.T) {
 			vm.Spec.Resources = corev1.ResourceRequirements{
 				Requests: map[corev1.ResourceName]resource.Quantity{
 					corev1.ResourceCPU:    resource.MustParse("1"),
-					corev1.ResourceMemory: resource.MustParse("1Gi"),
+					corev1.ResourceMemory: resource.MustParse("1280Mi"),
 				},
 				Limits: map[corev1.ResourceName]resource.Quantity{
 					corev1.ResourceCPU:    resource.MustParse("0"),
-					corev1.ResourceMemory: resource.MustParse("1Gi"),
+					corev1.ResourceMemory: resource.MustParse("1280Mi"),
 				},
 			}
 			return vm
@@ -106,11 +106,11 @@ func TestValidateVM(t *testing.T) {
 			vm.Spec.Resources = corev1.ResourceRequirements{
 				Requests: map[corev1.ResourceName]resource.Quantity{
 					corev1.ResourceCPU:    resource.MustParse("2"),
-					corev1.ResourceMemory: resource.MustParse("1Gi"),
+					corev1.ResourceMemory: resource.MustParse("1280Mi"),
 				},
 				Limits: map[corev1.ResourceName]resource.Quantity{
 					corev1.ResourceCPU:    resource.MustParse("2"),
-					corev1.ResourceMemory: resource.MustParse("1Gi"),
+					corev1.ResourceMemory: resource.MustParse("1280Mi"),
 				},
 			}
 			return vm
@@ -127,7 +127,7 @@ func TestValidateVM(t *testing.T) {
 				},
 				Limits: map[corev1.ResourceName]resource.Quantity{
 					corev1.ResourceCPU:    resource.MustParse("1"),
-					corev1.ResourceMemory: resource.MustParse("1Gi"),
+					corev1.ResourceMemory: resource.MustParse("1280Mi"),
 				},
 			}
 			return vm
@@ -140,7 +140,7 @@ func TestValidateVM(t *testing.T) {
 			vm.Spec.Resources = corev1.ResourceRequirements{
 				Requests: map[corev1.ResourceName]resource.Quantity{
 					corev1.ResourceCPU:    resource.MustParse("1"),
-					corev1.ResourceMemory: resource.MustParse("1Gi"),
+					corev1.ResourceMemory: resource.MustParse("1280Mi"),
 				},
 				Limits: map[corev1.ResourceName]resource.Quantity{
 					corev1.ResourceCPU:    resource.MustParse("1"),
@@ -462,6 +462,20 @@ func TestMutateVM(t *testing.T) {
 		assert: func(vm *virtv1alpha1.VirtualMachine) {
 			assert.Equal(t, "1Gi", vm.Spec.Instance.Memory.Size.String())
 		},
+	}, {
+		vm: func() *virtv1alpha1.VirtualMachine {
+			vm := oldVM.DeepCopy()
+			vm.Spec.Instance.CPU.DedicatedCPUPlacement = true
+			vm.Spec.Resources.Requests = nil
+			vm.Spec.Instance.Memory.Size = resource.MustParse("1Gi")
+			return vm
+		}(),
+		assert: func(vm *virtv1alpha1.VirtualMachine) {
+			assert.Equal(t, "1", vm.Spec.Resources.Requests.Cpu().String())
+			assert.Equal(t, "1", vm.Spec.Resources.Limits.Cpu().String())
+			assert.Equal(t, "1280Mi", vm.Spec.Resources.Requests.Memory().String())
+			assert.Equal(t, "1280Mi", vm.Spec.Resources.Limits.Memory().String())
+		},
 	}, {
 		vm: func() *virtv1alpha1.VirtualMachine {
 			vm := oldVM.DeepCopy()