Merge pull request #1848 from luissimas/linstor-remove-snapshots

Add `linstor.remove_snapshots` config option

Author: stgraber

doc/reference/storage_linstor.md

@@ -57,6 +57,9 @@ Restoring from older snapshots
   This method makes it possible to confirm whether a specific snapshot contains what you need.
   After determining the correct snapshot, you can {ref}`remove the newer snapshots <storage-edit-snapshots>` so that the snapshot you need is the latest one and you can restore it.
 
+  Alternatively, you can configure Incus to automatically discard the newer snapshots during restore.
+  To do so, set the [`linstor.remove_snapshots`](storage-linstor-vol-config) configuration for the volume (or the corresponding `volume.linstor.remove_snapshots` configuration on the storage pool for all volumes in the pool).
+
 ## Configuration options
 
 The following configuration options are available for storage pools that use the `linstor` driver and for storage volumes in these pools.
@@ -96,6 +99,7 @@ Key                               | Type      | Condition
 `drbd.on_no_quorum`               | string    |                                                   | -                                              | The DRBD policy to use on resources when quorum is lost (applied to the resource definition)
 `drbd.auto_diskful`               | string    |                                                   | -                                              | A duration string describing the time after which a primary diskless resource can be converted to diskful if storage is available on the node (applied to the resource definition)
 `drbd.auto_add_quorum_tiebreaker` | bool      |                                                   | `true`                                         | Whether to allow LINSTOR to automatically create diskless resources to act as quorum tiebreakers if needed (applied to the resource definition)
+`linstor.remove_snapshots`        | bool      |                                                   | same as `volume.zfs.remove_snapshots` or `false` | Remove snapshots as needed
 
 [^*]: {{snapshot_pattern_detail}}
 

internal/server/storage/drivers/driver_linstor_utils.go

@@ -54,6 +54,9 @@ const DrbdAutoDiskfulConfigKey = "drbd.auto_diskful"
 // DrbdAutoAddQuorumTiebreakerConfigKey represents the config key that describes whether DRBD will automatically create tiebreaker resources.
 const DrbdAutoAddQuorumTiebreakerConfigKey = "drbd.auto_add_quorum_tiebreaker"
 
+// LinstorRemoveSnapshotsConfigKey represents the config key that describes whether snapshots should be automatically removed with volumes.
+const LinstorRemoveSnapshotsConfigKey = "linstor.remove_snapshots"
+
 // LinstorAuxSnapshotPrefix represents the AuxProp prefix to map Incus and LINSTOR snapshots.
 const LinstorAuxSnapshotPrefix = "Aux/Incus/snapshot-name/"
 
@@ -1076,6 +1079,22 @@ func (d *linstor) drbdPropsFromConfig(config map[string]string) (map[string]stri
 	return props, nil
 }
 
+// getSnapshots retrieves all snapshots for a given resource definition name.
+func (d *linstor) getSnapshots(resourceDefinitionName string) ([]linstorClient.Snapshot, error) {
+	linstor, err := d.state.Linstor()
+	if err != nil {
+		return []linstorClient.Snapshot{}, err
+	}
+
+	snapshots, err := linstor.Client.Resources.GetSnapshots(context.TODO(), resourceDefinitionName)
+	if err != nil {
+		return snapshots, fmt.Errorf("Unable to get snapshots: %w", err)
+	}
+
+	return snapshots, nil
+}
+
+// rsyncMigrationType returns the migration types to use for a given content type.
 func (d *linstor) rsyncMigrationType(contentType ContentType) localMigration.Type {
 	var rsyncTransportType migration.MigrationFSType
 	var rsyncFeatures []string
@@ -1100,6 +1119,7 @@ func (d *linstor) rsyncMigrationType(contentType ContentType) localMigration.Typ
 	}
 }
 
