Skip to content

feat(exec): allow interactive (autodetected) shell - #3156

Open
pimvanpelt wants to merge 6 commits into
srl-labs:mainfrom
pimvanpelt:main
Open

feat(exec): allow interactive (autodetected) shell#3156
pimvanpelt wants to merge 6 commits into
srl-labs:mainfrom
pimvanpelt:main

Conversation

@pimvanpelt

Copy link
Copy Markdown
Contributor
This tool looks up the active lab containers (globally, or restricted
by --topo or --name), and allows the user to specify any container
with a substring match. It will use 'docker exec' to connect to, and
run a default shell, by imagename, or by the --shell flag.

If no containername is given, a list is provided. If the given substring
is ambiguous, the available choices are provided as a list.
    
Fixes #3155

This tool looks up the active lab containers (globally, or restricted
by --topo or --name), and allows the user to specify any container
with a substring match. It will use 'docker exec' to connect to, and
run a default shell, by imagename, or by the --shell flag.

If no containername is given, a list is provided. If the given substring
is ambiguous, the available choices are provided as a list.

Fixes srl-labs#3155
@pimvanpelt

Copy link
Copy Markdown
Contributor Author

This is a proof of concept, although it works well. It probably needs work for podman.

@pimvanpelt pimvanpelt changed the title feat(tools): add 'dc' to directly connect to a docker container feat(exec): allow interactive (autodetected) shell Apr 7, 2026
Comment thread cmd/exec.go Outdated
imageKey string
argv []string
}{
{"ceos", []string{"/usr/bin/Cli", "-p", "15"}},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nono no... please don't do it like that...
This is obviousely kind dependent... so make it a field in the kind implementation (and default kind) and expose it via a getter function... don't start all over again with polluting the code in every corner with kind specifics.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the default kind then might do something fuzzy stuff of trying bash first and sh there after or so...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let's discuss in #3155

Comment thread cmd/exec.go
case nodeKnown && node.Config().Shell != "":
shell = strings.Fields(node.Config().Shell)
case nodeKnown:
shell = node.ExecInteractiveShell()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

still execinteractiveshell is missleading...
also see my comment here:
2b5bb90#r181794850

Comment thread nodes/node.go
// ExecInteractiveShell returns the argv for the interactive shell to use
// when the user runs 'exec --interactive'. Nodes override this to provide
// their NOS CLI or a specific shell invocation.
ExecInteractiveShell() []string

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is rather GetInteractiveShellCommand... not ExecInteractiveShell ... nothing is executed really when calling this function

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.

3 participants