Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement deletion of local snapshots for S3 backups #202

Merged
merged 1 commit into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
30 changes: 30 additions & 0 deletions pkg/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -956,8 +956,14 @@ 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)
}

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 +973,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 +1359,25 @@ 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
}

// Create a new, local only snapshot object (no remote set!), so we don't accidentally delete the remote backup
return d.Snapshots.SnapDelete(ctx, &volume.Snapshot{Snapshot: snap.Snapshot})
}

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
Loading