Fix SIGHUP - #4113
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4113 +/- ##
==========================================
- Coverage 27.19% 27.18% -0.01%
==========================================
Files 677 677
Lines 46206 46210 +4
==========================================
- Hits 12564 12562 -2
- Misses 32442 32447 +5
- Partials 1200 1201 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
roman-khimov
left a comment
There was a problem hiding this comment.
Create an issue for neofs-testcases to test these scenarios.
| return fmt.Errorf("read configuration: %w", err) | ||
| } | ||
| if err := validateConfig(newCfg); err != nil { | ||
| return fmt.Errorf("validate configuration: %w", err) |
There was a problem hiding this comment.
not sure if running with an incorrect config is ok. maybe app should die? the only feedback we have there is log message, otherwise: app is healthy, and systemd thinks we are "Ready"
There was a problem hiding this comment.
Aren't we running with an old config in this case?
There was a problem hiding this comment.
yes, but we have a new config file (envs), and the app is still running with already unknown old config
There was a problem hiding this comment.
Agree. A failed reload can leave the runtime state out of sync with the configuration, so reporting READY=1 is misleading. I think stopping the node on reload failure would be a good idea.
roman-khimov
left a comment
There was a problem hiding this comment.
Create an issue for neofs-testcases to test these scenarios.
8848cd1 to
771b309
Compare
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 <andrey@nspcc.io>
Handle node attribute parsing errors through the common graceful shutdown path. Signed-off-by: Andrey Butusov <andrey@nspcc.io>
|
The gRPC-specific fixes no longer make sense because any SIGHUP reload failure now triggers a graceful node shutdown. |
Noticed some issues when adding a new configuration to gRPC.