Merge branch '382-snapshot-list-workaround' into 'master'

fix: keep the snapshot list with relevant values (#382)

Closes #382

See merge request postgres-ai/database-lab!539

Author: agneum

engine/internal/provision/pool/manager.go

@@ -69,16 +69,12 @@ func NewManager(runner runners.Runner, config ManagerConfig) (FSManager, error)
 
 	switch config.Pool.Mode {
 	case zfs.PoolMode:
-		osUser, err := user.Current()
+		zfsConfig, err := buildZFSConfig(config)
 		if err != nil {
-			return nil, errors.Wrap(err, "failed to get current user")
+			return nil, err
 		}
 
-		manager = zfs.NewFSManager(runner, zfs.Config{
-			Pool:              config.Pool,
-			PreSnapshotSuffix: config.PreSnapshotSuffix,
-			OSUsername:        osUser.Username,
-		})
+		manager = zfs.NewFSManager(runner, zfsConfig)
 
 	case lvm.PoolMode:
 		if manager, err = lvm.NewFSManager(runner, config.Pool); err != nil {
@@ -93,3 +89,41 @@ func NewManager(runner runners.Runner, config ManagerConfig) (FSManager, error)
 
 	return manager, nil
 }
+
+// BuildFromExistingManager prepares FSManager from an existing one.
+func BuildFromExistingManager(fsm FSManager, config ManagerConfig) (FSManager, error) {
+	switch manager := fsm.(type) {
+	case *zfs.Manager:
+		zfsConfig, err := buildZFSConfig(config)
+		if err != nil {
+			return nil, err
+		}
+
+		manager.UpdateConfig(zfsConfig)
+
+		fsm = manager
+
+	case *lvm.LVManager:
+		manager.UpdateConfig(config.Pool)
+
+		fsm = manager
+
+	default:
+		return nil, fmt.Errorf(`unsupported thin-clone manager: %T`, manager)
+	}
+
+	return fsm, nil
+}
+
+func buildZFSConfig(config ManagerConfig) (zfs.Config, error) {
+	osUser, err := user.Current()
+	if err != nil {
+		return zfs.Config{}, fmt.Errorf("failed to get current user: %w", err)
+	}
+
+	return zfs.Config{
+		Pool:              config.Pool,
+		PreSnapshotSuffix: config.PreSnapshotSuffix,
+		OSUsername:        osUser.Username,
+	}, nil
+}

engine/internal/provision/pool/pool_manager.go

@@ -259,6 +259,10 @@ func (pm *Manager) examineEntries(entries []os.DirEntry) (map[string]FSManager,
 
 	poolMappings := make(map[string]string)
 
+	pm.mu.Lock()
+	originalPools := pm.fsManagerPool
+	pm.mu.Unlock()
+
 	for _, entry := range entries {
 		if !entry.IsDir() {
 			continue
@@ -324,10 +328,19 @@ func (pm *Manager) examineEntries(entries []os.DirEntry) (map[string]FSManager,
 			}
 		}
 
-		fsm, err := NewManager(pm.runner, ManagerConfig{
+		var fsm FSManager
+
+		managerConfig := ManagerConfig{
 			Pool:              pool,
 			PreSnapshotSuffix: pm.cfg.PreSnapshotSuffix,
-		})
+		}
+
+		if originalFSM, ok := originalPools[pool.Name]; ok {
+			fsm, err = BuildFromExistingManager(originalFSM, managerConfig)
+		} else {
+			fsm, err = NewManager(pm.runner, managerConfig)
+		}
+
 		if err != nil {
 			log.Msg("failed to create clone manager:", err.Error())
 			continue

engine/internal/provision/thinclones/lvm/lvmanager.go

@@ -47,6 +47,11 @@ func (m *LVManager) Pool() *resources.Pool {
 	return m.pool
 }
 
+// UpdateConfig updates the manager's pool.
+func (m *LVManager) UpdateConfig(pool *resources.Pool) {
+	m.pool = pool
+}
+
 // CreateClone creates a new volume.
 func (m *LVManager) CreateClone(name, _ string) error {
 	return CreateVolume(m.runner, m.volumeGroup, m.logicalVolume, name, m.pool.ClonesDir())

engine/internal/provision/thinclones/zfs/zfs.go

@@ -159,9 +159,10 @@ type Config struct {
 // NewFSManager creates a new Manager instance for ZFS.
 func NewFSManager(runner runners.Runner, config Config) *Manager {
 	m := Manager{
-		runner: runner,
-		config: config,
-		mu:     &sync.Mutex{},
+		runner:    runner,
+		config:    config,
+		mu:        &sync.Mutex{},
+		snapshots: make([]resources.Snapshot, 0),
 	}
 
 	return &m
@@ -172,6 +173,11 @@ func (m *Manager) Pool() *resources.Pool {
 	return m.config.Pool
 }
 
+// UpdateConfig updates the manager's configuration.
+func (m *Manager) UpdateConfig(cfg Config) {
+	m.config = cfg
+}
+
 // CreateClone creates a new ZFS clone.
 func (m *Manager) CreateClone(cloneName, snapshotID string) error {
 	exists, err := m.cloneExists(cloneName)
@@ -316,14 +322,20 @@ func (m *Manager) CreateSnapshot(poolSuffix, dataStateAt string) (string, error)
 		return "", fmt.Errorf("failed to parse dataStateAt: %w", err)
 	}
 
-	m.addSnapshotToList(resources.Snapshot{
+	newSnapshot := resources.Snapshot{
 		ID:          snapshotName,
 		CreatedAt:   time.Now(),
 		DataStateAt: dataStateTime,
 		Pool:        m.config.Pool.Name,
-	})
+	}
+
+	if !strings.HasSuffix(snapshotName, m.config.PreSnapshotSuffix) {
+		m.addSnapshotToList(newSnapshot)
 
-	go m.RefreshSnapshotList()
+		log.Dbg("New snapshot:", newSnapshot)
+
+		m.RefreshSnapshotList()
+	}
 
 	return snapshotName, nil
 }
@@ -511,8 +523,11 @@ func (m *Manager) GetFilesystemState() (models.FileSystem, error) {
 
 // SnapshotList returns a list of snapshots.
 func (m *Manager) SnapshotList() []resources.Snapshot {
-	snapshotList := m.snapshots
-	return snapshotList
+	m.mu.Lock()
+	snapshots := m.snapshots
+	m.mu.Unlock()
+
+	return snapshots
 }
 
 // RefreshSnapshotList updates the list of snapshots.
@@ -559,20 +574,22 @@ func (m *Manager) getSnapshots() ([]resources.Snapshot, error) {
 
 func (m *Manager) addSnapshotToList(snapshot resources.Snapshot) {
 	m.mu.Lock()
-	m.snapshots = append(m.snapshots, snapshot)
+	m.snapshots = append([]resources.Snapshot{snapshot}, m.snapshots...)
 	m.mu.Unlock()
 }
 
 func (m *Manager) removeSnapshotFromList(snapshotName string) {
+	m.mu.Lock()
+
 	for i, snapshot := range m.snapshots {
 		if snapshot.ID == snapshotName {
-			m.mu.Lock()
-			m.snapshots = append(m.snapshots[:i], m.snapshots[i+1:]...)
-			m.mu.Unlock()
+			m.snapshots = append((m.snapshots)[:i], (m.snapshots)[i+1:]...)
 
 			break
 		}
 	}
+
+	m.mu.Unlock()
 }
 
 // ListFilesystems lists ZFS file systems (clones, pools).

engine/internal/provision/thinclones/zfs/zfs_test.go

@@ -199,7 +199,7 @@ func TestBuildingCommands(t *testing.T) {
 
 func TestSnapshotList(t *testing.T) {
 	t.Run("Snapshot list", func(t *testing.T) {
-		fsManager := NewFSManager(runnerMock{}, Config{})
+		fsManager := NewFSManager(runnerMock{}, Config{Pool: &resources.Pool{Name: "testPool"}})
 
 		require.Equal(t, 0, len(fsManager.SnapshotList()))
 
@@ -216,11 +216,11 @@ func TestSnapshotList(t *testing.T) {
 		fsManager.addSnapshotToList(snapshot3)
 
 		require.Equal(t, 3, len(fsManager.SnapshotList()))
-		require.Equal(t, []resources.Snapshot{{ID: "test1"}, {ID: "test2"}, {ID: "test3"}}, fsManager.SnapshotList())
+		require.Equal(t, []resources.Snapshot{{ID: "test3"}, {ID: "test2"}, {ID: "test1"}}, fsManager.SnapshotList())
 
 		fsManager.removeSnapshotFromList("test2")
 
 		require.Equal(t, 2, len(fsManager.SnapshotList()))
-		require.Equal(t, []resources.Snapshot{{ID: "test1"}, {ID: "test3"}}, fsManager.SnapshotList())
+		require.Equal(t, []resources.Snapshot{{ID: "test3"}, {ID: "test1"}}, fsManager.SnapshotList())
 	})
 }

engine/internal/retrieval/engine/postgres/snapshot/physical.go

@@ -703,7 +703,6 @@ func (p *PhysicalInitial) parseWAL(ctx context.Context, containerID string, pgVe
 		Cmd: []string{"sh", "-c", cmd},
 	})
 	if err != nil {
-		log.Dbg("failed to parse WAL: ", err)
 		return ""
 	}