fix: avoid forced snapshot cleanup (#231):

Take into account related clones while clean up snapshots.
check all snapshots that we are going to delete and skip those that have active dependent clones.

Author: agneum

pkg/services/provision/thinclones/zfs/zfs.go

@@ -10,6 +10,7 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"unicode"
 
 	"github.com/pkg/errors"
 
@@ -305,12 +306,21 @@ func (m *Manager) DestroySnapshot(snapshotName string) error {
 	return nil
 }
 
-// CleanupSnapshots destroys old snapshots considering retention limit.
+// CleanupSnapshots destroys old snapshots considering retention limit and related clones.
 func (m *Manager) CleanupSnapshots(retentionLimit int) ([]string, error) {
+	clonesCmd := fmt.Sprintf("zfs list -S clones -o name,origin -H -r %s", m.config.Pool.Name)
+
+	clonesOutput, err := m.runner.Run(clonesCmd)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to list snapshots")
+	}
+
+	busySnapshots := m.getBusySnapshotList(clonesOutput)
+
 	cleanupCmd := fmt.Sprintf(
-		"zfs list -t snapshot -H -o name -s %s -s creation -r %s | grep -v clone | head -n -%d "+
+		"zfs list -t snapshot -H -o name -s %s -s creation -r %s | grep -v clone | head -n -%d %s"+
 			"| xargs -n1 --no-run-if-empty zfs destroy -R ",
-		dataStateAtLabel, m.config.Pool.Name, retentionLimit)
+		dataStateAtLabel, m.config.Pool.Name, retentionLimit, excludeBusySnapshots(busySnapshots))
 
 	out, err := m.runner.Run(cleanupCmd)
 	if err != nil {
@@ -322,6 +332,52 @@ func (m *Manager) CleanupSnapshots(retentionLimit int) ([]string, error) {
 	return lines, nil
 }
 
+func (m *Manager) getBusySnapshotList(clonesOutput string) []string {
+	systemClones, userClones := make(map[string]string), make(map[string]struct{})
+
+	userClonePrefix := m.config.Pool.Name + "/" + util.ClonePrefix
+
+	for _, line := range strings.Split(clonesOutput, "\n") {
+		cloneLine := strings.FieldsFunc(line, unicode.IsSpace)
+
+		if len(cloneLine) != 2 || cloneLine[1] == "-" {
+			continue
+		}
+
+		if strings.HasPrefix(cloneLine[0], userClonePrefix) {
+			origin := cloneLine[1]
+
+			if idx := strings.Index(origin, "@"); idx != -1 {
+				origin = origin[:idx]
+			}
+
+			userClones[origin] = struct{}{}
+
+			continue
+		}
+
+		systemClones[cloneLine[0]] = cloneLine[1]
+	}
+
+	busySnapshots := make([]string, 0, len(userClones))
+
+	for userClone := range userClones {
+		busySnapshots = append(busySnapshots, systemClones[userClone])
+	}
+
+	return busySnapshots
+}
+
+// excludeBusySnapshots excludes snapshots that match a pattern by name.
+// The exclusion logic relies on the fact that snapshots have unique substrings (timestamps).
+func excludeBusySnapshots(busySnapshots []string) string {
+	if len(busySnapshots) == 0 {
+		return ""
+	}
+
+	return fmt.Sprintf("| grep -Ev '%s' ", strings.Join(busySnapshots, "|"))
+}
+
 // GetSessionState returns a state of a session.
 func (m *Manager) GetSessionState(name string) (*resources.SessionState, error) {
 	entries, err := m.listFilesystems(m.config.Pool.Name)

pkg/services/provision/thinclones/zfs/zfs_test.go

@@ -113,3 +113,47 @@ func TestFailedListClones(t *testing.T) {
 	assert.Nil(t, cloneNames)
 	assert.EqualError(t, err, "failed to list clones: runner error")
 }
+
+func TestBusySnapshotList(t *testing.T) {
+	m := Manager{config: Config{Pool: &resources.Pool{Name: "dblab_pool"}}}
+
+	out := `dblab_pool	-
+dblab_pool/clone_pre_20210127105215	dblab_pool@snapshot_20210127105215_pre
+dblab_pool/clone_pre_20210127113000	dblab_pool@snapshot_20210127113000_pre
+dblab_pool/clone_pre_20210127120000	dblab_pool@snapshot_20210127120000_pre
+dblab_pool/clone_pre_20210127123000	dblab_pool@snapshot_20210127123000_pre
+dblab_pool/clone_pre_20210127130000	dblab_pool@snapshot_20210127130000_pre
+dblab_pool/clone_pre_20210127133000	dblab_pool@snapshot_20210127133000_pre
+dblab_pool/clone_pre_20210127140000	dblab_pool@snapshot_20210127140000_pre
+dblab_pool/dblab_clone_6000	dblab_pool/clone_pre_20210127133000@snapshot_20210127133008
+dblab_pool/dblab_clone_6001	dblab_pool/clone_pre_20210127123000@snapshot_20210127133008
+`
+	expected := []string{"dblab_pool@snapshot_20210127133000_pre", "dblab_pool@snapshot_20210127123000_pre"}
+
+	list := m.getBusySnapshotList(out)
+	assert.Equal(t, expected, list)
+}
+
+func TestExcludingBusySnapshots(t *testing.T) {
+	testCases := []struct {
+		snapshotList []string
+		result       string
+	}{
+		{
+			snapshotList: []string{},
+			result:       "",
+		},
+		{
+			snapshotList: []string{"dblab_pool@snapshot_20210127133000_pre"},
+			result:       "| grep -Ev 'dblab_pool@snapshot_20210127133000_pre' ",
+		},
+		{
+			snapshotList: []string{"dblab_pool@snapshot_20210127133000_pre", "dblab_pool@snapshot_20210127123000_pre"},
+			result:       "| grep -Ev 'dblab_pool@snapshot_20210127133000_pre|dblab_pool@snapshot_20210127123000_pre' ",
+		},
+	}
+
+	for _, tc := range testCases {
+		assert.Equal(t, tc.result, excludeBusySnapshots(tc.snapshotList))
+	}
+}