Suspend k9s using CTRL-Z #2484

arajski · 2024-01-17T13:05:54Z

Description

Implements unix-like suspend functionality mentioned in #2013

At the moment, k9s supports Ctrl-C, which exits the application, however it's not possible to suspend the process and send it to the background. This PR implements that functionality, which is common among other tools, like vim.
User can now send the process to the background by pressing Ctrl-Z.
Once suspended, it can be resumed by either using fg or selecting a specific id from the jobs list (for example %1) in case there are multiple ones.

This behaviour can be enabled by setting allowSuspend to true.

Changes

remapped "Toggle Faults" action from Ctrl-Z to Ctrl-T (first one available that kind of made sense to me)
added suspend functionality to Ctrl-Z, which sends SIGTSTP signal to the current k9s process
added allowSuspend config to opt in to this behaviour
updated tests

TODO:

test on other platforms

Notes

This is my first contribution to this project, so happy to update things if there's anything missing/required. Thanks!

derailed

@arajski Thanks for this update Artur!
Biggest concern with this behavior is to make sure we're cool all supported platforms.

derailed · 2024-01-18T00:57:33Z

internal/config/json/schemas/k9s.json

@@ -13,6 +13,7 @@
 "maxConnRetry": { "type": "integer" },
 "readOnly": { "type": "boolean" },
 "noExitOnCtrlC": { "type": "boolean" },
+ "noSuspendOnCtrlZ": { "type": "boolean" },


Not keen on the negative naming here. Think folks may want to opt in just in case.
Perhaps allowSuspend? Ctrl-z is likely already understood from folks aware of this command.
To boot, it will default to false if left unset hence keeping current behavior.

agree on that one. I was looking at noExitOnCtrlC as a sort of naming convention here. Anyway - renamed to allowSuspend 👍

derailed · 2024-01-18T00:58:32Z

internal/ui/app.go

@@ -143,6 +144,7 @@ func (a *App) bindKeys() {
 KeyColon: NewKeyAction("Cmd", a.activateCmd, false),
 tcell.KeyCtrlR: NewKeyAction("Redraw", a.redrawCmd, false),
 tcell.KeyCtrlC: NewKeyAction("Quit", a.quitCmd, false),
+ tcell.KeyCtrlZ: NewKeyAction("Suspend", a.suspendCmd, false),


This should only be available if the option is set and likely we may not want to show it in the header all the time.

Moved it out of the initial set

derailed · 2024-01-18T01:00:47Z

internal/ui/app.go

+ return evt
+ }
+
+ if !a.Config.K9s.NoSuspendOnCtrlZ {


We would not be needed the check here if we don't enable the action in the first place?

derailed · 2024-01-18T01:01:53Z

internal/ui/app.go

+ }
+
+ if !a.Config.K9s.NoSuspendOnCtrlZ {
+ a.Suspend(func() {


This will require additional testing on various platforms to make sure we're cool on other non nix aka windows?

I've got Windows 10 as dual boot, so can give it a test. Is there are set of other recommended platforms/versions?
Some VMs/containers might be required for this.

derailed · 2024-01-18T01:05:01Z

internal/view/reference.go

@@ -40,7 +40,7 @@ func (r *Reference) Init(ctx context.Context) error {

 func (r *Reference) bindKeys(aa ui.KeyActions) {
 aa.Delete(ui.KeyShiftA, tcell.KeyCtrlS, tcell.KeyCtrlSpace, ui.KeySpace)
- aa.Delete(tcell.KeyCtrlW, tcell.KeyCtrlL, tcell.KeyCtrlZ)
+ aa.Delete(tcell.KeyCtrlW, tcell.KeyCtrlL)


This should now be KeyCtrlT since we renamed it ;(

Added KeyCtrlT back

derailed

@arajski Thank you for the updates Artur! LGTM
I think we need to test this out on linux, windows flavors, macOs.

arajski · 2024-01-24T00:44:27Z

@derailed seems like go doesn't support os signals on Windows without some hacky workarounds.
I was thinking about spawning a shell for anything other than linux and darwin OS.
From a user perspective it's going to look almost the same, except in order to return to k9s, you'll need to exit the shell instead of resuming the process from the background. Let me know if that makes sense.
Otherwise we can go with the subshell approach for everyone.

derailed · 2024-02-03T15:14:20Z

@arajski Thank you for the research Artur! I think for non nix users likely Ctrtl-z does not make much sense. I don't use windows much these days but isn't Ctrl-z equivalent to undo?
If so might be best to detect the os and disable that functionality while in windows land?

arajski · 2024-02-06T13:38:13Z

Done 👍 Tested on macOS Sonoma and Debian 12

derailed

@arajski Thanks for the checks and updates Artur.

derailed · 2024-02-15T17:20:59Z

internal/ui/app.go

@@ -146,6 +148,12 @@ func (a *App) bindKeys() {
 tcell.KeyCtrlU: NewSharedKeyAction("Clear Filter", a.clearCmd, false),
 tcell.KeyCtrlQ: NewSharedKeyAction("Clear Filter", a.clearCmd, false),
 }
+
+ if a.Config.K9s.AllowSuspend && runtime.GOOS != "windows" {


I think it might be best to make a call on the config and keep this functionality encapsulated vs exposing on the call site? Perhaps something a.Config.K9s.IsSuspendable()?

yep makes sense - done :)

derailed requested changes Jan 18, 2024

View reviewed changes

derailed added enhancement New feature or request question Further information is requested needs-tlc Pr needs additional updates labels Jan 18, 2024

derailed reviewed Jan 18, 2024

View reviewed changes

arajski added 8 commits February 5, 2024 22:53

added support for ctrl z

4677311

removed locks

c05d46b

remove comment

0df356a

updated config variable name

1213920

added CtrlT back

6585561

updated dir test

4c41f93

added KeyCtrlT to img_scan

cc167b6

remove windows support

33e727e

arajski force-pushed the feat/support_ctrl_z branch from 9497cc0 to 33e727e Compare February 5, 2024 14:55

arajski requested a review from derailed February 6, 2024 13:38

derailed requested changes Feb 15, 2024

View reviewed changes

moved the suspend check to config

b480030

arajski requested a review from derailed February 28, 2024 12:20

Merge branch 'master' into feat/support_ctrl_z

53d8339

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suspend k9s using CTRL-Z #2484

Suspend k9s using CTRL-Z #2484

arajski commented Jan 17, 2024 •

edited

derailed left a comment

derailed Jan 18, 2024

arajski Jan 18, 2024

derailed Jan 18, 2024

arajski Jan 18, 2024

derailed Jan 18, 2024

arajski Jan 18, 2024

derailed Jan 18, 2024

arajski Jan 18, 2024

derailed Jan 18, 2024

arajski Jan 18, 2024

derailed left a comment

arajski commented Jan 24, 2024

derailed commented Feb 3, 2024

arajski commented Feb 6, 2024

derailed left a comment

derailed Feb 15, 2024

arajski Feb 27, 2024

Suspend k9s using CTRL-Z #2484

Are you sure you want to change the base?

Suspend k9s using CTRL-Z #2484

Conversation

arajski commented Jan 17, 2024 • edited

Description

Changes

TODO:

Notes

derailed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derailed left a comment

Choose a reason for hiding this comment

arajski commented Jan 24, 2024

derailed commented Feb 3, 2024

arajski commented Feb 6, 2024

derailed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arajski commented Jan 17, 2024 •

edited