feat(nfs-server): add support to configure resource requirements on NFS Server (#92)

sai chaithanya · sai chaithanya · commit 7e0ba59eb38f · 2021-08-30T19:10:40.000+05:30
This PR adds support to configure resource requirements[requests &amp; limits]
for NFS Server via NFS StorageClass. Values specified in StorageClass annotation
will be propagated to NFS Server deployment.

**Sample NFS SC YAML**:
```yaml
apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  name: openebs-rwx
  annotations:
    openebs.io/cas-type: nfsrwx
    cas.openebs.io/config: |
      - name: NFSServerType
        value: "kernel"
      - name: BackendStorageClass
        value: "openebs-hostpath"
      #  NFSServerResourceRequests defines the resource requests for NFS Server
      - name: NFSServerResourceRequests
        value: |-
            memory: 50Mi
            cpu: 50m
      #  NFSServerResourceLimits defines the resource limits for NFS Server
      - name: NFSServerResourceLimits
        value: |-
            memory: 100Mi
            cpu: 100m
provisioner: openebs.io/nfsrwx
reclaimPolicy: Delete
```

Signed-off-by: mittachaitu &lt;sai.chaithanya@mayadata.io&gt;
diff --git a/Makefile b/Makefile
@@ -195,7 +195,7 @@ license-check:
 .PHONY: sanity-test
 sanity-test: sanity-test
 	@echo "--> Running sanity test";
-	go test -v -timeout 30m ./tests/...
+	go test -v -timeout 60m ./tests/...
 
 .PHONY: push
 push:
diff --git a/deploy/kubectl/openebs-nfs-provisioner.yaml b/deploy/kubectl/openebs-nfs-provisioner.yaml
@@ -188,6 +188,16 @@ metadata:
         value: "kernel"
       - name: BackendStorageClass
         value: "openebs-hostpath"
+      #  NFSServerResourceRequests defines the resource requests for NFS Server
+      #- name: NFSServerResourceRequests
+      #  value: |-
+      #      memory: 50Mi
+      #      cpu: 50m
+      #  NFSServerResourceLimits defines the resource limits for NFS Server
+      #- name: NFSServerResourceLimits
+      #  value: |-
+      #      memory: 100Mi
+      #      cpu: 100m
       #  LeaseTime defines the renewl period(in seconds) for client state
       #- name: LeaseTime
       #  value: 30
diff --git a/go.mod b/go.mod
@@ -30,15 +30,19 @@ replace (
 )
 
 require (
+	github.com/cloudflare/cfssl v1.6.0 // indirect
+	github.com/cloudflare/go-metrics v0.0.0-20151117154305-6a9aea36fb41 // indirect
 	github.com/ghodss/yaml v1.0.0
+	github.com/google/go-cmp v0.4.0
 	github.com/onsi/ginkgo v1.12.0
 	github.com/onsi/gomega v1.9.0
 	github.com/openebs/maya v1.12.1
 	github.com/pkg/errors v0.9.1
-	github.com/prometheus/client_golang v1.0.0
+	github.com/prometheus/client_golang v1.9.0
 	github.com/spf13/cobra v1.1.1
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.4.0
+	github.com/zmap/zlint v0.0.0-20190806154020-fd021b4cfbeb // indirect
 	k8s.io/api v0.17.3
 	k8s.io/apimachinery v0.17.3
 	k8s.io/client-go v11.0.0+incompatible
diff --git a/go.sum b/go.sum
diff --git a/provisioner/config.go b/provisioner/config.go
@@ -21,6 +21,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/ghodss/yaml"
 	mconfig "github.com/openebs/maya/pkg/apis/openebs.io/v1alpha1"
 	cast "github.com/openebs/maya/pkg/castemplate/v1alpha1"
 	"github.com/openebs/maya/pkg/util"
@@ -58,6 +59,12 @@ const (
 
 	// FSGroupID defines the permissions of nfs share volume
 	FSGroupID = "FSGID"
+
+	// NFSServerResourceRequests holds key name that represent NFS Resource Requests
+	NFSServerResourceRequests = "NFSServerResourceRequests"
+
+	// NFSServerResourceLimits holds key name that represent NFS Resource Limits
+	NFSServerResourceLimits = "NFSServerResourceLimits"
 )
 
 const (
@@ -192,6 +199,35 @@ func (c *VolumeConfig) GetFSGroupID() (*int64, error) {
 	return &fsGIDInt, nil
 }
 
+// GetNFSServerResourceRequirements fetches the resource(cpu & memory) request &
+// limits for NFS server from StorageClass only if specified
+func (c *VolumeConfig) GetNFSServerResourceRequirements() (*v1.ResourceRequirements, error) {
+	var err error
+	resourceRequirements := &v1.ResourceRequirements{}
+	resourceRequirements.Requests, err = c.getResourceList(NFSServerResourceRequests)
+	if err != nil {
+		return nil, err
+	}
+
+	resourceRequirements.Limits, err = c.getResourceList(NFSServerResourceLimits)
+	if err != nil {
+		return nil, err
+	}
+	return resourceRequirements, nil
+}
+
+// getResourceList is a utility function to extract resource list
+// and convert from map[string]interface{} to proper Go struct
+func (c *VolumeConfig) getResourceList(key string) (v1.ResourceList, error) {
+	var resourceList v1.ResourceList
+	dataStr := c.getValue(key)
+	err := yaml.Unmarshal([]byte(dataStr), &resourceList)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to marshal data %s", dataStr)
+	}
+	return resourceList, nil
+}
+
 //getValue is a utility function to extract the value
 // of the `key` from the ConfigMap object - which is
 // map[string]interface{map[string][string]}
diff --git a/provisioner/config_test.go b/provisioner/config_test.go
@@ -0,0 +1,170 @@
+/*
+Copyright 2021 The OpenEBS Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package provisioner
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/ghodss/yaml"
+	"github.com/google/go-cmp/cmp"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+)
+
+func TestGetResourceList(t *testing.T) {
+	tests := map[string]struct {
+		volumeConfig         *VolumeConfig
+		key                  string
+		expectedResourceList corev1.ResourceList
+		isErrExpected        bool
+	}{
+		"When NFS resource requests has only memory field": {
+			volumeConfig: &VolumeConfig{
+				options: map[string]interface{}{
+					NFSServerResourceRequests: map[string]string{
+						"value": func() string {
+							resourceList := make(map[corev1.ResourceName]resource.Quantity)
+							resourceList[corev1.ResourceMemory] = resource.MustParse("500M")
+							data, err := yaml.Marshal(resourceList)
+							if err != nil {
+								t.Errorf("failed to convert to YAML error %v", err)
+							}
+							return string(data)
+						}(),
+					},
+				},
+			},
+			key: NFSServerResourceRequests,
+			expectedResourceList: map[corev1.ResourceName]resource.Quantity{
+				corev1.ResourceMemory: resource.MustParse("500M"),
+			},
+		},
+		"When NFS resource requests has both memory and cpu": {
+			volumeConfig: &VolumeConfig{
+				options: map[string]interface{}{
+					NFSServerResourceRequests: map[string]string{
+						"value": func() string {
+							resourceList := make(map[corev1.ResourceName]resource.Quantity)
+							resourceList[corev1.ResourceMemory] = resource.MustParse("500M")
+							resourceList[corev1.ResourceCPU] = resource.MustParse("500m")
+							data, err := yaml.Marshal(resourceList)
+							if err != nil {
+								t.Errorf("failed to convert to YAML error %v", err)
+							}
+							return string(data)
+						}(),
+					},
+				},
+			},
+			key: NFSServerResourceRequests,
+			expectedResourceList: map[corev1.ResourceName]resource.Quantity{
+				corev1.ResourceMemory: resource.MustParse("500M"),
+				corev1.ResourceCPU:    resource.MustParse("500m"),
+			},
+		},
+		"When NFS resource limits has only memory specified": {
+			volumeConfig: &VolumeConfig{
+				options: map[string]interface{}{
+					"NFSResourceLimits": map[string]string{
+						"value": func() string {
+							resourceList := make(map[corev1.ResourceName]resource.Quantity)
+							resourceList[corev1.ResourceMemory] = resource.MustParse("150Mi")
+							data, err := yaml.Marshal(resourceList)
+							if err != nil {
+								t.Errorf("failed to convert to YAML error %v", err)
+							}
+							return string(data)
+						}(),
+					},
+				},
+			},
+			key: "NFSResourceLimits",
+			expectedResourceList: map[corev1.ResourceName]resource.Quantity{
+				corev1.ResourceMemory: resource.MustParse("150Mi"),
+			},
+		},
+		"When NFS resource limits has both memory & cpu specified": {
+			volumeConfig: &VolumeConfig{
+				options: map[string]interface{}{
+					"NFSResourceLimits": map[string]string{
+						"value": func() string {
+							resourceList := make(map[corev1.ResourceName]resource.Quantity)
+							resourceList[corev1.ResourceMemory] = resource.MustParse("150Mi")
+							resourceList[corev1.ResourceCPU] = resource.MustParse("150m")
+							data, err := yaml.Marshal(resourceList)
+							if err != nil {
+								t.Errorf("failed to convert to YAML error %v", err)
+							}
+							return string(data)
+						}(),
+					},
+				},
+			},
+			key: "NFSResourceLimits",
+			expectedResourceList: map[corev1.ResourceName]resource.Quantity{
+				corev1.ResourceMemory: resource.MustParse("150Mi"),
+				corev1.ResourceCPU:    resource.MustParse("150m"),
+			},
+		},
+		"When NFS resource limits are specified but requesting for requests": {
+			volumeConfig: &VolumeConfig{
+				options: map[string]interface{}{
+					"NFSResourceLimits": map[string]string{
+						"value": func() string {
+							resourceList := make(map[corev1.ResourceName]resource.Quantity)
+							resourceList[corev1.ResourceMemory] = resource.MustParse("150Mi")
+							resourceList[corev1.ResourceCPU] = resource.MustParse("150m")
+							data, err := yaml.Marshal(resourceList)
+							if err != nil {
+								t.Errorf("failed to convert to YAML error %v", err)
+							}
+							return string(data)
+						}(),
+					},
+				},
+			},
+			key:                  NFSServerResourceRequests,
+			expectedResourceList: nil,
+		},
+		"When invalid NFS resource requests are specified error should occur": {
+			volumeConfig: &VolumeConfig{
+				options: map[string]interface{}{
+					NFSServerResourceRequests: map[string]string{
+						"value": `memory: 50Ci`,
+					},
+				},
+			},
+			key:           NFSServerResourceRequests,
+			isErrExpected: true,
+		},
+	}
+
+	for name, test := range tests {
+		gotOutput, err := test.volumeConfig.getResourceList(test.key)
+		if test.isErrExpected && err == nil {
+			t.Errorf("%q test failed expected error to occur but got nil", name)
+		}
+		if !test.isErrExpected && err != nil {
+			t.Errorf("%q test failed expected error not to occur but got %v", name, err)
+		}
+		if !test.isErrExpected {
+			if !reflect.DeepEqual(test.expectedResourceList, gotOutput) {
+				t.Errorf("%q test has following diff %s", name, cmp.Diff(test.expectedResourceList, gotOutput))
+			}
+		}
+	}
+}
diff --git a/provisioner/helper_kernel_nfs_server.go b/provisioner/helper_kernel_nfs_server.go
@@ -92,6 +92,12 @@ type KernelNFSServerOptions struct {
 	// fsGID defines the filesystem group ID if set then nfs share
 	// volume permissions will be updated by OR'ing with rw-rw----
 	fsGroup *int64
+
+	// resources defines the request & limits of NFS server
+	// This will be populated from NFS StorageClass. If not
+	// specified resource limits will not be applied on NFS
+	// Server container
+	resources *corev1.ResourceRequirements
 }
 
 // validate checks that the required fields to create NFS Server
@@ -192,6 +198,7 @@ func (p *Provisioner) deleteBackendPVC(nfsServerOpts *KernelNFSServerOptions) er
 
 // createDeployment creates a new NFS Server Deployment for a given NFS PVC
 func (p *Provisioner) createDeployment(nfsServerOpts *KernelNFSServerOptions) error {
+	var resourceRequirements corev1.ResourceRequirements
 	klog.V(4).Infof("Creating Deployment")
 	if err := nfsServerOpts.validate(); err != nil {
 		return err
@@ -218,6 +225,9 @@ func (p *Provisioner) createDeployment(nfsServerOpts *KernelNFSServerOptions) er
 	nfsDeployLabelSelector := map[string]string{
 		"openebs.io/nfs-server": deployName,
 	}
+	if nfsServerOpts.resources != nil {
+		resourceRequirements = *nfsServerOpts.resources
+	}
 
 	//TODO
 	secContext := true
@@ -281,7 +291,8 @@ func (p *Provisioner) createDeployment(nfsServerOpts *KernelNFSServerOptions) er
 									MountPath: "/nfsshare",
 								},
 							},
-						),
+						).
+						WithResources(&resourceRequirements),
 				).
 				WithVolumeBuilders(
 					volume.NewBuilder().
diff --git a/provisioner/provisioner_kernel_nfs_server.go b/provisioner/provisioner_kernel_nfs_server.go
@@ -55,6 +55,12 @@ func (p *Provisioner) ProvisionKernalNFSServer(opts pvController.ProvisionOption
 		return nil, err
 	}
 
+	resources, err := volumeConfig.GetNFSServerResourceRequirements()
+	if err != nil {
+		klog.Errorf("Failed to get NFS server resource requirements(requests & limits) error: %s", err.Error())
+		return nil, err
+	}
+
 	//Extract the details to create a NFS Server
 	nfsServerOpts := &KernelNFSServerOptions{
 		pvName:                name,
@@ -68,6 +74,7 @@ func (p *Provisioner) ProvisionKernalNFSServer(opts pvController.ProvisionOption
 		pvcName:               pvc.Name,
 		pvcNamespace:          pvc.Namespace,
 		pvcUID:                string(pvc.UID),
+		resources:             resources,
 	}
 
 	nfsService, err := p.getNFSServerAddress(nfsServerOpts)
diff --git a/tests/nfs_server_resource_limits_test.go b/tests/nfs_server_resource_limits_test.go
