🚧 [WIP] (POC) feat: talosctl debug
#12392
Conversation
laurazard
force-pushed
the
poc-talos-debug
branch
8 times, most recently
from
ce722ef to
97f4bb2
Compare
laurazard
force-pushed
the
poc-talos-debug
branch
from
97f4bb2 to
431f4bc
Compare
WIP Signed-off-by: Laura Brehm <[email protected]>
Signed-off-by: Laura Brehm <[email protected]>
laurazard
force-pushed
the
poc-talos-debug
branch
3 times, most recently
from
95c259b to
79cd19e
Compare
|
@smira when you get a sec can you TAL and see if you have any early feedback/ideas? |
laurazard
force-pushed
the
poc-talos-debug
branch
from
79cd19e to
3da742a
Compare
In the first attempt at the `DebugContainer` API, a single gRPC endpoint was added (`rpc DebugContainer`) to receive the debug container spec, create and configure the container, and run it + handle IO streams. This patch splits this into two separate RPCs: - `DebugContainerCreate`, and - `DebugContainerRun` This results in a cleaner API with better separation of concerns. Signed-off-by: Laura Brehm <[email protected]>
laurazard
force-pushed
the
poc-talos-debug
branch
from
3da742a to
4d9c6ca
Compare
Signed-off-by: Laura Brehm <[email protected]>
| "/machine.MachineService/ImageList", | ||
| "/machine.MachineService/Kubeconfig", | ||
| "/machine.MachineService/List", | ||
| "/machine.MachineService/DebugContainer", |
| rpc ImageList(ImageListRequest) returns (stream ImageListResponse); | ||
| // ImagePull pulls an image into the CRI. | ||
| rpc ImagePull(ImagePullRequest) returns (ImagePullResponse); | ||
| // DebugContainerCreate |
There was a problem hiding this comment.
leaving some notes after the call:
probably we can move this out to a separate DebugService (I think it makes sense to be machine.DebugService) ?
There was a problem hiding this comment.
I don't remember, but I think apid_test should scream at you as you haven't specified RBAC for it.
If it doesn't scream, let's make sure it screams, and specify RBAC (os:admin).
There was a problem hiding this comment.
oops, I'm blind, I see RBAC changes, but let's double check if we move it to a separate proto, that the test picks it up
| return proxy.One2One, nil, status.Error(codes.InvalidArgument, "one-2-many proxying is not supported for COSI methods") | ||
| } | ||
|
|
||
| if strings.HasPrefix(fullMethodName, "/machine.MachineService/DebugContainer") { |
There was a problem hiding this comment.
turn this inside out, specify methods which support one2many proxying (probably /machine.MachineService and a couple of others ? and everything else should return an error
| } | ||
|
|
||
| // DebugContainerRun implements the machine.MachineServer interface. | ||
| func (s *Server) DebugContainerRun(srv machine.MachineService_DebugContainerRunServer) error { //nolint:gocyclo |
There was a problem hiding this comment.
as we separated the protos, we can separate the implementation into its own DebugServer Go struct which we can now "mix in" into the maintenance/normal mode client.
We should probably double-assert here on os:admin RBAC, as in maintenance mode we assign "os:admin" to SideroLink activated connections, and there is no middleware for RBAC
| func (s *Server) DebugContainerCreate(srv machine.MachineService_DebugContainerCreateServer) error { //nolint:gocyclo | ||
| ctx := srv.Context() | ||
|
|
||
| client, err := containerdapi.New(constants.SystemContainerdAddress, |
There was a problem hiding this comment.
food for thought: should we pick up the containerd to use automatically? if the "CRI" one is up, use it always? and use "system" one only if CRI is not up yet?
2 participants
Pull Request
What? (description)
Why? (reasoning)
Closes #8720
Acceptance
Please use the following checklist:
make conformance)make fmt)make lint)make docs)make unit-tests)