-
Notifications
You must be signed in to change notification settings - Fork 302
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
Add agentless mode to debug target pod when there isn't an agent running on target host #31
Conversation
Build debug-agent image from distroless image (aylei#30)
Looks awesome! I will give a review ASAP ;) |
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.
Thank you for your excellent work! It is generally looks good to me. I've left some comments to discuss the codes that we may want to refine it now.
pkg/plugin/cmd.go
Outdated
@@ -386,6 +479,7 @@ func (o *DebugOptions) Run() error { | |||
withCleanUp := func() error { | |||
return interrupt.Chain(nil, func() { | |||
if o.Fork { | |||
fmt.Println("delete forked pod.....") |
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.
Prefer fmt.Sprintln(o, "delete forked pod.....")
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.
Sorry, I made a typo in the comment. It should be fmt.Fprintln
pkg/plugin/cmd.go
Outdated
@@ -399,6 +493,15 @@ func (o *DebugOptions) Run() error { | |||
close(o.StopChannel) | |||
} | |||
} | |||
// delete agent pod | |||
if o.AgentLess && agentPod != nil { | |||
fmt.Println("delete agent pod.....") |
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.
ditto
pkg/plugin/cmd.go
Outdated
log.Println("waiting for container running...") | ||
event, err := watch.UntilWithoutRetry(ctx, watcher, conditions.PodRunning) | ||
if err != nil { | ||
fmt.Printf("w:%v", err) |
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.
ditto, and what is w
mean? maybe more specific
pkg/plugin/cmd.go
Outdated
return nil, err | ||
} | ||
|
||
fmt.Printf("Agent Pod info : \n\tName:%s\n\tNamespace:%s\n\tHostPort:%d\n\n", agentPod.GetObjectMeta().GetName(), agentPod.GetObjectMeta().GetNamespace(), o.AgentPort) |
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.
ditto
pkg/plugin/cmd.go
Outdated
@@ -149,6 +202,14 @@ func NewDebugCmd(streams genericclioptions.IOStreams) *cobra.Command { | |||
"Debug agent daemonset name when using port-forward") | |||
cmd.Flags().StringVar(&opts.DebugAgentNamespace, "daemonset-ns", opts.DebugAgentNamespace, | |||
"Debug agent namespace, default to 'default'") | |||
// flags used for agentless mode. | |||
cmd.Flags().BoolVar(&opts.AgentLess, "agent-less", false, "Whether to turn on agentless mode. Agentless mode: debug target pod if there isn't an agent running on the target host.") |
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.
"agentless" is more accurate
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.
And could we add a shorthand for this flag?
pkg/plugin/cmd.go
Outdated
defaultAgentImage = "aylei/debug-agent:latest" | ||
defaultAgentPodNamePrefix = "debug-agent-pod-" | ||
defaultAgentPodNamespace = "default" | ||
defaultAgentPodTemplate = ` |
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.
For type safety and flexibility, can we construct the Pod
object via the client-go interface instead of YAML template?
Thanks! I'll refine it ASAP! :) |
Hi aylei,the latest commit constructs the agent pod via client-go and refines the way to print message. But I'm not sure the way is good enough, so please help me check it again. By the way, I'm not skilled in Golang, so could you please tell me why |
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.
LGTM
@tkanng
So, generally, if some message is ought to notify the end user, it should use |
Got it! Thank you very much! |
Thank you! |
Hi aylei, this PR adds agentless mode to debug target container when there isn't agent pod running on the targetHost. In agentless mode, we don't need to deploy agent daemonset any more.
In this mode, we first start agent pod on target node, and then start debugging container. Both agent pod and debug container will be deleted when debug process exists.The size of debug-agent image is about 30MB, so the preparation time for starting agent pod might not be too long when the network is OK.
I've tested some cases without agent daemonset deployed:
In agentless mode ,it also supports changing agent port dynamically when port-forward mode is off, just like this:
In port-forward mode, it doesn't support changing agent port dynamically, just like this:
Agent pod listens on port 10027(inside container) and on port 10000(outside container), so
kubectl-debug
fails to connect pod's port 10000(inside container). In the future, we can fix this by adding some parameter to config agent listening port and config agent-pod yaml respectively. :)