Skip to content

Commit

Permalink
implement deletion of local snapshots for S3 backups
Browse files Browse the repository at this point in the history
When creating a remote backup in LINSTOR, we can remove the local snapshot
after shipping completes.

Signed-off-by: Moritz "WanzenBug" Wanzenböck <[email protected]>
  • Loading branch information
WanzenBug committed Jun 15, 2023
1 parent 28f1640 commit 60f3698
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

- Delete S3 backups in delete snapshot requests.
- Delete local snapshots when creating S3 backups when new "delete-local" parameter is true.

## [1.1.1] - 2023-06-05

Expand Down
2 changes: 2 additions & 0 deletions examples/k8s/volume-snapshot-class.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ parameters:
# See https://linbit.com/drbd-user-guide/linstor-guide-1_0-en/#s-shipping_snapshots-linstor for details
# on each parameter.
snap.linstor.csi.linbit.com/remote-name: snapshot-bucket
# Delete local copy of the snapshot after uploading completes
snap.linstor.csi.linbit.com/delete-local: "true"
snap.linstor.csi.linbit.com/allow-incremental: "false"
snap.linstor.csi.linbit.com/s3-bucket: snapshot-bucket
snap.linstor.csi.linbit.com/s3-endpoint: s3.us-west-1.amazonaws.com
Expand Down
41 changes: 41 additions & 0 deletions pkg/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -956,8 +956,21 @@ func (d Driver) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotReque
}).Debug("found existing snapshot")

if existingSnap.GetSourceVolumeId() == req.GetSourceVolumeId() {
err = d.maybeDeleteLocalSnapshot(ctx, existingSnap, params)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to delete local snapshot: %s", err)
}
if existingSnap.ReadyToUse && params.Type != volume.SnapshotTypeInCluster && params.DeleteLocal {
// Create a new, local only snapshot object, so we don't accidentally delete the remote backup
err := d.Snapshots.SnapDelete(ctx, &volume.Snapshot{Snapshot: existingSnap.Snapshot})
if err != nil {
return nil, status.Errorf(codes.Internal, "can't delete local snapshot: %s", err)
}
}

return &csi.CreateSnapshotResponse{Snapshot: &existingSnap.Snapshot}, nil
}

return nil, status.Errorf(codes.AlreadyExists, "can't use %q for snapshot name for volume %q, snapshot name is in use for volume %q",
req.GetName(), req.GetSourceVolumeId(), existingSnap.GetSourceVolumeId())
}
Expand All @@ -967,6 +980,11 @@ func (d Driver) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotReque
return nil, status.Errorf(codes.Internal, "failed to create snapshot: %v", err)
}

err = d.maybeDeleteLocalSnapshot(ctx, snap, params)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to delete local snapshot: %s", err)
}

return &csi.CreateSnapshotResponse{Snapshot: &snap.Snapshot}, nil
}

Expand Down Expand Up @@ -1348,6 +1366,29 @@ func (d Driver) createNewVolume(ctx context.Context, info *volume.Info, params *
}, nil
}

// maybeDeleteLocalSnapshot deletes the local portion of a snapshot according to their volume.SnapshotParameters.
// It will not delete a snapshot that is not ready, does not have a remote target, or where local deletion is disabled.
func (d Driver) maybeDeleteLocalSnapshot(ctx context.Context, snap *volume.Snapshot, params *volume.SnapshotParameters) error {
if !snap.ReadyToUse {
return nil
}

if params.Type == volume.SnapshotTypeInCluster {
return nil
}

if !params.DeleteLocal {
return nil
}

err := d.Snapshots.SnapDelete(ctx, &volume.Snapshot{Snapshot: snap.Snapshot})
if err != nil {
return fmt.Errorf("can't delete local snapshot: %s", err)
}

return nil
}

func missingAttr(methodCall, volumeID, attr string) error {
if volumeID == "" {
volumeID = "an unknown volume"
Expand Down
12 changes: 12 additions & 0 deletions pkg/volume/snapshot_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type SnapshotParameters struct {
Type SnapshotType `json:"type,omitempty"`
AllowIncremental bool `json:"allow-incremental"`
RemoteName string `json:"remote-name,omitempty"`
DeleteLocal bool `json:"delete-local,omitempty"`
S3Endpoint string `json:"s3-endpoint,omitempty"`
S3Bucket string `json:"s3-bucket,omitempty"`
S3SigningRegion string `json:"s3-signing-region,omitempty"`
Expand Down Expand Up @@ -51,6 +52,13 @@ func NewSnapshotParameters(params, secrets map[string]string) (*SnapshotParamete
}

p.Type = t
case "/delete-local":
b, err := strconv.ParseBool(v)
if err != nil {
return nil, err
}

p.DeleteLocal = b
case "/allow-incremental":
b, err := strconv.ParseBool(v)
if err != nil {
Expand Down Expand Up @@ -82,6 +90,10 @@ func NewSnapshotParameters(params, secrets map[string]string) (*SnapshotParamete
return nil, fmt.Errorf("snapshots of type `%s` require specifying a %s/remote-name", p.Type, linstor.SnapshotParameterNamespace)
}

if p.Type == SnapshotTypeInCluster && p.DeleteLocal {
return nil, fmt.Errorf("cannot use %s/delete-local with in-cluster snapshots", linstor.SnapshotParameterNamespace)
}

p.S3AccessKey = secrets["access-key"]
p.S3SecretKey = secrets["secret-key"]

Expand Down

0 comments on commit 60f3698

Please sign in to comment.