-
-
Notifications
You must be signed in to change notification settings - Fork 2k
set and use $port whenever $PROC_TYPE == 'web' #1341
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
Conversation
|
@josegonzalez I think we just missed this use case. Not sure we want to add a deploy test specifically for this though. Thoughts? |
dokku
Outdated
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.
The first if/elif stanza can probably be simplified a bit as well.
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.
How do you mean?
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.
Like, we check several times what the PROC_TYPE is, as well as two checks on BIND_EXTERNAL.
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.
I guess I meant, do you have a recommendation?
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.
Something like:
if [[ "$PROC_TYPE" == "web" ]]; then
port=${DOKKU_DOCKERFILE_PORT:=5000}
if [[ "$BIND_EXTERNAL" = "true" ]]; then
id=$(docker run -d -p $port -e PORT=$port $DOCKER_ARGS $IMAGE $START_CMD)
port=$(docker port $id $port | sed 's/[0-9.]*://')
ipaddr=127.0.0.1
else
id=$(docker run -d -e PORT=$port $DOCKER_ARGS $IMAGE $START_CMD)
ipaddr=$(docker inspect --format '{{ .NetworkSettings.IPAddress }}' $id)
fi
else
id=$(docker run -d $DOCKER_ARGS $IMAGE $START_CMD)
fi
shrug
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.
oh oh oh yeah sorry I was overthinking it. this makes sense.
|
No test needed imo. |
…ns-vhost set and use $port whenever $PROC_TYPE == 'web'
This honors
$DOKKU_DOCKERFILE_PORTwhen binding the docker container to an external IP.refs #1336