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 binds 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>

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.

changelogs/unreleased/121-pawanpraka1

@@ -0,0 +1 @@
+Fixes an issue where PVC was bound to unusable PV created using incorrect values provided in PVC/Storageclass

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

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

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

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 :-

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"`
+}

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

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.GetZFSVolumeState(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.GetZFSVolumeState(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}

pkg/zfs/volume.go

@@ -157,6 +157,21 @@ func GetZFSVolume(volumeID string) (*apis.ZFSVolume, error) {
 	return vol, err
 }
 
+// GetZFSVolumeState returns ZFSVolume OwnerNode and State for
+// the given volume. CreateVolume request may call it again and
+// again until volume is "Ready".
+func GetZFSVolumeState(volID string) (string, string, error) {
+	getOptions := metav1.GetOptions{}
+	vol, err := volbuilder.NewKubeclient().
+		WithNamespace(OpenEBSNamespace).Get(volID, getOptions)
+
+	if err != nil {
+		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 +183,7 @@ func UpdateZvolInfo(vol *apis.ZFSVolume) error {
 
 	newVol, err := volbuilder.BuildFrom(vol).
 		WithFinalizer(finalizers).
+		WithVolumeStatus(ZFSStatusReady).
 		WithLabels(labels).Build()
 
 	if err != nil {