Skip to content

Conversation

@dorser
Copy link
Contributor

@dorser dorser commented Nov 15, 2025

Summary

This PR implements #4167. This work aligns node debugger workflow with kubectl debug node. It provisions a busybox debugger container, attaches without running shell commands, and gracefully terminates the session on close (This also fixes the behavior when the window closes abruptly).

Related Issue

Implements #4167

Changes

  • Updated frontend/src/components/node/NodeShellAction.tsx to rename the entry point to “Debug Node” and use a debugger icon.
  • Updated frontend/src/helpers/clusterSettings.ts to default the node debugger image to busybox and run it in the default namespace.
  • Reworked frontend/src/components/node/NodeShellTerminal.tsx so the generated pod mirrors kubectl debug node (host mounts, tolerations, stdin/tty flags, attach URL) and so the session is cleaned up by sending exit on close/beforeunload/unmount.

Steps to Test

  1. Build and run Headlamp
  2. Navigate to any Linux node and click the “Debug Node” action.
  3. Verify that a node-debugger-* pod appears in the configured namespace, that the terminal attaches without launching a new shell command, and that closing the dialog/tab finishes the pod after an exit is sent.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 15, 2025
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 15, 2025
@dorser dorser force-pushed the dorser/node-debugger branch 4 times, most recently from f094d39 to a27ffc2 Compare November 16, 2025 10:29
@illume illume added kind/feature Categorizes issue or PR as related to a new feature. frontend Issues related to the frontend labels Nov 17, 2025
@joaquimrocha
Copy link
Contributor

@skoeva , can you test this PR?

@illume illume requested a review from Copilot November 19, 2025 11:43
Copilot finished reviewing on behalf of illume November 19, 2025 11:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the node debugging feature to align with kubectl debug node behavior. It transitions from a privileged shell-based approach using alpine to an attach-based debugger workflow using busybox, changes the default namespace from kube-system to default, and implements graceful session cleanup.

Key changes:

  • Replaces exec-based shell launching with attach-based debugging workflow
  • Changes default image from alpine to busybox and default namespace from kube-system to default
  • Implements exit command-based cleanup instead of explicit pod deletion
  • Updates UI labels from "Node Shell" to "Debug Node"

Reviewed Changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
frontend/src/components/node/NodeShellTerminal.tsx Core workflow refactoring: pod spec now mirrors kubectl debug node with host mounts, tolerations, stdin/tty flags, and attach URL; adds exit-based cleanup handlers
frontend/src/components/node/NodeShellAction.tsx Updates UI label to "Debug Node" and changes icon to mdi:bug
frontend/src/helpers/clusterSettings.ts Changes default image to busybox:latest and default namespace to 'default'
frontend/src/components/App/Settings/NodeShellSettings.tsx Updates settings UI to reflect new namespace default
frontend/src/i18n/locales/*/glossary.json Updates translations for "Node Shell" → "Debug Node" and error messages
frontend/src/i18n/locales/*/translation.json Updates namespace default description across all locales
frontend/src/components/App/Settings/snapshots/NodeShellSettings.Default.stories.storyshot Updates snapshot to reflect new defaults

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dorser dorser force-pushed the dorser/node-debugger branch from 27c0491 to 4e42520 Compare November 19, 2025 20:23
@joaquimrocha
Copy link
Contributor

@farodin91 , do you have any comments? I thought of asking you since you have contributed the node-shell changes.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2025
Signed-off-by: Dor Serero <dor.serero@gmail.com>
Signed-off-by: Dor Serero <dor.serero@gmail.com>
@dorser dorser force-pushed the dorser/node-debugger branch from 4e42520 to 9f4ec16 Compare November 23, 2025 09:24
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 23, 2025
@joaquimrocha
Copy link
Contributor

I have read more about the differences in debug vs the approach we had. I like how tit makes it closer to the kubectl debug.
I have also tested and it works fine. Thanks @dorser !

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 27, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dorser, joaquimrocha

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 27, 2025
@k8s-ci-robot k8s-ci-robot merged commit dff0ea0 into kubernetes-sigs:main Nov 27, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. frontend Issues related to the frontend kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants