fix(helm): Make log verbosity configurable #397
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7 tasks
|
Sorry for the noise, I changed |
mittachaitu
reviewed
Oct 26, 2021
There was a problem hiding this comment.
@ianroberts , Thanks for raising the PR, have provided a few comments
Make log verbosity level configurable via chart values, rather than hard-coding --v values (several components are currently set at --v=5, the most verbose level, intended for trace-level logging). For backwards compatibility the defaults are set to match the previous hard-coded values everywhere, but this can now be overridden via csiController.{attacher|provisioner|resizer|snapshotController}.logLevel for the controller (or csiController.logLevel to set the default for all four), csiNode.driverRegistrar.logLevel for the CSI node and cvcOperator.logLevel for the CVC operator.
Signed-off-by: Ian Roberts <[email protected]>
ianroberts
added a commit
to ianroberts/jiva-operator
that referenced
this pull request
Oct 27, 2021
This is a modification to openebs-archive#161 to reverse the order of the overriding between csiController.logLevel and csiController.<container>.logLevel following discussions on openebs-archive/cstor-operators#397. Now csiController.logLevel sets a default at the pod level, and csiController.*.logLevel overrides that default for the relevant pod. Signed-off-by: Ian Roberts <[email protected]>
4 tasks
Ab-hishek
approved these changes
Nov 19, 2021
mittachaitu
pushed a commit
to openebs-archive/jiva-operator
that referenced
this pull request
Nov 25, 2021
This is a modification to #161 to reverse the order of the overriding between csiController.logLevel and csiController.<container>.logLevel following discussions on openebs-archive/cstor-operators#397. Now csiController.logLevel sets a default at the pod level, and csiController.*.logLevel overrides that default for the relevant pod. Signed-off-by: Ian Roberts <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A number of components in the helm charts have hard-coded
--v=5options which can produce overly-verbose logging. I recently submitted openebs-archive/jiva-operator#161 to fix this in the Jiva chart and was asked by @prateekpandey14 to port the same changes here for cStor.What this PR does?:
This PR makes various
--vflags configurable via the values file/--set. To maintain backwards compatibility the defaults invalues.yamlremain at the previously hard-coded values but they can now be overridden viacsiController.logLevelto change all the CSI controller containers to the same level in one go. If this value is not set then we fall back to per-container settings, all of which default to "5"csiController.attacher.logLevelcsiController.provisioner.logLevelcsiController.resizer.logLevelcsiController.snapshotController.logLevelcsiNode.logLevelas above for the CSI node pod, though there is only one container-specific child value in this case:csiNode.driverRegistrar.logLevelfor the node pod driver registrarcvcOperator.logLevelfor the CVC operator (default level 2)Edit: following discussion this has now been changed so that the top-level per-pod settings provide the defaults in
values.yaml, and the per-container settings can be supplied to override the default.Note I have only added options for those cases where there is already a
--voption specified in the helm template. There may be other components that could accept--vbut where the chart does not currently pass one, but I don't use cStor myself (this PR is a port of changes I made to Jiva) and don't know the code so I've limited myself to the obviously non-breaking changes.Does this PR require any upgrade changes?:
No