-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
helm/web-deployment: add env vars for log level of autotrace_webhook #75
base: master
Are you sure you want to change the base?
Conversation
These environment variables come useful during debugging when the autotrace webhook is used. Signed-off-by: Antonio Gutierrez <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested changes but otherwise LGTM.
# autotrace-webhook binaries to log in debug level | ||
debug_autotrace: false | ||
# For the mini ones minikube, minishift set to true | ||
nodeport: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nodeport: false | |
nodeport: 0 |
Rather than a boolean I would make it a "optional" int value so the port can be adjusted to whatever the user wants. With a 0 value the nodePort would be disabled. Also Helm discourages heavy nesting. I'd probably opt for webDebugAutotrace
and webNodePort
. If we wanted dynamic assignment by the cluster of a nodePort then it might make sense to have a boolean and a numeric but it's probably over-engineering for a demo app.
@@ -11,7 +11,7 @@ spec: | |||
targetPort: 8080 | |||
selector: | |||
service: web | |||
{{ if .Values.nodeport }} | |||
{{ if .Values.web.nodeport }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No change required if this is numeric.
@@ -11,7 +11,7 @@ spec: | |||
targetPort: 8080 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
targetPort: 8080 | |
{{ if .Values.web.nodeport }} | |
nodePort: {{ .Values.web.nodeport }} | |
{{ else }} | |
targetPort: 8080 | |
{{ end }} |
|
||
web: | ||
# autotrace-webhook binaries to log in debug level | ||
debug_autotrace: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug_autotrace: false | |
debugAutotrace: false |
Ideally we'd maintain consistency with the rest of the config and not use snake case.
These environment variables come useful during debugging when the autotrace
webhook is used.
Signed-off-by: Antonio Gutierrez [email protected]