- fixed messages for memory validation

- if memory size is < 0 then QuantityToByte will return err
Ethan-2017 · Jun 6, 2018 · fb0fcc9 · fb0fcc9
1 parent 75978d1
commit fb0fcc9
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 30 deletions.
diff --git a/pkg/virt-api/validating-webhook/validating-webhook.go b/pkg/virt-api/validating-webhook/validating-webhook.go
@@ -324,20 +324,20 @@ func validateVirtualMachineSpec(field *k8sfield.Path, spec *v1.VirtualMachineSpe
 	// Validate memory size if values are not negative
 	if spec.Domain.Resources.Requests.Memory().Value() < 0 {
 		causes = append(causes, metav1.StatusCause{
-			Type:    metav1.CauseTypeFieldValueInvalid,
-			Message: fmt.Sprintf("%s is specified as negative value", field.Child("domain", "resources", "requests", "memory").String()),
-			Field:   field.Child("domain", "resources", "requests", "memory").String(),
+			Type: metav1.CauseTypeFieldValueInvalid,
+			Message: fmt.Sprintf("%s '%s': must be greater than or equal to 0.", field.Child("domain", "resources", "requests", "memory").String(),
+				spec.Domain.Resources.Requests.Memory()),
+			Field: field.Child("domain", "resources", "requests", "memory").String(),
 		})
-		// We won't process anything with negative requests.memory size
-		return causes
-	} else if spec.Domain.Resources.Limits.Memory().Value() < 0 {
+	}
+
+	if spec.Domain.Resources.Limits.Memory().Value() < 0 {
 		causes = append(causes, metav1.StatusCause{
-			Type:    metav1.CauseTypeFieldValueInvalid,
-			Message: fmt.Sprintf("%s is specified as negative value", field.Child("domain", "resources", "limits", "memory").String()),
-			Field:   field.Child("domain", "resources", "limits", "memory").String(),
+			Type: metav1.CauseTypeFieldValueInvalid,
+			Message: fmt.Sprintf("%s '%s': must be greater than or equal to 0.", field.Child("domain", "resources", "limits", "memory").String(),
+				spec.Domain.Resources.Limits.Memory()),
+			Field: field.Child("domain", "resources", "limits", "memory").String(),
 		})
-		// We won't process anything with negative limits.memory size
-		return causes
 	}
 
 	for _, volume := range spec.Volumes {

diff --git a/pkg/virt-launcher/virtwrap/api/converter.go b/pkg/virt-launcher/virtwrap/api/converter.go
@@ -371,7 +371,9 @@ func Convert_v1_VirtualMachine_To_api_Domain(vm *v1.VirtualMachine, domain *Doma
 	}
 
 	if v, ok := vm.Spec.Domain.Resources.Requests[k8sv1.ResourceMemory]; ok {
-		domain.Spec.Memory = QuantityToByte(v)
+		if domain.Spec.Memory, err = QuantityToByte(v); err != nil {
+			return err
+		}
 	}
 
 	volumes := map[string]*v1.Volume{}
@@ -505,12 +507,15 @@ func SecretToLibvirtSecret(vm *v1.VirtualMachine, secretName string) string {
 	return fmt.Sprintf("%s-%s-%s---", secretName, vm.Namespace, vm.Name)
 }
 
-func QuantityToByte(quantity resource.Quantity) Memory {
+func QuantityToByte(quantity resource.Quantity) (Memory, error) {
 	memorySize, _ := quantity.AsInt64()
+	if memorySize < 0 {
+		return Memory{Unit: "B"}, fmt.Errorf("Memory size '%s' must be greater than or equal to 0", quantity.String())
+	}
 	return Memory{
 		Value: uint64(memorySize),
 		Unit:  "B",
-	}
+	}, nil
 }
 
 func boolToOnOff(value *bool, defaultOn bool) string {

diff --git a/pkg/virt-launcher/virtwrap/api/converter_test.go b/pkg/virt-launcher/virtwrap/api/converter_test.go
@@ -464,37 +464,45 @@ var _ = Describe("Converter", func() {
 		It("should calculate memory in bytes", func() {
 			By("specifying memory 64M")
 			m64, _ := resource.ParseQuantity("64M")
-			memory := QuantityToByte(m64)
+			memory, err := QuantityToByte(m64)
 			Expect(memory.Value).To(Equal(uint64(64000000)))
 			Expect(memory.Unit).To(Equal("B"))
+			Expect(err).ToNot(HaveOccurred())
 
 			By("specifying memory 64Mi")
 			mi64, _ := resource.ParseQuantity("64Mi")
-			Expect(QuantityToByte(mi64).Value).To(Equal(uint64(67108864)))
+			memory, err = QuantityToByte(mi64)
+			Expect(memory.Value).To(Equal(uint64(67108864)))
+			Expect(err).ToNot(HaveOccurred())
 
 			By("specifying memory 3G")
-			g4, _ := resource.ParseQuantity("3G")
-			Expect(QuantityToByte(g4).Value).To(Equal(uint64(3000000000)))
+			g3, _ := resource.ParseQuantity("3G")
+			memory, err = QuantityToByte(g3)
+			Expect(memory.Value).To(Equal(uint64(3000000000)))
+			Expect(err).ToNot(HaveOccurred())
 
 			By("specifying memory 3Gi")
-			gi4, _ := resource.ParseQuantity("3Gi")
-			Expect(QuantityToByte(gi4).Value).To(Equal(uint64(3221225472)))
+			gi3, _ := resource.ParseQuantity("3Gi")
+			memory, err = QuantityToByte(gi3)
+			Expect(memory.Value).To(Equal(uint64(3221225472)))
+			Expect(err).ToNot(HaveOccurred())
 
 			By("specifying memory 45Gi")
 			gi45, _ := resource.ParseQuantity("45Gi")
-			Expect(QuantityToByte(gi45).Value).To(Equal(uint64(48318382080)))
+			memory, err = QuantityToByte(gi45)
+			Expect(memory.Value).To(Equal(uint64(48318382080)))
+			Expect(err).ToNot(HaveOccurred())
 
 			By("specifying memory 451231 bytes")
 			b451231, _ := resource.ParseQuantity("451231")
-			Expect(QuantityToByte(b451231).Value).To(Equal(uint64(451231)))
-
-			By("specifying memory which exceeds int64 size")
-			exceeds, _ := resource.ParseQuantity("1Pi")
-			Expect(QuantityToByte(exceeds).Value).To(Equal(uint64(0)))
-
-			By("specifying invalid memory 23Inv")
-			inv23, _ := resource.ParseQuantity("23Inv")
-			Expect(QuantityToByte(inv23).Value).To(Equal(uint64(0)))
+			memory, err = QuantityToByte(b451231)
+			Expect(memory.Value).To(Equal(uint64(451231)))
+			Expect(err).ToNot(HaveOccurred())
+
+			By("specyfing negative memory size -45Gi")
+			m45gi, _ := resource.ParseQuantity("-45Gi")
+			_, err = QuantityToByte(m45gi)
+			Expect(err).To(HaveOccurred())
 		})
 	})
 })