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

Init container emits non-error messages to stderr #1801

Closed
skpy opened this issue Jan 3, 2022 · 10 comments
Closed

Init container emits non-error messages to stderr #1801

skpy opened this issue Jan 3, 2022 · 10 comments

Comments

@skpy
Copy link
Contributor

skpy commented Jan 3, 2022

What happened:
The amazon-vpc-cni-init container invokes set -x in init.sh which emits output to stderr. Our log collection and monitoring system (DataDog) sees these messages as errors (because they're coming from stderr, not stdout) and trips our alerting threshold for number of errors per unit of time whenever the init container runs.

I have implemented a log processor in DataDog to forcibly set all amazon-vpc-cni-init logs as INFO, which avoids the superfluous alerts at the expense of masking any real errors. I could also modify our existing alerts to exclude amazon-vpc-cni-init, then make a new alert specifically for amazon-vpc-cni-init, if necessary.

It might also be worth adding -s to the curl commands (1 and 2) to suppress the status output from curl, which also emits to stderr.

Environment:
EKS 1.21

  • Kubernetes version (use kubectl version): Server Version: version.Info{Major:"1", Minor:"21+", GitVersion:"v1.21.2-eks-06eac09", GitCommit:"5f6d83fe4cb7febb5f4f4e39b3b2b64ebbbe3e97", GitTreeState:"clean", BuildDate:"2021-09-13T14:20:15Z", GoVersion:"go1.16.5", Compiler:"gc", Platform:"linux/amd64"}
  • CNI Version v1.10.1-eksbuild.1
@jaypipes
Copy link
Contributor

jaypipes commented Jan 3, 2022

@skpy What's up, Skippy? :)

@jaypipes
Copy link
Contributor

jaypipes commented Jan 3, 2022

I agree that it's generally poor Bash hygiene to have any non-error output be directed to stderr. We could either do BASH_XTRACEFD=1 at the start of the init script or we could simply remove the use of set -x entirely (as it's mostly a debug/troubleshooting need and not something that should be in production scripts). @jayanthvn what do you think?

@skpy
Copy link
Contributor Author

skpy commented Jan 3, 2022

I can trivially submit a PR to effect the removal of -x (and add -Ss to both curl calls, consistent with other curl invocations within this repo); but I don't have a good way to building, testing, and validating this yet.

@jaypipes
Copy link
Contributor

jaypipes commented Jan 3, 2022

I can trivially submit a PR to effect the removal of -x (and add -Ss to both curl calls, consistent with other curl invocations within this repo); but I don't have a good way to building, testing, and validating this yet.

ack, totes. I'd be good with that. just checking with Jayanth if he's good with it too.

@cgchinmay
Copy link
Contributor

@skpy Thanks for bringing this up. I will talk to @jayanthvn and can actively work on this from next week. Feel free to raise a PR, if you have anything in mind. I can take it from there.

@jayanthvn
Copy link
Contributor

jayanthvn commented Jan 3, 2022

I agree that it's generally poor Bash hygiene to have any non-error output be directed to stderr. We could either do BASH_XTRACEFD=1 at the start of the init script or we could simply remove the use of set -x entirely (as it's mostly a debug/troubleshooting need and not something that should be in production scripts). @jayanthvn what do you think?

I agree with you @jaypipes we can remove the set -x. We do have logs in stdout if anything fails during init. Also I have an open PR where I am moving the init and entry point script to golang :)- #1726

@skpy
Copy link
Contributor Author

skpy commented Jan 4, 2022

Thanks for merging my PR! Out of curiosity, when do you think this will land in a release?

@jayanthvn
Copy link
Contributor

@skpy - I have milestoned the PR for next upcoming release of v1.11.0. It is planned for Jan/Feb but will provide you an estimated date by next week.

@orsenthil orsenthil added this to the v1.10.2 milestone Jan 14, 2022
@orsenthil
Copy link
Member

The PR #1802 is merged.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@cgchinmay cgchinmay modified the milestones: v1.10.2, v1.11.0 Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants