From ce44d86726bc1634b7f0ea861c54023b01f378cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Mik=C3=B3?= Date: Mon, 27 Jul 2026 15:59:07 +0200 Subject: [PATCH] feat(taints): add a picker for widely used taints Adding a taint meant typing a long key by hand (node-role.kubernetes.io/control-plane, node.kubernetes.io/out-of-service, the cloud-provider spot taints). A typo produces a taint nothing tolerates, silently. Press p in the taint editor's list to pick from 12 real, documented taints, each with a note on what applying it does. Selecting one fills the add-row key, value and effect and focuses the key field, so a preset is a starting point rather than a commitment: all three fields stay editable and the taint still goes through the normal validation and staging path. p mirrors a for manual add. A ctrl binding was avoidable here, and the app-wide "." presets key was unusable: a taint key legitimately contains dots, so the add-row inputs must keep it. The node-controller-managed condition taints (not-ready, unreachable, memory-pressure, disk-pressure, pid-pressure, network-unavailable) are deliberately excluded -- the controller adds and removes them from live node conditions, so a hand-applied copy is either redundant or fought over. A test enforces the exclusion. Both taint overlays share one case in handleOverlayKeySecondary and renderOverlayContentExtended and split in handleTaintOverlayKey / renderTaintOverlay: those dispatchers sit at the gocyclo cap, and view_overlays.go at the file-length cap. --- docs/keybindings.md | 2 +- internal/app/app_types.go | 1 + internal/app/overlay_hintbar.go | 2 + internal/app/taint_editor.go | 3 + internal/app/taint_editor_keys.go | 3 + internal/app/taint_presets.go | 149 +++++++++++++++++++++++++++ internal/app/taint_presets_test.go | 131 +++++++++++++++++++++++ internal/app/update_overlays.go | 4 +- internal/app/view_overlays.go | 4 +- internal/model/taint_presets.go | 70 +++++++++++++ internal/model/taint_presets_test.go | 66 ++++++++++++ 11 files changed, 430 insertions(+), 5 deletions(-) create mode 100644 internal/app/taint_presets.go create mode 100644 internal/app/taint_presets_test.go create mode 100644 internal/model/taint_presets.go create mode 100644 internal/model/taint_presets_test.go diff --git a/docs/keybindings.md b/docs/keybindings.md index c8e16bc7..41b777a2 100644 --- a/docs/keybindings.md +++ b/docs/keybindings.md @@ -1056,7 +1056,7 @@ The Scale overlay edits the HPA's `spec.minReplicas` / `spec.maxReplicas` (the H `e` ConfigMap Editor, `v` Describe, `E` Edit, `D` Delete, `l` Labels / Annotations, `P` Permissions, `b` Debug Pod, `V` Events ### Node Actions -`c` Cordon/Uncordon (toggle schedulability), `n` Drain, `t` Taints (editor: mark taints for removal with `space`, add with `a`, apply with `enter`), `s` Shell, `v` Describe, `E` Edit, `b` Debug Pod, `V` Events +`c` Cordon/Uncordon (toggle schedulability), `n` Drain, `t` Taints (editor: mark taints for removal with `space`, add with `a`, pick a common taint with `p`, apply with `enter`), `s` Shell, `v` Describe, `E` Edit, `b` Debug Pod, `V` Events Drain streams the eviction progress live: in PTY terminal mode (default on macOS/Linux) the `kubectl drain` output renders in lfk's embedded scrollable terminal; in Exec mode the host terminal is handed over. The same applies to Drain Node on a Karpenter NodeClaim. diff --git a/internal/app/app_types.go b/internal/app/app_types.go index 78d4020e..7f1de14e 100644 --- a/internal/app/app_types.go +++ b/internal/app/app_types.go @@ -106,6 +106,7 @@ const ( overlayLogTopProfile // single-select log format profile picker for Log Top overlayLogTopColumns // show/hide and reorder column picker for Log Top overlayTaintEditor // node taint editor (action menu key t on a Node) + overlayTaintPresets // common-taint picker over the taint editor (p key) ) // whoCanState groups the reverse-RBAC ("Who-Can") fields so they live diff --git a/internal/app/overlay_hintbar.go b/internal/app/overlay_hintbar.go index 8412f263..2f881751 100644 --- a/internal/app/overlay_hintbar.go +++ b/internal/app/overlay_hintbar.go @@ -147,6 +147,8 @@ func (m Model) overlayHintBarSelector() string { return m.renderHints(copyFieldPickerHints()) case overlayTaintEditor: return m.renderHints(m.taintEditorHints()) + case overlayTaintPresets: + return m.renderHints(taintPresetHints()) case overlayPortForward: return m.renderHints([]ui.HintEntry{ {Key: "j/k", Desc: "select port"}, diff --git a/internal/app/taint_editor.go b/internal/app/taint_editor.go index fd50daa6..b19c3424 100644 --- a/internal/app/taint_editor.go +++ b/internal/app/taint_editor.go @@ -47,6 +47,9 @@ type taintEditorState struct { addEff int // index into model.ValidTaintEffects loading bool seq int // fetch sequence — stale taintsLoadedMsg are dropped + // Common-taint picker (overlayTaintPresets), opened over the editor. + presetCursor int + presetScroll int } // taintsLoadedMsg carries the fetched node taints for the editor. diff --git a/internal/app/taint_editor_keys.go b/internal/app/taint_editor_keys.go index e9c000a6..56c1664a 100644 --- a/internal/app/taint_editor_keys.go +++ b/internal/app/taint_editor_keys.go @@ -28,6 +28,8 @@ func (m Model) handleTaintEditorKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { case "a": p.focus = taintFocusKey return m, nil + case "p": + return m.openTaintPresets() case " ": if p.cursor < 0 || p.cursor >= n { return m, nil @@ -175,6 +177,7 @@ func (m Model) taintEditorHints() []ui.HintEntry { return []ui.HintEntry{ {Key: "space", Desc: "mark remove"}, {Key: "a", Desc: "add"}, + {Key: "p", Desc: "common taints"}, {Key: "j/k", Desc: "navigate"}, {Key: "enter", Desc: "apply"}, {Key: "esc", Desc: "cancel"}, diff --git a/internal/app/taint_presets.go b/internal/app/taint_presets.go new file mode 100644 index 00000000..af8441d4 --- /dev/null +++ b/internal/app/taint_presets.go @@ -0,0 +1,149 @@ +package app + +import ( + "slices" + + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" + + "github.com/janosmiko/lfk/internal/model" + "github.com/janosmiko/lfk/internal/ui" +) + +// handleTaintOverlayKey and renderTaintOverlay route the editor and its +// picker as one feature. The dispatchers they hang off (handleOverlayKeySecondary, +// renderOverlayContentExtended) sit at the gocyclo cap, so the two overlays +// share one case there and split here instead. +func (m Model) handleTaintOverlayKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { + if m.overlay == overlayTaintPresets { + return m.handleTaintPresetKey(msg) + } + return m.handleTaintEditorKey(msg) +} + +func (m Model) renderTaintOverlay() (string, int, int) { + if m.overlay == overlayTaintPresets { + return m.renderOverlayTaintPresets() + } + return m.renderOverlayTaintEditor() +} + +// openTaintPresets opens the common-taint picker over the editor. The editor +// state stays live underneath so Esc returns to the list exactly as it was. +func (m Model) openTaintPresets() (tea.Model, tea.Cmd) { + m.taintEditor.presetCursor = 0 + m.taintEditor.presetScroll = 0 + m.overlay = overlayTaintPresets + return m, nil +} + +// closeTaintPresets returns to the editor without adopting anything — the +// picker writes to the add-row only on an explicit selection. +func (m *Model) closeTaintPresets() { + m.overlay = overlayTaintEditor +} + +// taintPresetsMaxVisible caps the picker rows shown at once, shared by the +// renderer and the scroll clamp so the scrollbar and cursor stay in sync. +func (m Model) taintPresetsMaxVisible() int { + return min(12, max(m.height-12, 3)) +} + +// handleTaintPresetKey drives the common-taint picker: vim navigation, Enter +// to adopt the highlighted preset, Esc to back out. +func (m Model) handleTaintPresetKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { + p := &m.taintEditor + n := len(model.CommonTaints) + visible := m.taintPresetsMaxVisible() + switch msg.String() { + case "esc", "q": + m.closeTaintPresets() + return m, nil + case "enter": + return m.applyTaintPreset() + case "j", "down": + if p.presetCursor < n-1 { + p.presetCursor++ + } + case "k", "up": + if p.presetCursor > 0 { + p.presetCursor-- + } + case "g", "home": + p.presetCursor = 0 + case "G", "end": + p.presetCursor = max(n-1, 0) + case "ctrl+d": + p.presetCursor = min(p.presetCursor+visible/2, max(n-1, 0)) + case "ctrl+u": + p.presetCursor = max(p.presetCursor-visible/2, 0) + case "ctrl+f", "pgdown", "shift+down": + p.presetCursor = min(p.presetCursor+visible, max(n-1, 0)) + case "ctrl+b", "pgup", "shift+up": + p.presetCursor = max(p.presetCursor-visible, 0) + } + p.presetCursor = max(0, min(p.presetCursor, n-1)) + overlayListScroll(&p.presetScroll, p.presetCursor, n, visible) + return m, nil +} + +// applyTaintPreset copies the highlighted preset into the add-row and returns +// to the editor with the key field focused. The preset is a starting point, +// not a commitment: all three fields stay editable and the taint still goes +// through the normal validation and staging path. +func (m Model) applyTaintPreset() (tea.Model, tea.Cmd) { + p := &m.taintEditor + if p.presetCursor < 0 || p.presetCursor >= len(model.CommonTaints) { + m.closeTaintPresets() + return m, nil + } + t := model.CommonTaints[p.presetCursor].Taint + p.addKey = t.Key + p.addVal = t.Value + p.addEff = max(slices.Index(model.ValidTaintEffects, t.Effect), 0) + p.focus = taintFocusKey + m.closeTaintPresets() + return m, nil +} + +// renderOverlayTaintPresets paints the common-taint picker: each preset in +// kubectl notation with a note on what applying it does. +func (m Model) renderOverlayTaintPresets() (string, int, int) { + if !m.taintEditor.active { + return "", 0, 0 + } + p := m.taintEditor + items := make([]ui.OverlayListItem, len(model.CommonTaints)) + for i, preset := range model.CommonTaints { + items[i] = ui.OverlayListItem{ + Name: preset.Taint.String(), + Description: preset.Desc, + } + } + cfg := ui.OverlayListConfig{ + Title: "Common taints", + Cursor: p.presetCursor, + ShowDescription: true, + Scroll: p.presetScroll, + MaxVisible: m.taintPresetsMaxVisible(), + } + + overlayW := ui.OverlayListWidth(items, cfg, max(m.width-10, 1)) + innerW := max(overlayW-4, 1) + for i := range items { + items[i].Name = ui.Truncate(items[i].Name, innerW) + } + + content := ui.RenderOverlayList(items, cfg, innerW) + overlayH := min(lipgloss.Height(content)+2, max(m.height-4, 3)) + return content, overlayW, overlayH +} + +// taintPresetHints is the picker's bottom hint bar. +func taintPresetHints() []ui.HintEntry { + return []ui.HintEntry{ + {Key: "j/k", Desc: "navigate"}, + {Key: "enter", Desc: "use taint"}, + {Key: "esc", Desc: "back"}, + } +} diff --git a/internal/app/taint_presets_test.go b/internal/app/taint_presets_test.go new file mode 100644 index 00000000..2b53dd2b --- /dev/null +++ b/internal/app/taint_presets_test.go @@ -0,0 +1,131 @@ +package app + +import ( + "testing" + + tea "github.com/charmbracelet/bubbletea" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/janosmiko/lfk/internal/model" +) + +// taintPresetKey drives the preset picker, which owns its own overlay and so +// routes through the overlay dispatcher rather than the editor's handler. +func taintPresetKey(t *testing.T, m Model, keys ...string) Model { + t.Helper() + for _, k := range keys { + var msg tea.KeyMsg + switch k { + case "enter": + msg = tea.KeyMsg{Type: tea.KeyEnter} + case "esc": + msg = tea.KeyMsg{Type: tea.KeyEsc} + default: + msg = tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(k)} + } + mdl, _ := m.handleTaintPresetKey(msg) + var ok bool + m, ok = mdl.(Model) + require.True(t, ok) + } + return m +} + +func TestTaintEditor_POpensPresetPicker(t *testing.T) { + m := openTaintEditorLoaded(t, taintEditorTestModel()) + m = taintKey(t, m, "p") + + assert.Equal(t, overlayTaintPresets, m.overlay) + assert.True(t, m.taintEditor.active, "the editor stays alive underneath") +} + +// Selecting a preset fills all three add-row fields and drops the user on the +// key field, so every part stays editable before staging. +func TestTaintPresets_SelectFillsAddRowAndFocusesKey(t *testing.T) { + m := openTaintEditorLoaded(t, taintEditorTestModel()) + m = taintKey(t, m, "p") + m = taintPresetKey(t, m, "j", "enter") + + want := model.CommonTaints[1].Taint + assert.Equal(t, overlayTaintEditor, m.overlay) + assert.Equal(t, want.Key, m.taintEditor.addKey) + assert.Equal(t, want.Value, m.taintEditor.addVal) + assert.Equal(t, want.Effect, model.ValidTaintEffects[m.taintEditor.addEff]) + assert.Equal(t, taintFocusKey, m.taintEditor.focus) +} + +// A picked preset is a starting point, not a commitment: it must still go +// through the normal validation and staging path. +func TestTaintPresets_PickedPresetStages(t *testing.T) { + m := openTaintEditorLoaded(t, taintEditorTestModel()) + before := len(m.taintEditor.rows) + m = taintKey(t, m, "p") + m = taintPresetKey(t, m, "enter") + m = taintKey(t, m, "enter") + + require.Len(t, m.taintEditor.rows, before+1) + staged := m.taintEditor.rows[before] + assert.True(t, staged.staged) + assert.Equal(t, model.CommonTaints[0].Taint.String(), staged.taint.String()) +} + +// Esc backs out to the editor without adopting the highlighted preset — the +// picker only writes to the add-row on an explicit selection. +func TestTaintPresets_EscAdoptsNothing(t *testing.T) { + m := openTaintEditorLoaded(t, taintEditorTestModel()) + m = taintKey(t, m, "p") + m = taintPresetKey(t, m, "j", "esc") + + assert.Equal(t, overlayTaintEditor, m.overlay) + assert.Empty(t, m.taintEditor.addKey, "esc must not fill the add-row") + assert.Equal(t, taintFocusList, m.taintEditor.focus) +} + +func TestTaintPresets_CursorClampsAtBothEnds(t *testing.T) { + m := openTaintEditorLoaded(t, taintEditorTestModel()) + m = taintKey(t, m, "p") + + m = taintPresetKey(t, m, "k") + assert.Equal(t, 0, m.taintEditor.presetCursor, "cursor clamps at the top") + + m = taintPresetKey(t, m, "G") + assert.Equal(t, len(model.CommonTaints)-1, m.taintEditor.presetCursor) + + m = taintPresetKey(t, m, "j") + assert.Equal(t, len(model.CommonTaints)-1, m.taintEditor.presetCursor, "cursor clamps at the bottom") + + m = taintPresetKey(t, m, "g") + assert.Equal(t, 0, m.taintEditor.presetCursor) +} + +func TestRenderOverlayTaintPresets_ShowsTaintsAndDescriptions(t *testing.T) { + m := openTaintEditorLoaded(t, taintEditorTestModel()) + m = taintKey(t, m, "p") + + content, w, h := m.renderOverlayTaintPresets() + require.NotEmpty(t, content) + assert.Positive(t, w) + assert.Positive(t, h) + + plain := stripANSI(content) + assert.Contains(t, plain, "node-role.kubernetes.io/control-plane:NoSchedule") + assert.Contains(t, plain, "Reserve control-plane nodes") +} + +// The picker's hints must describe the picker, not the editor underneath. +func TestTaintPresets_HintBar(t *testing.T) { + m := openTaintEditorLoaded(t, taintEditorTestModel()) + m = taintKey(t, m, "p") + + hints := stripANSI(m.overlayHintBar()) + assert.Contains(t, hints, "use taint") +} + +// The editor's own hint bar must advertise the picker, or nobody finds it. +func TestTaintEditor_HintBarAdvertisesPresets(t *testing.T) { + m := openTaintEditorLoaded(t, taintEditorTestModel()) + + hints := stripANSI(m.overlayHintBar()) + assert.Contains(t, hints, "common taints") +} diff --git a/internal/app/update_overlays.go b/internal/app/update_overlays.go index 7786e642..11627a3a 100644 --- a/internal/app/update_overlays.go +++ b/internal/app/update_overlays.go @@ -267,8 +267,8 @@ func (m Model) handleOverlayKeySecondary(msg tea.KeyMsg) (tea.Model, tea.Cmd, bo case overlayCopyField: mdl, cmd := m.handleCopyFieldPickerKey(msg) return mdl, cmd, true - case overlayTaintEditor: - mdl, cmd := m.handleTaintEditorKey(msg) + case overlayTaintEditor, overlayTaintPresets: + mdl, cmd := m.handleTaintOverlayKey(msg) return mdl, cmd, true } return m, nil, false diff --git a/internal/app/view_overlays.go b/internal/app/view_overlays.go index 8e472091..2c6c89c3 100644 --- a/internal/app/view_overlays.go +++ b/internal/app/view_overlays.go @@ -323,8 +323,8 @@ func (m Model) renderOverlayContentExtended() (string, int, int, bool) { case overlayCopyField: c, w, h := m.renderOverlayCopyField() return c, w, h, true - case overlayTaintEditor: - c, w, h := m.renderOverlayTaintEditor() + case overlayTaintEditor, overlayTaintPresets: + c, w, h := m.renderTaintOverlay() return c, w, h, true case overlayLogTopGroupBy: c, w, h := m.renderLogTopGroupByOverlay() diff --git a/internal/model/taint_presets.go b/internal/model/taint_presets.go new file mode 100644 index 00000000..69695369 --- /dev/null +++ b/internal/model/taint_presets.go @@ -0,0 +1,70 @@ +package model + +// TaintPreset is a well-known taint offered by the editor's picker, with a +// short note on what applying it does. +type TaintPreset struct { + Taint Taint + Desc string +} + +// CommonTaints are widely used taints a user applies by hand, offered as +// starting points in the taint editor — the keys are long and easy to +// mistype, and a typo silently produces a taint nothing tolerates. +// +// The node-controller-managed condition taints (not-ready, unreachable, +// memory-pressure, disk-pressure, pid-pressure, network-unavailable) are +// deliberately absent: the controller adds and removes them from live node +// conditions, so a hand-applied copy is either redundant or fought over. +// +// Every entry is a real taint documented by Kubernetes or by the platform +// that applies it; values stay editable after picking. +var CommonTaints = []TaintPreset{ + { + Taint{Key: "node-role.kubernetes.io/control-plane", Effect: "NoSchedule"}, + "Reserve control-plane nodes", + }, + { + Taint{Key: "CriticalAddonsOnly", Value: "true", Effect: "NoSchedule"}, + "System addons only (AKS, GKE, kubeadm)", + }, + { + Taint{Key: "dedicated", Value: "", Effect: "NoSchedule"}, + "Dedicate nodes to one workload — set the value", + }, + { + Taint{Key: "node.kubernetes.io/unschedulable", Effect: "NoSchedule"}, + "Cordon: block new pods, keep running ones", + }, + { + Taint{Key: "node.kubernetes.io/out-of-service", Value: "nodeshutdown", Effect: "NoExecute"}, + "Shut-down node: force pod eviction and volume detach", + }, + { + Taint{Key: "nvidia.com/gpu", Value: "present", Effect: "NoSchedule"}, + "Reserve GPU nodes for GPU workloads", + }, + { + Taint{Key: "node.cilium.io/agent-not-ready", Effect: "NoSchedule"}, + "Hold pods until the Cilium agent is ready", + }, + { + Taint{Key: "node.cloudprovider.kubernetes.io/uninitialized", Effect: "NoSchedule"}, + "Hold pods until the cloud controller initializes the node", + }, + { + Taint{Key: "karpenter.sh/disrupted", Effect: "NoSchedule"}, + "Karpenter is disrupting this node", + }, + { + Taint{Key: "cloud.google.com/gke-spot", Value: "true", Effect: "NoSchedule"}, + "GKE Spot nodes: opt-in workloads only", + }, + { + Taint{Key: "kubernetes.azure.com/scalesetpriority", Value: "spot", Effect: "NoSchedule"}, + "AKS Spot nodes: opt-in workloads only", + }, + { + Taint{Key: "eks.amazonaws.com/compute-type", Value: "fargate", Effect: "NoSchedule"}, + "EKS Fargate nodes", + }, +} diff --git a/internal/model/taint_presets_test.go b/internal/model/taint_presets_test.go new file mode 100644 index 00000000..fd60eb31 --- /dev/null +++ b/internal/model/taint_presets_test.go @@ -0,0 +1,66 @@ +package model + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Every preset must pass the same validation a hand-typed taint faces — +// a preset that cannot be staged is worse than no preset. +func TestCommonTaints_AllValid(t *testing.T) { + require.NotEmpty(t, CommonTaints) + for _, p := range CommonTaints { + t.Run(p.Taint.String(), func(t *testing.T) { + assert.NoError(t, ValidateTaint(p.Taint)) + assert.NotEmpty(t, p.Desc, "each preset explains what it does") + }) + } +} + +// A node cannot carry two taints with the same key+effect, so offering two +// presets with one identity would guarantee a rejected staging. +func TestCommonTaints_NoDuplicateIdentities(t *testing.T) { + seen := make(map[taintIdentity]bool, len(CommonTaints)) + for _, p := range CommonTaints { + id := taintIdentity{p.Taint.Key, p.Taint.Effect} + assert.False(t, seen[id], "duplicate preset identity: %s", p.Taint.String()) + seen[id] = true + } +} + +// The node controller owns the condition taints from live node conditions. +// Offering them for hand-application invites a fight with the controller. +func TestCommonTaints_ExcludeControllerManagedConditions(t *testing.T) { + managed := []string{ + "node.kubernetes.io/not-ready", + "node.kubernetes.io/unreachable", + "node.kubernetes.io/memory-pressure", + "node.kubernetes.io/disk-pressure", + "node.kubernetes.io/pid-pressure", + "node.kubernetes.io/network-unavailable", + } + for _, p := range CommonTaints { + for _, m := range managed { + assert.NotEqual(t, m, p.Taint.Key, + "%s is node-controller managed and must not be a preset", m) + } + } +} + +// dedicated= is the one preset intentionally shipped without a value: it is a +// convention whose value names the workload, so the picker must leave the user +// to fill it in rather than inventing one. +func TestCommonTaints_DedicatedHasNoValue(t *testing.T) { + for _, p := range CommonTaints { + if p.Taint.Key == "dedicated" { + assert.Empty(t, p.Taint.Value) + assert.True(t, strings.Contains(p.Desc, "value"), + "description must tell the user to set the value") + return + } + } + t.Fatal("dedicated preset missing") +}