when restart deployment kube-ovn-controller the kubectl ko log loss #2508
Conversation
changluyi
force-pushed
the
restart_kube_ovn_controller_show_log
branch
from
9771134
to
afe4eca
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| restartCmd := fmt.Sprintf("kubectl rollout restart deployment %s -n kube-system", name) | ||
| _, err := exec.Command("bash", "-c", restartCmd).CombinedOutput() | ||
| framework.ExpectNoError(err, fmt.Sprintf("restart %s failed", name)) | ||
| } |
There was a problem hiding this comment.
The code patch adds a new function RestartSystemDeployment that restarts a Kubernetes deployment in the kube-system namespace. The function takes two arguments: the name of the deployment to restart and a boolean flag to enable logging.
There are a few potential issues with this code:
-
The function assumes that
kubectlandkubectl koare installed on the system where the code is running. It may be better to check for these dependencies and provide more informative error messages if they are not found. -
The function logs the output of the
kubectl ko logcommand, but it does not check the exit status of the command. If the command fails, the function will continue executing and attempt to restart the deployment anyway. -
The function uses
CombinedOutput()to capture the output of thekubectlcommands, which means that both stdout and stderr will be combined into a single byte slice. This may make it harder to parse the output of the commands later on. -
The function does not return any values or provide any feedback to the caller about whether the deployment was successfully restarted or not.
To address these issues, you could consider:
-
Adding checks for the presence of
kubectlandkubectl ko, and returning an error if they are not found. -
Checking the exit status of the
kubectl ko logcommand, and returning an error if it fails. -
Using separate
StdoutPipe()andStderrPipe()methods to capture the stdout and stderr streams separately. -
Returning an error from the function if the deployment restart fails, or providing some other way for the caller to determine whether the restart was successful or not.
|
kube-ovn-controller also logs to the file /var/log/kube-ovn/kube-ovn-controller.log. We can collect logs from this file on each node. Printing kube-ovn-controller logs during the e2e running will mess up the ginkgo output. We should collect and print the logs after the e2e completes. |
What type of this PR
Examples of user facing changes:
Which issue(s) this PR fixes:
Fixes #(issue-number)