kubernetes-retired / contrib

[EOL] This is a place for various components in the Kubernetes ecosystem that aren't part of the Kubernetes core.
Apache License 2.0
2.46k stars 1.68k forks source link

Directly write on-start and on-change script output to stdout #2866

Closed lenalebt closed 6 years ago

lenalebt commented 6 years ago

We had a problem with a hanging on-start script, and peer-finder waited for the script to terminate before writing it's output to stdout. This change directly writes the output to stdout as it happens, making it way easier to debug hanging startup scripts.

k8s-ci-robot commented 6 years ago

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/devel/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
k8s-ci-robot commented 6 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lenalebt To fully approve this pull request, please assign additional approvers. We suggest the following additional approver: eparis

Assign the PR to them by writing /assign @eparis in a comment when ready.

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: - **[OWNERS](https://github.com/kubernetes/contrib/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
lenalebt commented 6 years ago

/assign eparis

k8s-ci-robot commented 6 years ago

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/devel/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
lenalebt commented 6 years ago

I just realized that there have been formatting issues, which I resolved now. Anything I can do to get this merged :)?

lenalebt commented 6 years ago

We realized internally that this approach has some drawbacks; it sometimes does not recognize that the connected command terminates and then hangs. We have a solution (by my coworker) that solves these issues. We will create a new pull request for that.

andrenarchy commented 6 years ago

@lenalebt did your alternative solution work out in the end? I just created #2944 since we ran into the same issue. If you found out more it'd be great to share it in the issue. Thanks! :)

lenalebt commented 6 years ago

Hi @andrenarchy - indeed, we have a more-or-less working solution. I am not 100% sure what exactly we changed, but I have added the source we used to compile our own version as a gist here: https://gist.github.com/lenalebt/9b2ed4abe659fe78a8f68b796d24fad7

Maybe you can use it to solve your issue, or post a new PR for this?