feat(zfspv): pvc should be bound only if volume has been created.

The controller does not check whether the volume has been created or not and return successful. Which in turn bounds the pvc to the pv. The PVC should not bound until corresponding zfs volume has been created. Now controller will check the ZFSVolume CR state to be "Ready" before returning successful. The CSI will retry the CreateVolume request when it will get a error reply and when the ZFS node agent creates the ZFS volume and sets the ZFSVolume CR state to be "Ready", the controller will return success for the CreateVolume Request and then PVC will be bound. Signed-off-by: Pawan <pawan@mayadata.io>
openebs · May 19, 2020 · bfdc6df · bfdc6df
1 parent 2f19a66
commit bfdc6df
Show file tree

Hide file tree

Showing 10 changed files with 103 additions and 5 deletions.
diff --git a/README.md b/README.md
@@ -314,6 +314,8 @@ Spec:
   Pool Name:      zfspv-pool
   Recordsize:     4k
   Volume Type:    DATASET
+Status:
+  State: Ready
 Events:           <none>
 ```
 
@@ -488,7 +490,7 @@ Name:         pvc-e1230d2c-b32a-48f7-8b76-ca335b253dcd
 Namespace:    openebs
 Labels:       kubernetes.io/nodename=zfspv-node1
 Annotations:  <none>
-API Version:  openebs.io/v1alpha1
+API Version:  zfs.openebs.io/v1alpha1
 Kind:         ZFSVolume
 Metadata:
   Creation Timestamp:  2019-11-22T09:49:29Z
@@ -505,6 +507,8 @@ Spec:
   Pool Name:      zfspv-pool
   Snap Name:      pvc-34133838-0d0d-11ea-96e3-42010a800114@snapshot-3cbd5e59-4c6f-4bd6-95ba-7f72c9f12fcd
   Volume Type:    DATASET
+Status:
+  State: Ready
 Events:           <none>
 
 Here you can note that this resource has Snapname field which tells that this volume is created from that snapshot.

diff --git a/changelogs/unreleased/121-pawanpraka1 b/changelogs/unreleased/121-pawanpraka1
@@ -0,0 +1 @@
+enhancement to bound the pvc only if volume has been created
diff --git a/deploy/sample/zfspvcr.yaml b/deploy/sample/zfspvcr.yaml
@@ -1,4 +1,4 @@
-apiVersion: openebs.io/v1alpha1
+apiVersion: zfs.openebs.io/v1alpha1
 kind: ZFSVolume
 metadata:
   name: pvc-34133838-0d0d-11ea-96e3-42010a800114
@@ -16,3 +16,5 @@ spec:
   recordsize: 8k
   thinProvision: "off"
   volumeType: DATASET
+status:
+  state: Ready
diff --git a/deploy/yamls/zfsvolume-crd.yaml b/deploy/yamls/zfsvolume-crd.yaml
@@ -32,6 +32,10 @@ spec:
     description: Size of the volume
     name: Size
     type: string
+  - JSONPath: .status.state
+    description: Status of the volume
+    name: Status
+    type: string
   - JSONPath: .spec.volblocksize
     description: volblocksize of volume
     name: volblocksize
@@ -216,6 +220,18 @@ spec:
           - poolName
           - volumeType
           type: object
+        status:
+          properties:
+            state:
+              description: State specifies the current state of the volume provisioning
+                request. The state "Pending" means that the volume creation request
+                has not processed yet. The state "Ready" means that the volume has
+                been created and it is ready for the use.
+              enum:
+              - Pending
+              - Ready
+              type: string
+          type: object
       required:
       - spec
       type: object

diff --git a/deploy/zfs-operator.yaml b/deploy/zfs-operator.yaml
@@ -53,6 +53,10 @@ spec:
     description: Size of the volume
     name: Size
     type: string
+  - JSONPath: .status.state
+    description: Status of the volume
+    name: Status
+    type: string
   - JSONPath: .spec.volblocksize
     description: volblocksize of volume
     name: volblocksize
@@ -237,6 +241,18 @@ spec:
           - poolName
           - volumeType
           type: object
+        status:
+          properties:
+            state:
+              description: State specifies the current state of the volume provisioning
+                request. The state "Pending" means that the volume creation request
+                has not processed yet. The state "Ready" means that the volume has
+                been created and it is ready for the use.
+              enum:
+              - Pending
+              - Ready
+              type: string
+          type: object
       required:
       - spec
       type: object

diff --git a/docs/import-existing-volume.md b/docs/import-existing-volume.md
@@ -151,6 +151,8 @@ spec:
   ownerNodeID: pawan-3 # should be the nodename where ZPOOL is running
   poolName: zfspv-pool # poolname where the volume is present
   volumeType: DATASET # whether it is a DATASET or ZVOL
+Status:
+  State: Ready
 ```
 
 Modify the parameters :-

diff --git a/pkg/apis/openebs.io/zfs/v1alpha1/zfsvolume.go b/pkg/apis/openebs.io/zfs/v1alpha1/zfsvolume.go
@@ -30,6 +30,7 @@ import (
 // +kubebuilder:printcolumn:name="ZPool",type=string,JSONPath=`.spec.poolName`,description="ZFS Pool where the volume is created"
 // +kubebuilder:printcolumn:name="Node",type=string,JSONPath=`.spec.ownerNodeID`,description="Node where the volume is created"
 // +kubebuilder:printcolumn:name="Size",type=string,JSONPath=`.spec.capacity`,description="Size of the volume"
+// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.state`,description="Status of the volume"
 // +kubebuilder:printcolumn:name="volblocksize",type=string,JSONPath=`.spec.volblocksize`,description="volblocksize of volume"
 // +kubebuilder:printcolumn:name="recordsize",type=string,JSONPath=`.spec.recordsize`,description="recordsize of created zfs dataset"
 // +kubebuilder:printcolumn:name="Filesystem",type=string,JSONPath=`.spec.fsType`,description="filesystem created on the volume"
@@ -39,7 +40,8 @@ type ZFSVolume struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
-	Spec VolumeInfo `json:"spec"`
+	Spec   VolumeInfo `json:"spec"`
+	Status VolStatus  `json:"status,omitempty"`
 }
 
 // MountInfo contains the volume related info
