Skip to content

Conversation

@joamaki
Copy link
Contributor

@joamaki joamaki commented Oct 6, 2025

Let's move the shell implementation into cilium/hive so it can be easily used in other projects as well.

The main changes from cilium/cilium:

  • Single package as it's not that much code (unless I missed the real reason why they were separate)
  • Cell is now ServerCell and takes as argument the default path for the socket
  • ServerCell creates its own job group. The providing of job.Group as part of cell.Module is only in pkg/hive of cilium/cilium. Maybe should change that...
  • Creating the base directories for the unix socket file is now up to the application so we don't accidentally create the directories with wrong permissions
  • ServerCell prints a info log message when it starts that includes the socket path
  • ShellCmd now takes the default socket path and a function to print the greeting
  • The server now cancels the context to interrupt execution if writes to the connection fail
  • Auto-completion is now supported via the shell as well

Second commit extends the example application to add in the shell server and client.

@joamaki joamaki requested a review from a team as a code owner October 6, 2025 09:19
@joamaki joamaki requested review from pippolo84 and removed request for a team October 6, 2025 09:19
Copy link
Member

@mhofstetter mhofstetter left a comment

Choose a reason for hiding this comment

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

Thanks, makes sense!

Copy link
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

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

Overall LGTM, some nits and cosmetic suggestions left inline

Copy link

@smagnani96 smagnani96 left a comment

Choose a reason for hiding this comment

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

LGTM, minor nit (Cilium reference in the code).

@joamaki joamaki force-pushed the pr/joamaki/shell branch 8 times, most recently from 0bf32c5 to 3fe7e53 Compare October 20, 2025 15:03
joamaki and others added 3 commits October 21, 2025 09:37
Port over the shell server and client from cilium/cilium so it
is easier to use in other projects.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
Adapted from Dylan's prior work.

Authored-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
Add the shell server to the example application and add the
shell command.

To start the example application:

  go run ./example

In another terminal connect to the example application shell:

  go run ./example shell

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki joamaki merged commit 03494cb into main Oct 21, 2025
1 check passed
@joamaki joamaki deleted the pr/joamaki/shell branch October 21, 2025 07:38
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.

6 participants