fix(manager): keep skipped daemon records in the store

A missing config file does not prove the daemon behind the record is
dead. Keep the record so a later restart can re-adopt the daemon once
the config is restored; only drop the in-memory registration.

Signed-off-by: Iaroslav Geraskin <iaroslav@reflection.ai>

Author: iaroslav-reflection

pkg/manager/manager.go

@@ -314,9 +314,9 @@ func (m *Manager) recoverDaemons(ctx context.Context,
 	// Records whose on-disk artifacts are damaged — e.g. a daemon that was
 	// persisted but whose config file never made it to disk. One damaged
 	// record must not abort the whole walk: returning an error here fails
-	// NewSnapshotter, and since the record is never pruned the snapshotter
-	// crash-loops on every restart until the database is repaired by hand.
-	var quarantined []*daemon.Daemon
+	// NewSnapshotter, crash-looping the snapshotter on every restart until
+	// the database is repaired by hand. The record itself is kept: restoring
+	// the config file lets a later restart re-adopt the daemon.
 	if err := m.store.WalkDaemons(ctx, func(s *daemon.ConfigState) error {
 		if s.FsDriver != m.FsDriver {
 			return nil
@@ -340,8 +340,8 @@ func (m *Manager) recoverDaemons(ctx context.Context,
 		if d.States.FsDriver == config.FsDriverFusedev {
 			cfg, err := daemonconfig.NewDaemonConfig(d.States.FsDriver, d.ConfigFile(""))
 			if err != nil {
-				log.L.Errorf("Quarantining daemon %s: failed to reload configuration %s, %s", d.ID(), d.ConfigFile(""), err)
-				quarantined = append(quarantined, d)
+				log.L.Errorf("Skipping recovery of daemon %s: failed to reload configuration %s, %s", d.ID(), d.ConfigFile(""), err)
+				m.daemonCache.Remove(d)
 				//nolint:nilerr
 				return nil
 			}
@@ -395,17 +395,5 @@ func (m *Manager) recoverDaemons(ctx context.Context,
 		return errors.Wrapf(err, "walk daemons to reconnect")
 	}
 
-	// Prune quarantined records outside the walk since the bucket must not be
-	// mutated during ForEach. A daemon without its configuration cannot be
-	// recovered or managed; dropping the record keeps the next restart from
-	// tripping over it again, and orphan process cleanup is handled by the
-	// regular vestige clearing.
-	for _, d := range quarantined {
-		m.daemonCache.Remove(d)
-		if err := m.store.DeleteDaemon(d.ID()); err != nil {
-			log.L.WithError(err).Warnf("failed to prune quarantined daemon record %s", d.ID())
-		}
-	}
-
 	return nil
 }

pkg/manager/manager_test.go

@@ -18,7 +18,7 @@ import (
 	"github.com/containerd/nydus-snapshotter/pkg/store"
 )
 
-func TestRecoverDaemonsQuarantinesRecordsWithoutConfig(t *testing.T) {
+func TestRecoverDaemonsSkipsRecordsWithoutConfig(t *testing.T) {
 	rootDir := t.TempDir()
 	db, err := store.NewDatabase(rootDir)
 	require.NoError(t, err)
@@ -69,14 +69,22 @@ func TestRecoverDaemonsQuarantinesRecordsWithoutConfig(t *testing.T) {
 	assert.Empty(t, recovering)
 	assert.Empty(t, live)
 
-	// The damaged records are pruned so the next restart does not trip over
-	// them again; the unrelated record is untouched.
+	// The damaged records stay in the store: a missing config file does not
+	// prove the daemon is dead, and keeping the record lets a later restart
+	// re-adopt the daemon once the file is restored.
 	remaining := make(map[string]string)
 	require.NoError(t, m.store.WalkDaemons(context.Background(), func(s *daemon.ConfigState) error {
 		remaining[s.ID] = s.FsDriver
 		return nil
 	}))
-	assert.Equal(t, map[string]string{other.ID(): config.FsDriverFscache}, remaining)
+	expected := map[string]string{other.ID(): config.FsDriverFscache}
+	for _, d := range poisoned {
+		expected[d.ID()] = config.FsDriverFusedev
+	}
+	assert.Equal(t, expected, remaining)
+
+	// Only the in-memory registration is dropped, so no half-initialized
+	// daemon without its configuration lingers in the cache.
 	for _, d := range poisoned {
 		assert.Nil(t, m.daemonCache.GetByDaemonID(d.ID(), nil))
 	}