Support static nginx port when deploying without an application VHOST #1476

michaelshobbs · 2015-09-18T02:11:59Z

example output with no $DOKKU_ROOT/VHOST and an IP in $DOKKU_ROOT/HOSTNAME (also can be accomplished by setting NO_VHOST=1)
note: we always use the contents of $DOKKU_ROOT/HOSTNAME for urls output in the aforementioned cases

$ git push dokku@dokku.me:node-js-app master
Pseudo-terminal will not be allocated because stdin is not a terminal.
Counting objects: 454, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (365/365), done.
Writing objects: 100% (454/454), 218.70 KiB | 0 bytes/s, done.
Total 454 (delta 79), reused 391 (delta 51)
-----> Cleaning up...
-----> Building node-js-app from herokuish...
-----> Fetching custom buildpack
-----> Node.js app detected
...
-----> All checks successful!
=====> node-js-app container output:
       Node app is running at localhost:5000
=====> end node-js-app container output
-----> Running post-deploy
=====> unsupported vhost config found. disabling vhost support
-----> Setting config vars
       NO_VHOST: 1
-----> no nginx port set. setting to random open high port
-----> Setting config vars
       DOKKU_NGINX_PORT: 22578
-----> Creating http nginx.conf
-----> Running nginx-pre-reload
       Reloading nginx
=====> Application deployed:
       http://10.0.0.2:32824 (container)
       http://10.0.0.2:22578 (nginx)

To dokku@dokku.me:node-js-app
 * [new branch]      master -> master

josegonzalez · 2015-09-18T02:55:02Z

plugins/common/functions

We should be a bit more professional here ;)

Bahahahaha!

EDIT: You do read my code!!!!! ;P

I don't just click merge as soon as I see a green button :P

…function

…ame stanza if we have no hosts"

michaelshobbs · 2015-09-18T14:28:38Z

I'm not particularly happy with how this ended up. Any feedback on the nginx_build_config function would be welcomed.

josegonzalez · 2015-09-18T19:56:27Z

docs/nginx.md

Should we add port as an option to nginx:enable so that users can modify the port to their preferred value?

eh? 🤷

just set the config var...?

Fine by me :)

This reduces deploy output by moving `config:get` calls out of the main loop. Since the values are the same for each iteration, it does not make sense to re-retrieve them each time.

Because of how plugin commands are implemented, their output can be incredibly verbose. Rather than executing even the `set -eo pipefail` parts of a plugin, we immediately check if the command is implemented by a plugin. If it is not, then we continue on as normal. One side-effect of this change is that plugin commands need to be duplicated again: - once in the command array - once for the actual body of the command - once in the help output This is also quite hackish, and probably not the best way to decrease trace output. Note that we drop approximately 2k lines worth of logs with this change.

michaelshobbs · 2015-09-18T21:18:04Z

I'm working on some more refactoring

michaelshobbs · 2015-09-18T22:48:43Z

plugins/config/functions

it was impossible to support transitions from NO_VHOST=1 to nothing and maintain a single function that told the truth about an app's vhost state. so we catch NO_VHOST in unset and set it to 0

michaelshobbs · 2015-09-18T22:52:27Z

I think this is a step in the right direction. Could definitely use some more time to breakdown nginx_build_config but let's not hold up 0.4.0 for this.

u2mejc · 2015-09-18T23:25:51Z

plugins/common/functions

I think your if statement is missing the brackets to evaluate this and you have a continue in an if statement?

it twerks:

$ get_open_port() { > while true; do > local port=$(shuf -i 80-81 -n 1) > if ! nc -z 0.0.0.0 $port; then > echo "open port: $port" > return 0 > else > echo "failed port: $port" > continue > fi > done > } $ get_open_port failed port: 80 failed port: 80 open port: 81

…ore explicit

michaelshobbs · 2015-09-18T23:55:22Z

plugins/common/functions

$ dpkg -S /usr/bin/shuf coreutils: /usr/bin/shuf

just showing what provides shuf

…h-nginx-novhost

josegonzalez · 2015-09-19T21:50:11Z

yey passes

nginx proxy without VHOST

initial pass at nginx proxy without VHOST. closes #1123

4dd4d7c

josegonzalez reviewed Sep 18, 2015
View reviewed changes

michaelshobbs added 4 commits September 17, 2015 21:15

cleanup unnecessary failure message

de6ccf4

fixing tests

943e9b3

[ci skip] update nginx docs

89dd783

be explicit about our template path since others are calling us as a …

f84c2bf

…function

michaelshobbs force-pushed the 1123_mh-nginx-novhost branch from 1be6ebf to f84c2bf Compare September 18, 2015 04:56

michaelshobbs added 3 commits September 17, 2015 22:32

include new port variables in custom nginx templates

e07ae7a

redirect includes port now

9efa8be

set SSL_VHOSTS to HOSTNAME if NO_VHOST is set and filter out server_n…

6e7ca94

…ame stanza if we have no hosts"

michaelshobbs changed the title ~~[WIP] initial pass at nginx proxy without VHOST~~ nginx proxy without VHOST Sep 18, 2015

josegonzalez reviewed Sep 18, 2015
View reviewed changes

josegonzalez added 6 commits September 18, 2015 16:08

Reduce deploy trace output for multi-container applications

b9ecb32

This reduces deploy output by moving `config:get` calls out of the main loop. Since the values are the same for each iteration, it does not make sense to re-retrieve them each time.

Retrieve default docker args a single time

1e543af

Drop another ~1k lines from DOKKU_TRACE output

467264d

Add help commands to docker-options and tar plugins

d3c218f

Consolidate ipv4/ipv6 regex declarations behing nginx-vhosts plugin

f914e62

michaelshobbs reviewed Sep 18, 2015
View reviewed changes

michaelshobbs force-pushed the 1123_mh-nginx-novhost branch from 7ad572f to bf3cab7 Compare September 18, 2015 22:49

michaelshobbs mentioned this pull request Sep 18, 2015

Reduce trace output #1474

Closed

michaelshobbs force-pushed the 1123_mh-nginx-novhost branch from bf3cab7 to 876173d Compare September 18, 2015 23:15

u2mejc reviewed Sep 18, 2015
View reviewed changes

create functions for app/global vhost detection. make VHOST setting m…

3e18d9a

…ore explicit

michaelshobbs force-pushed the 1123_mh-nginx-novhost branch from 876173d to 3e18d9a Compare September 18, 2015 23:49

michaelshobbs reviewed Sep 18, 2015
View reviewed changes

Merge remote-tracking branch 'origin/reduce-trace-output' into 1123_m…

fd44635

…h-nginx-novhost

michaelshobbs force-pushed the 1123_mh-nginx-novhost branch from 08418db to fd44635 Compare September 19, 2015 00:34

josegonzalez added a commit that referenced this pull request Sep 19, 2015

Merge pull request #1476 from progrium/1123_mh-nginx-novhost

5bb62c0

nginx proxy without VHOST

josegonzalez merged commit 5bb62c0 into master Sep 19, 2015

josegonzalez deleted the 1123_mh-nginx-novhost branch September 19, 2015 21:54

josegonzalez changed the title ~~nginx proxy without VHOST~~ Support static nginx port when deploying without an application VHOST Sep 19, 2015

Uh oh!

Support static nginx port when deploying without an application VHOST #1476

Support static nginx port when deploying without an application VHOST #1476

Uh oh!

Conversation

michaelshobbs commented Sep 18, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelshobbs commented Sep 18, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelshobbs commented Sep 18, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelshobbs commented Sep 18, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josegonzalez commented Sep 19, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants