chore(backendPvcTimeout): option to configure BackendPvcTimeout using env Var (#84)

* chore(backendPvcTimeout): configure backendPvcTimeout through env var OPENEBS_IO_NFS_SERVER_BACKEND_PVC_TIMEOUT
* unit test to verify backendPvcTimeout value
* updating helm chart and deployment yaml to configure backendPvcTimeout

Signed-off-by: mayank <[email protected]>

Author: mynktl

deploy/helm/charts/Chart.yaml

@@ -4,7 +4,7 @@ description: Helm chart for OpenEBS Dynamic NFS PV. For instructions to install
 type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
-version: 0.5.0
+version: 0.5.1
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application.
 appVersion: 0.5.0

deploy/helm/charts/README.md

@@ -129,6 +129,7 @@ helm install openebs-nfs openebs-nfs/nfs-provisioner --namespace openebs --creat
 | `nfsProvisioner.tolerations`          | NFS Provisioner pod toleration values         | `""`                            |
 | `nfsProvisioner.nfsServerNamespace`          | NFS server namespace         | `"openebs"`                            |
 | `nfsProvisioner.nfsServerNodeAffinity`       | NFS Server node affinity rules                | `""`                          |
+| `nfsProvisioner.nfsBackendPvcTimeout`       | Timeout for backend PVC binding in seconds                | `"60"`                          |
 | `nfsStorageClass.backendStorageClass` | StorageClass to be used to provision the backend volume. If not specified, the default StorageClass is used. | `""`                         |
 | `nfsStorageClass.isDefaultClass`      | Make 'openebs-kernel-nfs' the default StorageClass | `"false"`                         |
 | `nfsStorageClass.reclaimPolicy`       | ReclaimPolicy for NFS PVs                      | `"Delete"`                     |

deploy/helm/charts/templates/deployment.yaml

@@ -99,6 +99,10 @@ spec:
             - name: OPENEBS_IO_NFS_SERVER_NODE_AFFINITY
               value: "{{ .Values.nfsProvisioner.nfsServerNodeAffinity }}"
             {{- end }}
+           {{- if .Values.nfsProvisioner.nfsBackendPvcTimeout }}
+            - name: OPENEBS_IO_NFS_SERVER_BACKEND_PVC_TIMEOUT
+              value: "{{ .Values.nfsProvisioner.nfsBackendPvcTimeout }}"
+            {{- end }}
           # Process name used for matching is limited to the 15 characters
           # present in the pgrep output.
           # So fullname can't be used here with pgrep (>15 chars).A regular expression

deploy/kubectl/openebs-nfs-provisioner.yaml

@@ -143,6 +143,9 @@ spec:
         # leader election is enabled.
         #- name: LEADER_ELECTION_ENABLED
         #  value: "true"
+        # Set Timeout for backend PVC to bound, Default value is 60 seconds
+        #- name: OPENEBS_IO_NFS_SERVER_BACKEND_PVC_TIMEOUT
+        #  value: "60"
         # Process name used for matching is limited to the 15 characters
         # present in the pgrep output.
         # So fullname can't be used here with pgrep (>15 chars).A regular expression

provisioner/env.go

@@ -20,7 +20,7 @@ import (
 	menv "github.com/openebs/maya/pkg/env/v1alpha1"
 )
 
-//This file defines the environement variable names that are specific
+//This file defines the environment variable names that are specific
 // to this provisioner. In addition to the variables defined in this file,
 // provisioner also uses the following:
 //   OPENEBS_NAMESPACE
@@ -54,6 +54,9 @@ const (
 
 	// NodeAffinityKey holds the env name representing Node affinity rules
 	NodeAffinityKey menv.ENVKey = "OPENEBS_IO_NFS_SERVER_NODE_AFFINITY"
+
+	// NFSBackendPvcTimeout defines env name to store BackendPvcBoundTimeout value
+	NFSBackendPvcTimeout menv.ENVKey = "OPENEBS_IO_NFS_SERVER_BACKEND_PVC_TIMEOUT"
 )
 
 var (
@@ -94,3 +97,7 @@ func getNFSServerImage() string {
 func getNfsServerNodeAffinity() string {
 	return menv.Get(NodeAffinityKey)
 }
+
+func getBackendPvcTimeout() string {
+	return menv.Get(NFSBackendPvcTimeout)
+}

provisioner/helper_kernel_nfs_server.go

@@ -53,9 +53,9 @@ const (
 	//RPCBindPort set the RPC Bind Port
 	RPCBindPort = 111
 
-	// BackendPvcBoundTimeout defines the timeout for PVC Bound check
+	// DefaultBackendPvcBoundTimeout defines the timeout for PVC Bound check.
 	// set to 60 seconds
-	BackendPvcBoundTimeout = 60 * time.Second
+	DefaultBackendPvcBoundTimeout = 60
 )
 
 var (
@@ -423,7 +423,7 @@ func (p *Provisioner) deleteService(nfsServerOpts *KernelNFSServerOptions) error
 	svcName := "nfs-" + nfsServerOpts.pvName
 	klog.V(4).Infof("Verifying if Service(%v) for NFS storage exists.", svcName)
 
-	//Check if the Serivce still exists. It could have been removed
+	//Check if the Service still exists. It could have been removed
 	// or never created due to a provisioning create failure.
 	_, err := p.kubeClient.CoreV1().
 		Services(p.serverNamespace).
@@ -493,7 +493,7 @@ func (p *Provisioner) createNFSServer(nfsServerOpts *KernelNFSServerOptions) err
 		return errors.Wrapf(err, "failed to initialize NFS Storage Deployment for RWX PVC{%v}", nfsServerOpts.pvName)
 	}
 
-	err = waitForPvcBound(p.kubeClient, p.serverNamespace, "nfs-"+nfsServerOpts.pvName, BackendPvcBoundTimeout)
+	err = waitForPvcBound(p.kubeClient, p.serverNamespace, "nfs-"+nfsServerOpts.pvName, p.backendPvcTimeout)
 	if err != nil {
 		return err
 	}

provisioner/helper_kernel_nfs_server_test.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"os"
 	"testing"
+	"time"
 
 	errors "github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
@@ -655,8 +656,9 @@ func TestGetNFSServerAddress(t *testing.T) {
 				backendStorageClass: "test1-sc",
 			},
 			provisioner: &Provisioner{
-				kubeClient:      fake.NewSimpleClientset(),
-				serverNamespace: "nfs-server-ns1",
+				kubeClient:        fake.NewSimpleClientset(),
+				serverNamespace:   "nfs-server-ns1",
+				backendPvcTimeout: 60 * time.Second,
 			},
 			expectedServiceIP:     "nfs-test1-pv.nfs-server-ns1.svc.cluster.local",
 			shouldBoundBackendPvc: true,
@@ -670,9 +672,10 @@ func TestGetNFSServerAddress(t *testing.T) {
 				backendStorageClass: "test2-sc",
 			},
 			provisioner: &Provisioner{
-				kubeClient:      fake.NewSimpleClientset(),
-				serverNamespace: "nfs-server-ns2",
-				useClusterIP:    true,
+				kubeClient:        fake.NewSimpleClientset(),
+				serverNamespace:   "nfs-server-ns2",
+				useClusterIP:      true,
+				backendPvcTimeout: 60 * time.Second,
 			},
 			// Since we are using fake clients there won't be ClusterIP on service
 			// so expecting for empty value
@@ -688,9 +691,30 @@ func TestGetNFSServerAddress(t *testing.T) {
 				backendStorageClass: "test3-sc",
 			},
 			provisioner: &Provisioner{
-				kubeClient:      fake.NewSimpleClientset(),
-				serverNamespace: "nfs-server-ns3",
-				useClusterIP:    false,
+				kubeClient:        fake.NewSimpleClientset(),
+				serverNamespace:   "nfs-server-ns3",
+				useClusterIP:      false,
+				backendPvcTimeout: 60 * time.Second,
+			},
+			// Since we are using fake clients there won't be ClusterIP on service
+			// so expecting for empty value
+			expectedServiceIP:     "",
+			isErrExpected:         true,
+			shouldBoundBackendPvc: false,
+		},
+		"when provisioner configured with very low backendPvcTimeout value": {
+			// NOTE: Populated only fields required for test
+			options: &KernelNFSServerOptions{
+				provisionerNS:       "openebs",
+				pvName:              "test3-pv",
+				capacity:            "5G",
+				backendStorageClass: "test3-sc",
+			},
+			provisioner: &Provisioner{
+				kubeClient:        fake.NewSimpleClientset(),
+				serverNamespace:   "nfs-server-ns3",
+				useClusterIP:      false,
+				backendPvcTimeout: 1 * time.Second,
 			},
 			// Since we are using fake clients there won't be ClusterIP on service
 			// so expecting for empty value

provisioner/provisioner.go

@@ -33,7 +33,9 @@ package provisioner
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
+	"time"
 
 	"github.com/openebs/maya/pkg/alertlog"
 
@@ -67,6 +69,14 @@ func NewProvisioner(stopCh chan struct{}, kubeClient *clientset.Clientset) (*Pro
 	if len(strings.TrimSpace(namespace)) == 0 {
 		return nil, fmt.Errorf("Cannot start Provisioner: failed to get namespace")
 	}
+
+	backendPvcTimeoutStr := getBackendPvcTimeout()
+	backendPvcTimeoutVal, err := strconv.Atoi(backendPvcTimeoutStr)
+	if err != nil || backendPvcTimeoutVal == 0 {
+		klog.Warningf("Invalid backendPvcTimeout value=%s, using default value %d", backendPvcTimeoutStr, DefaultBackendPvcBoundTimeout)
+		backendPvcTimeoutVal = DefaultBackendPvcBoundTimeout
+	}
+
 	kubeInformerFactory := kubeinformers.NewSharedInformerFactory(kubeClient, 0)
 	k8sNodeInformer := kubeInformerFactory.Core().V1().Nodes().Informer()
 
@@ -86,10 +96,11 @@ func NewProvisioner(stopCh chan struct{}, kubeClient *clientset.Clientset) (*Pro
 				Value: getDefaultNFSServerType(),
 			},
 		},
-		useClusterIP:  menv.Truthy(ProvisionerNFSServerUseClusterIP),
-		k8sNodeLister: listersv1.NewNodeLister(k8sNodeInformer.GetIndexer()),
-		nodeAffinity:  getNodeAffinityRules(),
-		pvTracker:     pvTracker,
+		useClusterIP:      menv.Truthy(ProvisionerNFSServerUseClusterIP),
+		k8sNodeLister:     listersv1.NewNodeLister(k8sNodeInformer.GetIndexer()),
+		nodeAffinity:      getNodeAffinityRules(),
+		pvTracker:         pvTracker,
+		backendPvcTimeout: time.Duration(backendPvcTimeoutVal) * time.Second,
 	}
 	p.getVolumeConfig = p.GetVolumeConfig
 

provisioner/types.go

@@ -18,6 +18,8 @@ limitations under the License.
 package provisioner
 
 import (
+	"time"
+
 	mconfig "github.com/openebs/maya/pkg/apis/openebs.io/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
 	clientset "k8s.io/client-go/kubernetes"
@@ -57,6 +59,9 @@ type Provisioner struct {
 
 	// pvTracker to track in-progress provisioning request
 	pvTracker ProvisioningTracker
+
+	// backendPvcTimeout defines timeout for backend PVC Bound check
+	backendPvcTimeout time.Duration
 }
 
 //VolumeConfig struct contains the merged configuration of the PVC