Add entrypoint.sh to abstract around metadatad binary. #40
Conversation
igorpeshansky
force-pushed
the
master
branch
from
70449da to
5aa2078
Compare
pkg/docker/Dockerfile-xenial
Outdated
| ARG version=0.0.13-5 | ||
| EXPOSE 8000 | ||
|
|
||
| ARG version=0.0.13-3 |
There was a problem hiding this comment.
Is this way out of date now?
pkg/docker/docker-entrypoint.sh
Outdated
| #!/bin/sh | ||
| set -e | ||
|
|
||
| # first arg is a flag. |
There was a problem hiding this comment.
Nit: capitalize "First".
pkg/docker/docker-entrypoint.sh
Outdated
| set -e | ||
|
|
||
| # first arg is a flag. | ||
| if [ "${1:0:1}" = '-' ]; then |
There was a problem hiding this comment.
Is this a bash-ism? If so, should the shebang be #!/bin/bash instead?
pkg/docker/docker-entrypoint.sh
Outdated
| set -e | ||
|
|
||
| # first arg is a flag. | ||
| if [ "${1:0:1}" = '-' ]; then |
There was a problem hiding this comment.
What if there isn't a first argument?
There was a problem hiding this comment.
I'm not sure what you mean. Please see if the latest update addresses your concerns.
There was a problem hiding this comment.
I was worried that a missing $1 would cause it to error out when nounset is in effect, but that seems to be ignored for substring substitutions (i.e., echo "${1}" would produce a "1: unbound variable" error, but echo ${1:0:1} will happily print an empty string). So never mind.
There was a problem hiding this comment.
@bmoyles0117 Just noticed that this is a bash-ism, and you set the shell to /bin/sh. I got Bad substitution errors on this line in the latest container.
I've sent #161 to fix.
bmoyles0117
force-pushed
the
bmoyles0117-docker-entrypoint
branch
from
65b8b2c to
1b1c892
Compare
bmoyles0117
changed the title
pkg/docker/Dockerfile-xenial
Outdated
| && apt-get install -f -y \ | ||
| && rm -rf /var/lib/apt/lists/* \ | ||
| && rm -rf /stackdriver-metadata.deb | ||
| ADD entrypoint.sh /entrypoint.sh |
There was a problem hiding this comment.
This is a local file — let's use COPY.
| # $ docker run -it {image:tag} /bin/bash | ||
| # | ||
| # Default behavior uses CMD defined in Dockerfile. | ||
| # $ docker run -it {image:tag} |
There was a problem hiding this comment.
This comment is not related to the following if, so let's add a blank line after it.
pkg/docker/entrypoint.sh
Outdated
| set -- /opt/stackdriver/metadata/sbin/metadatad "$@" | ||
| fi | ||
|
|
||
| exec "$@" No newline at end of file |
There was a problem hiding this comment.
Let's keep the trailing newline...
There was a problem hiding this comment.
Please understand that I don't do this intentionally :)
pkg/docker/docker-entrypoint.sh
Outdated
| set -e | ||
|
|
||
| # first arg is a flag. | ||
| if [ "${1:0:1}" = '-' ]; then |
There was a problem hiding this comment.
I was worried that a missing $1 would cause it to error out when nounset is in effect, but that seems to be ignored for substring substitutions (i.e., echo "${1}" would produce a "1: unbound variable" error, but echo ${1:0:1} will happily print an empty string). So never mind.
4 participants
No description provided.