Skip to content

Fix SIGHUP - #4113

Open
End-rey wants to merge 3 commits into
masterfrom
fix-sighup
Open

Fix SIGHUP#4113
End-rey wants to merge 3 commits into
masterfrom
fix-sighup

Conversation

@End-rey

@End-rey End-rey commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Noticed some issues when adding a new configuration to gRPC.

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 106 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.23%. Comparing base (68883b0) to head (290f5ff).

Files with missing lines Patch % Lines
cmd/neofs-node/grpc.go 0.00% 60 Missing ⚠️
cmd/neofs-node/config.go 0.00% 35 Missing ⚠️
cmd/neofs-node/attributes.go 0.00% 7 Missing ⚠️
cmd/neofs-node/netmap.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4113      +/-   ##
==========================================
- Coverage   27.26%   27.23%   -0.03%     
==========================================
  Files         678      678              
  Lines       46301    46347      +46     
==========================================
  Hits        12622    12622              
- Misses      32461    32507      +46     
  Partials     1218     1218              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roman-khimov roman-khimov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create an issue for neofs-testcases to test these scenarios.

Comment thread cmd/neofs-node/config.go
return fmt.Errorf("read configuration: %w", err)
}
if err := validateConfig(newCfg); err != nil {
return fmt.Errorf("validate configuration: %w", err)

@carpawell carpawell Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't we running with an old config in this case?

@carpawell carpawell Aug 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but we have a new config file (envs), and the app is still running with already unknown old config

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am fully ok with stopping

@carpawell carpawell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conflicts

End-rey added 3 commits August 7, 2026 21:52
Build replacement gRPC servers and bind newly added endpoints before stopping
existing servers. This keeps the current Public API available when the new TLS
configuration is invalid or a new endpoint is occupied. Track the configuration
snapshot associated with running servers. Update it only after a successful
reload, so a failed rebind is retried on the next SIGHUP instead of reusing a
stopped server.

Signed-off-by: Andrey Butusov <andrey@nspcc.io>
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>
Return attribute parsing errors to the configuration reloader instead of
terminating the process. Restore the previous node attributes when the updated
attributes cannot be parsed.

Signed-off-by: Andrey Butusov <andrey@nspcc.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants