Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect klog to write to file instead of stderr to avoid blocking #505

Merged
merged 1 commit into from
Jun 15, 2020
Merged

Conversation

pepov
Copy link
Contributor

@pepov pepov commented May 26, 2020

Description

Redirects klog to file instead of stderr to avoid blocking.

Acceptance tests

  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch? (If so, please include the test log in a gist)

I've ran the suite and it failed with a seemingly unrelated issue, that maybe connected to:
#494

The log output:
https://gist.github.com/pepov/1b53980c5389df3ac3451c74585070da

Update: tests passed the second time:
https://gist.github.com/pepov/8b5a172240a99b331da87b6e125bd1ca

References

This is the original issue that this PR is trying to work around:
#498

@ghost ghost added the size/XS label May 26, 2020
@pepov
Copy link
Contributor Author

pepov commented May 28, 2020

sorry for the mentions, but could someone please help pointing me to someone who would possibly be willing to take a look at this pr and the underlying issue as it's a blocker on EKS @mcuadros @jrhouston

#498

@krzysztof-miemiec
Copy link
Contributor

krzysztof-miemiec commented Jun 6, 2020

Your PR, combined with my PR (#525) seems to work "in production" - I guess that my solution regarding concurrency issue is not the most optimal, but hey.. it works after all! 😄🎉

@hashicorp-cla
Copy link

hashicorp-cla commented Jun 8, 2020

CLA assistant check
All committers have signed the CLA.

@jtyr
Copy link

jtyr commented Jun 9, 2020

I can confirm that this PR combined together with the PR #525 solved my problem with the stuck helm provider! ❤️

Copy link
Contributor

@jrhouston jrhouston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch, this is a nasty bug. Thanks a lot for investigating this @pepov! There is definitely a deeper problem here that I'm going to investigate - if there is a problem with klog writing to stderr then this could surface in the other kubernetes providers too.

For now we can accept this as a bandaid to stop the provider from hanging. The branch just needs to be rebased as we just moved this repo and there's some path conflicts.

@pepov
Copy link
Contributor Author

pepov commented Jun 15, 2020

@jrhouston rebased on master, thx for looking into this!

@jrhouston jrhouston merged commit ce0fd07 into hashicorp:master Jun 15, 2020
@ghost
Copy link

ghost commented Jul 16, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Jul 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants