Merge pull request #625 from imeoer/revert-config

Revert "feat: Remove DumpFile operations"

Author: imeoer

config/daemonconfig/daemonconfig.go

@@ -9,9 +9,9 @@ package daemonconfig
 
 import (
 	"encoding/json"
+	"os"
 	"reflect"
 	"strings"
-	"sync"
 
 	"github.com/pkg/errors"
 
@@ -36,6 +36,7 @@ type DaemonConfig interface {
 	StorageBackend() (StorageBackendType, *BackendConfig)
 	UpdateMirrors(mirrorsConfigDir, registryHost string) error
 	DumpString() (string, error)
+	DumpFile(path string) error
 }
 
 // Daemon configurations factory
@@ -121,14 +122,19 @@ type DeviceConfig struct {
 	} `json:"cache"`
 }
 
-var configRWMutex sync.RWMutex
+// For nydusd as FUSE daemon. Serialize Daemon info and persist to a json file
+// We don't have to persist configuration file for fscache since its configuration
+// is passed through HTTP API.
+func DumpConfigFile(c interface{}, path string) error {
+	if config.IsBackendSourceEnabled() {
+		c = serializeWithSecretFilter(c)
+	}
+	b, err := json.Marshal(c)
+	if err != nil {
+		return errors.Wrapf(err, "marshal config")
+	}
 
-type SupplementInfoInterface interface {
-	GetImageID() string
-	GetSnapshotID() string
-	IsVPCRegistry() bool
-	GetLabels() map[string]string
-	GetParams() map[string]string
+	return os.WriteFile(path, b, 0600)
 }
 
 func DumpConfigString(c interface{}) (string, error) {
@@ -137,22 +143,20 @@ func DumpConfigString(c interface{}) (string, error) {
 }
 
 // Achieve a daemon configuration from template or snapshotter's configuration
-func SupplementDaemonConfig(c DaemonConfig, info SupplementInfoInterface) error {
-
-	configRWMutex.Lock()
-	defer configRWMutex.Unlock()
+func SupplementDaemonConfig(c DaemonConfig, imageID, snapshotID string,
+	vpcRegistry bool, labels map[string]string, params map[string]string) error {
 
-	image, err := registry.ParseImage(info.GetImageID())
+	image, err := registry.ParseImage(imageID)
 	if err != nil {
-		return errors.Wrapf(err, "parse image %s", info.GetImageID())
+		return errors.Wrapf(err, "parse image %s", imageID)
 	}
 
 	backendType, _ := c.StorageBackend()
 
 	switch backendType {
 	case backendTypeRegistry:
 		registryHost := image.Host
-		if info.IsVPCRegistry() {
+		if vpcRegistry {
 			registryHost = registry.ConvertToVPCHost(registryHost)
 		} else if registryHost == "docker.io" {
 			// For docker.io images, we should use index.docker.io
@@ -166,8 +170,8 @@ func SupplementDaemonConfig(c DaemonConfig, info SupplementInfoInterface) error
 		// If no auth is provided, don't touch auth from provided nydusd configuration file.
 		// We don't validate the original nydusd auth from configuration file since it can be empty
 		// when repository is public.
-		keyChain := auth.GetRegistryKeyChain(registryHost, info.GetImageID(), info.GetLabels())
-		c.Supplement(registryHost, image.Repo, info.GetSnapshotID(), info.GetParams())
+		keyChain := auth.GetRegistryKeyChain(registryHost, imageID, labels)
+		c.Supplement(registryHost, image.Repo, snapshotID, params)
 		c.FillAuth(keyChain)
 
 	// Localfs and OSS backends don't need any update,

config/daemonconfig/fscache.go

@@ -9,6 +9,7 @@ package daemonconfig
 import (
 	"encoding/json"
 	"os"
+	"path"
 
 	"github.com/containerd/log"
 	"github.com/containerd/nydus-snapshotter/pkg/auth"
@@ -120,3 +121,11 @@ func (c *FscacheDaemonConfig) FillAuth(kc *auth.PassKeyChain) {
 func (c *FscacheDaemonConfig) DumpString() (string, error) {
 	return DumpConfigString(c)
 }
+
+func (c *FscacheDaemonConfig) DumpFile(f string) error {
+	if err := os.MkdirAll(path.Dir(f), 0755); err != nil {
+		return err
+	}
+
+	return DumpConfigFile(c, f)
+}

config/daemonconfig/fuse.go

@@ -9,6 +9,7 @@ package daemonconfig
 import (
 	"encoding/json"
 	"os"
+	"path"
 
 	"github.com/pkg/errors"
 
@@ -91,7 +92,12 @@ func (c *FuseDaemonConfig) StorageBackend() (string, *BackendConfig) {
 }
 
 func (c *FuseDaemonConfig) DumpString() (string, error) {
-	configRWMutex.Lock()
-	defer configRWMutex.Unlock()
 	return DumpConfigString(c)
 }
+
+func (c *FuseDaemonConfig) DumpFile(f string) error {
+	if err := os.MkdirAll(path.Dir(f), 0755); err != nil {
+		return err
+	}
+	return DumpConfigFile(c, f)
+}

docs/optimize_nydus_image.md

@@ -108,7 +108,7 @@ Nydus provides a [nydusify](https://github.com/dragonflyoss/nydus/blob/master/do
 We can install the `nydusify` cli tool from the nydus package.
 
 ```console
-VERSION=v2.1.5
+VERSION=v2.3.0
 
 wget https://github.com/dragonflyoss/nydus/releases/download/$VERSION/nydus-static-$VERSION-linux-amd64.tgz
 tar -zxvf nydus-static-$VERSION-linux-amd64.tgz

integration/Dockerfile

@@ -3,7 +3,7 @@ ARG CONTAINERD_PROJECT=/containerd
 ARG RUNC_VER=1.1.4
 ARG NYDUS_SNAPSHOTTER_PROJECT=/nydus-snapshotter
 ARG DOWNLOADS_MIRROR="https://github.com"
-ARG NYDUS_VER=2.2.5
+ARG NYDUS_VER=2.3.0
 ARG NERDCTL_VER=1.7.6
 ARG DELVE_VER=1.23.0
 ARG GO_VER=1.22.5-bookworm

pkg/daemon/daemon.go

@@ -87,21 +87,6 @@ type Daemon struct {
 	state types.DaemonState
 }
 
-type NydusdSupplementInfo struct {
-	DaemonState ConfigState
-	ImageID     string
-	SnapshotID  string
-	Vpc         bool
-	Labels      map[string]string
-	Params      map[string]string
-}
-
-func (s *NydusdSupplementInfo) GetImageID() string           { return s.ImageID }
-func (s *NydusdSupplementInfo) GetSnapshotID() string        { return s.SnapshotID }
-func (s *NydusdSupplementInfo) IsVPCRegistry() bool          { return s.Vpc }
-func (s *NydusdSupplementInfo) GetLabels() map[string]string { return s.Labels }
-func (s *NydusdSupplementInfo) GetParams() map[string]string { return s.Params }
-
 func (d *Daemon) Lock() {
 	d.mu.Lock()
 }
@@ -265,7 +250,12 @@ func (d *Daemon) sharedFusedevMount(rafs *rafs.Rafs) error {
 		return err
 	}
 
-	c := d.Config
+	c, err := daemonconfig.NewDaemonConfig(d.States.FsDriver, d.ConfigFile(rafs.SnapshotID))
+	if err != nil {
+		return errors.Wrapf(err, "Failed to reload instance configuration %s",
+			d.ConfigFile(rafs.SnapshotID))
+	}
+
 	cfg, err := c.DumpString()
 	if err != nil {
 		return errors.Wrap(err, "dump instance configuration")
@@ -290,7 +280,12 @@ func (d *Daemon) sharedErofsMount(ra *rafs.Rafs) error {
 		return errors.Wrapf(err, "failed to create fscache work dir %s", ra.FscacheWorkDir())
 	}
 
-	c := d.Config
+	c, err := daemonconfig.NewDaemonConfig(d.States.FsDriver, d.ConfigFile(ra.SnapshotID))
+	if err != nil {
+		log.L.Errorf("Failed to reload daemon configuration %s, %s", d.ConfigFile(ra.SnapshotID), err)
+		return err
+	}
+
 	cfgStr, err := c.DumpString()
 	if err != nil {
 		return err
@@ -655,29 +650,3 @@ func NewDaemon(opt ...NewDaemonOpt) (*Daemon, error) {
 
 	return d, nil
 }
-
-func (d *Daemon) MountByAPI() error {
-	rafs := d.RafsCache.Head()
-	if rafs == nil {
-		return errors.Wrapf(errdefs.ErrNotFound, "daemon %s no rafs instance associated", d.ID())
-	}
-	client, err := d.GetClient()
-	if err != nil {
-		return errors.Wrapf(err, "mount instance %s", rafs.SnapshotID)
-	}
-	bootstrap, err := rafs.BootstrapFile()
-	if err != nil {
-		return err
-	}
-	c := d.Config
-	cfg, err := c.DumpString()
-	if err != nil {
-		return errors.Wrap(err, "dump instance configuration")
-	}
-	err = client.Mount("/", bootstrap, cfg)
-	if err != nil {
-		return errors.Wrapf(err, "mount rafs instance MountByAPI()")
-	}
-	return nil
-
-}

pkg/filesystem/fs.go

@@ -130,19 +130,6 @@ func NewFileSystem(ctx context.Context, opt ...NewFSOpt) (*Filesystem, error) {
 			if err != nil {
 				return errors.Wrapf(err, "get filesystem manager for daemon %s", d.States.ID)
 			}
-
-			supplementInfo, err := fsManager.GetInfo(d.ID())
-			if err != nil {
-				return errors.Wrap(err, "GetInfo failed")
-			}
-
-			cfg := d.Config
-			err = daemonconfig.SupplementDaemonConfig(cfg, supplementInfo)
-			if err != nil {
-				return errors.Wrap(err, "supplement configuration")
-			}
-			d.Config = cfg
-
 			if err := fsManager.StartDaemon(d); err != nil {
 				return errors.Wrapf(err, "start daemon %s", d.ID())
 			}
@@ -245,6 +232,7 @@ func (fs *Filesystem) Mount(ctx context.Context, snapshotID string, labels map[s
 		// Instance already exists, how could this happen? Can containerd handle this case?
 		return nil
 	}
+
 	fsDriver := config.GetFsDriver()
 	if label.IsTarfsDataLayer(labels) {
 		fsDriver = config.FsDriverBlockdev
@@ -314,25 +302,34 @@ func (fs *Filesystem) Mount(ctx context.Context, snapshotID string, labels map[s
 			daemonconfig.WorkDir:   workDir,
 			daemonconfig.CacheDir:  cacheDir,
 		}
-		supplementInfo := &daemon.NydusdSupplementInfo{
-			DaemonState: d.States,
-			ImageID:     imageID,
-			SnapshotID:  snapshotID,
-			Vpc:         false,
-			Labels:      labels,
-			Params:      params,
-		}
 		cfg := deepcopy.Copy(*fsManager.DaemonConfig).(daemonconfig.DaemonConfig)
-		err = daemonconfig.SupplementDaemonConfig(cfg, supplementInfo)
+		err = daemonconfig.SupplementDaemonConfig(cfg, imageID, snapshotID, false, labels, params)
 		if err != nil {
 			return errors.Wrap(err, "supplement configuration")
 		}
-		if errs := fsManager.AddSupplementInfo(supplementInfo); errs != nil {
-			return errors.Wrapf(err, "AddSupplementInfo failed %s", d.States.ID)
-		}
+
 		// TODO: How to manage rafs configurations on-disk? separated json config file or DB record?
 		// In order to recover erofs mount, the configuration file has to be persisted.
-		d.Config = cfg
+		var configSubDir string
+		if useSharedDaemon {
+			configSubDir = snapshotID
+		} else {
+			// Associate daemon config object when creating a new daemon object to avoid
+			// reading disk file again and again.
+			// For shared daemon, each rafs instance has its own configuration, so we don't
+			// attach a config interface to daemon in this case.
+			d.Config = cfg
+		}
+
+		err = cfg.DumpFile(d.ConfigFile(configSubDir))
+		if err != nil {
+			if errors.Is(err, errdefs.ErrAlreadyExists) {
+				log.L.Debugf("Configuration file %s already exits", d.ConfigFile(configSubDir))
+			} else {
+				return errors.Wrap(err, "dump daemon configuration file")
+			}
+		}
+
 		d.AddRafsInstance(rafs)
 
 		// if publicKey is not empty we should verify bootstrap file of image
@@ -599,6 +596,10 @@ func (fs *Filesystem) initSharedDaemon(fsManager *manager.Manager) (err error) {
 	// it is loaded when requesting mount api
 	// Dump the configuration file since it is reloaded when recovering the nydusd
 	d.Config = *fsManager.DaemonConfig
+	err = d.Config.DumpFile(d.ConfigFile(""))
+	if err != nil && !errors.Is(err, errdefs.ErrAlreadyExists) {
+		return errors.Wrapf(err, "dump configuration file %s", d.ConfigFile(""))
+	}
 
 	if err := fsManager.StartDaemon(d); err != nil {
 		return errors.Wrap(err, "start shared daemon")

pkg/manager/daemon_adaptor.go

@@ -44,16 +44,6 @@ func (m *Manager) StartDaemon(d *daemon.Daemon) error {
 	if err := cmd.Start(); err != nil {
 		return err
 	}
-	fsDriver := config.GetFsDriver()
-	isSharedFusedev := fsDriver == config.FsDriverFusedev && config.GetDaemonMode() == config.DaemonModeShared
-	useSharedDaemon := fsDriver == config.FsDriverFscache || isSharedFusedev
-
-	if !useSharedDaemon {
-		errs := d.MountByAPI()
-		if errs != nil {
-			return errors.Wrapf(err, "failed to mount")
-		}
-	}
 
 	d.Lock()
 	defer d.Unlock()
@@ -165,6 +155,10 @@ func (m *Manager) BuildDaemonCommand(d *daemon.Daemon, bin string, upgrade bool)
 				return nil, errors.Wrapf(err, "locate bootstrap %s", bootstrap)
 			}
 
+			cmdOpts = append(cmdOpts,
+				command.WithConfig(d.ConfigFile("")),
+				command.WithBootstrap(bootstrap),
+			)
 			if config.IsBackendSourceEnabled() {
 				configAPIPath := fmt.Sprintf(endpointGetBackend, d.States.ID)
 				cmdOpts = append(cmdOpts,

pkg/manager/manager.go

@@ -197,23 +197,6 @@ func (m *Manager) AddDaemon(daemon *daemon.Daemon) error {
 	return nil
 }
 
-func (m *Manager) AddSupplementInfo(supplementInfo *daemon.NydusdSupplementInfo) error {
-	m.mu.Lock()
-	defer m.mu.Unlock()
-	if err := m.store.AddInfo(supplementInfo); err != nil {
-		return errors.Wrapf(err, "add supplementInfo %s", supplementInfo.DaemonState.ID)
-	}
-	return nil
-}
-
-func (m *Manager) GetInfo(daemonID string) (*daemon.NydusdSupplementInfo, error) {
-	info, err := m.store.GetInfo(daemonID)
-	if err != nil {
-		return nil, errors.Wrapf(err, "add supplementInfo %s", daemonID)
-	}
-	return info, nil
-}
-
 func (m *Manager) UpdateDaemon(daemon *daemon.Daemon) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
@@ -339,7 +322,13 @@ func (m *Manager) recoverDaemons(ctx context.Context,
 		}
 
 		if d.States.FsDriver == config.FsDriverFusedev {
-			d.Config = *m.DaemonConfig
+			cfg, err := daemonconfig.NewDaemonConfig(d.States.FsDriver, d.ConfigFile(""))
+			if err != nil {
+				log.L.Errorf("Failed to reload daemon configuration %s, %s", d.ConfigFile(""), err)
+				return err
+			}
+
+			d.Config = cfg
 		}
 
 		state, err := d.GetState()

pkg/manager/store.go

@@ -29,9 +29,6 @@ type Store interface {
 	WalkRafsInstances(ctx context.Context, cb func(*rafs.Rafs) error) error
 
 	NextInstanceSeq() (uint64, error)
-
-	AddInfo(supplementInfo *daemon.NydusdSupplementInfo) error
-	GetInfo(daemonID string) (*daemon.NydusdSupplementInfo, error)
 }
 
 var _ Store = &store.DaemonRafsStore{}