List all k8s debuggable containers #84
Conversation
Signed-off-by: Evan Harris <[email protected]>
Signed-off-by: Evan Harris <[email protected]>
| return | ||
| } | ||
|
|
||
| if commandParams.ActionListDebuggableContainers { |
There was a problem hiding this comment.
@eharris128 what was the reason to move commandParams.ActionListDebuggableContainers here? Was the intent to support listing debuggable containers across all pods in the (selected) namespace?
There was a problem hiding this comment.
Yes exactly. Although what you point out below was neglected with this approach.
| } | ||
|
|
||
| for cname, iname := range result { | ||
| xc.Out.Info("debuggable.container", ovars{"name": cname, "image": iname}) |
There was a problem hiding this comment.
@eharris128 if the intent is to show all debuggable containers across all pods then we need the pod name for each record we are printing here.
There was a problem hiding this comment.
Agreed. Definitely a necessary extension.
| "ec.count": len(pod.Spec.EphemeralContainers), | ||
| }).Debug("target pod info") | ||
|
|
||
| if commandParams.ActionListDebuggableContainers { |
There was a problem hiding this comment.
@eharris128 it might still be good to have an option to list all debuggable containers in a given pod, but if we have both options (to list them for all pods and for a specific pod) then we'll need a separate flag for this, so you can explicitly select what option you want.
There was a problem hiding this comment.
Yeah supporting both paths - per pod and across pods seems reasonable to me.
I do not remember how the --list-debuggable-containers flag performs right now (or before my change) if one passed in --pod as well.
Feels like adding a new flag to differentiate by one pod versus all pods would be the clearest UX.
There was a problem hiding this comment.
@eharris128 how about --list-debuggable-pod-containers
There was a problem hiding this comment.
--list-debuggable-pod-containers For this flag we would require users to provide a --pod as well?
Making sure I follow.
|
@eharris128 merging... do you mind adding the enhancements in a new PR |
Yes for sure. I mentioned above but |
It doesn't need to be an explicit pod parameter. It can be the default pod name based on the logic that's there already where we "ensure" the pod name. If a pod flag is not specified we pick the first available pod. |
@eharris128 We already have the |
Yup makes sense. I will add in a |
2 participants
What
make fmtagainst syscall filesWhy
How Tested
Screencast.from.2024-11-07.15-20-10.webm