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

chore: support debug shell for advanced development #9201

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

frezbo
Copy link
Member

@frezbo frezbo commented Aug 19, 2024

Support dropping into a very minimal debug shell.

sudo -E --preserve-env=HOME _out/talosctl-linux-amd64 cluster create --provisioner=qemu $REGISTRY_MIRROR_FLAGS --controlplanes=1 --workers=0 --with-bootloader=false --with-debug-shell

@dsseng dsseng self-requested a review August 19, 2024 18:36
Copy link
Member

@dsseng dsseng left a comment

Choose a reason for hiding this comment

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

Overall good, will rebase eudev stuff on top of this and try looking further

Dockerfile Outdated Show resolved Hide resolved
@@ -113,6 +114,14 @@ func (check *preflightCheckContext) swtpmExecutable(ctx context.Context) error {
return nil
}

func (check *preflightCheckContext) numberOfNodesWhenDebugShellEnabled(ctx context.Context) error {
if check.options.WithDebugShell && len(check.request.Nodes.ControlPlaneNodes())+len(check.request.Nodes.WorkerNodes()) > 1 {
return fmt.Errorf("error: --with-debug-shell is not supported with more than one node")
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to restrict this? I believe there's zero issue having multiple sockets, accessible like socat - UNIX-CONNECT:/tmp/tl-test-home/.talos/clusters/talos-default/talos-default-controlplane-1.serial

Copy link
Member Author

Choose a reason for hiding this comment

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

oh this was when i was using graphical window to debug, then more than one is a little messy 😅

Makefile Show resolved Hide resolved
@dsseng dsseng self-assigned this Oct 5, 2024
@dsseng
Copy link
Member

dsseng commented Oct 5, 2024

@frezbo Won't you mind if I push subsequent changes I do to this PR and finalize it to be merged? It helps debugging udev+selinux, so might help us in the future as well. Thanks for implementing this feature!

@frezbo
Copy link
Member Author

frezbo commented Oct 5, 2024

@frezbo Won't you mind if I push subsequent changes I do to this PR and finalize it to be merged? It helps debugging udev+selinux, so might help us in the future as well. Thanks for implementing this feature!

👍

frezbo and others added 4 commits October 6, 2024 14:24
Support dropping into a very minimal debug shell.

```bash
sudo -E --preserve-env=HOME _out/talosctl-linux-amd64 cluster create --provisioner=qemu $REGISTRY_MIRROR_FLAGS --controlplanes=1 --workers=0 --with-bootloader=false --with-debug-shell
```

Signed-off-by: Noel Georgi <[email protected]>
Actually we don't have any issues creating multiple sockets

Signed-off-by: Dmitry Sharshakov <[email protected]>
@dsseng dsseng marked this pull request as ready for review October 6, 2024 12:25
Signed-off-by: Dmitry Sharshakov <[email protected]>
@dsseng
Copy link
Member

dsseng commented Oct 10, 2024

@frezbo could you please take a look at my changes?

@@ -42,6 +42,8 @@ ARG PKG_CNI
ARG PKG_FLANNEL_CNI
ARG PKG_TALOSCTL_CNI_BUNDLE_INSTALL

ARG DEBUG_TOOLS_SOURCE
Copy link
Member Author

Choose a reason for hiding this comment

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

this is not used anywhere below, so all the debug stuff would get copied always which we don't want

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants