Skip to content

Commit

Permalink
rbd: set SnapshotGroupID on each Snapshot of a VolumeGroupSnapshot
Browse files Browse the repository at this point in the history
Without the SnapshotGroupID in the Snapshot object, Kubernetes CSI does
not know that the Snapshot belongs to a group. In that case, it allows
the deletion of the Snapshot, which should be denied.

Signed-off-by: Niels de Vos <[email protected]>
  • Loading branch information
nixpanic authored and mergify[bot] committed Nov 6, 2024
1 parent ec1e7a4 commit cea8bf8
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 10 deletions.
6 changes: 6 additions & 0 deletions internal/rbd/group/group_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,12 @@ func NewVolumeGroupSnapshot(
}

volumeMap[handle] = name

// store the CSI ID of the group in the snapshot attributes
snapErr = snapshot.SetVolumeGroup(ctx, creds, vgs.id)
if snapErr != nil {
return nil, fmt.Errorf("failed to set volume group ID %q for snapshot %q: %w", vgs.id, name, snapErr)
}
}

j, err := vgs.getJournal(ctx)
Expand Down
5 changes: 0 additions & 5 deletions internal/rbd/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,6 @@ func (mgr *rbdManager) GetSnapshotByID(ctx context.Context, id string) (types.Sn
}
}

// FIXME: The snapshot will have RbdImageName set to the image that was
// used as source. This is not correct for group snapshots images, and
// need to be fixed. See rbdVolume.NewSnapshotByID() for more details.
snapshot.RbdImageName = snapshot.RbdSnapName

return snapshot, nil
}

Expand Down
12 changes: 12 additions & 0 deletions internal/rbd/rbd_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ type rbdSnapshot struct {
// RbdSnapName is the name of the RBD snapshot backing this rbdSnapshot
SourceVolumeID string
RbdSnapName string

// groupID is the CSI volume group ID where this snapshot belongs to
groupID string
}

// imageFeature represents required image features and value.
Expand Down Expand Up @@ -1054,6 +1057,15 @@ func genSnapFromSnapID(
}
}

if imageAttributes.GroupID != "" {
rbdSnap.groupID = imageAttributes.GroupID
// FIXME: The snapshot will have RbdImageName set to the image
// that was used as source. This is not correct for group
// snapshots images, and need to be fixed. See
// rbdVolume.NewSnapshotByID() for more details.
rbdSnap.RbdImageName = rbdSnap.RbdSnapName
}

err = rbdSnap.Connect(cr)
if err != nil {
return rbdSnap, fmt.Errorf("failed to connect to %q: %w",
Expand Down
34 changes: 29 additions & 5 deletions internal/rbd/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,12 @@ func (rbdSnap *rbdSnapshot) ToCSI(ctx context.Context) (*csi.Snapshot, error) {
}

return &csi.Snapshot{
SizeBytes: rbdSnap.VolSize,
SnapshotId: rbdSnap.VolID,
SourceVolumeId: rbdSnap.SourceVolumeID,
CreationTime: timestamppb.New(*created),
ReadyToUse: true,
SizeBytes: rbdSnap.VolSize,
SnapshotId: rbdSnap.VolID,
SourceVolumeId: rbdSnap.SourceVolumeID,
CreationTime: timestamppb.New(*created),
ReadyToUse: true,
GroupSnapshotId: rbdSnap.groupID,
}, nil
}

Expand Down Expand Up @@ -315,3 +316,26 @@ func (rv *rbdVolume) NewSnapshotByID(

return snap, nil
}

func (rbdSnap *rbdSnapshot) SetVolumeGroup(ctx context.Context, cr *util.Credentials, groupID string) error {
vi := util.CSIIdentifier{}
err := vi.DecomposeCSIID(rbdSnap.VolID)
if err != nil {
return err
}

j, err := snapJournal.Connect(rbdSnap.Monitors, rbdSnap.RadosNamespace, cr)
if err != nil {
return fmt.Errorf("snapshot %q failed to connect to journal: %w", rbdSnap, err)
}
defer j.Destroy()

err = j.StoreGroupID(ctx, rbdSnap.Pool, vi.ObjectUUID, groupID)
if err != nil {
return fmt.Errorf("failed to set volume group ID for snapshot %q: %w", rbdSnap, err)
}

rbdSnap.groupID = groupID

return nil
}
5 changes: 5 additions & 0 deletions internal/rbd/types/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"time"

"github.com/container-storage-interface/spec/lib/go/csi"

"github.com/ceph/ceph-csi/internal/util"
)

type Snapshot interface {
Expand All @@ -35,4 +37,7 @@ type Snapshot interface {
ToCSI(ctx context.Context) (*csi.Snapshot, error)

GetCreationTime(ctx context.Context) (*time.Time, error)

// SetVolumeGroup sets the CSI volume group ID in the snapshot.
SetVolumeGroup(ctx context.Context, creds *util.Credentials, vgID string) error
}

0 comments on commit cea8bf8

Please sign in to comment.