feat(hook): load hook configuration using file instead of configmap (#120)

This commit does the following changes:
* updating provisioner to load hook configuration from pre-defined file '/etc/nfs-provisioner-hook/config'
* updating hook design document to include changes on loading hook configuration using file
* bdd(hook): updating hook BDD test to mount configmap in nfs-provisioner deployment

Signed-off-by: mayank <mayank.patel@mayadata.io>

Author: mynktl

deploy/kubectl/openebs-nfs-provisioner.yaml

@@ -140,8 +140,6 @@ spec:
         - name: OPENEBS_IO_NFS_SERVER_IMG
           value: openebs/nfs-server-alpine:ci
         # OPENEBS_IO_NFS_HOOK_CONFIGMAP defines configmap to use for hook
-        #- name: OPENEBS_IO_NFS_HOOK_CONFIGMAP
-        #  value: "nfs-hook"
         # LEADER_ELECTION_ENABLED is used to enable/disable leader election. By default
         # leader election is enabled.
         #- name: LEADER_ELECTION_ENABLED
@@ -178,6 +176,16 @@ spec:
           limits:
             cpu: 200m
             memory: 200M
+        # uncomment below lines to use nfs-hooks
+        # volumeMounts:
+        #   - mountPath: /etc/nfs-provisioner
+        #     name: hook-config
+      # uncomment below lines to use nfs-hooks
+      # volumes:
+        # - name: hook-config
+          # configMap:
+           #  name: hook-config
+
 ---
 #Sample storage classes for OpenEBS Local PV
 apiVersion: storage.k8s.io/v1

docs/design/hooks.md

@@ -39,10 +39,10 @@ I should be able to configure nfs-provisioner to set the provided annotation and
 I should be able to configure nfs-provisioner to set the provided annotation and finalizer on specific resources created by nfs-provisioner.
 
 ### Proposed Implementation
-NFS-Provisioner will use user-provided Configmap to learn hooks configuration. These hooks will be executed on volume provisioning or deleting events to set the given information on provided resources. 
+NFS-Provisioner will use user-provided Config file to learn hooks configuration. These hooks will be executed on volume provisioning or deleting events to set the given information on provided resources. 
 
 ### High-Level Design
-User will deploy nfs-provisioner with Configmap having information about hook information. This Configmap should exist in the same namespace in which nfs-provisioner is deployed. Sample nfs-provisioner deployment config is as below:
+NFS Provisioner will load the hook configuration from the file located at pre-defined path. User can create a configmap with hook configuration and mount it at pre-defined path. Sample nfs-provisioner deployment config is as below:
 
 ```yaml
     spec:
@@ -61,9 +61,16 @@ User will deploy nfs-provisioner with Configmap having information about hook in
             - test `pgrep "^provisioner-nfs.*"` = 1
           initialDelaySeconds: 30
           periodSeconds: 60
+        volumeMounts:
+          - mountPath: /etc/nfs-provisioner
+            name: hook-config
+      volumes:
+        - name: hook-config
+          configMap:
+            name: hook-config
 ```
 
-User needs to provide Configmap name as a value of `OPENEBS_IO_NFS_HOOK_CONFIGMAP` environment variable.
+User needs to create a configmap named 'hook-config' in provisioner's namespace.
 
 ### Low-Level Design
 #### Hook Definition
@@ -192,7 +199,7 @@ type Provisioner struct {
 ```
 
 #### Configmap structure
-User needs to create hook Configmap resource in nfs-provisioner namespace. Hook configuration needs to be provided in data field **config**.
+User needs to create hook Configmap resource in nfs-provisioner namespace. Hook configuration needs to be provided in data field **hook-config**.
 Configmap needs be defined as below:
 
 ```yaml
@@ -202,7 +209,7 @@ metadata:
   name: hook-config
   namespace: openebs
 data:
-  config: |
+  hook-config: |
     hooks:
       addOrUpdateEntriesOnCreateVolumeEvent:
         backendPV:
@@ -274,7 +281,8 @@ For initial development, following template variables will be supported:
 
 
 #### NFS Provisioner changes
-If NFS Provisioner is configured with environment variable **OPENEBS_IO_NFS_HOOK_CONFIGMAP** set then Provisioner needs to lookup the provided Configmap in nfs-provisioner namespace. If Configmap exists then provisioner needs to initialize **Provisioner** with hook configuration provided in Configmap.
+NFS Provisioner will initialize the hook configuration using pre-defined hook config file(*/etc/nfs-provisioner/hook-config*). If hook config file is not found then NFS Provisioner will skip the initialization of hooks and continue with provisioning.
+
 NFS Provisioner executes two events, volume provisioning, and volume deletion. On these two events provisioner needs to execute all the hooks as per the given hook Action.
 
 NFS Provisioner will initialize the hook on startup. If Hook configuration is invalid then NFS Provisioner will throw the error and exit.

provisioner/config.go

@@ -19,20 +19,20 @@ package provisioner
 
 import (
 	"context"
+	"github.com/pkg/errors"
+	"io/ioutil"
+	"os"
 	"strconv"
 	"strings"
 
 	"github.com/ghodss/yaml"
+	nfshook "github.com/openebs/dynamic-nfs-provisioner/pkg/hook"
 	mconfig "github.com/openebs/maya/pkg/apis/openebs.io/v1alpha1"
 	cast "github.com/openebs/maya/pkg/castemplate/v1alpha1"
 	"github.com/openebs/maya/pkg/util"
-	"k8s.io/klog/v2"
-
-	//"github.com/pkg/errors"
-	errors "github.com/pkg/errors"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	//storagev1 "k8s.io/api/storage/v1"
+	"k8s.io/klog/v2"
 )
 
 const (
@@ -66,6 +66,15 @@ const (
 
 	// NFSServerResourceLimits holds key name that represent NFS Resource Limits
 	NFSServerResourceLimits = "NFSServerResourceLimits"
+
+	// HookConfigFileName represent file name for hook configuration
+	HookConfigFileName = "hook-config"
+
+	// ConfigDirectory defines directory to store config files specific to NFS provisioner
+	ConfigDirectory = "/etc/nfs-provisioner"
+
+	// HookConfigFilePath defines path for hook config file
+	HookConfigFilePath = ConfigDirectory + "/" + HookConfigFileName
 )
 
 const (
@@ -265,3 +274,49 @@ func GetNFSServerTypeFromPV(pv *v1.PersistentVolume) string {
 	//TODO extract this from PV annotations
 	return "kernel"
 }
+
+// hookConfigFileExist check if hook config file exists or not
+func hookConfigFileExist() (bool, error) {
+	// HookConfigFilePath
+	_, err := os.Stat(HookConfigFilePath)
+	if err == nil {
+		return true, nil
+	}
+
+	if os.IsNotExist(err) {
+		return false, nil
+	}
+
+	return false, err
+}
+
+// initializeHook read the hook config file and update the given hook variable
+// return value:
+// 	- nil
+// 		- If hook config file doesn't exists
+// 		- If hook config file is parsed and given hook variable is updated
+// 	- error
+// 		- If hook config is invalid
+func initializeHook(hook **nfshook.Hook) error {
+	hookFileExists, err := hookConfigFileExist()
+	if err != nil {
+		return errors.Errorf("failed to check hook config file, err=%s", err)
+	}
+
+	if !hookFileExists {
+		return nil
+	}
+
+	data, err := ioutil.ReadFile(HookConfigFilePath)
+	if err != nil {
+		return errors.Errorf("failed to read hook config file, err=%s", err)
+	}
+
+	hookObj, err := nfshook.ParseHooks(data)
+	if err != nil {
+		return err
+	}
+
+	*hook = hookObj
+	return nil
+}

provisioner/env.go

@@ -60,9 +60,6 @@ const (
 
 	// NFSServerImagePullSecret defines the env name to store the name of the image pull secret
 	NFSServerImagePullSecret menv.ENVKey = "OPENEBS_IO_NFS_SERVER_IMAGE_PULL_SECRET"
-
-	// NFSHookConfigMapName defines env variable name to hold hook configmap name
-	NFSHookConfigMapName menv.ENVKey = "OPENEBS_IO_NFS_HOOK_CONFIGMAP"
 )
 
 var (
@@ -111,7 +108,3 @@ func getBackendPvcTimeout() string {
 func getNfsServerImagePullSecret() string {
 	return menv.GetOrDefault(NFSServerImagePullSecret, "")
 }
-
-func getHookConfigMapName() string {
-	return menv.Get(NFSHookConfigMapName)
-}

provisioner/provisioner.go

@@ -51,20 +51,15 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	kubeinformers "k8s.io/client-go/informers"
-	"k8s.io/client-go/kubernetes"
 	listersv1 "k8s.io/client-go/listers/core/v1"
 	v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
 
 	nfshook "github.com/openebs/dynamic-nfs-provisioner/pkg/hook"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientset "k8s.io/client-go/kubernetes"
 )
 
 var (
 	NodeAffinityRulesMismatchEvent = "No matching nodes found for given affinity rules"
-
-	// HookConfigMapDataField defines configmap data key for hook config
-	HookConfigMapDataField = "config"
 )
 
 // NewProvisioner will create a new Provisioner object and initialize
@@ -89,12 +84,9 @@ func NewProvisioner(ctx context.Context, kubeClient *clientset.Clientset) (*Prov
 	nfsServerNs := getNfsServerNamespace()
 
 	var hook *nfshook.Hook
-	hookCmap := getHookConfigMapName()
-	if hookCmap != "" {
-		err := initializeHook(kubeClient, ctx, namespace, hookCmap, &hook)
-		if err != nil {
-			return nil, errors.Errorf("failed to initialize hooks, err={%s}", err)
-		}
+	err = initializeHook(&hook)
+	if err != nil {
+		return nil, errors.Errorf("failed to initialize hooks, err={%s}", err)
 	}
 
 	pvTracker := NewProvisioningTracker()
@@ -302,22 +294,3 @@ func sendEventOrIgnore(pvcName, pvName, capacity, stgType, method string) {
 		SetCategory(method).
 		SetVolumeCapacity(capacity).Send()
 }
-
-func initializeHook(kubeClient kubernetes.Interface, ctx context.Context, namespace, hookCmap string, hook **nfshook.Hook) error {
-	cmapObj, err := kubeClient.CoreV1().ConfigMaps(namespace).Get(ctx, hookCmap, metav1.GetOptions{})
-	if err != nil {
-		return errors.Wrapf(err, "failed to fetch hook configmap=%s", hookCmap)
-	}
-
-	data, ok := cmapObj.Data[HookConfigMapDataField]
-	if !ok {
-		return errors.Errorf("hook configmap=%s doesn't have data field=%s", hookCmap, HookConfigMapDataField)
-	}
-
-	hookObj, err := nfshook.ParseHooks([]byte(data))
-	if err != nil {
-		return err
-	}
-	*hook = hookObj
-	return nil
-}

tests/nfs_hook_test.go

@@ -112,6 +112,7 @@ var _ = Describe("TEST NFS HOOK", func() {
 
 		backendPVName      = ""
 		hookConfigMapName  = "nfs-hook"
+		hookVolumeName     = "nfs-hook-vol"
 		hook               *nfshook.Hook
 		NFSProvisionerName = "openebs-nfs-provisioner"
 		openebsNamespace   = "openebs"
@@ -128,7 +129,7 @@ var _ = Describe("TEST NFS HOOK", func() {
 			cmap.Name = hookConfigMapName
 			cmap.Namespace = openebsNamespace
 			cmap.Data = map[string]string{
-				"config": string(data),
+				provisioner.HookConfigFileName: string(data),
 			}
 
 			err = Client.createConfigMap(&cmap)
@@ -154,15 +155,26 @@ var _ = Describe("TEST NFS HOOK", func() {
 			)
 
 			By("updating the deployment")
-			nsEnv := corev1.EnvVar{
-				Name:  string(provisioner.NFSHookConfigMapName),
-				Value: hookConfigMapName,
-			}
+			deploy.Spec.Template.Spec.Volumes = append(deploy.Spec.Template.Spec.Volumes,
+				corev1.Volume{
+					Name: hookVolumeName,
+					VolumeSource: corev1.VolumeSource{
+						ConfigMap: &corev1.ConfigMapVolumeSource{
+							LocalObjectReference: corev1.LocalObjectReference{
+								Name: hookConfigMapName,
+							},
+						},
+					},
+				},
+			)
 
-			deploy.Spec.Template.Spec.Containers[0].Env = append(
-				deploy.Spec.Template.Spec.Containers[0].Env,
-				nsEnv,
+			deploy.Spec.Template.Spec.Containers[0].VolumeMounts = append(deploy.Spec.Template.Spec.Containers[0].VolumeMounts,
+				corev1.VolumeMount{
+					Name:      hookVolumeName,
+					MountPath: provisioner.ConfigDirectory,
+				},
 			)
+
 			_, err = Client.updateDeployment(deploy)
 			Expect(err).To(BeNil(), "while updating deployment %s/%s", openebsNamespace, NFSProvisionerName)
 
@@ -335,16 +347,30 @@ var _ = Describe("TEST NFS HOOK", func() {
 			Expect(err).To(BeNil(), "while fetching deployment %s/%s", openebsNamespace, NFSProvisionerName)
 
 			By("updating the provisioner deployment")
+			// Removing volumeMount
 			idx := 0
-			for idx < len(deploy.Spec.Template.Spec.Containers[0].Env) {
-				if deploy.Spec.Template.Spec.Containers[0].Env[idx].Name == string(provisioner.NFSHookConfigMapName) {
+			for idx < len(deploy.Spec.Template.Spec.Containers[0].VolumeMounts) {
+				if deploy.Spec.Template.Spec.Containers[0].VolumeMounts[idx].Name == hookVolumeName {
+					break
+				}
+				idx++
+			}
+			deploy.Spec.Template.Spec.Containers[0].VolumeMounts = append(deploy.Spec.Template.Spec.Containers[0].VolumeMounts[:idx],
+				deploy.Spec.Template.Spec.Containers[0].VolumeMounts[idx+1:]...)
+
+			// Removing volume
+			idx = 0
+			for idx < len(deploy.Spec.Template.Spec.Volumes) {
+				if deploy.Spec.Template.Spec.Volumes[idx].Name == hookVolumeName {
 					break
 				}
 				idx++
 			}
-			deploy.Spec.Template.Spec.Containers[0].Env = append(deploy.Spec.Template.Spec.Containers[0].Env[:idx], deploy.Spec.Template.Spec.Containers[0].Env[idx+1:]...)
+			deploy.Spec.Template.Spec.Volumes = append(deploy.Spec.Template.Spec.Volumes[:idx],
+				deploy.Spec.Template.Spec.Volumes[idx+1:]...)
+
 			_, err = Client.updateDeployment(deploy)
-			Expect(err).To(BeNil(), "while updateingupdating deployment %s/%s", openebsNamespace, NFSProvisionerName)
+			Expect(err).To(BeNil(), "while updating deployment %s/%s", openebsNamespace, NFSProvisionerName)
 
 			By("waiting for deployment rollout")
 			err = Client.waitForDeploymentRollout(OpenEBSNamespace, NFSProvisionerName)