Conversation
There was a problem hiding this comment.
Pull request overview
Improves error reporting for kubeadm operations in the v20260301 kubeadm action implementations by capturing and surfacing command output when kubeadm join / kubeadm reset fail.
Changes:
- Switch
kubeadm joinexecution toCombinedOutput()and append stdout/stderr to the returned error. - Switch
kubeadm resetexecution toCombinedOutput()and append stdout/stderr to the returned error.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| components/kubeadm/v20260301/join.go | Captures kubeadm join output and includes it in failure errors. |
| components/kubeadm/v20260301/reset.go | Captures kubeadm reset output and includes it in failure errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| if output, err := utilexec.New().CommandContext(ctx, kubeadmCommand, "join", "--config", joinConfig, "-v", "5").CombinedOutput(); err != nil { | ||
| return status.Errorf(codes.Internal, "kubeadm join: %s\n%s", err, string(output)) |
There was a problem hiding this comment.
CombinedOutput() buffers all kubeadm stdout/stderr in memory and then embeds it verbatim into the gRPC status message. This makes the returned error unbounded in size (potentially causing high memory usage and/or exceeding gRPC message/log size limits). Consider truncating the captured output (e.g., last N KB/lines) and/or logging full output separately, and only including a bounded summary in the returned error message.
| if err := utilexec.New().CommandContext(ctx, kubeadmCommand, "join", "--config", joinConfig, "-v", "5").Run(); err != nil { | ||
| return status.Errorf(codes.Internal, "kubeadm join: %s", err) | ||
| if output, err := utilexec.New().CommandContext(ctx, kubeadmCommand, "join", "--config", joinConfig, "-v", "5").CombinedOutput(); err != nil { | ||
| return status.Errorf(codes.Internal, "kubeadm join: %s\n%s", err, string(output)) |
There was a problem hiding this comment.
The error formatting always adds a newline and string(output), even when the command produced no output. This can result in noisy/odd error strings (trailing blank line). Consider conditionally appending the output only when it is non-empty (and trimming whitespace) so the error message stays clean.
| return status.Errorf(codes.Internal, "kubeadm join: %s\n%s", err, string(output)) | |
| trimmedOutput := strings.TrimSpace(string(output)) | |
| if trimmedOutput != "" { | |
| return status.Errorf(codes.Internal, "kubeadm join: %s\n%s", err, trimmedOutput) | |
| } | |
| return status.Errorf(codes.Internal, "kubeadm join: %s", err) |
| if output, err := utilexec.New().CommandContext( | ||
| ctx, kubeadmBinary, "reset", "--force", "-v", "5", | ||
| ).Run(); err != nil { | ||
| return status.Errorf(codes.Internal, "kubeadm reset: %s", err) | ||
| ).CombinedOutput(); err != nil { | ||
| return status.Errorf(codes.Internal, "kubeadm reset: %s\n%s", err, string(output)) |
There was a problem hiding this comment.
CombinedOutput() captures the full kubeadm output into memory and then returns it in the gRPC error. Since kubeadm can be quite verbose (especially with -v 5), this can cause large allocations and very large error payloads/log lines. Consider bounding/truncating the output and/or emitting the full output to logs instead of returning it verbatim.
| ).CombinedOutput(); err != nil { | ||
| return status.Errorf(codes.Internal, "kubeadm reset: %s\n%s", err, string(output)) |
There was a problem hiding this comment.
The returned error string always includes a newline plus string(output), even if output is empty. Consider only appending trimmed output when it is non-empty to avoid trailing blank lines in the error message.
This pull request improves error reporting for
kubeadm joinandkubeadm resetoperations by including the command output in error messages. This will help with debugging by providing more context when these commands fail.Enhanced error reporting for kubeadm commands:
runJoinmethod injoin.goto useCombinedOutput()and include the output of thekubeadm joincommand in error messages.runResetmethod inreset.goto useCombinedOutput()and include the output of thekubeadm resetcommand in error messages.