Skip to content

windows: use/accept platform-specific signal for stopping agent - #26780

Merged
tgross merged 1 commit into
mainfrom
gh26775-api-tests-windows
Sep 17, 2025
Merged

tgross merged 1 commit into
mainfrom
gh26775-api-tests-windows

Conversation

@tgross

@tgross tgross commented Sep 16, 2025 •

Copy link
Copy Markdown
Member

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: #26775

Contributor Checklist

  • Changelog Entry If this PR changes user-facing behavior, please generate and add a
    changelog entry using the make cl command.
    • no changelog, as this is a test-only fix
  • Testing Please add tests to cover any new functionality or to demonstrate bug fixes and
    ensure regressions will be caught.
  • Documentation If the change impacts user-facing functionality such as the CLI, API, UI,
    and job configuration, please update the Nomad website documentation to reflect this. Refer to
    the website README for docs guidelines. Please also consider whether the
    change requires notes within the upgrade guide.

Reviewer Checklist

  • Backport Labels Please add the correct backport labels as described by the internal
    backporting document.
  • Commit Type Ensure the correct merge method is selected which should be "squash and merge"
    in the majority of situations. The main exceptions are long-lived feature branches or merges where
    history should be preserved.
  • Enterprise PRs If this is an enterprise only PR, please add any required changelog entry
    within the public repository.
  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

@tgross

tgross commented Sep 16, 2025 •

Copy link
Copy Markdown
Member Author

I've been doing some testing and discovered that on Windows the signal handler is also ignoring os.Interrupt, so I've had to switch it out for syscall.SIGINT just to get it to catch the signal at all. But the tests still reporting failure and I can't yet figure out why:

==> Starting Nomad agent...
==> Nomad agent configuration:

       Advertise Addrs: HTTP: 172.31.25.30:50317; RPC: 172.31.25.30:50318; Serf: 172.31.25.30:50319
            Bind Addrs: HTTP: [0.0.0.0:50317]; RPC: 0.0.0.0:50318; Serf: 0.0.0.0:50319
                Client: false
             Log Level: ERROR
               Node Id: b9dfa3a1-ddc3-743e-ff7c-a02c42d2c79b
                Region: global (DC: dc1)
                Server: true
               Version: 1.10.6-dev

==> Nomad agent started! Log data will stream in below:

2025-09-16T20:04:00.543Z [ERROR] failed to send event to windows eventlog, timed out: event="&{2  1}"
=== RUN   TestVariables_SimpleCRUD/1_fail_create_when_no_items
=== RUN   TestVariables_SimpleCRUD/2_create_sv1
=== RUN   TestVariables_SimpleCRUD/2_create_sv2
=== RUN   TestVariables_SimpleCRUD/4_update_sv1
=== RUN   TestVariables_SimpleCRUD/5_list_vars
=== RUN   TestVariables_SimpleCRUD/5a_list_vars_opts
=== RUN   TestVariables_SimpleCRUD/5b_prefixlist
=== RUN   TestVariables_SimpleCRUD/6_delete_sv1
=== RUN   TestVariables_SimpleCRUD/7_list_vars_after_delete
==> Caught signal: interrupt
2025-09-16T20:04:02.709Z [ERROR] failed to send event to windows eventlog, timed out: event="&{3  1}"
FAIL    github.com/hashicorp/nomad/api  3.102s

If we call s.cmd.Process.Kill() instead of sending the SIGINT, the tests do get marked as passing, so there's definitely something specific to this problem going on here.

@tgross
tgross force-pushed the gh26775-api-tests-windows branch from b17abc2 to 3954a16 Compare September 16, 2025 20:51
@tgross
tgross force-pushed the gh26775-api-tests-windows branch from 3954a16 to 7223f9a Compare September 17, 2025 14:05
@tgross
tgross force-pushed the gh26775-api-tests-windows branch from 7223f9a to ccb00c1 Compare September 17, 2025 14:07
@tgross tgross added type/bug and removed theme/testing Test related issues labels Sep 17, 2025
@tgross tgross changed the title testing: use platform-specific signal for stopping test server use/accept platform-specific signal for stopping agent Sep 17, 2025
@tgross

tgross commented Sep 17, 2025

Copy link
Copy Markdown
Member Author

@chrisroberts discovered that we needed to call SetConsoleCtrlHandler as well, so this is now working as expected.

@tgross tgross changed the title use/accept platform-specific signal for stopping agent windows: use/accept platform-specific signal for stopping agent Sep 17, 2025
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: #26775

Co-authored-by: Chris Roberts <croberts@hashicorp.com>

@jrasell jrasell 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.

LGTM and although the Windows stuff looks a little magical to me, it seems to track given what I've read in this PR, the issue, and on the Microsoft learn website about the functions.

@tgross
tgross merged commit 4e75e99 into main Sep 17, 2025
38 checks passed
@tgross
tgross deleted the gh26775-api-tests-windows branch September 17, 2025 15:32
@github-actions

Copy link
Copy Markdown

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jan 16, 2026

This branch was successfully deployed

1 active deployment
Preview – nomad-ui — 55c12e67 Deployed Sep 17, 2025 by vercel[bot]
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API tests cannot be run on Windows

2 participants