@@ -198,3 +200,12 @@ type VolumeInfo struct {
 	// Default Value: ext4.
 	FsType string `json:"fsType,omitempty"`
 }
+
+type VolStatus struct {
+	// State specifies the current state of the volume provisioning request.
+	// The state "Pending" means that the volume creation request has not
+	// processed yet. The state "Ready" means that the volume has been created
+	// and it is ready for the use.
+	// +kubebuilder:validation:Enum=Pending;Ready
+	State string `json:"state,omitempty"`
+}
diff --git a/pkg/builder/volbuilder/build.go b/pkg/builder/volbuilder/build.go
@@ -160,6 +160,12 @@ func (b *Builder) WithVolumeType(vtype string) *Builder {
 	return b
 }
 
+// WithVolumeStatus sets ZFSVolume status
+func (b *Builder) WithVolumeStatus(status string) *Builder {
+	b.volume.Object.Status.State = status
+	return b
+}
+
 // WithFsType sets filesystem for the ZFSVolume
 func (b *Builder) WithFsType(fstype string) *Builder {
 	b.volume.Object.Spec.FsType = fstype

diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go
@@ -110,6 +110,7 @@ func CreateZFSVolume(req *csi.CreateVolumeRequest) (string, error) {
 		WithThinProv(tp).
 		WithOwnerNode(selected).
 		WithVolumeType(vtype).
+		WithVolumeStatus(zfs.ZFSStatusPending).
 		WithFsType(fstype).
 		WithCompression(compression).Build()
 
@@ -161,7 +162,9 @@ func CreateZFSClone(req *csi.CreateVolumeRequest, snapshot string) (string, erro
 	selected := snap.Spec.OwnerNodeID
 
 	volObj, err := volbuilder.NewBuilder().
-		WithName(volName).Build()
+		WithName(volName).
+		WithVolumeStatus(zfs.ZFSStatusPending).
+		Build()
 
 	volObj.Spec = snap.Spec
 	volObj.Spec.SnapName = snapshot
@@ -187,12 +190,22 @@ func (cs *controller) CreateVolume(
 	volName := req.GetName()
 	pool := req.GetParameters()["poolname"]
 	size := req.GetCapacityRange().RequiredBytes
+	contentSource := req.GetVolumeContentSource()
 
 	if err = cs.validateVolumeCreateReq(req); err != nil {
 		return nil, err
 	}
 
-	contentSource := req.GetVolumeContentSource()
+	selected, state, err := zfs.GetZFSVolumeStatus(req.Name)
+
+	if err == nil {
+		// ZFSVolume CR has been created, check if it is in Ready state
+		if state == zfs.ZFSStatusReady {
+			goto CreateVolumeResponse
+		}
+		return nil, status.Errorf(codes.Internal, "volume %s creation is Pending", volName)
+	}
+
 	if contentSource != nil && contentSource.GetSnapshot() != nil {
 		snapshotID := contentSource.GetSnapshot().GetSnapshotId()
 
@@ -205,6 +218,18 @@ func (cs *controller) CreateVolume(
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
+	_, state, err = zfs.GetZFSVolumeStatus(req.Name)
+
+	if err != nil {
+		return nil, status.Errorf(codes.Internal, "createvolume: failed to fetch the volume %v", err.Error())
+	}
+
+	if state == zfs.ZFSStatusPending {
+		return nil, status.Errorf(codes.Internal, "volume %s is being created", volName)
+	}
+
+CreateVolumeResponse:
+
 	sendEventOrIgnore(volName, strconv.FormatInt(int64(size), 10), "zfs-localpv", analytics.VolumeProvision)
 
 	topology := map[string]string{zfs.ZFSTopologyKey: selected}

diff --git a/pkg/zfs/volume.go b/pkg/zfs/volume.go
@@ -157,6 +157,20 @@ func GetZFSVolume(volumeID string) (*apis.ZFSVolume, error) {
 	return vol, err
 }
 
+// GetZFSVolumeStatus returns ZFSVolume status
+func GetZFSVolumeStatus(volID string) (string, string, error) {
+	getOptions := metav1.GetOptions{}
+	vol, err := volbuilder.NewKubeclient().
+		WithNamespace(OpenEBSNamespace).Get(volID, getOptions)
+
+	if err != nil {
+		logrus.Errorf("Get ZFSVolume failed %s err: %s", vol.Name, err.Error())
+		return "", "", err
+	}
+
+	return vol.Spec.OwnerNodeID, vol.Status.State, nil
+}
+
 // UpdateZvolInfo updates ZFSVolume CR with node id and finalizer
 func UpdateZvolInfo(vol *apis.ZFSVolume) error {
 	finalizers := []string{ZFSFinalizer}
@@ -168,6 +182,7 @@ func UpdateZvolInfo(vol *apis.ZFSVolume) error {
 
 	newVol, err := volbuilder.BuildFrom(vol).
 		WithFinalizer(finalizers).
+		WithVolumeStatus(ZFSStatusReady).
 		WithLabels(labels).Build()
 
 	if err != nil {