-
Notifications
You must be signed in to change notification settings - Fork 24
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 rpaas debug feature with custom image #136
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.
Some in-line request changes 💅 but LGTM
&cli.BoolFlag{ | ||
Name: "interactive", | ||
Aliases: []string{"I", "stdin"}, | ||
Usage: "pass STDIN to container", | ||
}, | ||
&cli.BoolFlag{ | ||
Name: "tty", | ||
Aliases: []string{"t"}, | ||
Usage: "allocate a pseudo-TTY", | ||
}, |
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.
I'd force the interactive mode (tty + interactive) in this sub-command... I think there's no other user case to use it without an interactive session.
if status.State.Terminated != nil { | ||
req := m.kcs.CoreV1().Pods(instance.Namespace).GetLogs(args.Pod, &corev1.PodLogOptions{Container: debugContainerName}) | ||
return logs.DefaultConsumeRequest(req, args.Stdout) |
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.
It's possible I overlooked something however I really didn't get it... Why should we see the logs if pod is the termination phase?
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.
Cause it's possible to send a command and not enter in interact mode with debug container. In this case scenario container enter in terminated state and code throw to user container logs related to the command sent (this feature its similar how kubectl debug works in this scenario)
@@ -0,0 +1,165 @@ | |||
package web |
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.
Missing license header.
Co-authored-by: Claudio Netto <nettoclaudio@ufrj.br>
Co-authored-by: Claudio Netto <nettoclaudio@ufrj.br>
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
cmd/plugin/rpaasv2/cmd/debug.go
Outdated
if nerr != nil { | ||
closeErr, ok := nerr.(*websocket.CloseError) | ||
if !ok { | ||
done <- fmt.Errorf("ERROR: receveid an unexpected error while reading messages: %w", 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.
Small typo:
"ERROR: received an unexpected..."
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.
🦅 eyes :)
thanks!
No description provided.