Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/build,os/signal: TestDetectNohup and TestNohup fail on replacement darwin LUCI builders #63875

Open
prattmic opened this issue Nov 1, 2023 · 7 comments
Open

x/build,os/signal: TestDetectNohup and TestNohup fail on replacement darwin LUCI builders #63875

prattmic opened this issue Nov 1, 2023 · 7 comments
Assignees
@prattmic
Labels
Builders x/build issues (builders, bots, dashboards) compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Unreleased

Comments

@prattmic
Copy link
Member

prattmic commented Nov 1, 2023

At present, LUCI is using temporary loaner Macs for darwin builders. They are getting replaced with macOS VMs. On the new bots, the os/signal TestDetectNohup and TestNohup fail like so:

=== RUN   TestDetectNohup
    signal_test.go:323: ran test with -check_sighup_ignored under nohup and it failed: expected success.
        Error: exit status 127
        Output:
        nohup: can't detach from console: Inappropriate ioctl for device
--- FAIL: TestDetectNohup (0.02s)

=== RUN   TestNohup/nohup-2
=== RUN   TestNohup/nohup-1
    signal_test.go:503: ran test with -send_uncaught_sighup=1 under nohup and it failed: expected success.
        Error: exit status 127
        Output:
        nohup: can't detach from console: Inappropriate ioctl for device
--- FAIL: TestNohup/nohup-1 (0.01s)
    signal_test.go:503: ran test with -send_uncaught_sighup=2 under nohup and it failed: expected success.
        Error: exit status 127
        Output:
        nohup: can't detach from console: Inappropriate ioctl for device
--- FAIL: TestNohup/nohup-2 (0.01s)
--- FAIL: TestNohup (0.02s)

I ran into this before, but did not really understand what was going on.

I believe that this is related to the way the swarming bot is running under launchd on the new bots. This same failure can be reproduced using a similar service:

/Library/LaunchDaemons/test.plist:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
    <dict>
        <key>Label</key>
        <string>org.golang.build.test</string>
        <key>ProgramArguments</key>
        <array>
            <string>/tmp/test.sh</string>
        </array>
        <key>RunAtLoad</key>
        <true/>
        <key>KeepAlive</key>
        <true/>
        <key>StandardOutPath</key>
        <string>/tmp/test.log</string>
        <key>StandardErrorPath</key>
        <string>/tmp/test.log</string>
    </dict>
</plist>

/tmp/test.sh:

#!/bin/bash
/bin/echo foo
nohup /bin/echo bar

$ sudo launchctl load /Library/LaunchDaemons/test.plist
$ sudo cat /tmp/test.log
foo
nohup: can't detach from console: Inappropriate ioctl for device

I believe this is related to the Execution Context of different types of launchd services. My working theory is that nohup wants to move into the "Background" bootstrap namespace, but this type of service does not have a "Background" bootstrap namespace.

I believe this should work OK using either a "pre-login launchd agent" or "per-user launchd agent" (I believe that the existing loaner Macs use the latter), but I have not had much luck reliably converting to one of those service types so far. Since this is a fairly minor problem, I am simply documenting for now to unblock the rest of this migration.
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Nov 1, 2023
@gopherbot gopherbot added this to the Unreleased milestone Nov 1, 2023
@gopherbot
Copy link

Change https://go.dev/cl/538698 mentions this issue: os/signal: skip nohup tests on darwin builders

@bcmills bcmills changed the title x/build: os/signal: TestDetectNohup and TestNohup fail on replacement darwin LUCI builders x/build,os/signal: TestDetectNohup and TestNohup fail on replacement darwin LUCI builders Nov 1, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 1, 2023
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 2, 2023
gopherbot pushed a commit that referenced this issue Nov 2, 2023
@prattmic
os/signal: skip nohup tests on darwin builders
a334c45 
The new LUCI builders have a temporary limitation that breaks nohup.
Skip nohup tests there.

For #63875.

Cq-Include-Trybots: luci.golang.try:gotip-darwin-arm64_13
Change-Id: Ia9ffecea7310f84a21f6138d8f8cdfc5e1392307
Reviewed-on: https://go-review.googlesource.com/c/go/+/538698
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
@prattmic
Copy link
Member Author

prattmic commented Nov 2, 2023

@gopherbot Please backport to 1.20 and 1.21. This test needs to avoid failures on release branches as well.

This was referenced Nov 2, 2023
Closed
Closed
@gopherbot
Copy link

Backport issue(s) opened: #63910 (for 1.20), #63911 (for 1.21).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@dmitshur dmitshur added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 8, 2023
@gopherbot
Copy link

Change https://go.dev/cl/546376 mentions this issue: [release-branch.go1.20] os/signal: skip nohup tests on darwin builders

@gopherbot
Copy link

Change https://go.dev/cl/546022 mentions this issue: [release-branch.go1.21] os/signal: skip nohup tests on darwin builders

@cagedmantis
Copy link
Contributor

Is this issue waiting on anything to be resolved?

gopherbot pushed a commit that referenced this issue Dec 8, 2023
@prattmic @cagedmantis
[release-branch.go1.21] os/signal: skip nohup tests on darwin builders
bbab863 
The new LUCI builders have a temporary limitation that breaks nohup.
Skip nohup tests there.

For #63875.
Fixes #63911.

Cq-Include-Trybots: luci.golang.try:go1.21-darwin-amd64_13
Change-Id: Ia9ffecea7310f84a21f6138d8f8cdfc5e1392307
Reviewed-on: https://go-review.googlesource.com/c/go/+/538698
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
(cherry picked from commit a334c45)
Reviewed-on: https://go-review.googlesource.com/c/go/+/546022
gopherbot pushed a commit that referenced this issue Dec 8, 2023
@prattmic @cagedmantis
[release-branch.go1.20] os/signal: skip nohup tests on darwin builders
8cb86b5 
The new LUCI builders have a temporary limitation that breaks nohup.
Skip nohup tests there.

For #63875.
Fixes #63910.

Cq-Include-Trybots: luci.golang.try:go1.20-darwin-amd64_13
Change-Id: Ia9ffecea7310f84a21f6138d8f8cdfc5e1392307
Reviewed-on: https://go-review.googlesource.com/c/go/+/538698
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
(cherry picked from commit a334c45)
Reviewed-on: https://go-review.googlesource.com/c/go/+/546376
TryBot-Bypass: Carlos Amedee <carlos@golang.org>
@prattmic
Copy link
Member Author

prattmic commented Dec 8, 2023

We should fix the builders so that this test can actually run and the skips can be reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@prattmic prattmic

Labels
Builders x/build issues (builders, bots, dashboards) compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
Go Compiler / Runtime
Status: Todo
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@cagedmantis @prattmic @dmitshur @gopherbot @cherrymui