Capture log to error when executing command #1947
Conversation
|
Thanks for submitting this pull request 🎉. The team will review it soon and get back to you. If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document. |
e-sumin
force-pushed
the
pod_command_executor_exec_error
branch
from
873d704
to
defd856
Compare
|
@pavannd1 @PrasadG193 could you please review ? |
pkg/kube/log_tail.go
Outdated
| @@ -20,6 +20,8 @@ import ( | |||
| "strings" | |||
| ) | |||
|
|
|||
| const LogTailDefaultLength = 10 | |||
There was a problem hiding this comment.
| const LogTailDefaultLength = 10 | |
| const logTailDefaultLength = 10 |
No need to export unless required
| @@ -100,7 +102,6 @@ func (s *PodCommandExecutorTestSuite) TestPodRunnerExec(c *C) { | |||
| ctx := context.Background() | |||
| cli := fake.NewSimpleClientset() | |||
|
|
|||
| //simulatedError := errors.New("SimulatedError") | |||
There was a problem hiding this comment.
Missed removing commented lines during code cleanup?
e-sumin
force-pushed
the
pod_command_executor_exec_error
branch
from
378b372
to
539ea68
Compare
e-sumin
force-pushed
the
pod_command_executor_exec_error
branch
from
539ea68
to
4b2e41d
Compare
|
@PrasadG193 @pavannd1 could you please check the changes ? |
|
Hi @e-sumin, |
| Stdout: stdout, | ||
| Stderr: stderr, | ||
| Stdout: stdoutTail, | ||
| Stderr: stderrTail, |
There was a problem hiding this comment.
sorry, I don't have previous context in this task. What happens if we don't specify stdout, where exactly NewLogTail writes the logs?
There was a problem hiding this comment.
LogTail collects only last lines of log, and keeps them internally. If error occurs, we create error which contains log tail. Thus, whether or not the caller passed the stdout/stderr writers, in case of error he will get an access to last log lines.
There was a problem hiding this comment.
thank you. What about
I just wanted to know, do we not use this way to execing a command into a pod when we use kube.Exec API?
There was a problem hiding this comment.
Sorry, forgot to answer this part - nope, currently we are not using and it's only part of PodController / PodCommandExecutor way of execution. Because for proper utilization of such error handling, the invoker should be prepared for such kind of error. But of course we can extend kube.Exec invokers and handle such erros.
There was a problem hiding this comment.
Right, I think we should do the same kind of handling even for kube.exec to make sure we are not seeing diff kinds of behaviour between creating a new pod and executing command in that and executing command in an already created pod.
Should we create an issue to track this?
There was a problem hiding this comment.
Probably you are right. Should I create an issue or you will do it ?
Change Overview
When executing command in pod, we'd like to have generalized errors handling.
As a high level of execution error, we will have error with last log lines.
Callers who does know what are they execute, are able to produce more specific error basing on data containing in logs.
Pull request type
Please check the type of change your PR introduces:
Test Plan