-
Notifications
You must be signed in to change notification settings - Fork 20
Add end-to-end exec support across controller, worker, and CLI #359
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
|
Is the idea here is to be able to run commands inside VMs which doesn't have SSH enabled? If SSH enabled VM is an option then Orchard controller then controller as an SSH jump host might be an option. |
|
You’ve got it—the driver is exactly those cases where SSH isn’t available or desirable. Orchard’s exec path lets us reach VMs without opening SSH ports or distributing keys, and it keeps everything on the controller’s existing channel (auth, audit, policy). If a VM already exposes SSH and you’re comfortable running a jump host, that can work, but it reintroduces credential management, extra network surface, and more moving parts. Exec avoids that overhead while still giving us the command execution capability we need. That's the thinking, anyway! |
|
I think we should start implementing #332 as a simple non-interactive endpoint first. You pass the command to execute, and optionally a pre-determined standard input contents, and get a This should solve most of the day-to-day use-cases and wouldn't require the user to deal with WebSockets. Would that work for you? |
|
Makes sense to me! I agree that simple commands should be simple to integrate with (and only interactive should need to upgrade to a websocket). |
|
@edigaryev thinking this through there are two neat ways we can slice the minimal API: Option 1 · Simplicity First Option 2 · Upgradable That extra hop gives us a stable session resource we can reuse later—to reconnect to a long-running command, expose metadata (exit code, duration), or negotiate a WebSocket by hitting the same URL with Option 1 is dead simple today; Option 2 is almost as simple but buys a clear path to interactivity and reconnection without revisiting the public contract. Does Option 2 line up with what you had in mind, or would you rather we keep to Option 1 for the first cut? |
If merged, this PR will:
internal/command/exec/*.go,pkg/client/vms.go)internal/controller/api_vms_exec.go,internal/controller/api_rpc_exec.go,internal/controller/api_rpc_watch.go,pkg/client/rpc.go,rpc/constants.go)internal/worker/exec.go,internal/worker/rpc.go,internal/worker/rpcv2.go,internal/worker/worker.go)internal/execstream/frame.go,rpc/guestagent/guestagent.proto,rpc/guestagent/*.pb.go)pkg/resource/v1/v1.go,pkg/resource/v1/worker.go,pkg/resource/v1/watch_instruction.go,internal/controller/api_controller.go)/bin/echo(internal/tests/integration_test.go)Testing:
go test ./...go test ./internal/tests -tags=integration--interactiveexec with--ttyto confirm resize and stdin handling