add validation to resource requirement (#685)

* add validation for resource requirement Signed-off-by: Stephanie <[email protected]> * address review comment Signed-off-by: Stephanie <[email protected]>
devfile · Nov 18, 2021 · 959f3c8 · 959f3c8
1 parent dfec9a4
commit 959f3c8
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 0 deletions.
diff --git a/pkg/validation/components.go b/pkg/validation/components.go
@@ -60,6 +60,41 @@ func ValidateComponents(components []v1alpha2.Component) (returnedErr error) {
 					returnedErr = multierror.Append(returnedErr, reservedEnvErr)
 				}
 			}
+			var memoryLimit, cpuLimit, memoryRequest, cpuRequest resource.Quantity
+			if component.Container.MemoryLimit != "" {
+				memoryLimit, err = resource.ParseQuantity(component.Container.MemoryLimit)
+				if err != nil {
+					parseQuantityErr := &ParsingResourceRequirementError{resource: MemoryLimit, cmpName: component.Name, errMsg: err.Error()}
+					returnedErr = multierror.Append(returnedErr, parseQuantityErr)
+				}
+			}
+			if component.Container.CpuLimit != "" {
+				cpuLimit, err = resource.ParseQuantity(component.Container.CpuLimit)
+				if err != nil {
+					parseQuantityErr := &ParsingResourceRequirementError{resource: CpuLimit, cmpName: component.Name, errMsg: err.Error()}
+					returnedErr = multierror.Append(returnedErr, parseQuantityErr)
+				}
+			}
+			if component.Container.MemoryRequest != "" {
+				memoryRequest, err = resource.ParseQuantity(component.Container.MemoryRequest)
+				if err != nil {
+					parseQuantityErr := &ParsingResourceRequirementError{resource: MemoryRequest, cmpName: component.Name, errMsg: err.Error()}
+					returnedErr = multierror.Append(returnedErr, parseQuantityErr)
+				} else if !memoryLimit.IsZero() && memoryRequest.Cmp(memoryLimit) > 0 {
+					invalidResourceRequest := &InvalidResourceRequestError{cmpName: component.Name, errMsg: fmt.Sprintf("memoryRequest is greater than memoryLimit.")}
+					returnedErr = multierror.Append(returnedErr, invalidResourceRequest)
+				}
+			}
+			if component.Container.CpuRequest != "" {
+				cpuRequest, err = resource.ParseQuantity(component.Container.CpuRequest)
+				if err != nil {
+					parseQuantityErr := &ParsingResourceRequirementError{resource: CpuRequest, cmpName: component.Name, errMsg: err.Error()}
+					returnedErr = multierror.Append(returnedErr, parseQuantityErr)
+				} else if !cpuLimit.IsZero() && cpuRequest.Cmp(cpuLimit) > 0 {
+					invalidResourceRequest := &InvalidResourceRequestError{cmpName: component.Name, errMsg: fmt.Sprintf("cpuRequest is greater than cpuLimit.")}
+					returnedErr = multierror.Append(returnedErr, invalidResourceRequest)
+				}
+			}
 
 			// if annotation is not empty and dedicatedPod is false
 			if component.Container.Annotation != nil && component.Container.DedicatedPod != nil && !(*component.Container.DedicatedPod) {

diff --git a/pkg/validation/components_test.go b/pkg/validation/components_test.go
@@ -31,6 +31,24 @@ func generateDummyContainerComponent(name string, volMounts []v1alpha2.VolumeMou
 			}}}
 }
 
