From 67787f68d2d83990cc1691599b604510583f151a Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Fri, 5 Jan 2024 03:39:51 +0700 Subject: [PATCH 01/13] Fix Signal Issue on Windows - [+] fix(signals.go): add build tags for non-windows systems - [+] fix(signals_test.go): add build tags for non-windows systems - [+] feat(signals_windows.go): add build tags for windows systems - [+] feat(signals_windows_test.go): add build tags for windows systems --- internal/legacy/signals/signals.go | 5 + internal/legacy/signals/signals_test.go | 5 + internal/legacy/signals/signals_windows.go | 96 +++++++++++ .../legacy/signals/singals_windows_test.go | 163 ++++++++++++++++++ 4 files changed, 269 insertions(+) create mode 100644 internal/legacy/signals/signals_windows.go create mode 100644 internal/legacy/signals/singals_windows_test.go diff --git a/internal/legacy/signals/signals.go b/internal/legacy/signals/signals.go index c447c46e..377e30c5 100644 --- a/internal/legacy/signals/signals.go +++ b/internal/legacy/signals/signals.go @@ -1,3 +1,8 @@ +//go:build !windows +// +build !windows + +// Note: this build on unix systems + /* Copyright 2021 The Kubernetes Authors. diff --git a/internal/legacy/signals/signals_test.go b/internal/legacy/signals/signals_test.go index 70f17879..8e6a255d 100644 --- a/internal/legacy/signals/signals_test.go +++ b/internal/legacy/signals/signals_test.go @@ -1,3 +1,8 @@ +//go:build !windows +// +build !windows + +// Note: this build on unix systems + /* Copyright 2021 The Kubernetes Authors. diff --git a/internal/legacy/signals/signals_windows.go b/internal/legacy/signals/signals_windows.go new file mode 100644 index 00000000..9f85b9f5 --- /dev/null +++ b/internal/legacy/signals/signals_windows.go @@ -0,0 +1,96 @@ +//go:build windows +// +build windows + +// Note: this build on windows systems + +/* +Copyright 2021 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, + //syscall.SIGTSTP: 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 +// 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() + } +} diff --git a/internal/legacy/signals/singals_windows_test.go b/internal/legacy/signals/singals_windows_test.go new file mode 100644 index 00000000..46a99104 --- /dev/null +++ b/internal/legacy/signals/singals_windows_test.go @@ -0,0 +1,163 @@ +//go:build windows +// +build windows + +// Note: this build on windows systems + +/* +Copyright 2021 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. + sigTests := []sigTest{ + // { + // signal: syscall.SIGIO, + // expected: []string{ + // fmt.Sprintf("Encoutered signal: %s", syscall.SIGIO.String()), + // }, + // terminate: false, + // }, + { + 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 + // signals.SignalChannel <- syscall.SIGSYS + // signals.SignalChannel <- syscall.SIGIO + // 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.SIGTTOU + // signals.SignalChannel <- syscall.SIGCHLD + 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") +} From 667a209c0dc239c039feca68269c4d75c6b47d69 Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Fri, 5 Jan 2024 04:00:21 +0700 Subject: [PATCH 02/13] Fix Typo - [+] refactor(signals_windows_test.go): Rename file from singals_windows_test.go to signals_windows_test.go --- .../signals/{singals_windows_test.go => signals_windows_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename internal/legacy/signals/{singals_windows_test.go => signals_windows_test.go} (100%) diff --git a/internal/legacy/signals/singals_windows_test.go b/internal/legacy/signals/signals_windows_test.go similarity index 100% rename from internal/legacy/signals/singals_windows_test.go rename to internal/legacy/signals/signals_windows_test.go From 1a578c9a8f87ac2b9bb4b8cfe7da87bab682536b Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Fri, 5 Jan 2024 03:39:51 +0700 Subject: [PATCH 03/13] Fix Signal Issue on Windows - [+] fix(signals.go): add build tags for non-windows systems - [+] fix(signals_test.go): add build tags for non-windows systems - [+] feat(signals_windows.go): add build tags for windows systems - [+] feat(signals_windows_test.go): add build tags for windows systems --- internal/legacy/signals/signals.go | 5 + internal/legacy/signals/signals_test.go | 5 + internal/legacy/signals/signals_windows.go | 96 +++++++++++ .../legacy/signals/singals_windows_test.go | 163 ++++++++++++++++++ 4 files changed, 269 insertions(+) create mode 100644 internal/legacy/signals/signals_windows.go create mode 100644 internal/legacy/signals/singals_windows_test.go diff --git a/internal/legacy/signals/signals.go b/internal/legacy/signals/signals.go index c447c46e..377e30c5 100644 --- a/internal/legacy/signals/signals.go +++ b/internal/legacy/signals/signals.go @@ -1,3 +1,8 @@ +//go:build !windows +// +build !windows + +// Note: this build on unix systems + /* Copyright 2021 The Kubernetes Authors. diff --git a/internal/legacy/signals/signals_test.go b/internal/legacy/signals/signals_test.go index 70f17879..8e6a255d 100644 --- a/internal/legacy/signals/signals_test.go +++ b/internal/legacy/signals/signals_test.go @@ -1,3 +1,8 @@ +//go:build !windows +// +build !windows + +// Note: this build on unix systems + /* Copyright 2021 The Kubernetes Authors. diff --git a/internal/legacy/signals/signals_windows.go b/internal/legacy/signals/signals_windows.go new file mode 100644 index 00000000..9f85b9f5 --- /dev/null +++ b/internal/legacy/signals/signals_windows.go @@ -0,0 +1,96 @@ +//go:build windows +// +build windows + +// Note: this build on windows systems + +/* +Copyright 2021 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, + //syscall.SIGTSTP: 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 +// 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() + } +} diff --git a/internal/legacy/signals/singals_windows_test.go b/internal/legacy/signals/singals_windows_test.go new file mode 100644 index 00000000..46a99104 --- /dev/null +++ b/internal/legacy/signals/singals_windows_test.go @@ -0,0 +1,163 @@ +//go:build windows +// +build windows + +// Note: this build on windows systems + +/* +Copyright 2021 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. + sigTests := []sigTest{ + // { + // signal: syscall.SIGIO, + // expected: []string{ + // fmt.Sprintf("Encoutered signal: %s", syscall.SIGIO.String()), + // }, + // terminate: false, + // }, + { + 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 + // signals.SignalChannel <- syscall.SIGSYS + // signals.SignalChannel <- syscall.SIGIO + // 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.SIGTTOU + // signals.SignalChannel <- syscall.SIGCHLD + 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") +} From dccf08c33b345f234c56ab250bae8e7d3e3ce682 Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Fri, 5 Jan 2024 04:00:21 +0700 Subject: [PATCH 04/13] Fix Typo - [+] refactor(signals_windows_test.go): Rename file from singals_windows_test.go to signals_windows_test.go --- .../signals/{singals_windows_test.go => signals_windows_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename internal/legacy/signals/{singals_windows_test.go => signals_windows_test.go} (100%) diff --git a/internal/legacy/signals/singals_windows_test.go b/internal/legacy/signals/signals_windows_test.go similarity index 100% rename from internal/legacy/signals/singals_windows_test.go rename to internal/legacy/signals/signals_windows_test.go From ef0b26cec3106dc652c89a132634abb334469fe8 Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Fri, 5 Jan 2024 16:30:23 +0700 Subject: [PATCH 05/13] Chore [PR] [Signals] Note about complexity and Fix Typo - [+] chore(pr.go): Add note about complexity and suggest splitting into smaller functions - [+] chore(signals.go): Update note to specify that it builds on Unix/Linux systems - [+] chore(signals_test.go): Update note to specify that it builds on Unix/Linux systems --- cmd/kpromo/cmd/pr/pr.go | 2 ++ internal/legacy/signals/signals.go | 2 +- internal/legacy/signals/signals_test.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/kpromo/cmd/pr/pr.go b/cmd/kpromo/cmd/pr/pr.go index bc698539..0f6d6fe1 100644 --- a/cmd/kpromo/cmd/pr/pr.go +++ b/cmd/kpromo/cmd/pr/pr.go @@ -158,6 +158,8 @@ func init() { } } +// Note: whoever made this, it's too complex and hard to read for human and machine, +// it's better to split it into smaller functions because this go not a python func runPromote(opts *promoteOptions) error { // Check the cmd line opts if err := opts.Validate(); err != nil { diff --git a/internal/legacy/signals/signals.go b/internal/legacy/signals/signals.go index 377e30c5..08b48a17 100644 --- a/internal/legacy/signals/signals.go +++ b/internal/legacy/signals/signals.go @@ -1,7 +1,7 @@ //go:build !windows // +build !windows -// Note: this build on unix systems +// Note: this build on unix/linux systems /* Copyright 2021 The Kubernetes Authors. diff --git a/internal/legacy/signals/signals_test.go b/internal/legacy/signals/signals_test.go index 8e6a255d..673aa209 100644 --- a/internal/legacy/signals/signals_test.go +++ b/internal/legacy/signals/signals_test.go @@ -1,7 +1,7 @@ //go:build !windows // +build !windows -// Note: this build on unix systems +// Note: this build on unix/linux systems /* Copyright 2021 The Kubernetes Authors. From b66ad0fcb8927813f511aeec118cf2b26d94f0a4 Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Fri, 5 Jan 2024 21:26:52 +0700 Subject: [PATCH 06/13] Chore [PR] [Signals] Update Note - [+] chore(pr.go): Update note about complexity and suggest splitting into smaller functions --- cmd/kpromo/cmd/pr/pr.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/kpromo/cmd/pr/pr.go b/cmd/kpromo/cmd/pr/pr.go index 0f6d6fe1..ec7318f6 100644 --- a/cmd/kpromo/cmd/pr/pr.go +++ b/cmd/kpromo/cmd/pr/pr.go @@ -158,8 +158,9 @@ func init() { } } -// Note: whoever made this, it's too complex and hard to read for human and machine, -// it's better to split it into smaller functions because this go not a python +// Note: This function is overly complex and difficult to read, both for humans and machines. +// 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 { From fd344e4ce0fa26c274aa7ee145cbd5f1e13edd71 Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Fri, 5 Jan 2024 21:28:46 +0700 Subject: [PATCH 07/13] Chore [Internal] [Signal] Update Copyright Year - [+] chore(signals): update copyright year to 2024 in signals_windows.go and signals_windows_test.go files --- internal/legacy/signals/signals_windows.go | 2 +- internal/legacy/signals/signals_windows_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/legacy/signals/signals_windows.go b/internal/legacy/signals/signals_windows.go index 9f85b9f5..8b745e5b 100644 --- a/internal/legacy/signals/signals_windows.go +++ b/internal/legacy/signals/signals_windows.go @@ -4,7 +4,7 @@ // Note: this build on windows systems /* -Copyright 2021 The Kubernetes Authors. +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. diff --git a/internal/legacy/signals/signals_windows_test.go b/internal/legacy/signals/signals_windows_test.go index 46a99104..99afe4a3 100644 --- a/internal/legacy/signals/signals_windows_test.go +++ b/internal/legacy/signals/signals_windows_test.go @@ -4,7 +4,7 @@ // Note: this build on windows systems /* -Copyright 2021 The Kubernetes Authors. +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. From a48f2f00e80ed9e301666b5edf22521b6bcc9b73 Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Fri, 5 Jan 2024 21:34:47 +0700 Subject: [PATCH 08/13] Chore [Internal] [Signal Test] Update Note - [+] test(signals_windows_test.go): add comments to explain why certain signals are not tested --- internal/legacy/signals/signals_windows_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/legacy/signals/signals_windows_test.go b/internal/legacy/signals/signals_windows_test.go index 99afe4a3..0b9973e2 100644 --- a/internal/legacy/signals/signals_windows_test.go +++ b/internal/legacy/signals/signals_windows_test.go @@ -43,6 +43,8 @@ func TestLogSignal(t *testing.T) { // Create multiple tests. // NOTE: We are unable to observe SIGKILL or SIGSTOP, therefore we will not // test with these syscalls. + // Another NOTE by @H0llyW00dzZ: SIGIO,SIGSYS,SIGTTOU,SIGCHLD is not available on Windows + // System, so we will not test with these syscalls. sigTests := []sigTest{ // { // signal: syscall.SIGIO, From 65feb8b311f677d7c6b990eee2f15a1fab14a210 Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Fri, 5 Jan 2024 21:41:07 +0700 Subject: [PATCH 09/13] Chore [CMD] [Kpromo] [CMD] [PR] Add Author Note - [+] refactor(pr.go): split runPromote function into smaller functions for better readability and maintainability --- cmd/kpromo/cmd/pr/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/kpromo/cmd/pr/pr.go b/cmd/kpromo/cmd/pr/pr.go index ec7318f6..9ecdf9b0 100644 --- a/cmd/kpromo/cmd/pr/pr.go +++ b/cmd/kpromo/cmd/pr/pr.go @@ -158,7 +158,7 @@ func init() { } } -// Note: This function is overly complex and difficult to read, both for humans and machines. +// Another NOTE by @H0llyW00dzZ: This function is overly complex and difficult to read, both for humans and machines. // 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 { From ecfffa1a78ab6517f25ab0e1c28e7da1ac0789ab Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Sat, 20 Jan 2024 08:21:28 +0700 Subject: [PATCH 10/13] Update Note - [+] test(signals_windows_test.go): remove unnecessary comments and update comment about unavailable signals on Windows --- internal/legacy/signals/signals_windows_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/legacy/signals/signals_windows_test.go b/internal/legacy/signals/signals_windows_test.go index 0b9973e2..e3dcf66b 100644 --- a/internal/legacy/signals/signals_windows_test.go +++ b/internal/legacy/signals/signals_windows_test.go @@ -1,8 +1,6 @@ //go:build windows // +build windows -// Note: this build on windows systems - /* Copyright 2024 The Kubernetes Authors. @@ -43,8 +41,7 @@ func TestLogSignal(t *testing.T) { // Create multiple tests. // NOTE: We are unable to observe SIGKILL or SIGSTOP, therefore we will not // test with these syscalls. - // Another NOTE by @H0llyW00dzZ: SIGIO,SIGSYS,SIGTTOU,SIGCHLD is not available on Windows - // System, so we will not test with these syscalls. + // SIGIO, SIGSYS, SIGTTOU, and SIGCHLD are not available on Windows sigTests := []sigTest{ // { // signal: syscall.SIGIO, From 81ca4597a59e200caa5b02a0da294c1e761b51ec Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Sat, 20 Jan 2024 08:24:53 +0700 Subject: [PATCH 11/13] Update Note & remove commented out code - [+] chore(signals_windows.go): remove commented out code for SIGTSTP signal --- internal/legacy/signals/signals_windows.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/legacy/signals/signals_windows.go b/internal/legacy/signals/signals_windows.go index 8b745e5b..fb620e54 100644 --- a/internal/legacy/signals/signals_windows.go +++ b/internal/legacy/signals/signals_windows.go @@ -1,8 +1,6 @@ //go:build windows // +build windows -// Note: this build on windows systems - /* Copyright 2024 The Kubernetes Authors. @@ -39,7 +37,6 @@ var ( syscall.SIGQUIT: true, syscall.SIGTERM: true, syscall.SIGSEGV: true, - //syscall.SIGTSTP: true, } // ExitChannel is for gracefully terminating the LogSignals() function. ExitChannel = make(chan bool, 1) From 2c817bf360e00810c3a21e8a9a78cdd000398c46 Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Sat, 20 Jan 2024 08:27:26 +0700 Subject: [PATCH 12/13] Remove Comment Code - [+] test(signals_windows_test.go): comment out unused signal tests - [+] test(signals_windows_test.go): comment out unused signal tests --- internal/legacy/signals/signals_windows_test.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/internal/legacy/signals/signals_windows_test.go b/internal/legacy/signals/signals_windows_test.go index e3dcf66b..011914dd 100644 --- a/internal/legacy/signals/signals_windows_test.go +++ b/internal/legacy/signals/signals_windows_test.go @@ -43,13 +43,6 @@ func TestLogSignal(t *testing.T) { // test with these syscalls. // SIGIO, SIGSYS, SIGTTOU, and SIGCHLD are not available on Windows sigTests := []sigTest{ - // { - // signal: syscall.SIGIO, - // expected: []string{ - // fmt.Sprintf("Encoutered signal: %s", syscall.SIGIO.String()), - // }, - // terminate: false, - // }, { signal: syscall.SIGALRM, expected: []string{ @@ -136,8 +129,6 @@ func TestLogSignals(t *testing.T) { // the SignalChannel will block and the test will fail. signals.SignalChannel <- syscall.SIGBUS signals.SignalChannel <- syscall.SIGALRM - // signals.SignalChannel <- syscall.SIGSYS - // signals.SignalChannel <- syscall.SIGIO // Force exit. signals.Stop() wg.Wait() @@ -151,8 +142,6 @@ func TestLogSignals(t *testing.T) { 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.SIGTTOU - // signals.SignalChannel <- syscall.SIGCHLD signals.SignalChannel <- syscall.SIGPIPE // Pass an exit signal. signals.SignalChannel <- syscall.SIGINT From f3e8576b9eec454b076cf7ee6486c6850c1ee8cb Mon Sep 17 00:00:00 2001 From: H0llyW00dzZ Date: Sat, 20 Jan 2024 08:40:21 +0700 Subject: [PATCH 13/13] Update Comment - [+] chore(pr.go): update comment to reference issue #1177 and clarify code readability concerns --- cmd/kpromo/cmd/pr/pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/kpromo/cmd/pr/pr.go b/cmd/kpromo/cmd/pr/pr.go index 9ecdf9b0..9e7ef92d 100644 --- a/cmd/kpromo/cmd/pr/pr.go +++ b/cmd/kpromo/cmd/pr/pr.go @@ -158,7 +158,7 @@ func init() { } } -// Another NOTE by @H0llyW00dzZ: This function is overly complex and difficult to read, both for humans and machines. +// Issue #1177: This function is overly complex and difficult to read, both for humans and machines. // 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 {