Added WaitGroups to prevent stderr/stdout from being empty in error logs #10694
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/M
spowelljr
changed the title
spowelljr
changed the title
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
|
Can one of the admins verify this patch? |
|
kvm2 Driver Times for Minikube (PR 10694): 69.9s 68.5s 68.8s Averages Time Per Log docker Driver Times for Minikube (PR 10694): 27.1s 27.0s 27.2s Averages Time Per Log |
|
kvm2 Driver Times for Minikube (PR 10694): 66.8s 67.4s 69.0s Averages Time Per Log docker Driver Times for Minikube (PR 10694): 28.1s 26.0s 26.6s Averages Time Per Log |
| @@ -254,11 +256,12 @@ func (k *Bootstrapper) init(cfg config.ClusterConfig) error { | |||
| } | |||
| return errors.Wrap(err, "wait") | |||
| } | |||
| kw.Close() | |||
There was a problem hiding this comment.
I think this close should be after Wait
There was a problem hiding this comment.
When we get to this line, all the logs are in the pipeline but may not have finished being processed, so we close the pipeline. Closing the pipeline doesn't kill it, instead it writes EOF to the pipe and then prevents new writes. Once the pipeline is emptied it hits the EOF and calls wg.Done() (line 322) and then the wg.Wait() on the next line is unblocked.
tl;dr: If they were in the other order it would block forever because the pipeline processing would be waiting for a EOF which is only added on kw.Close().
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh, spowelljr The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Done, didn't make any changes to the files. |
Added WaitGroups to prevent stderr/stdout from being empty in error logs
Closes #10570
Issue was sometimes error logs were being written out, but stderr was empty. The cause was that there were no WaitGroups to ensure that the logs had finished piping before outputting the error, sometimes resulting with no or partial stderr logs. This fix implements WaitGroups to ensure that the logs have finished piping before outputting the error.