helm-charts
helm-charts copied to clipboard
newrelic
[newrelic-logging] Adjust order of env list in daemonset to fix dependency reference
Is this a new chart
No
What this PR does / why we need it:
When using persistentVolume mode, the env variable FB_DB references NODE_NAME. This reference is currently not resolved, because NODE_NAME is defined after FB_DB. Therefore, only a single database file named '$(NODE_NAME)-fb.db' is created in the persistent volume, instead of one file for each node. #1408 traced the problem down to this change, which moved the definition of NODE_NAME after the definition of FB_DB.
This PR fixes this, by moving the definition of NODE_NAME before the definition of FB_DB again.
The behavior of kubernetes for dependent environment variables is documented here.
Which issue this PR fixes
- fixes #1408
Special notes for your reviewer:
I was thinking about a test that checks if NODE_NAME is defined before FB_DB, but I wasn't able to come up with a way to do this using helm-unittest.
Checklist
- [x] Chart Version bumped
- [ ] Variables are documented in the README.md
- [x] Title of the PR starts with chart name (e.g.
[mychartname])
Hi @jsubirat,
Thanks for the feedback! I didn't know that Fluent Bit is able to resolve environment variables. Doing it that way is of course a lot more elegant. I updated the PR according to your suggestions.
Hi @nluedema , We have validated your PR and found that Fluent Bit is not resolving the variable name ${NODE_NAME} as it doesn't support nested environment variable interpolation. So we agreed internally to do with your previous code changes for now even though it keeps order dependency.
Can you update code to previous one with curved braces and also add a comment mentioning the order dependency to have that variable resolved. So we can proceed with merging your PR.
@nr-rkallempudi @jsubirat I updated the PR to only include the initial changes. Let me know what you think! A pity that nested environment variable interpolation doesn't work.