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.

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

Fix Signal Issue on Windows #1159

Closed
wants to merge 15 commits into from
Closed
3 changes: 3 additions & 0 deletions cmd/kpromo/cmd/pr/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ func init() {
}
}

// Issue #1177: This function is overly complex and difficult to read, both for humans and machines.
Copy link
Member

Choose a reason for hiding this comment

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

add the full like and drop this comment

// It would be better to split it into smaller functions, as this is Go, not Python, and Go
// encourages simpler, more readable code.
func runPromote(opts *promoteOptions) error {
// Check the cmd line opts
if err := opts.Validate(); err != nil {
Expand Down
5 changes: 5 additions & 0 deletions internal/legacy/signals/signals.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
//go:build !windows
// +build !windows

// Note: this build on unix/linux systems
H0llyW00dzZ marked this conversation as resolved.
Show resolved Hide resolved

/*
Copyright 2021 The Kubernetes Authors.

Expand Down
5 changes: 5 additions & 0 deletions internal/legacy/signals/signals_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
//go:build !windows
// +build !windows

// Note: this build on unix/linux systems
H0llyW00dzZ marked this conversation as resolved.
Show resolved Hide resolved

/*
Copyright 2021 The Kubernetes Authors.

Expand Down
93 changes: 93 additions & 0 deletions internal/legacy/signals/signals_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
//go:build windows
// +build windows

/*
Copyright 2024 The Kubernetes Authors.

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 signals

import (
"os"
"os/signal"
"syscall"

"github.com/sirupsen/logrus"
)

var (
// ExitSignals are used to determine if an incoming os.Signal should cause termination.
ExitSignals = map[os.Signal]bool{
syscall.SIGHUP: true,
syscall.SIGINT: true,
syscall.SIGABRT: true,
syscall.SIGILL: true,
syscall.SIGQUIT: true,
syscall.SIGTERM: true,
syscall.SIGSEGV: true,
}
// ExitChannel is for gracefully terminating the LogSignals() function.
ExitChannel = make(chan bool, 1)
// SignalChannel is for listening to OS signals.
SignalChannel = make(chan os.Signal, 1)
// Debug is defined globally for mocking logrus.Debug statements.
Debug func(args ...interface{}) = logrus.Debug
)

// Watch concurrently logs debug statements when encountering interrupt signals from the OS.
// This function relies on the os/signal package which may not capture SIGKILL and SIGSTOP.
func Watch() {
Debug("Watching for OS Signals...")
// Observe all signals, excluding SIGKILL and SIGSTOP.
signal.Notify(SignalChannel)
// Continuously log signals.
go LogSignals()
}

// Stop gracefully terminates the concurrent signal logging.
// TODO: @tylerferrara Currently we don't gracefully exit, since the Auditor is designed
H0llyW00dzZ marked this conversation as resolved.
Show resolved Hide resolved
// to run indefinitely. In the future, we should enable graceful termination to allow
// this to be called from a Shutdown() function.
func Stop() {
ExitChannel <- true
}

// LogSignals continuously prints logging statements for each signal it observes,
// exiting only when observing an exit signal.
func LogSignals() {
for {
select {
case sig := <-SignalChannel:
LogSignal(sig)
case <-ExitChannel:
// Gracefully terminate.
return
}
}
}

// LogSignal prints a logging statements for the given signal,
// exiting only when observing an exit signal.
func LogSignal(sig os.Signal) {
Debug("Encoutered signal: ", sig.String())
// Handle exit signals.
if _, found := ExitSignals[sig]; found {
Debug("Exiting from signal: ", sig.String())
// If we get here, an exit signal was seen. We must handle this by forcing
// the program to exit. Without this, the program would ignore all exit signals
// except SIGKILL.
Stop()
}
}
151 changes: 151 additions & 0 deletions internal/legacy/signals/signals_windows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
//go:build windows
// +build windows

/*
Copyright 2024 The Kubernetes Authors.

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 signals_test

import (
"fmt"
"os"
"sync"
"syscall"
"testing"

"github.com/stretchr/testify/require"

"sigs.k8s.io/promo-tools/v4/internal/legacy/signals"
)

func TestLogSignal(t *testing.T) {
// Define what a test looks like.
type sigTest struct {
signal os.Signal // Incoming signal to observe.
expected []string // Expected logs to be produced.
terminate bool // Determine if signals.LogSignals() should return.
}
// Create multiple tests.
// NOTE: We are unable to observe SIGKILL or SIGSTOP, therefore we will not
// test with these syscalls.
// SIGIO, SIGSYS, SIGTTOU, and SIGCHLD are not available on Windows
sigTests := []sigTest{
{
signal: syscall.SIGALRM,
expected: []string{
fmt.Sprintf("Encoutered signal: %s", syscall.SIGALRM.String()),
},
terminate: false,
},
{
signal: syscall.SIGALRM,
expected: []string{
fmt.Sprintf("Encoutered signal: %s", syscall.SIGALRM.String()),
},
terminate: false,
},
{
signal: syscall.SIGQUIT,
expected: []string{
fmt.Sprintf("Encoutered signal: %s", syscall.SIGQUIT.String()),
fmt.Sprintf("Exiting from signal: %s", syscall.SIGQUIT.String()),
},
terminate: true,
},
{
signal: syscall.SIGINT,
expected: []string{
fmt.Sprintf("Encoutered signal: %s", syscall.SIGINT.String()),
fmt.Sprintf("Exiting from signal: %s", syscall.SIGINT.String()),
},
terminate: true,
},
{
signal: syscall.SIGABRT,
expected: []string{
fmt.Sprintf("Encoutered signal: %s", syscall.SIGABRT.String()),
fmt.Sprintf("Exiting from signal: %s", syscall.SIGABRT.String()),
},
terminate: true,
},
}
// Capture all logs.
logs := []string{}
// Mock logrus.Debug statements.
signals.Debug = func(args ...interface{}) {
logs = append(logs, fmt.Sprint(args...))
}
// Determine if LogSignal invoked Stop().
terminated := func() bool {
return <-signals.ExitChannel
}
// Used for enforcing defaults before each test.
reset := func() {
logs = []string{}
}
// Run through all tests.
for _, st := range sigTests {
reset()
// Log the test signal.
signals.LogSignal(st.signal)
// Ensure the logs are correct.
require.EqualValues(t, st.expected, logs, "Unexpected signal observation logs.")
if st.terminate {
// Ensure Stop() was invoked if the test specifies.
require.True(t, terminated(), "LogSignal did not terminate on exit signal.")
}
}
}

// TestLogSignals ensures LogSignals() can handle multiple incoming signals and terminates either by
// receiving an exit signal or explicit call to Stop().
func TestLogSignals(t *testing.T) {
// Capture concurrent function termination.
wg := sync.WaitGroup{}
terminated := false
// Ensure the test waits for logSignals to finish executing.
logSignals := func() {
signals.LogSignals()
terminated = true
wg.Done()
}
// Start logging.
wg.Add(1)
go logSignals()
// Pass multiple non-exit signals, ensuring LogSignals is consuming each. Otherwise,
// the SignalChannel will block and the test will fail.
signals.SignalChannel <- syscall.SIGBUS
signals.SignalChannel <- syscall.SIGALRM
// Force exit.
signals.Stop()
wg.Wait()
// Ensure termination happened.
require.True(t, terminated, "LogSignals() did not terminate on call to Stop()")

// Reset termination bool for new test.
terminated = false
// Start logging.
wg.Add(1)
go logSignals()
// Pass multiple non-exit signals, ensuring LogSignals is consuming each. Otherwise,
// the SignalChannel will block and the test will fail.
signals.SignalChannel <- syscall.SIGPIPE
// Pass an exit signal.
signals.SignalChannel <- syscall.SIGINT
wg.Wait()
// Ensure termination happened.
require.True(t, terminated, "LogSignals() did not terminate when given an exit signal")
}