+// parseVolumeType parses a string into a volume type.
 func (d *linstor) parseVolumeType(s string) (*VolumeType, bool) {
 	for _, volType := range d.Info().VolumeTypes {
 		if s == string(volType) {
@@ -1110,6 +1130,7 @@ func (d *linstor) parseVolumeType(s string) (*VolumeType, bool) {
 	return nil, false
 }
 
+// parseContentType parses a string into a volume type.
 func (d *linstor) parseContentType(s string) (*ContentType, bool) {
 	for _, contentType := range []ContentType{ContentTypeFS, ContentTypeBlock, ContentTypeISO} {
 		if s == string(contentType) {
@@ -1120,6 +1141,7 @@ func (d *linstor) parseContentType(s string) (*ContentType, bool) {
 	return nil, false
 }
 
+// generateUUIDWithPrefix generates a new UUID (without "-") and appends it to the configured volume prefix.
 func (d *linstor) generateUUIDWithPrefix() string {
 	return d.config[LinstorVolumePrefixConfigKey] + strings.ReplaceAll(uuid.NewString(), "-", "")
 }

internal/server/storage/drivers/driver_linstor_volumes.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"io/fs"
 	"os"
+	"slices"
 	"strings"
 	"time"
 
@@ -74,6 +75,7 @@ func (d *linstor) commonVolumeRules() map[string]func(value string) error {
 		DrbdOnNoQuorumConfigKey:              validate.Optional(validate.IsOneOf("io-error", "suspend-io")),
 		DrbdAutoDiskfulConfigKey:             validate.Optional(validate.IsMinimumDuration(time.Minute)),
 		DrbdAutoAddQuorumTiebreakerConfigKey: validate.Optional(validate.IsBool),
+		LinstorRemoveSnapshotsConfigKey:      validate.Optional(validate.IsBool),
 	}
 }
 
@@ -774,9 +776,56 @@ func (d *linstor) RestoreVolume(vol Volume, snapshotName string, op *operations.
 		return err
 	}
 
-	// TODO: check if more recent snapshots exist and delete them if the user
-	// configure the storage pool to allow for such deletions. Otherwise, return
-	// a graceful error
+	resourceDefinition, err := d.getResourceDefinition(vol, false)
+	if err != nil {
+		return err
+	}
+
+	// Since LINSTOR only supports rolling back a resource definition to its latest
+	// snapshot, we need to check if the given snapshot is the latest one.
+	existingSnapshots, err := d.getSnapshots(resourceDefinition.Name)
+	if err != nil {
+		return err
+	}
+
+	// Sort all snapshots by creation date in descending order.
+	slices.SortFunc(existingSnapshots, func(a linstorClient.Snapshot, b linstorClient.Snapshot) int {
+		return a.Snapshots[0].CreateTimestamp.Compare(b.Snapshots[0].CreateTimestamp.Time) * -1
+	})
+
+	linstorSnapshotName, ok := resourceDefinition.Props[LinstorAuxSnapshotPrefix+snapshotName]
+	if !ok {
+		return fmt.Errorf("Could not find snapshot name mapping for volume %s", vol.Name())
+	}
+
+	snapshotMap, err := d.getSnapshotMap(vol)
+	if err != nil {
+		return err
+	}
+
+	// Get all snapshots taken after the one we're trying to restore.
+	snapshots := []string{}
+	for _, s := range existingSnapshots {
+		if s.Name == linstorSnapshotName {
+			break
+		}
+
+		snapshots = append(snapshots, snapshotMap[s.Name])
+	}
+
+	// Check if snapshot removal is allowed.
+	if len(snapshots) > 0 {
+		if util.IsFalseOrEmpty(vol.ExpandedConfig(LinstorRemoveSnapshotsConfigKey)) {
+			return fmt.Errorf("Snapshot %q cannot be restored due to subsequent snapshot(s). Set %s to override", snapshotName, LinstorRemoveSnapshotsConfigKey)
+		}
+
+		// Setup custom error to tell the backend what to delete.
+		err := ErrDeleteSnapshots{}
+		err.Snapshots = snapshots
+		return err
+	}
+
+	// Restore the snapshot.
 	err = d.restoreVolume(vol, snapVol)
 	if err != nil {
 		return err
@@ -1151,11 +1200,6 @@ func (d *linstor) CreateVolumeFromMigration(vol Volume, conn io.ReadWriteCloser,
 func (d *linstor) VolumeSnapshots(vol Volume, op *operations.Operation) ([]string, error) {
 	var snapshots []string
 
-	linstor, err := d.state.Linstor()
-	if err != nil {
-		return snapshots, err
-	}
-
 	resourceDefinition, err := d.getResourceDefinition(vol, false)
 	if err != nil {
 		return snapshots, err
@@ -1167,9 +1211,9 @@ func (d *linstor) VolumeSnapshots(vol Volume, op *operations.Operation) ([]strin
 	}
 
 	// Get the snapshots.
-	linstorSnapshots, err := linstor.Client.Resources.GetSnapshots(context.TODO(), resourceDefinition.Name)
+	linstorSnapshots, err := d.getSnapshots(resourceDefinition.Name)
 	if err != nil {
-		return snapshots, fmt.Errorf("Unable to get snapshots: %w", err)
+		return snapshots, err
 	}
 
 	for _, snapshot := range linstorSnapshots {

test/suites/storage_driver_linstor.sh

@@ -92,9 +92,20 @@ test_storage_driver_linstor() {
     incus storage volume set "incustest-$(basename "${INCUS_DIR}")-pool1" c1 size 500MiB
     incus storage volume unset "incustest-$(basename "${INCUS_DIR}")-pool1" c1 size
 
+    # Validate that we can restore to previous snapshots given that linstor.remove_snapshots is set
+    incus storage volume create "incustest-$(basename "${INCUS_DIR}")-pool1" c3
+    incus storage volume snapshot create "incustest-$(basename "${INCUS_DIR}")-pool1" c3 snap0
+    incus storage volume snapshot create "incustest-$(basename "${INCUS_DIR}")-pool1" c3 snap1
+    ! incus storage volume snapshot restore "incustest-$(basename "${INCUS_DIR}")-pool1" c3 snap0 || false
+    incus storage volume set "incustest-$(basename "${INCUS_DIR}")-pool1" c3 linstor.remove_snapshots=true
+    incus storage volume snapshot restore "incustest-$(basename "${INCUS_DIR}")-pool1" c3 snap0 || false
+    incus storage volume list "incustest-$(basename "${INCUS_DIR}")-pool1" | grep snap0
+    ! incus storage volume list "incustest-$(basename "${INCUS_DIR}")-pool1" | grep snap1 || false
+
+    # Cleanup
     incus storage volume delete "incustest-$(basename "${INCUS_DIR}")-pool1" c1
     incus storage volume delete "incustest-$(basename "${INCUS_DIR}")-pool1" c2
-
+    incus storage volume delete "incustest-$(basename "${INCUS_DIR}")-pool1" c3
     incus image delete testimage
     incus profile device remove default root
     incus storage delete "incustest-$(basename "${INCUS_DIR}")-pool1"