From d449eb9275593d6d741243a3091f0bb4ea2c8501 Mon Sep 17 00:00:00 2001 From: Andrey Butusov Date: Mon, 10 Aug 2026 20:07:00 +0300 Subject: [PATCH 1/2] node: validate and apply configuration on SIGHUP Validate the newly read configuration before applying it. Invalid configuration now aborts reload before services are modified. Stop the node when configuration reload fails, preventing it from running with a runtime state that can differ from the configuration file or environment. Systemd restarts the node according to the service restart policy. Signed-off-by: Andrey Butusov --- CHANGELOG.md | 1 + cmd/neofs-node/config.go | 103 ++++++++++++++++++++------------------- 2 files changed, 55 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91fa9fe6da..5fa52759a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Changelog for NeoFS Node ### Fixed - SN could panic on forwarding GET/HEAD/RANGE request (#4120) +- Storage node shuts down when SIGHUP configuration reload fails (#4113) ### Changed - SNs exchange TLS certificates on inter-node connections (#4097) diff --git a/cmd/neofs-node/config.go b/cmd/neofs-node/config.go index 833bd72568..22bbf80338 100644 --- a/cmd/neofs-node/config.go +++ b/cmd/neofs-node/config.go @@ -624,7 +624,6 @@ func (c *cfg) needBootstrap() bool { } func (c *cfg) configWatcher(ctx context.Context) { - var err error ch := make(chan os.Signal, 1) signal.Notify(ch, syscall.SIGHUP) @@ -637,74 +636,80 @@ func (c *cfg) configWatcher(ctx context.Context) { c.log.Warn("failed to notify systemd about reloading", zap.Error(err)) } - oldMetrics := writeMetricConfig(c.appCfg) - oldProfiler := writeProfilerConfig(c.appCfg) - oldGRPC := writeGRPCConfig(c.appCfg) + if err := c.reloadConfig(); err != nil { + c.internalErr <- fmt.Errorf("configuration reload: %w", err) + return + } - c.appCfg, err = config.New(config.WithConfigFile(c.appCfg.Path())) - if err != nil { - c.log.Error("configuration reading", zap.Error(err)) - continue + c.log.Info("configuration has been reloaded successfully") + + if err := sdnotify.Send(sdnotify.Ready); err != nil { + c.log.Warn("failed to notify systemd about readiness after reload", zap.Error(err)) } + case <-ctx.Done(): + return + } + } +} - // Prometheus and pprof +//nolint:contextcheck // Reloading HTTP services does not receive a request context. +func (c *cfg) reloadConfig() error { + oldCfg := c.appCfg + oldMetrics := writeMetricConfig(oldCfg) + oldProfiler := writeProfilerConfig(oldCfg) + oldGRPC := writeGRPCConfig(oldCfg) - // nolint:contextcheck - c.reloadMetricsAndPprof(oldMetrics, oldProfiler) + newCfg, err := config.New(config.WithConfigFile(oldCfg.Path())) + if err != nil { + return fmt.Errorf("read configuration: %w", err) + } + if err := validateConfig(newCfg); err != nil { + return fmt.Errorf("validate configuration: %w", err) + } + c.appCfg = newCfg - // Logger + // Prometheus and pprof - err = c.logLevel.UnmarshalText([]byte(c.appCfg.Logger.Level)) - if err != nil { - c.log.Error("invalid logger level configuration", zap.Error(err)) - continue - } + c.reloadMetricsAndPprof(oldMetrics, oldProfiler) - // Policer + // Logger - c.policer.Reload(c.policerOpts()...) + if err := c.logLevel.UnmarshalText([]byte(c.appCfg.Logger.Level)); err != nil { + return fmt.Errorf("set logger level: %w", err) + } - // Storage Engine + // Policer - var rcfg engine.ReConfiguration - for _, optsWithID := range c.shardOpts() { - rcfg.AddShard(optsWithID.configID, optsWithID.shOpts) - } + c.policer.Reload(c.policerOpts()...) - err = c.cfgObject.cfgLocalStorage.localStorage.Reload(rcfg) - if err != nil { - c.log.Error("storage engine configuration update", zap.Error(err)) - continue - } + // Storage Engine - // Morph + var rcfg engine.ReConfiguration + for _, optsWithID := range c.shardOpts() { + rcfg.AddShard(optsWithID.configID, optsWithID.shOpts) + } - c.cli.Reload(client.WithEndpoints(c.appCfg.FSChain.Endpoints)) + if err := c.cfgObject.cfgLocalStorage.localStorage.Reload(rcfg); err != nil { + return fmt.Errorf("update storage engine configuration: %w", err) + } - // Node + // Morph - err = c.reloadNodeAttributes() - if err != nil { - c.log.Error("invalid node attributes configuration", zap.Error(err)) - continue - } + c.cli.Reload(client.WithEndpoints(c.appCfg.FSChain.Endpoints)) - // gRPC + // Node - if err = reloadGRPC(c, oldGRPC); err != nil { - c.log.Error("gRPC configuration reload", zap.Error(err)) - continue - } + if err := c.reloadNodeAttributes(); err != nil { + return fmt.Errorf("update node attributes: %w", err) + } - c.log.Info("configuration has been reloaded successfully") + // gRPC - if err := sdnotify.Send(sdnotify.Ready); err != nil { - c.log.Warn("failed to notify systemd about readiness after reload", zap.Error(err)) - } - case <-ctx.Done(): - return - } + if err := reloadGRPC(c, oldGRPC); err != nil { + return fmt.Errorf("reload gRPC configuration: %w", err) } + + return nil } // writeSystemAttributes writes app version as defined at compilation From 771b309d125269556d0d339eb3e2dd27404f438a Mon Sep 17 00:00:00 2001 From: Andrey Butusov Date: Mon, 10 Aug 2026 20:07:00 +0300 Subject: [PATCH 2/2] node: handle invalid node attributes on SIGHUP Handle node attribute parsing errors through the common graceful shutdown path. Signed-off-by: Andrey Butusov --- CHANGELOG.md | 1 + cmd/neofs-node/attributes.go | 14 +++++++++----- cmd/neofs-node/netmap.go | 7 +++++-- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fa52759a6..d8368774dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Changelog for NeoFS Node ### Fixed - SN could panic on forwarding GET/HEAD/RANGE request (#4120) - Storage node shuts down when SIGHUP configuration reload fails (#4113) +- Storage node shuts down gracefully on invalid node attributes during SIGHUP (#4113) ### Changed - SNs exchange TLS certificates on inter-node connections (#4097) diff --git a/cmd/neofs-node/attributes.go b/cmd/neofs-node/attributes.go index 2a3546eeb3..802a67b8f9 100644 --- a/cmd/neofs-node/attributes.go +++ b/cmd/neofs-node/attributes.go @@ -8,24 +8,26 @@ import ( "go.uber.org/zap" ) -func parseAttributes(c *cfg) { +func parseAttributes(c *cfg) error { if c.appCfg.Node.Relay { - return + return nil } - fatalOnErr(attributes.ReadNodeAttributes(&c.cfgNodeInfo.localInfo, c.appCfg.Node.Attributes)) + if err := attributes.ReadNodeAttributes(&c.cfgNodeInfo.localInfo, c.appCfg.Node.Attributes); err != nil { + return err + } // expand UN/LOCODE attribute if any found; keep user's attributes // if any conflicts appear locAttr := c.cfgNodeInfo.localInfo.LOCODE() if locAttr == "" { - return + return nil } record, err := getRecord(locAttr) if err != nil { - fatalOnErr(fmt.Errorf("could not get locode record from DB: %w", err)) + return fmt.Errorf("could not get locode record from DB: %w", err) } countryCode := locAttr[:locodedb.CountryCodeLen] @@ -84,6 +86,8 @@ func parseAttributes(c *cfg) { } else { setIfNotEmpty(n.SetSubdivisionName, record.SubDivName) } + + return nil } func getRecord(lc string) (locodedb.Record, error) { diff --git a/cmd/neofs-node/netmap.go b/cmd/neofs-node/netmap.go index 0a9f5e1cb9..36b98722f0 100644 --- a/cmd/neofs-node/netmap.go +++ b/cmd/neofs-node/netmap.go @@ -141,7 +141,7 @@ func initNetmapService(c *cfg) { network.WriteToNodeInfo(c.localAddr, &c.cfgNodeInfo.localInfo) c.cfgNodeInfo.localInfo.SetPublicKey(c.key.PublicKey().Bytes()) - parseAttributes(c) + fatalOnErr(parseAttributes(c)) c.cfgNodeInfo.localInfo.SetOffline() c.cfgNodeInfo.localInfoLock.Unlock() @@ -478,11 +478,14 @@ func (c *cfg) reloadNodeAttributes() error { c.cfgNodeInfo.localInfo.SetAttributes(nil) err := writeSystemAttributes(c) + if err == nil { + err = parseAttributes(c) + } if err != nil { + c.cfgNodeInfo.localInfo.SetAttributes(oldAttrs) c.cfgNodeInfo.localInfoLock.Unlock() return err } - parseAttributes(c) newAttrs := c.cfgNodeInfo.localInfo.GetAttributes()