From 4e75e99f1aabd7ba65e628f94fe84477723e599e Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 17 Sep 2025 11:32:20 -0400 Subject: [PATCH] windows: use/accept platform-specific signal for stopping agent (#26780) On Windows, the `os.Process.Signal` method returns an error when sending `os.Interrupt` (SIGINT) because it isn't implemented. This causes test servers in the `testutil` packages to break on Windows. Use the platform specific syscalls to generate the SIGINT instead. The agent's signal handler also did not correctly handle the Ctrl-C because we were masking os.Interrupt instead of SIGINT. Fixes: https://github.com/hashicorp/nomad/issues/26775 Co-authored-by: Chris Roberts --- .changelog/26780.txt | 3 ++ api/internal/testutil/server.go | 6 ++-- api/internal/testutil/server_default.go | 17 ++++++++++++ api/internal/testutil/server_windows.go | 37 +++++++++++++++++++++++++ command/agent/command.go | 4 +-- testutil/server.go | 19 +++++++------ testutil/server_default.go | 17 ++++++++++++ testutil/server_windows.go | 37 +++++++++++++++++++++++++ 8 files changed, 127 insertions(+), 13 deletions(-) create mode 100644 .changelog/26780.txt create mode 100644 api/internal/testutil/server_default.go create mode 100644 api/internal/testutil/server_windows.go create mode 100644 testutil/server_default.go create mode 100644 testutil/server_windows.go diff --git a/.changelog/26780.txt b/.changelog/26780.txt new file mode 100644 index 000000000..a02955533 --- /dev/null +++ b/.changelog/26780.txt @@ -0,0 +1,3 @@ +```release-note:bug +windows: Fixed a bug where agents would not gracefully shut down on Ctrl-C +``` diff --git a/api/internal/testutil/server.go b/api/internal/testutil/server.go index f31deac01..59799326d 100644 --- a/api/internal/testutil/server.go +++ b/api/internal/testutil/server.go @@ -240,7 +240,9 @@ func NewTestServer(t testing.TB, cb ServerConfigCallback) *TestServer { // Stop stops the test Nomad server, and removes the Nomad data // directory once we are done. func (s *TestServer) Stop() { - defer func() { _ = os.RemoveAll(s.Config.DataDir) }() + s.t.Cleanup(func() { + _ = os.RemoveAll(s.Config.DataDir) + }) // wait for the process to exit to be sure that the data dir can be // deleted on all platforms. @@ -251,7 +253,7 @@ func (s *TestServer) Stop() { }() // kill and wait gracefully - err := s.cmd.Process.Signal(os.Interrupt) + err := s.gracefulStop() must.NoError(s.t, err) select { diff --git a/api/internal/testutil/server_default.go b/api/internal/testutil/server_default.go new file mode 100644 index 000000000..44b01145e --- /dev/null +++ b/api/internal/testutil/server_default.go @@ -0,0 +1,17 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +//go:build !windows + +package testutil + +import ( + "os" +) + +// gracefulStop performs a platform-specific graceful stop. On non-Windows this +// uses the Go API for SIGINT +func (s *TestServer) gracefulStop() error { + err := s.cmd.Process.Signal(os.Interrupt) + return err +} diff --git a/api/internal/testutil/server_windows.go b/api/internal/testutil/server_windows.go new file mode 100644 index 000000000..8cfb2e354 --- /dev/null +++ b/api/internal/testutil/server_windows.go @@ -0,0 +1,37 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +//go:build windows + +package testutil + +import ( + "fmt" + "syscall" +) + +var ( + kernel32 = syscall.NewLazyDLL("kernel32.dll") + procSetCtrlHandler = kernel32.NewProc("SetConsoleCtrlHandler") + procGenCtrlEvent = kernel32.NewProc("GenerateConsoleCtrlEvent") +) + +// gracefulStop performs a platform-specific graceful stop. On Windows the Go +// API does not implement SIGINT even though it's supported on Windows via +// CTRL_C_EVENT +func (s *TestServer) gracefulStop() error { + // note: err is always non-nil from these proc Call methods because it's + // always populated from GetLastError and you need to check the result + // returned against the docs. + pid := s.cmd.Process.Pid + result, _, err := procSetCtrlHandler.Call(0, 1) + if result == 0 { + return fmt.Errorf("failed to modify handlers for ctrl-c on pid %d: %w", pid, err) + } + + result, _, err = procGenCtrlEvent.Call(syscall.CTRL_C_EVENT, uintptr(pid)) + if result == 0 { + return fmt.Errorf("failed to send ctrl-C event to pid %d: %w", pid, err) + } + return nil +} diff --git a/command/agent/command.go b/command/agent/command.go index f466751ee..71a06860f 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -1081,7 +1081,7 @@ func (c *Command) handleSignals() int { signalCh := make(chan os.Signal, 4) defer signal.Stop(signalCh) - signal.Notify(signalCh, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGPIPE) + signal.Notify(signalCh, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGPIPE) // Signal readiness only once signal handlers are setup sdSock, err := openNotify() @@ -1120,7 +1120,7 @@ func (c *Command) handleSignals() int { } return c.terminateGracefully(signalCh, sdSock) - case os.Interrupt: + case syscall.SIGINT: if !c.agent.GetConfig().LeaveOnInt { return 1 } diff --git a/testutil/server.go b/testutil/server.go index a800c3c2d..9fc784820 100644 --- a/testutil/server.go +++ b/testutil/server.go @@ -29,6 +29,7 @@ import ( "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/helper/discover" "github.com/hashicorp/nomad/helper/pointer" + "github.com/shoenig/test/must" ) // TestServerConfig is the main server configuration struct. @@ -268,21 +269,21 @@ func NewTestServer(t testing.TB, cb ServerConfigCallback) *TestServer { // Stop stops the test Nomad server, and removes the Nomad data // directory once we are done. func (s *TestServer) Stop() { - defer os.RemoveAll(s.Config.DataDir) + s.t.Cleanup(func() { + _ = os.RemoveAll(s.Config.DataDir) + }) // wait for the process to exit to be sure that the data dir can be // deleted on all platforms. done := make(chan struct{}) go func() { defer close(done) - - s.cmd.Wait() + _ = s.cmd.Wait() }() // kill and wait gracefully - if err := s.cmd.Process.Signal(os.Interrupt); err != nil { - s.t.Errorf("err: %s", err) - } + err := s.gracefulStop() + must.NoError(s.t, err) select { case <-done: @@ -291,9 +292,9 @@ func (s *TestServer) Stop() { s.t.Logf("timed out waiting for process to gracefully terminate") } - if err := s.cmd.Process.Kill(); err != nil { - s.t.Errorf("err: %s", err) - } + err = s.cmd.Process.Kill() + must.NoError(s.t, err, must.Sprint("failed to kill process")) + select { case <-done: case <-time.After(5 * time.Second): diff --git a/testutil/server_default.go b/testutil/server_default.go new file mode 100644 index 000000000..2242b1edd --- /dev/null +++ b/testutil/server_default.go @@ -0,0 +1,17 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +//go:build !windows + +package testutil + +import ( + "os" +) + +// gracefulStop performs a platform-specific graceful stop. On non-Windows this +// uses the Go API for SIGINT +func (s *TestServer) gracefulStop() error { + err := s.cmd.Process.Signal(os.Interrupt) + return err +} diff --git a/testutil/server_windows.go b/testutil/server_windows.go new file mode 100644 index 000000000..925ba7dd3 --- /dev/null +++ b/testutil/server_windows.go @@ -0,0 +1,37 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +//go:build windows + +package testutil + +import ( + "fmt" + "syscall" +) + +var ( + kernel32 = syscall.NewLazyDLL("kernel32.dll") + procSetCtrlHandler = kernel32.NewProc("SetConsoleCtrlHandler") + procGenCtrlEvent = kernel32.NewProc("GenerateConsoleCtrlEvent") +) + +// gracefulStop performs a platform-specific graceful stop. On Windows the Go +// API does not implement SIGINT even though it's supported on Windows via +// CTRL_C_EVENT +func (s *TestServer) gracefulStop() error { + // note: err is always non-nil from these proc Call methods because it's + // always populated from GetLastError and you need to check the result + // returned against the docs. + pid := s.cmd.Process.Pid + result, _, err := procSetCtrlHandler.Call(0, 1) + if result == 0 { + return fmt.Errorf("failed to modify handlers for ctrl-c on pid %d: %w", pid, err) + } + + result, _, err = procGenCtrlEvent.Call(syscall.CTRL_C_EVENT, uintptr(pid)) + if result == 0 { + return fmt.Errorf("failed to send ctrl-C event to pid %d: %w", pid, err) + } + return nil +}