From 2511dd4cd339dc21dd8432ca410882d2b72db305 Mon Sep 17 00:00:00 2001
From: Praveen M <m.praveen@ibm.com>
Date: Wed, 18 Oct 2023 13:45:53 +0530
Subject: [PATCH] cephfs: remove snapshot protect/unprotect
This commit eliminates the code for protecting and unprotecting
snapshots, as the functionality to protect and unprotect snapshots
is being deprecated.
Signed-off-by: Praveen M <m.praveen@ibm.com>
---
internal/cephfs/controllerserver.go | 53 ++-----------------
internal/cephfs/core/clone.go | 60 ++-------------------
internal/cephfs/core/snapshot.go | 81 -----------------------------
3 files changed, 8 insertions(+), 186 deletions(-)
diff --git a/internal/cephfs/controllerserver.go b/internal/cephfs/controllerserver.go
index d1c126fc3d15..39feb1fdf975 100644
--- a/internal/cephfs/controllerserver.go
+++ b/internal/cephfs/controllerserver.go
@@ -750,7 +750,7 @@ func (cs *ControllerServer) ControllerExpandVolume(
// CreateSnapshot creates the snapshot in backend and stores metadata
// in store
//
-//nolint:gocognit,gocyclo,cyclop // golangci-lint did not catch this earlier, needs to get fixed late
+//nolint:gocyclo,cyclop // golangci-lint did not catch this earlier, needs to get fixed late
func (cs *ControllerServer) CreateSnapshot(
ctx context.Context,
req *csi.CreateSnapshotRequest,
@@ -830,54 +830,19 @@ func (cs *ControllerServer) CreateSnapshot(
}
defer cs.VolumeLocks.Release(sourceVolID)
snapName := req.GetName()
- sid, snapInfo, err := store.CheckSnapExists(ctx, parentVolOptions, cephfsSnap, cs.ClusterName, cs.SetMetadata, cr)
+ sid, _, err := store.CheckSnapExists(ctx, parentVolOptions, cephfsSnap, cs.ClusterName, cs.SetMetadata, cr)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
- // check are we able to retrieve the size of parent
- // ceph fs subvolume info command got added in 14.2.10 and 15.+
- // as we are not able to retrieve the parent size we are rejecting the
- // request to create snapshot.
- // TODO: For this purpose we could make use of cached clusterAdditionalInfo
- // too.
volClient := core.NewSubVolume(parentVolOptions.GetConnection(), &parentVolOptions.SubVolume,
parentVolOptions.ClusterID, cs.ClusterName, cs.SetMetadata)
info, err := volClient.GetSubVolumeInfo(ctx)
- if err != nil {
- // Check error code value against ErrInvalidCommand to understand the cluster
- // support it or not, It's safe to evaluate as the filtering
- // is already done from GetSubVolumeInfo() and send out the error here.
- if errors.Is(err, cerrors.ErrInvalidCommand) {
- return nil, status.Error(
- codes.FailedPrecondition,
- "subvolume info command not supported in current ceph cluster")
- }
- if sid != nil {
- errDefer := store.UndoSnapReservation(ctx, parentVolOptions, *sid, snapName, cr)
- if errDefer != nil {
- log.WarningLog(ctx, "failed undoing reservation of snapshot: %s (%s)",
- requestName, errDefer)
- }
- }
-
- return nil, status.Error(codes.Internal, err.Error())
- }
metadata := k8s.GetSnapshotMetadata(req.GetParameters())
if sid != nil {
- // check snapshot is protected
- protected := true
snapClient := core.NewSnapshot(parentVolOptions.GetConnection(), sid.FsSnapshotName,
parentVolOptions.ClusterID, cs.ClusterName, cs.SetMetadata, &parentVolOptions.SubVolume)
- if !(snapInfo.Protected == core.SnapshotIsProtected) {
- err = snapClient.ProtectSnapshot(ctx)
- if err != nil {
- protected = false
- log.WarningLog(ctx, "failed to protect snapshot of snapshot: %s (%s)",
- sid.FsSnapshotName, err)
- }
- }
// Update snapshot-name/snapshot-namespace/snapshotcontent-name details on
// subvolume snapshot as metadata in case snapshot already exist
@@ -894,7 +859,7 @@ func (cs *ControllerServer) CreateSnapshot(
SnapshotId: sid.SnapshotID,
SourceVolumeId: req.GetSourceVolumeId(),
CreationTime: sid.CreationTime,
- ReadyToUse: protected,
+ ReadyToUse: true,
},
}, nil
}
@@ -968,10 +933,6 @@ func (cs *ControllerServer) doSnapshot(
return snap, fmt.Errorf("failed to get snapshot info for snapshot:%s", snapID)
}
snap.CreationTime = timestamppb.New(snap.CreatedAt)
- err = snapClient.ProtectSnapshot(ctx)
- if err != nil {
- log.ErrorLog(ctx, "failed to protect snapshot %s %v", snapID, err)
- }
// Set snapshot-name/snapshot-namespace/snapshotcontent-name details
// on subvolume snapshot as metadata on create
@@ -1006,8 +967,6 @@ func (cs *ControllerServer) validateSnapshotReq(ctx context.Context, req *csi.Cr
// DeleteSnapshot deletes the snapshot in backend and removes the
// snapshot metadata from store.
-//
-//nolint:gocyclo,cyclop // TODO: reduce complexity
func (cs *ControllerServer) DeleteSnapshot(
ctx context.Context,
req *csi.DeleteSnapshotRequest,
@@ -1102,12 +1061,6 @@ func (cs *ControllerServer) DeleteSnapshot(
}
snapClient := core.NewSnapshot(volOpt.GetConnection(), sid.FsSnapshotName,
volOpt.ClusterID, cs.ClusterName, cs.SetMetadata, &volOpt.SubVolume)
- if snapInfo.Protected == core.SnapshotIsProtected {
- err = snapClient.UnprotectSnapshot(ctx)
- if err != nil {
- return nil, status.Error(codes.Internal, err.Error())
- }
- }
needsDelete, err := store.UnrefSelfInSnapshotBackedVolumes(ctx, volOpt, sid.SnapshotID)
if err != nil {
diff --git a/internal/cephfs/core/clone.go b/internal/cephfs/core/clone.go
index 8f3a75a4d280..e30e0f08ee38 100644
--- a/internal/cephfs/core/clone.go
+++ b/internal/cephfs/core/clone.go
@@ -18,7 +18,6 @@ package core
import (
"context"
- "errors"
"fmt"
cerrors "github.com/ceph/ceph-csi/internal/cephfs/errors"
@@ -73,43 +72,20 @@ func (s *subVolumeClient) CreateCloneFromSubvolume(
return err
}
- var (
- // if protectErr is not nil we will delete the snapshot as the protect fails
- protectErr error
- // if cloneErr is not nil we will unprotect the snapshot and delete the snapshot
- cloneErr error
- )
- defer func() {
- if protectErr != nil {
- err = snapClient.DeleteSnapshot(ctx)
- if err != nil {
- log.ErrorLog(ctx, "failed to delete snapshot %s %v", snapshotID, err)
- }
- }
+ // if cloneErr is not nil we will unprotect the snapshot and delete the snapshot
+ var cloneErr error
+
+ defer func() {
if cloneErr != nil {
if err = s.PurgeVolume(ctx, true); err != nil {
log.ErrorLog(ctx, "failed to delete volume %s: %v", s.VolID, err)
}
- if err = snapClient.UnprotectSnapshot(ctx); err != nil {
- // In case the snap is already unprotected we get ErrSnapProtectionExist error code
- // in that case we are safe and we could discard this error and we are good to go
- // ahead with deletion
- if !errors.Is(err, cerrors.ErrSnapProtectionExist) {
- log.ErrorLog(ctx, "failed to unprotect snapshot %s %v", snapshotID, err)
- }
- }
if err = snapClient.DeleteSnapshot(ctx); err != nil {
log.ErrorLog(ctx, "failed to delete snapshot %s %v", snapshotID, err)
}
}
}()
- protectErr = snapClient.ProtectSnapshot(ctx)
- if protectErr != nil {
- log.ErrorLog(ctx, "failed to protect snapshot %s %v", snapshotID, protectErr)
-
- return protectErr
- }
cloneErr = snapClient.CloneSnapshot(ctx, s.SubVolume)
if cloneErr != nil {
log.ErrorLog(ctx, "failed to clone snapshot %s %s to %s %v", parentvolOpt.VolID, snapshotID, s.VolID, cloneErr)
@@ -139,16 +115,6 @@ func (s *subVolumeClient) CreateCloneFromSubvolume(
}
// As we completed clone, remove the intermediate snap
- if err = snapClient.UnprotectSnapshot(ctx); err != nil {
- // In case the snap is already unprotected we get ErrSnapProtectionExist error code
- // in that case we are safe and we could discard this error and we are good to go
- // ahead with deletion
- if !errors.Is(err, cerrors.ErrSnapProtectionExist) {
- log.ErrorLog(ctx, "failed to unprotect snapshot %s %v", snapshotID, err)
-
- return err
- }
- }
if err = snapClient.DeleteSnapshot(ctx); err != nil {
log.ErrorLog(ctx, "failed to delete snapshot %s %v", snapshotID, err)
@@ -166,24 +132,8 @@ func (s *subVolumeClient) CleanupSnapshotFromSubvolume(
// identified during PVC-PVC cloning.
snapShotID := s.VolID
snapClient := NewSnapshot(s.conn, snapShotID, s.clusterID, s.clusterName, s.enableMetadata, parentVol)
- snapInfo, err := snapClient.GetSnapshotInfo(ctx)
- if err != nil {
- if errors.Is(err, cerrors.ErrSnapNotFound) {
- return nil
- }
-
- return err
- }
-
- if snapInfo.Protected == SnapshotIsProtected {
- err = snapClient.UnprotectSnapshot(ctx)
- if err != nil {
- log.ErrorLog(ctx, "failed to unprotect snapshot %s %v", snapShotID, err)
- return err
- }
- }
- err = snapClient.DeleteSnapshot(ctx)
+ err := snapClient.DeleteSnapshot(ctx)
if err != nil {
log.ErrorLog(ctx, "failed to delete snapshot %s %v", snapShotID, err)
diff --git a/internal/cephfs/core/snapshot.go b/internal/cephfs/core/snapshot.go
index b1994266efbb..11c853fa9b8d 100644
--- a/internal/cephfs/core/snapshot.go
+++ b/internal/cephfs/core/snapshot.go
@@ -30,12 +30,6 @@ import (
"github.com/golang/protobuf/ptypes/timestamp"
)
-// autoProtect points to the snapshot auto-protect feature of
-// the subvolume.
-const (
- autoProtect = "snapshot-autoprotect"
-)
-
// SnapshotClient is the interface that holds the signature of snapshot methods
// that interacts with CephFS snapshot API's.
type SnapshotClient interface {
@@ -45,10 +39,6 @@ type SnapshotClient interface {
DeleteSnapshot(ctx context.Context) error
// GetSnapshotInfo returns the snapshot info of the subvolume.
GetSnapshotInfo(ctx context.Context) (SnapshotInfo, error)
- // ProtectSnapshot protects the snapshot of the subvolume.
- ProtectSnapshot(ctx context.Context) error
- // UnprotectSnapshot unprotects the snapshot of the subvolume.
- UnprotectSnapshot(ctx context.Context) error
// CloneSnapshot clones the snapshot of the subvolume.
CloneSnapshot(ctx context.Context, cloneVolOptions *SubVolume) error
// SetAllSnapshotMetadata set all the metadata from arg parameters on
@@ -139,7 +129,6 @@ type SnapshotInfo struct {
CreatedAt time.Time
CreationTime *timestamp.Timestamp
HasPendingClones string
- Protected string
}
// GetSnapshotInfo returns the snapshot info of the subvolume.
@@ -169,80 +158,10 @@ func (s *snapshotClient) GetSnapshotInfo(ctx context.Context) (SnapshotInfo, err
}
snap.CreatedAt = info.CreatedAt.Time
snap.HasPendingClones = info.HasPendingClones
- snap.Protected = info.Protected
return snap, nil
}
-// ProtectSnapshot protects the snapshot of the subvolume.
-func (s *snapshotClient) ProtectSnapshot(ctx context.Context) error {
- // If "snapshot-autoprotect" feature is present, The ProtectSnapshot
- // call should be treated as a no-op.
- if checkSubvolumeHasFeature(autoProtect, s.Features) {
- return nil
- }
- fsa, err := s.conn.GetFSAdmin()
- if err != nil {
- log.ErrorLog(ctx, "could not get FSAdmin: %s", err)
-
- return err
- }
-
- err = fsa.ProtectSubVolumeSnapshot(s.FsName, s.SubvolumeGroup, s.VolID, s.SnapshotID)
- if err != nil {
- if errors.Is(err, rados.ErrObjectExists) {
- return nil
- }
- log.ErrorLog(
- ctx,
- "failed to protect subvolume snapshot %s %s in fs %s with error: %s",
- s.VolID,
- s.SnapshotID,
- s.FsName,
- err)
-
- return err
- }
-
- return nil
-}
-
-// UnprotectSnapshot unprotects the snapshot of the subvolume.
-func (s *snapshotClient) UnprotectSnapshot(ctx context.Context) error {
- // If "snapshot-autoprotect" feature is present, The UnprotectSnapshot
- // call should be treated as a no-op.
- if checkSubvolumeHasFeature(autoProtect, s.Features) {
- return nil
- }
- fsa, err := s.conn.GetFSAdmin()
- if err != nil {
- log.ErrorLog(ctx, "could not get FSAdmin: %s", err)
-
- return err
- }
-
- err = fsa.UnprotectSubVolumeSnapshot(s.FsName, s.SubvolumeGroup, s.VolID,
- s.SnapshotID)
- if err != nil {
- // In case the snap is already unprotected we get ErrSnapProtectionExist error code
- // in that case we are safe and we could discard this error.
- if errors.Is(err, rados.ErrObjectExists) {
- return nil
- }
- log.ErrorLog(
- ctx,
- "failed to unprotect subvolume snapshot %s %s in fs %s with error: %s",
- s.VolID,
- s.SnapshotID,
- s.FsName,
- err)
-
- return err
- }
-
- return nil
-}
-
// CloneSnapshot clones the snapshot of the subvolume.
func (s *snapshotClient) CloneSnapshot(
ctx context.Context,