Skip to content

Shut down on SIGTERM - #590

Merged
gab-arrobo merged 1 commit into
omec-project:mainfrom
martin-mat:sigterm-shutdown
Oct 2, 2026
Merged

gab-arrobo merged 1 commit into
omec-project:mainfrom
martin-mat:sigterm-shutdown

Conversation

@martin-mat

Copy link
Copy Markdown
Contributor

Fixes #589.

runWebUIAndNFConfig created its context with context.WithCancel(context.Background()), and nothing cancelled it on a signal. So the NFConfig server's graceful shutdown (srv.Shutdown with a 5 s timeout) never ran on SIGTERM: run as PID 1 the process was ended by the signal at once, and under a shell it was killed after the grace period.

  • server.go: the context comes from signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM). When NFConfig returns, for whatever reason, the WebUI is stopped too, and the function waits for it before returning, so the process exits 0.
  • backend/webui_service/webui_init.go: the WebUI server is shut down on cancellation, with the same 5 s timeout as the NFConfig server. http.ErrServerClosed from its ListenAndServe is no longer passed to Fatalln, which would otherwise end the shutdown with exit 1.
  • server_test.go: a test sends the process SIGTERM and checks that both parts stop and runWebUIAndNFConfig returns nil.

Checked with go test ./... (the new test three times in a row) and golangci-lint with the repository's configuration. On kind, with the image built from this branch running in the SD-Core chart as PID 1 (exec, as omec-project/sdcore-helm-charts#192 does for other functions): simapp's provisioning against it succeeds, and on a scale-down the pod exits with code 0 after 0.9 s, logging NFConfig context cancelled, shutting down server., WebUI shutting down due to context cancel and WebUI and NFConfig stopped. Before, it exited with 143 at once and logged nothing.

The metrics server on :8080 still uses http.ListenAndServe without a handle and is simply ended with the process.

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The WebUI shutdown path has a startup race that can bypass Shutdown.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds graceful SIGTERM and interrupt handling so WebUI and NFConfig shut down cleanly.

Changes:

  • Cancels both services on termination signals and waits for WebUI shutdown.
  • Gracefully shuts down the WebUI HTTP server.
  • Adds SIGTERM lifecycle coverage.
File Description
server.go Coordinates signal cancellation and service shutdown.
server_test.go Tests SIGTERM handling.
backend/​webui_service/​webui_init.go Adds WebUI server shutdown logic.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread backend/webui_service/webui_init.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

Signal handling, coordinated shutdown, error handling, and regression coverage are consistent and complete.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@gab-arrobo

gab-arrobo commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

@martin-mat, please see this comment: omec-project/simapp#300 (comment)

Nothing connected SIGTERM to the context that stops the servers, so the
NFConfig server's graceful shutdown never ran: run as PID 1, the
process was ended by the signal at once (and under a shell it was
killed after the grace period).

The context now comes from signal.NotifyContext, so SIGTERM and Ctrl-C
stop both servers. The WebUI server is shut down as well, with the same
5 s timeout as the NFConfig server; http.ErrServerClosed from its
ListenAndServe is no longer fatal, and runWebUIAndNFConfig waits for
the WebUI to stop before returning, so the process exits 0. The WebUI
waits for its server's configuration to end before shutting it down,
so a signal during configuration cannot leave a server listening.

Fixes omec-project#589

Signed-off-by: Martin Matyas <martin.matyas@tieto.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The unrelated version promotion would trigger the release pipeline and publish 3.1.4.

Review effort: Balanced
Findings: 1 High severity

Open (1)

Comment thread VERSION
@gab-arrobo
gab-arrobo enabled auto-merge (squash) October 2, 2026 13:27
@gab-arrobo
gab-arrobo merged commit e2927c1 into omec-project:main Oct 2, 2026
13 checks passed
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.

SIGTERM does not trigger the webui's shutdown

3 participants