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 "Mark Range": reduce maximum namespaces in favorites, fix shadowing of ctrl+space #2927

Merged
merged 6 commits into from
Nov 9, 2024

Conversation

zkck
Copy link
Contributor

@zkck zkck commented Oct 23, 2024

Fixes #2187 (at least "Mark Range")

Hello!

The "Mark Range" command seems to have remained broken. I used delve to investigate the problem, and it seems that the problem is in func namespaceActions: we lookup the missing key 10 in ui.NumKeys, giving back 0 (empty value for int), corresponding to tcell.KeyCtrlSpace, and breaking "Mark Range".

This may be due to the increase to MaxFavoriteNS added by #2881: number keys are added for each namespace in favorites plus the all namespace, so MaxFavoriteNS should be 9. I reduced MaxFavoriteNS back to 9, and added a check in func namespaceActions for when we go beyond ui.Key9, while removing the need for the ui.NumKeys.

Thanks again for k9s! Let me know if you see any issues with my PR :)

Copy link
Owner

@derailed derailed left a comment

Choose a reason for hiding this comment

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

@gerhard Nice catch! Thank you for this update

@@ -589,7 +589,12 @@ func (b *Browser) namespaceActions(aa *ui.KeyActions) {
if ns == client.NamespaceAll {
continue
}
aa.Add(ui.NumKeys[index], ui.NewKeyAction(ns, b.switchNamespaceCmd, true))
numKey := ui.Key0 + tcell.Key(index)
Copy link
Owner

Choose a reason for hiding this comment

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

I think all we need here is to make sure the key is within the index range.

		if k, ok := ui.NumKeys[index]; ok {
			aa.Add(k, ui.NewKeyAction(ns, b.switchNamespaceCmd, true))
			b.namespaces[index] = ns
			index++
		} else {
			log.Warn().Msgf("No more keys available for namespace %s: %d of %d]. Skipping...", ns, index, data.MaxFavoritesNS)
		}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright 👍 will change it. However maybe the index++ needs to be after the if/else block, otherwise index will just remain stuck on a missing key and wouldn't correspond with the namespace index. Or we can just assume that keys are no longer available and break after the warning

Copy link
Contributor Author

@zkck zkck Oct 31, 2024

Choose a reason for hiding this comment

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

Added the check, here we can see the output (I set MaxFavoriteNS back to 10 to test this)

Screenshot from 2024-10-31 10-06-12

Copy link
Owner

@derailed derailed left a comment

Choose a reason for hiding this comment

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

@zkck Thanks for the updates!

internal/view/browser.go Show resolved Hide resolved
@derailed derailed merged commit 99d47ab into derailed:master Nov 9, 2024
3 checks passed
thorbenbelow pushed a commit to thorbenbelow/k9s that referenced this pull request Nov 10, 2024
…wing of ctrl+space (derailed#2927)

* fix: avoid creating a ctrl+space key when index out of range

* fix: reduce maximum favorite namespaces to 9, making space for "all"

* fix: correct index incrementation

* refactor: remove usage of NumKeys

* feat: check for favorite namespace index in NumKeys

* feat: break when out of number keys, increment index when slot found
@derailed derailed mentioned this pull request Nov 10, 2024
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Nov 19, 2024
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [derailed/k9s](https://github.com/derailed/k9s) | patch | `v0.32.5` -> `v0.32.7` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>derailed/k9s (derailed/k9s)</summary>

### [`v0.32.7`](https://github.com/derailed/k9s/releases/tag/v0.32.7)

[Compare Source](derailed/k9s@v0.32.6...v0.32.7)

<img src="https://raw.githubusercontent.com/derailed/k9s/master/assets/k9s.png" align="center" width="800" height="auto"/>

### Release v0.32.7
#### Notes

Thank you to all that contributed with flushing out issues and enhancements for K9s!
I'll try to mark some of these issues as fixed. But if you don't mind grab the latest rev
and see if we're happier with some of the fixes!
If you've filed an issue please help me verify and close.

Your support, kindness and awesome suggestions to make K9s better are, as ever, very much noted and appreciated!
Also big thanks to all that have allocated their own time to help others on both slack and on this repo!!

As you may know, K9s is not pimped out by corps with deep pockets, thus if you feel K9s is helping your Kubernetes journey,
please consider joining our [sponsorship program](https://github.com/sponsors/derailed) and/or make some noise on social! [@&#8203;kitesurfer](https://twitter.com/kitesurfer)

On Slack? Please join us [K9slackers](https://join.slack.com/t/k9sers/shared_invite/enQtOTA5MDEyNzI5MTU0LWQ1ZGI3MzliYzZhZWEyNzYxYzA3NjE0YTk1YmFmNzViZjIyNzhkZGI0MmJjYzhlNjdlMGJhYzE2ZGU1NjkyNTM)

#### Maintenance Release!

***

#### Videos Are In The Can!

Please dial [K9s Channel](https://www.youtube.com/channel/UC897uwPygni4QIjkPCpgjmw) for up coming content...

-   [K9s v0.31.0 Configs+Sneak peek](https://youtu.be/X3444KfjguE)
-   [K9s v0.30.0 Sneak peek](https://youtu.be/mVBc1XneRJ4)
-   [Vulnerability Scans](https://youtu.be/ULkl0MsaidU)

***

#### Resolved Issues

-   [#&#8203;2970](derailed/k9s#2970) Ctrl-z on events view causes runtime error in v0.32.6
-   [#&#8203;2969](derailed/k9s#2969) When using impersonation user information and permissions not preserved when switching context
-   [#&#8203;2966](derailed/k9s#2966) Go to the Contexts page and filter, contexts that are matched will be filtered ou
-   [#&#8203;2962](derailed/k9s#2962) Small colour/filtering related bug
-   [#&#8203;2961](derailed/k9s#2961) Drain node with the -disable-eviction
-   [#&#8203;2958](derailed/k9s#2958) Restart count in container view associated with the wrong container
-   [#&#8203;2945](derailed/k9s#2945) Could we add ServiceAccount Column in v1/POD view

***

#### Contributed MRs

Please be sure to give `Big Thanks!` and `ATTA Girls/Boys!` to all the fine contributors for making K9s better for all of us!!

-   [#&#8203;2968](derailed/k9s#2968) Update go version to 1.23.X in README
-   [#&#8203;2964](derailed/k9s#2964) feat(dao,used-by-cmd): check imagePullSecrets as well
-   [#&#8203;2960](derailed/k9s#2960) Put log levels in order in cmd help

***

<img src="https://raw.githubusercontent.com/derailed/k9s/master/assets/imhotep_logo.png" width="32" height="auto"/> © 2024 Imhotep Software LLC. All materials licensed under [Apache v2.0](http://www.apache.org/licenses/LICENSE-2.0)

### [`v0.32.6`](https://github.com/derailed/k9s/releases/tag/v0.32.6)

[Compare Source](derailed/k9s@v0.32.5...v0.32.6)

<img src="https://raw.githubusercontent.com/derailed/k9s/master/assets/k9s.png" align="center" width="800" height="auto"/>

### Release v0.32.6
#### Notes

Thank you to all that contributed with flushing out issues and enhancements for K9s!
I'll try to mark some of these issues as fixed. But if you don't mind grab the latest rev
and see if we're happier with some of the fixes!
If you've filed an issue please help me verify and close.

Your support, kindness and awesome suggestions to make K9s better are, as ever, very much noted and appreciated!
Also big thanks to all that have allocated their own time to help others on both slack and on this repo!!

As you may know, K9s is not pimped out by corps with deep pockets, thus if you feel K9s is helping your Kubernetes journey,
please consider joining our [sponsorship program](https://github.com/sponsors/derailed) and/or make some noise on social! [@&#8203;kitesurfer](https://twitter.com/kitesurfer)

On Slack? Please join us [K9slackers](https://join.slack.com/t/k9sers/shared_invite/enQtOTA5MDEyNzI5MTU0LWQ1ZGI3MzliYzZhZWEyNzYxYzA3NjE0YTk1YmFmNzViZjIyNzhkZGI0MmJjYzhlNjdlMGJhYzE2ZGU1NjkyNTM)

#### Maintenance Release!

***

#### Videos Are In The Can!

Please dial [K9s Channel](https://www.youtube.com/channel/UC897uwPygni4QIjkPCpgjmw) for up coming content...

-   [K9s v0.31.0 Configs+Sneak peek](https://youtu.be/X3444KfjguE)
-   [K9s v0.30.0 Sneak peek](https://youtu.be/mVBc1XneRJ4)
-   [Vulnerability Scans](https://youtu.be/ULkl0MsaidU)

***

#### Resolved Issues

-   [#&#8203;2947](derailed/k9s#2947) CTRL+Z causes k9s to crash
-   [#&#8203;2938](derailed/k9s#2938) Critical Vulnerability CVE-2024-41110 in v26.0.1 of docker included in k9s
-   [#&#8203;2929](derailed/k9s#2929) conflicting plugins shortcuts
-   [#&#8203;2896](derailed/k9s#2896) Add a plugin to disable/enable a keda ScaledObject
-   [#&#8203;2811](derailed/k9s#2811) Dockerfile build step fails due to misaligned Go versions (1.21.5 vs 1.22.0)
-   [#&#8203;2767](derailed/k9s#2767) Manually triggered jobs don't get automatically cleaned up
-   [#&#8203;2761](derailed/k9s#2761) Enable "jump to owner" for more kinds
-   [#&#8203;2754](derailed/k9s#2754) Plugins not loaded/shown in UI
-   [#&#8203;2747](derailed/k9s#2747) Combining context and namespace switching only works sporadically (e.g. ":pod foo-ns [@&#8203;ctx-dev](https://github.com/ctx-dev)")
-   [#&#8203;2746](derailed/k9s#2746) k9s does not display "\[::]" string in its logs
-   [#&#8203;2738](derailed/k9s#2738) "Faults" view should show all Terminating pods

***

#### Contributed MRs

Please be sure to give `Big Thanks!` and `ATTA Girls/Boys!` to all the fine contributors for making K9s better for all of us!!

-   [#&#8203;2937](derailed/k9s#2937) Adding Argo Rollouts plugin version for PowerShell
-   [#&#8203;2935](derailed/k9s#2935) fix: show all terminating pods in Faults view ([#&#8203;2738](derailed/k9s#2738))
-   [#&#8203;2933](derailed/k9s#2933) chore: broken url in build-status tag in the readme.md
-   [#&#8203;2932](derailed/k9s#2932) fix: add kubeconfig if k9s is launched with --kubeconfig
-   [#&#8203;2930](derailed/k9s#2930) fixed conflicting plugin shortcuts, and added 2 new plugins
-   [#&#8203;2927](derailed/k9s#2927) Fix "Mark Range": reduce maximum namespaces in favorites, fix shadowing of ctrl+space
-   [#&#8203;2926](derailed/k9s#2926) chore(plugins,remove-finalizers): make sure the resources api group is respected
-   [#&#8203;2921](derailed/k9s#2921) feat: Add plugins for kubectl node-shell
-   [#&#8203;2920](derailed/k9s#2920) eat: added StartupProbes status (S) to the PROBES column in the container render
-   [#&#8203;2914](derailed/k9s#2914) Adding eks-node-viewer plugin
-   [#&#8203;2898](derailed/k9s#2898) Add argocd plugin to community plugins
-   [#&#8203;2896](derailed/k9s#2896) feat(2896): Add toggle keda plugin
-   [#&#8203;2890](derailed/k9s#2890) Update README.md
-   [#&#8203;2881](derailed/k9s#2881) Fix Mark-Range command: ensure that NS Favorite doesn't exceed the limit
-   [#&#8203;2861](derailed/k9s#2861) chore: fix function name
-   [#&#8203;2856](derailed/k9s#2856) fix internal/render/hpa.go merge issue
-   [#&#8203;2848](derailed/k9s#2848) Include sidecar containers requests and limits
-   [#&#8203;2844](derailed/k9s#2844) Update README GO Version Required
-   [#&#8203;2830](derailed/k9s#2830) update tview to fix log escaping problem completely
-   [#&#8203;2822](derailed/k9s#2822) Adding HolmesGPT plugin
-   [#&#8203;2821](derailed/k9s#2821) Add a spark-operator plugin
-   [#&#8203;2817](derailed/k9s#2817) Add comment about Escape keybinding
-   [#&#8203;2812](derailed/k9s#2812) fix: align build image Go version with go.mod
-   [#&#8203;2795](derailed/k9s#2795) add new plugin current-ctx-terminal
-   [#&#8203;2791](derailed/k9s#2791) Add leading space to Kubernetes context suggestions
-   [#&#8203;2789](derailed/k9s#2789) Create kubectl-get-in-shell.yaml
-   [#&#8203;2788](derailed/k9s#2788) Update README.md plugin format
-   [#&#8203;2787](derailed/k9s#2787) Update helm-purge.yaml
-   [#&#8203;2786](derailed/k9s#2786) Update README.md with plugin dangerous field
-   [#&#8203;2780](derailed/k9s#2780) install copyright file into correct location
-   [#&#8203;2775](derailed/k9s#2775) fix freebsd build failure
-   [#&#8203;2780](derailed/k9s#2780) install copyright file into correct location
-   [#&#8203;2772](derailed/k9s#2772) proper handle OwnerReference for manually created job
-   [#&#8203;2771](derailed/k9s#2771) feat: add duplik8s plugin
-   [#&#8203;2770](derailed/k9s#2770) feat: allow plugins block in plugin files
-   [#&#8203;2765](derailed/k9s#2765) fix: Shellin -> ShellIn
-   [#&#8203;2763](derailed/k9s#2763) enable "jump to owner" for more kinds
-   [#&#8203;2755](derailed/k9s#2755) Loki plugin
-   [#&#8203;2751](derailed/k9s#2751) container logs should be escaped when printed
-   [#&#8203;2750](derailed/k9s#2750) fix: should switching ctx before ns

***

<img src="https://raw.githubusercontent.com/derailed/k9s/master/assets/imhotep_logo.png" width="32" height="auto"/> © 2024 Imhotep Software LLC. All materials licensed under [Apache v2.0](http://www.apache.org/licenses/LICENSE-2.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Commands Mark Range and Mark Clear do not work
2 participants