+// generateDummyContainerComponentWithResourceRequirement returns a dummy container component with resource requirement for testing
+func generateDummyContainerComponentWithResourceRequirement(name, memoryLimit, memoryRequest, cpuLimit, cpuRequest string) v1alpha2.Component {
+	image := "docker.io/maven:latest"
+
+	return v1alpha2.Component{
+		Name: name,
+		ComponentUnion: v1alpha2.ComponentUnion{
+			Container: &v1alpha2.ContainerComponent{
+				Container: v1alpha2.Container{
+					Image:         image,
+					MemoryLimit:   memoryLimit,
+					MemoryRequest: memoryRequest,
+					CpuLimit:      cpuLimit,
+					CpuRequest:    cpuRequest,
+				},
+			}}}
+}
+
 // generateDummyVolumeComponent returns a dummy volume component for testing
 func generateDummyVolumeComponent(name, size string) v1alpha2.Component {
 
@@ -236,6 +254,9 @@ func TestValidateComponents(t *testing.T) {
 	pluginOverridesFromMainDevfile := attributes.Attributes{}.PutString(ImportSourceAttribute,
 		"uri: http://127.0.0.1:8080").PutString(PluginOverrideAttribute, "main devfile")
 	invalidURIErrWithImportAttributes := ".*invalid URI for request, imported from uri: http://127.0.0.1:8080, in plugin overrides from main devfile"
+	invalidCpuRequest := ".*cpuRequest is greater than cpuLimit."
+	invalidMemoryRequest := ".*memoryRequest is greater than memoryLimit."
+	quantityParsingErr := "error parsing .* requirement for component.*"
 
 	tests := []struct {
 		name       string
@@ -310,6 +331,30 @@ func TestValidateComponents(t *testing.T) {
 				generateDummyContainerComponent("name1", nil, []v1alpha2.Endpoint{endpointUrl18080, endpointUrl28080}, nil, v1alpha2.Annotation{}, false),
 			},
 		},
+		{
+			name: "Valid containers with valid resource requirement",
+			components: []v1alpha2.Component{
+				generateDummyContainerComponentWithResourceRequirement("name1", "1024Mi", "512Mi", "1024Mi", "512Mi"),
+				generateDummyContainerComponentWithResourceRequirement("name2", "", "512Mi", "", "512Mi"),
+				generateDummyContainerComponentWithResourceRequirement("name3", "1024Mi", "", "1024Mi", ""),
+				generateDummyContainerComponentWithResourceRequirement("name4", "", "", "", ""),
+			},
+		},
+		{
+			name: "Invalid containers with resource limit smaller than resource requested",
+			components: []v1alpha2.Component{
+				generateDummyContainerComponentWithResourceRequirement("name1", "512Mi", "1024Mi", "", ""),
+				generateDummyContainerComponentWithResourceRequirement("name2", "", "", "512Mi", "1024Mi"),
+			},
+			wantErr: []string{invalidMemoryRequest, invalidCpuRequest},
+		},
+		{
+			name: "Invalid container with resource quantity parsing error",
+			components: []v1alpha2.Component{
+				generateDummyContainerComponentWithResourceRequirement("name1", "512invalid", "", "", ""),
+			},
+			wantErr: []string{quantityParsingErr},
+		},
 		{
 			name: "Valid container with deployment, service and ingress annotations",
 			components: []v1alpha2.Component{

diff --git a/pkg/validation/errors.go b/pkg/validation/errors.go
@@ -161,6 +161,36 @@ func (e *InvalidProjectCheckoutRemoteError) Error() string {
 	return fmt.Sprintf("unable to find the checkout remote %s in the remotes for %s %s", e.checkoutRemote, e.objectType, e.objectName)
 }
 
+type ResourceRequirementType string
+
+const (
+	MemoryLimit   ResourceRequirementType = "memoryLimit"
+	CpuLimit      ResourceRequirementType = "cpuLimit"
+	MemoryRequest ResourceRequirementType = "memoryRequest"
+	CpuRequest    ResourceRequirementType = "cpuRequest"
+)
+
+//ParsingResourceRequirementError returns an error if failed to parse a resource requirement
+type ParsingResourceRequirementError struct {
+	resource ResourceRequirementType
+	cmpName  string
+	errMsg   string
+}
+
+func (e *ParsingResourceRequirementError) Error() string {
+	return fmt.Sprintf("error parsing %s requirement for component %s: %s", e.resource, e.cmpName, e.errMsg)
+}
+
+//InvalidResourceRequestError returns an error if resource limit < resource requested
+type InvalidResourceRequestError struct {
+	cmpName string
+	errMsg  string
+}
+
+func (e *InvalidResourceRequestError) Error() string {
+	return fmt.Sprintf("invalid resource request for component %s: %s", e.cmpName, e.errMsg)
+}
+
 type AnnotationType string
 
 const (

diff --git a/pkg/validation/validation-rule.md b/pkg/validation/validation-rule.md
@@ -35,6 +35,7 @@ Common rules for all components types:
 1. the container components must reference a valid volume component if it uses volume mounts, and the volume components are unique
 2. `PROJECT_SOURCE` or `PROJECTS_ROOT` are reserved environment variables defined under env, cannot be defined again in `env`
 3. the annotations should not have conflict values for same key, except deployment annotations and service annotations set for a container with `dedicatedPod=true`
+4. resource requirements, e.g. `cpuLimit`, `cpuRequest`, `memoryLimit`, `memoryRequest`, must be in valid quantity format; and the resource requested must be less than the resource limit (if specified).
 
 #### Plugin Component
 - Commands in plugins components share the same commands validation rules as listed above. Validation occurs after overriding and merging, in flattened devfile