Skip to content

Commit

Permalink
fix(controller): prevent zfs volume cr deletion if snapshot exists
Browse files Browse the repository at this point in the history
Signed-off-by: sinhaashish <[email protected]>
  • Loading branch information
sinhaashish committed Jan 10, 2025
1 parent c239bc7 commit 4365666
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 18 deletions.
31 changes: 23 additions & 8 deletions pkg/driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,18 @@ func (cs *controller) DeleteVolume(
unlock := cs.volumeLock.LockVolume(volumeID)
defer unlock()

// Fetch the list of snapshot for the given volume
snapList, err := zfs.GetSnapshotForVolume(volumeID)
if err != nil {
return nil, status.Errorf(
codes.NotFound,
"failed to handle delete volume request for {%s}, "+
"validation failed checking for snapshots. Error: %s",
req.VolumeId,
err.Error(),
)
}

// verify if the volume has already been deleted
vol, err := zfs.GetVolume(volumeID)
if vol != nil && vol.DeletionTimestamp != nil {
Expand All @@ -524,14 +536,17 @@ func (cs *controller) DeleteVolume(
return nil, status.Error(codes.Internal, "can not delete, volume creation is in progress")
}

// Delete the corresponding ZV CR
err = zfs.DeleteVolume(volumeID)
if err != nil {
return nil, errors.Wrapf(
err,
"failed to handle delete volume request for {%s}",
volumeID,
)
// Delete the corresponding ZV CR only if there are no snapshots present for the volume

if len(snapList.Items) == 0 {
err = zfs.DeleteVolume(volumeID)
if err != nil {
return nil, errors.Wrapf(
err,
"failed to handle delete volume request for {%s}",
volumeID,
)
}
}

sendEventOrIgnore("", volumeID, vol.Spec.Capacity, analytics.VolumeDeprovision)
Expand Down
9 changes: 9 additions & 0 deletions pkg/zfs/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,3 +441,12 @@ func IsVolumeReady(vol *apis.ZFSVolume) bool {

return false
}

// GetSnapshotForVolume fetches all the snapshots for the given volume
func GetSnapshotForVolume(volumeID string) (*apis.ZFSSnapshotList, error) {
listOptions := metav1.ListOptions{
LabelSelector: ZFSVolKey + "=" + volumeID,
}
snapList, err := snapbuilder.NewKubeclient().WithNamespace(OpenEBSNamespace).List(listOptions)
return snapList, err
}
44 changes: 41 additions & 3 deletions tests/provision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ import (

var _ = Describe("[zfspv] TEST VOLUME PROVISIONING", func() {
Context("App is deployed with zfs driver", func() {
It("Running zfs volume Creation Test", volumeCreationTest)
It("Running zfs volume Creation Test with custom node id", Label("custom-node-id"), volumeCreationTest)
//It("Running zfs volume Creation Test", volumeCreationTest)
//It("Running zfs volume Creation Test with custom node id", Label("custom-node-id"), volumeCreationTest)
It("Running zfs volume Deletion Test", volumeDeletionTest)
})
})

Expand Down Expand Up @@ -87,7 +88,6 @@ func blockVolCreationTest() {
By("Creating and deploying app pod", createDeployVerifyBlockApp)
By("verifying ZFSVolume object", VerifyZFSVolume)
By("verifying ZFSVolume property change", VerifyZFSVolumePropEdit)
By("Deleting application deployment")

createSnapshot(pvcName, snapName)
verifySnapshotCreated(snapName)
Expand All @@ -106,7 +106,45 @@ func blockVolCreationTest() {
By("Deleting storage class", deleteStorageClass)
}

func blockVolDeletionTest() {
By("Creating default storage class", createStorageClass)
By("creating and verifying PVC bound status", createAndVerifyBlockPVC)

By("Creating and deploying app pod", createDeployVerifyBlockApp)
By("verifying ZFSVolume object", VerifyZFSVolume)

createSnapshot(pvcName, snapName)
verifySnapshotCreated(snapName)

By("Deleting main application deployment")
deleteAppDeployment(appName)

By("Deleting main pvc")
deletePVC(pvcName)

By("Verifying ZFSVolume object after pvc deletion when snapshot is present", VerifyZFSVolume)

By("Creating clone from the snapshot")
createClone(clonePvcName, snapName, scObj.Name)
By("Creating and deploying clone app pod", createDeployVerifyCloneApp)

By("Deleting clone application deployment")
deleteAppDeployment(cloneAppName)

deleteSnapshot(pvcName, snapName)
deletePVC(clonePvcName)

//deleteZV(zvName)

By("Deleting storage class", deleteStorageClass)
}

func volumeCreationTest() {
By("Running volume creation test", fsVolCreationTest)
By("Running block volume creation test", blockVolCreationTest)

}

func volumeDeletionTest() {
By("Running volume deletion test", blockVolDeletionTest)
}
28 changes: 21 additions & 7 deletions tests/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ package tests

import (
"fmt"
"k8s.io/klog/v2"
"os/exec"
"time"

"k8s.io/klog/v2"

"github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"

Expand Down Expand Up @@ -470,19 +471,19 @@ func resizeAndVerifyPVC() {
}
func createDeployVerifyApp() {
ginkgo.By("creating and deploying app pod")
createAndDeployAppPod(appName)
createAndDeployAppPod(appName, pvcName)
time.Sleep(30 * time.Second)
ginkgo.By("verifying app pod is running", verifyAppPodRunning)
}

func createDeployVerifyCloneApp() {
ginkgo.By("creating and deploying app pod")
createAndDeployAppPod(cloneAppName)
time.Sleep(30 * time.Second)
createAndDeployAppPod(cloneAppName, clonePvcName)
time.Sleep(60 * time.Second)
ginkgo.By("verifying app pod is running", verifyAppPodRunning)
}

func createAndDeployAppPod(appname string) {
func createAndDeployAppPod(appname, pvcName string) {
var err error
ginkgo.By("building a busybox app pod deployment using above zfs volume")
deployObj, err = deploy.NewBuilder().
Expand Down Expand Up @@ -529,7 +530,7 @@ func createAndDeployAppPod(appname string) {
WithVolumeBuilders(
k8svolume.NewBuilder().
WithName("datavol1").
WithPVCSource(pvcObj.Name),
WithPVCSource(pvcName),
),
).
Build()
Expand Down Expand Up @@ -609,7 +610,7 @@ func createAndDeployBlockAppPod() {
}

func createDeployVerifyBlockApp() {
ginkgo.By("creating and deploying app pod", createAndDeployBlockAppPod)
createAndDeployBlockAppPod()
time.Sleep(30 * time.Second)
ginkgo.By("verifying app pod is running", verifyAppPodRunning)
}
Expand Down Expand Up @@ -646,6 +647,19 @@ func deletePVC(pvcname string) {
gomega.Expect(status).To(gomega.Equal(true), "while trying to get deleted pvc")
}

func deleteZV(zvname string) {
err := PVCClient.WithNamespace(OpenEBSNamespace).Delete(zvname, &metav1.DeleteOptions{})
gomega.Expect(err).To(
gomega.BeNil(),
"while deleting pvc {%s} in namespace {%s}",
zvname,
OpenEBSNamespace,
)
ginkgo.By("verifying deleted zv")
status := IsPVCDeletedEventually(zvname)
gomega.Expect(status).To(gomega.Equal(true), "while trying to get deleted zv")
}

func getStoragClassParams() []map[string]string {
return []map[string]string{
{
Expand Down

0 comments on commit 4365666

Please sign in to comment.