fix(volume): Fix Error Handling On Creation Failure (#51)

- Persistent volume provision failure handling

Co-authored-by: Vakul Gupta <[email protected]>
Co-authored-by: Vakul Gupta <[email protected]>
Co-authored-by: Vakul Gupta <[email protected]>

.github/workflows/build.yml

@@ -65,10 +65,10 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v2
 
-      - name: Set up Go 1.14
+      - name: Set up Go 1.16
         uses: actions/setup-go@v2
         with:
-          go-version: 1.14.7
+          go-version: 1.16.5
 
       - name: Setup Minikube-Kubernetes
         uses: manusa/[email protected]

.github/workflows/pull_request.yml

@@ -72,10 +72,10 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v2
 
-      - name: Set up Go 1.14
+      - name: Set up Go 1.16
         uses: actions/setup-go@v2
         with:
-          go-version: 1.14.7
+          go-version: 1.16.5
 
       - name: Setup Minikube-Kubernetes
         uses: manusa/[email protected]

deploy/device-operator.yaml

@@ -130,6 +130,7 @@ spec:
                 enum:
                 - Pending
                 - Ready
+                - Failed
                 type: string
             type: object
         required:

deploy/yamls/devicevolume-crd.yaml

@@ -109,6 +109,7 @@ spec:
                 enum:
                 - Pending
                 - Ready
+                - Failed
                 type: string
             type: object
         required:

pkg/apis/openebs.io/device/v1alpha1/devicevolume.go

@@ -77,7 +77,7 @@ type VolStatus struct {
 	// 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
+	// +kubebuilder:validation:Enum=Pending;Ready;Failed
 	State string `json:"state,omitempty"`
 
 	// Error denotes the error occurred during provisioning a volume.

pkg/apis/openebs.io/device/v1alpha1/devicevolumefunctions.go

@@ -0,0 +1,23 @@
+/*
+Copyright © 2019 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 v1alpha1
+
+import "fmt"
+
+func (e *VolumeError) Error() string {
+	return fmt.Sprintf("%s - %s", e.Code, e.Message)
+}

pkg/device/device-util.go

@@ -159,7 +159,10 @@ func CreateVolume(vol *apis.DeviceVolume) error {
 	disk, start, err := findBestPart(diskMetaName, capacityMiB)
 	if err != nil {
 		klog.Errorf("findBestPart Failed")
-		return err
+		return &apis.VolumeError{
+			Code:    apis.InsufficientCapacity,
+			Message: err.Error(),
+		}
 	}
 	return createPartAndWipeFS(disk, start, partitionName, capacityMiB, diskMetaName)
 }

pkg/device/volume.go

@@ -129,17 +129,21 @@ func GetDeviceVolumeState(volID string) (string, string, error) {
 }
 
 // UpdateVolInfo updates DeviceVolume CR with node id and finalizer
-func UpdateVolInfo(vol *apis.DeviceVolume) error {
-	finalizers := []string{DeviceFinalizer}
-	labels := map[string]string{DeviceNodeKey: NodeID}
-
+func UpdateVolInfo(vol *apis.DeviceVolume, state string) error {
+	klog.Infof("Upadting the DeviceVol status to : %s", state)
 	if vol.Finalizers != nil {
 		return nil
 	}
 
+	var finalizers []string
+	labels := map[string]string{DeviceNodeKey: NodeID}
+	switch state {
+	case DeviceStatusReady:
+		finalizers = append(finalizers, DeviceFinalizer)
+	}
 	newVol, err := volbuilder.BuildFrom(vol).
 		WithFinalizer(finalizers).
-		WithVolumeStatus(DeviceStatusReady).
+		WithVolumeStatus(state).
 		WithLabels(labels).Build()
 
 	if err != nil {

pkg/mgmt/volume/volume.go

@@ -84,13 +84,25 @@ func (c *VolController) syncVol(vol *apis.DeviceVolume) error {
 		}
 		return err
 	}
-	// if finalizer is not set then it means we are creating
-	// the volume. And if it is set then volume has already been
-	// created and this event is for property change only.
-	if vol.Status.State != device.DeviceStatusReady {
+
+	// if status is not Pending then we are just ignoring the event.
+	switch vol.Status.State {
+	case device.DeviceStatusFailed:
+		klog.Warningf("Skipping retrying device volume provisioning as its already in failed state: %+v", vol.Status.Error)
+		return nil
+	case device.DeviceStatusReady:
+		klog.Info("device volume already provisioned")
+		return nil
+	}
+
+	// if the status Pending means we will try to create the volume
+	if vol.Status.State == device.DeviceStatusPending {
 		err = device.CreateVolume(vol)
 		if err == nil {
-			err = device.UpdateVolInfo(vol)
+			err = device.UpdateVolInfo(vol, device.DeviceStatusReady)
+		} else if custError, ok := err.(*apis.VolumeError); ok && custError.Code == apis.InsufficientCapacity {
+			vol.Status.Error = custError
+			return device.UpdateVolInfo(vol, device.DeviceStatusFailed)
 		}
 	}
 	return err

tests/provision_test.go

@@ -57,7 +57,14 @@ func blockVolCreationTest() {
 	By("Deleting storage class", deleteStorageClass)
 }
 
+func volCreationFailTest() {
+	By("Create and Verify DeviceVol resource", CreateDeviceVolume)
+	By("Checking the state of DeviceVol", IsDeviceVolStatusFailedEventually)
+	By("Deleting DeviceVol", deleteDeviceVol)
+}
+
 func volumeCreationTest() {
 	By("Running volume creation test", fsVolCreationTest)
 	By("Running block volume creation test", blockVolCreationTest)
+	By("Running the volume creation failed test", volCreationFailTest)
 }

tests/suite_test.go

@@ -22,6 +22,7 @@ import (
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"github.com/openebs/device-localpv/pkg/builder/nodebuilder"
 	"github.com/openebs/device-localpv/pkg/builder/volbuilder"
 	"github.com/openebs/device-localpv/tests/deploy"
 	"github.com/openebs/device-localpv/tests/pod"
@@ -43,6 +44,7 @@ const (
 
 var (
 	DeviceClient     *volbuilder.Kubeclient
+	NodeClient       *nodebuilder.Kubeclient
 	SCClient         *sc.Kubeclient
 	PVCClient        *pvc.Kubeclient
 	DeployClient     *deploy.Kubeclient
@@ -52,6 +54,7 @@ var (
 	LocalProvisioner = "device.csi.openebs.io"
 	pvcName          = "devicepv-pvc"
 	appName          = "busybox-devicepv"
+	DeviceVolName    = "pvc-123"
 
 	nsObj            *corev1.Namespace
 	scObj            *storagev1.StorageClass
@@ -76,6 +79,7 @@ func init() {
 	DeployClient = deploy.NewKubeClient(deploy.WithKubeConfigPath(KubeConfigPath))
 	PodClient = pod.NewKubeClient(pod.WithKubeConfigPath(KubeConfigPath))
 	DeviceClient = volbuilder.NewKubeclient(volbuilder.WithKubeConfigPath(KubeConfigPath))
+	NodeClient = nodebuilder.NewKubeclient(nodebuilder.WithKubeConfigPath(KubeConfigPath))
 }
 
 func TestSource(t *testing.T) {

tests/utils.go

@@ -22,6 +22,7 @@ import (
 	"github.com/onsi/ginkgo"
 	"github.com/onsi/gomega"
 
+	"github.com/openebs/device-localpv/pkg/builder/volbuilder"
 	"github.com/openebs/device-localpv/pkg/device"
 	"github.com/openebs/device-localpv/tests/container"
 	"github.com/openebs/device-localpv/tests/deploy"
@@ -398,3 +399,74 @@ func deletePVC(pvcname string) {
 	status := IsPVCDeletedEventually(pvcname)
 	gomega.Expect(status).To(gomega.Equal(true), "while trying to get deleted pvc")
 }
+
+// IsDeviceVolDeletedEventually : After deleted check if resource still exists
+func IsDeviceVolDeletedEventually() bool {
+	ginkgo.By("Fetching device volume to check if its deleted")
+	return gomega.Eventually(func() bool {
+		_, err := DeviceClient.WithNamespace(OpenEBSNamespace).Get(DeviceVolName, metav1.GetOptions{})
+		return k8serrors.IsNotFound(err)
+	},
+		120, 5).
+		Should(gomega.BeTrue())
+
+}
+
+// CreateDeviceVolume : Create new DeviceVol resource
+func CreateDeviceVolume() {
+
+	nodeList, err := NodeClient.List(metav1.ListOptions{})
+	gomega.Expect(err).To(gomega.BeNil(), "while retrieving DeviceNode")
+	gomega.Expect(len(nodeList.Items) > 0).To(gomega.BeTrue(), "while retrieving DeviceNode")
+
+	ginkgo.By("Building DeviceVol")
+	capacity := "109951162777600" // 100Ti
+	volObj, err := volbuilder.NewBuilder().
+		WithName(DeviceVolName).
+		WithCapacity(capacity).
+		WithDeviceName(DeviceMetaName).
+		WithOwnerNode(nodeList.Items[0].ObjectMeta.Name).
+		WithVolumeStatus(device.DeviceStatusPending).Build()
+
+	ginkgo.By("Creating the above DeviceVol")
+	volObj, err = DeviceClient.WithNamespace(OpenEBSNamespace).Create(volObj)
+	gomega.Expect(err).To(
+		gomega.BeNil(),
+		"while creating DeviceVol {%s} in namespace {%s}",
+		DeviceVolName,
+		OpenEBSNamespace,
+	)
+
+	_, err = DeviceClient.WithNamespace(OpenEBSNamespace).Get(DeviceVolName, metav1.GetOptions{})
+	gomega.Expect(err).To(
+		gomega.BeNil(),
+		"while retrieving DeviceVol {%s} in namespace {%s}",
+		DeviceVolName,
+		OpenEBSNamespace,
+	)
+}
+
+// IsDeviceVolStatusFailedEventually : Checks for the DeviceVol status and waits till it becomes failed
+func IsDeviceVolStatusFailedEventually() {
+	status := gomega.Eventually(func() bool {
+		deviceVol, _ := DeviceClient.WithNamespace(OpenEBSNamespace).Get(DeviceVolName, metav1.GetOptions{})
+		return deviceVol.Status.State == device.DeviceStatusFailed
+	},
+		120, 5).
+		Should(gomega.BeTrue())
+
+	gomega.Expect(status).To(gomega.Equal(true), "while checking the status of DeviceVol")
+	ginkgo.By("DeviceVol is in failed state")
+}
+
+func deleteDeviceVol() {
+	err := DeviceClient.WithNamespace(OpenEBSNamespace).Delete(DeviceVolName)
+	gomega.Expect(err).To(
+		gomega.BeNil(),
+		"while deleting deviceVol {%s} in namespace {%s}",
+		DeviceVolName,
+		OpenEBSNamespace,
+	)
+	status := IsDeviceVolDeletedEventually()
+	gomega.Expect(status).To(gomega.Equal(true), "while trying to get deleted DeviceVol")
+}