Parameterize sidecar image name for Kubernetes scripts. #117
Conversation
| spec: | ||
| containers: | ||
| - name: sidecar | ||
| image: gcr.io/stackdriver-prometheus/stackdriver-prometheus-sidecar:${SIDECAR_IMAGE_TAG} |
There was a problem hiding this comment.
This script is used in the public docs. We shouldn't change the API. Can you preserve the current value as a default value?
There was a problem hiding this comment.
Sure, moved it over to patch.sh.
kube/full/deploy.sh
Outdated
|
|
||
| # Override to use a different Docker image version for the sidecar. | ||
| # Override to use a different Docker image name or version for the sidecar. | ||
| export SIDECAR_IMAGE_NAME=${SIDECAR_IMAGE_NAME:-'gcr.io/stackdriver-kubernetes-1337/stackdriver-prometheus-sidecar'} |
There was a problem hiding this comment.
This is changing the default value in kube/patch.sh. Can you justify the change?
There was a problem hiding this comment.
I moved var over to patch.sh, so it should be backwards compatible now.
jkohen
left a comment
jkohen
left a comment
There was a problem hiding this comment.
I'm still trying to understand the reason to change the default image name. Can you provide the reasoning?
davidbtucker
left a comment
davidbtucker
left a comment
There was a problem hiding this comment.
Fixed typos.
jkohen
left a comment
jkohen
left a comment
There was a problem hiding this comment.
It's still different, it should be gcr.io/stackdriver-prometheus/stackdriver-prometheus-sidecar, right? Can you please test the script so we are sure it still works? This is part of the public documentation and how most users try our Stackdriver integration first, so it is important that it works.
I think you were looking at an old snapshot. In any case, I've tested the latest version and it seems to work. |
3 participants
No description provided.