diff --git a/src/libraries/go/lib/pkg/nvkit/servers/BUILD.bazel b/src/libraries/go/lib/pkg/nvkit/servers/BUILD.bazel index 97121f458..8efddc298 100644 --- a/src/libraries/go/lib/pkg/nvkit/servers/BUILD.bazel +++ b/src/libraries/go/lib/pkg/nvkit/servers/BUILD.bazel @@ -20,6 +20,7 @@ go_library( srcs = [ "grpc.go", "server.go", + "shutdown.go", ], importpath = "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/nvkit/servers", visibility = ["//visibility:public"], @@ -28,6 +29,7 @@ go_library( "//src/libraries/go/lib/pkg/nvkit/clients", "//src/libraries/go/lib/pkg/nvkit/errors", "//src/libraries/go/lib/pkg/nvkit/servers/utils", + "//src/libraries/go/lib/pkg/nvkit/shutdown", "//src/libraries/go/lib/pkg/nvkit/tracing", "@com_github_go_kit_kit//log", "@com_github_go_kit_kit//transport/grpc", @@ -60,12 +62,14 @@ go_test( "grpc_shutdown_test.go", "grpc_test.go", "server_test.go", + "shutdown_test.go", ], data = ["//src/libraries/go/lib/pkg/nvkit/test/certs"], embed = [":servers"], deps = [ "//src/libraries/go/lib/pkg/nvkit/auth", "//src/libraries/go/lib/pkg/nvkit/errors", + "//src/libraries/go/lib/pkg/nvkit/shutdown", "//src/libraries/go/lib/pkg/nvkit/tracing", "@com_github_go_kit_kit//log", "@com_github_grpc_ecosystem_grpc_gateway_v2//runtime", diff --git a/src/libraries/go/lib/pkg/nvkit/servers/grpc.go b/src/libraries/go/lib/pkg/nvkit/servers/grpc.go index 9bb02ec08..fd483336f 100644 --- a/src/libraries/go/lib/pkg/nvkit/servers/grpc.go +++ b/src/libraries/go/lib/pkg/nvkit/servers/grpc.go @@ -28,7 +28,6 @@ import ( "os/signal" "strings" "sync" - "syscall" "time" // By default, it sets `GOMEMLIMIT` to 90% of cgroup's memory limit. @@ -58,6 +57,7 @@ import ( "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/nvkit/clients" "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/nvkit/errors" "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/nvkit/servers/utils" + "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/nvkit/shutdown" "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/nvkit/tracing" ) @@ -536,17 +536,12 @@ func (g *grpcServer) Run() error { }) } { - // This function just sits and waits for ctrl-C. + // This function just sits and waits for a shutdown signal. cancelInterrupt := make(chan struct{}) svrGroup.Add(func() error { c := make(chan os.Signal, 1) - signal.Notify(c, syscall.SIGINT, syscall.SIGTERM) - select { - case sig := <-c: - return fmt.Errorf("received signal %s", sig) - case <-cancelInterrupt: - return nil - } + signal.Notify(c, shutdown.Signals()...) + return awaitShutdownSignal(c, cancelInterrupt) }, func(error) { close(cancelInterrupt) }) diff --git a/src/libraries/go/lib/pkg/nvkit/servers/shutdown.go b/src/libraries/go/lib/pkg/nvkit/servers/shutdown.go new file mode 100644 index 000000000..0a2078018 --- /dev/null +++ b/src/libraries/go/lib/pkg/nvkit/servers/shutdown.go @@ -0,0 +1,43 @@ +/* +SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package servers + +import ( + "os" + + "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/nvkit/shutdown" +) + +// awaitShutdownSignal blocks until a shutdown signal arrives or the run group +// tears this actor down, and is the run group's signal actor. +// +// A signal is reported as a *shutdown.SignalError. The run group has no way to +// end an actor other than returning an error, so a graceful stop and a crash +// reach the caller through the same channel; the typed error is what lets them +// be told apart with errors.Is instead of by matching the message. +// +// cancelInterrupt closing means another actor already failed and the group is +// shutting this one down, which is not this actor's error to report. +func awaitShutdownSignal(signals <-chan os.Signal, cancelInterrupt <-chan struct{}) error { + select { + case sig := <-signals: + return shutdown.NewSignalError(sig) + case <-cancelInterrupt: + return nil + } +} diff --git a/src/libraries/go/lib/pkg/nvkit/servers/shutdown_test.go b/src/libraries/go/lib/pkg/nvkit/servers/shutdown_test.go new file mode 100644 index 000000000..4a6e1a649 --- /dev/null +++ b/src/libraries/go/lib/pkg/nvkit/servers/shutdown_test.go @@ -0,0 +1,88 @@ +/* +SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package servers + +import ( + "errors" + "os" + "syscall" + "testing" + "time" + + "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/nvkit/shutdown" +) + +func TestAwaitShutdownSignal_ReportsSignalAsShutdown(t *testing.T) { + for _, sig := range []os.Signal{syscall.SIGTERM, syscall.SIGINT} { + t.Run(sig.String(), func(t *testing.T) { + signals := make(chan os.Signal, 1) + signals <- sig + + err := awaitShutdownSignal(signals, make(chan struct{})) + + if err == nil { + t.Fatal("awaitShutdownSignal() = nil, want a shutdown error") + } + if !errors.Is(err, shutdown.ErrSignal) { + t.Errorf("errors.Is(%v, shutdown.ErrSignal) = false, want true", err) + } + if !shutdown.IsSignalError(err) { + t.Errorf("shutdown.IsSignalError(%v) = false, want true", err) + } + + var signalErr *shutdown.SignalError + if errors.As(err, &signalErr) && signalErr.Signal != sig { + t.Errorf("Signal = %v, want %v", signalErr.Signal, sig) + } + }) + } +} + +func TestAwaitShutdownSignal_ReturnsNilWhenInterruptedByRunGroup(t *testing.T) { + // Another actor failed first and the run group is tearing this one down. + // That is not this actor's error to report. + cancelInterrupt := make(chan struct{}) + close(cancelInterrupt) + + if err := awaitShutdownSignal(make(chan os.Signal), cancelInterrupt); err != nil { + t.Errorf("awaitShutdownSignal() = %v, want nil", err) + } +} + +func TestAwaitShutdownSignal_BlocksUntilSomethingHappens(t *testing.T) { + signals := make(chan os.Signal, 1) + done := make(chan error, 1) + + go func() { done <- awaitShutdownSignal(signals, make(chan struct{})) }() + + select { + case err := <-done: + t.Fatalf("awaitShutdownSignal() returned %v before any signal arrived", err) + case <-time.After(50 * time.Millisecond): + } + + signals <- syscall.SIGTERM + select { + case err := <-done: + if !errors.Is(err, shutdown.ErrSignal) { + t.Errorf("errors.Is(%v, shutdown.ErrSignal) = false, want true", err) + } + case <-time.After(time.Second): + t.Fatal("awaitShutdownSignal() did not return after a signal arrived") + } +} diff --git a/src/libraries/go/lib/pkg/nvkit/shutdown/BUILD.bazel b/src/libraries/go/lib/pkg/nvkit/shutdown/BUILD.bazel new file mode 100644 index 000000000..5eab29d9e --- /dev/null +++ b/src/libraries/go/lib/pkg/nvkit/shutdown/BUILD.bazel @@ -0,0 +1,29 @@ +# SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "shutdown", + srcs = ["shutdown.go"], + importpath = "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/nvkit/shutdown", + visibility = ["//visibility:public"], +) + +go_test( + name = "shutdown_test", + srcs = ["shutdown_test.go"], + embed = [":shutdown"], +) diff --git a/src/libraries/go/lib/pkg/nvkit/shutdown/shutdown.go b/src/libraries/go/lib/pkg/nvkit/shutdown/shutdown.go new file mode 100644 index 000000000..9396fc465 --- /dev/null +++ b/src/libraries/go/lib/pkg/nvkit/shutdown/shutdown.go @@ -0,0 +1,125 @@ +/* +SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package shutdown tells a graceful stop apart from a crash. +// +// The servers run group models a shutdown signal as a terminal error, so an +// expected stop and a genuine failure both reach callers as ordinary errors. +// Every consumer therefore had to decide which was which, and each did it by +// matching the message text against the SIGINT wording. SIGTERM stringifies as +// "terminated" rather than "interrupt", so those checks classified every +// SIGTERM -- and SIGTERM is how Kubernetes asks a container to stop -- as a +// crash, turning routine pod terminations into panics. +// +// This package is the single place that decision lives. Producers report a +// signal with NewSignalError so consumers can use errors.Is; consumers call +// IsSignalError, which also recognizes the historical wording so it keeps +// working against builds that predate the typed error. +package shutdown + +import ( + "errors" + "os" + "strings" + "syscall" +) + +// ErrSignal is the sentinel every shutdown-signal error matches under +// errors.Is, so callers can classify a stop without depending on the concrete +// error type or on its message. +var ErrSignal = errors.New("shutdown signal") + +// signalErrorPrefix is the historical wording of the run group's terminal +// error. It is part of the contract: consumers pinned to a lib revision that +// predates this package, and consumers whose servers package comes from the +// separate nvcf-go module, only ever see the text. +const signalErrorPrefix = "received signal " + +// handledSignals are the signals a server installs a handler for. SIGTERM is +// what Kubernetes sends to stop a container, so it is the one production +// actually sees; SIGINT only shows up in local runs. +var handledSignals = []os.Signal{syscall.SIGINT, syscall.SIGTERM} + +// Signals returns the signals a server should stop on, for passing to +// signal.Notify. Sharing the list with IsSignalError keeps the set a server +// listens for and the set callers forgive from drifting apart. +func Signals() []os.Signal { + return append([]os.Signal(nil), handledSignals...) +} + +// SignalError reports that a server stopped because it received a shutdown +// signal rather than because something failed. +type SignalError struct { + Signal os.Signal +} + +// NewSignalError returns the error a server should report when sig stops it. +func NewSignalError(sig os.Signal) *SignalError { + return &SignalError{Signal: sig} +} + +// Error preserves the historical wording so consumers still matching on text +// keep working. +func (e *SignalError) Error() string { + if e.Signal == nil { + return strings.TrimSpace(signalErrorPrefix) + } + return signalErrorPrefix + e.Signal.String() +} + +// Is reports SignalError as ErrSignal so errors.Is classifies any shutdown +// signal without the caller naming a particular one. +func (e *SignalError) Is(target error) bool { + return target == ErrSignal +} + +// IsSignalError reports whether err is a server stopping on a shutdown signal +// rather than failing. +// +// A typed SignalError matches directly. Anything else falls back to the +// historical wording, derived from the signal names rather than hard-coded, so +// that neither SIGINT nor SIGTERM can be mistaken for a crash. The fallback is +// still text matching and cannot be exact: an unrelated error that happens to +// embed "received signal terminated" is read as a shutdown. Producers inside +// this repo should return NewSignalError so their consumers never rely on it. +func IsSignalError(err error) bool { + if err == nil { + return false + } + if errors.Is(err, ErrSignal) { + return true + } + + msg := strings.ToLower(err.Error()) + for _, sig := range handledSignals { + if strings.Contains(msg, signalErrorPrefix+strings.ToLower(sig.String())) { + return true + } + } + return false +} + +// IsFatal reports whether a terminal error from a server or root command is a +// genuine failure, rather than nil or the expected response to a shutdown +// signal. It is what a caller deciding whether to panic should ask. +// +// The nil case is the reason this exists rather than being spelled out at each +// call site: IsSignalError(nil) is false, so the obvious hand-written form +// treats a clean exit as fatal unless the caller remembers the nil check. +func IsFatal(err error) bool { + return err != nil && !IsSignalError(err) +} diff --git a/src/libraries/go/lib/pkg/nvkit/shutdown/shutdown_test.go b/src/libraries/go/lib/pkg/nvkit/shutdown/shutdown_test.go new file mode 100644 index 000000000..0a7490049 --- /dev/null +++ b/src/libraries/go/lib/pkg/nvkit/shutdown/shutdown_test.go @@ -0,0 +1,194 @@ +/* +SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package shutdown + +import ( + "errors" + "fmt" + "os" + "syscall" + "testing" +) + +func TestIsSignalError(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + { + name: "nil is not a shutdown", + err: nil, + want: false, + }, + { + name: "typed SIGTERM", + err: NewSignalError(syscall.SIGTERM), + want: true, + }, + { + name: "typed SIGINT", + err: NewSignalError(syscall.SIGINT), + want: true, + }, + { + name: "wrapped typed error", + err: fmt.Errorf("server stopped: %w", NewSignalError(syscall.SIGTERM)), + want: true, + }, + { + // The wording an older pinned lib, or the separate nvcf-go copy of + // nvkit, still produces. Recognizing it is why the fallback exists. + name: "untyped SIGTERM wording", + err: errors.New("received signal terminated"), + want: true, + }, + { + name: "untyped SIGINT wording", + err: errors.New("received signal interrupt"), + want: true, + }, + { + name: "untyped wording wrapped", + err: fmt.Errorf("run group: %w", errors.New("received signal terminated")), + want: true, + }, + { + name: "signal-shaped message for a signal we do not handle", + err: errors.New("received signal hangup"), + want: false, + }, + { + name: "unrelated failure", + err: errors.New("listen tcp :8080: bind: address already in use"), + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsSignalError(tt.err); got != tt.want { + t.Errorf("IsSignalError(%v) = %t, want %t", tt.err, got, tt.want) + } + }) + } +} + +func TestSignalErrorMatchesSentinel(t *testing.T) { + err := NewSignalError(syscall.SIGTERM) + + if !errors.Is(err, ErrSignal) { + t.Errorf("errors.Is(%v, ErrSignal) = false, want true", err) + } + if errors.Is(errors.New("boom"), ErrSignal) { + t.Error("errors.Is(unrelated error, ErrSignal) = true, want false") + } +} + +func TestSignalErrorMessageMatchesHistoricalWording(t *testing.T) { + // Consumers pinned to an older lib still compare against this text, so the + // wording is part of the contract and must not drift. + if got, want := NewSignalError(syscall.SIGTERM).Error(), "received signal terminated"; got != want { + t.Errorf("Error() = %q, want %q", got, want) + } +} + +func TestSignalErrorUnwrapsToSignal(t *testing.T) { + err := NewSignalError(syscall.SIGTERM) + + var signalErr *SignalError + if !errors.As(err, &signalErr) { + t.Fatalf("errors.As(%v, *SignalError) = false, want true", err) + } + if signalErr.Signal != syscall.SIGTERM { + t.Errorf("Signal = %v, want %v", signalErr.Signal, syscall.SIGTERM) + } +} + +func TestSignalErrorWithoutSignalDoesNotPanic(t *testing.T) { + // A zero value should degrade to a plain message rather than panicking on + // a nil os.Signal. + if got := (&SignalError{}).Error(); got == "" { + t.Error("Error() on zero value = empty string, want a message") + } +} + +func TestSignalsAreTheOnesHandlersInstall(t *testing.T) { + want := []os.Signal{syscall.SIGINT, syscall.SIGTERM} + + got := Signals() + if len(got) != len(want) { + t.Fatalf("Signals() = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("Signals()[%d] = %v, want %v", i, got[i], want[i]) + } + } +} + +func TestSignalsReturnsACopy(t *testing.T) { + // Callers pass the result straight to signal.Notify; a shared backing array + // would let one of them corrupt the list for everyone. + first := Signals() + first[0] = syscall.SIGHUP + + if second := Signals(); second[0] != syscall.SIGINT { + t.Errorf("Signals()[0] = %v after caller mutation, want %v", second[0], syscall.SIGINT) + } +} + +func TestIsFatal(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + { + // The guard that makes IsFatal worth having: !IsSignalError(nil) + // is true, so a call site spelling this out by hand panics on a + // clean exit if it forgets the nil check. + name: "nil is a clean exit", + err: nil, + want: false, + }, + { + name: "typed shutdown signal", + err: NewSignalError(syscall.SIGTERM), + want: false, + }, + { + name: "untyped shutdown wording", + err: errors.New("received signal terminated"), + want: false, + }, + { + name: "genuine failure", + err: errors.New("listen tcp :8080: bind: address already in use"), + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsFatal(tt.err); got != tt.want { + t.Errorf("IsFatal(%v) = %t, want %t", tt.err, got, tt.want) + } + }) + } +}