Allow proxying arbitrary ports and starting daemons, more explicit nodejs support, fixes #3456 #4057
Conversation
|
Please take a look and see what you think @tyler36 @torenware @hchonov @davereid-pfg and anybody else interested in nodejs support. |
|
Download the artifacts for this pull request: |
|
I've just read the updated docs, which look good so far. A couple of general question about how this will work:
I'm going to try to get this to work with a trivial install of something, but that's what I have for the first pass. |
|
Thanks @torenware
|
IIRC
True. |
|
I'm starting up with these additions to config.yaml: # 4057 tests
web_extra_daemons:
- name: "http-1"
command: "node_modules/.bin/vite"
directory: /var/www/html/frontend
web_extra_exposed_ports:
- name: node-vite
container_port: 5173
http_port: 9998
https_port: 5147
hooks:
post-start:
- exec: "cd frontend; npm install"First issue: I set So ddev falls over. I change the first portion of the config to: web_extra_daemons:
- name: "http-1"
command: "cd frontend; node_modules/.bin/vite"
directory: /var/www/htmlThis also kills ddev good and dead: Sounds like the command must be something that can be exec'd. I'll write a short script and see if I can get around this. Which works. |
|
Working config: # 4057 tests
web_extra_daemons:
- name: "http-1"
command: "./launch-vite"
directory: /var/www/html
web_extra_exposed_ports:
- name: node-vite
container_port: 5173
http_port: 9998
https_port: 5173
web_environment:
# Used by my simple PHP page:
- VITE_PRIMARY_PORT=5173
- VITE_PROJECT_DIR=frontend
hooks:
post-start:
- exec: "cd frontend; npm install"
- exec: "frontend/node_modules/.bin/vite -v"
This required an "adapter script" so it could be exec'd, which contains: #! /usr/bin/env bash
cd frontend
exec node_modules/.bin/vite "$@"My canary script at public/index.php prints out: as it should. So, issues found:
I'll post my test project to GitHub so folks can use it during the test of this PR. But vite is a fainting goat of a program, and is a fair test of the feature, I think. |
|
Test program up on GitHub. Simple "proof of life" test: checks for an install directory for the JavaScript app, and whether the expected port is actually listening. Nothing really Vite specific done at all, so you should be able to test about any container that gets started up with a bit of fiddling. |
|
Thanks so much for looking at this so carefully!
instead of the more natural web_extra_daemons:
- name: "http-1"
command: "node_modules/.bin/vite"
directory: /var/www/html/frontendPlease try some simple things like running |
That's what I thought too. BECAUSE IT TRIED IT AND IT DIDN'T WORK ;-) Yeah, that might be the reason. Using the syntax from the docs, I got the errors: Not sure how you tested it, but the code as it's currently written definitely set the working directory wrong. You have my test program; try changing my config to the original, and watch the smoke emerge. |
|
Thanks for making https://github.com/torenware/ddev-pr-4057 - I clicked through to it earlier, but it doesn't say how you should set it up, what the expectations are, how you'd test it, where BTW, you can't do things like You can validate the configurations used for supervisord by Again, it probably makes sense to start with simple things that don't have dependencies like |
|
-I do see why your test passed however. The data- app.WebExtraDaemons = []WebExtraDaemon{
{Name: "FirstDaemon", Command: "php -S 0.0.0.0:3000", Directory: "/var/www/html"},
{Name: "SecondDaemon", Command: "php -S 0.0.0.0:4000", Directory: "/var/www/html/sub"},
}Looks like the tests initially crashed because node_modules ran in the wrong dir. If it runs first, then: web_extra_daemons:
- name: "http-1"
command: "node_modules/.bin/vite"
directory: /var/www/html/frontenddoes run correctly. So I stand corrected. Note that if node_modules isn't populated, you do get the full crash. Since you're not enforcing |
|
Updated the test program. There really is nothing to do except run |
|
Thanks so much for your engagement on this @torenware .
Do you have suggestions on how to get people to do the |
|
Some ramblings for what to do around this. The biggest stability issue right now is how the app responds when Beyond that:
Probably you can restrict parse out Then you get to node_modules specifically, since the most likely reason the binary or script is not found is due to an install script not having run. The next most likely is that a non-native binary is sitting at the end of
I'm having difficulty thinking of a scenario where a node-based daemon would not have a package.json file deployed with it. Maybe check for it in the current directory and warn if it's absent? There may be some new-fangled package managers for JS that don't use one, but it's very uncommon. Even package managers that support this (e.g., pnpm IIRC) create a package.json file for backwards compatibility. |
|
The problem you had was that supervisord apparently doesn't change the directory until after it finds the binary, so an absolute path to the binary is required. I updated docs (will push shortly) and did a PR for your test repo. I do note that you have the Thanks for the good thinking/rambling about node usage.
|
| ```yaml | ||
| hooks: | ||
| post-start: | ||
| - exec: "nohup /var/www/html/frontend/node_modules/.bin/vite &" |
There was a problem hiding this comment.
Probably not the example to use. I'm reasonably sure this would not work; it's something I tried a few weeks back for ddev-viteserve
|
|
||
| For example, you could use this configuration to run two instances of the nodejs http-server serving different directories: | ||
|
|
||
| ```yaml | ||
| web_extra_daemons: | ||
| - name: "http-1" | ||
| command: "node_modules/.bin/http-server -p 3000" | ||
| command: "/var/www/html/node_modules/.bin/http-server -p 3000" |
There was a problem hiding this comment.
Better to allow relative commands, and check the existence of a relative path before you pass it to supervisord for processing. Also: you can use ddev w/o being aware that the host's dev directory is mapped to /var/www/html.
| directory: /var/www/html | ||
| - name: "http-2" | ||
| command: "node_modules/.bin/http-server /var/www/html/sub -p 3000" | ||
| command: "/var/www/html/node_modules/.bin/http-server /var/www/html/sub -p 3000" |
There was a problem hiding this comment.
I'm not sure I've seen anybody use http_server like this in the wild. Odd example.
| site := TestSites[0] | ||
|
|
||
| testcommon.ClearDockerEnv() | ||
| app := new(DdevApp) |
There was a problem hiding this comment.
Legal, but not really idiomatic Go. Better &DdevApp{}, especially if you're going to set anything in the structure immediately anyway.
| return errors.Errorf("web container not found") | ||
| return fmt.Errorf("web container for %s not found", app.Name) | ||
| } | ||
| if container.State != "running" { |
There was a problem hiding this comment.
Good this. Since containers will fail to start on occasion, and you want that output where users will immediately see it.
Welcome to JavaScript world. It's Dante-esqe "Abandon all hope all ye that enter here". The problem is that
Actually harder than it looks. The vite example currently in the draft docs actually won't work; I've tried doing exactly that.
Supervisord is, remember, an implementation detail. It's clearly subject to problems if it is given bad input. Worth filtering at least a little before giving it control.
Better to support relative paths and pre-flight them. Remember the case of not running
True. |
|
Hmm, I tested the post-start hook on your repo and it worked... That's where it came from. |
Best test more; it doesn't stay up. It falls over if it actually does anything, which was hell to debug. Also, shutting it down would be a PITA if you started it that way. In I don't know if the problem is specific to vite (or even vite 2), or if more scripts would also die with only nohup and a ham sandwich. But I know vite is picker than that. |
|
I'll use a different example. |
rfay
force-pushed
the
20220728_proxy_arbitrary_ports
branch
from
04b9ed8
to
cdf879e
Compare
|
If you do get frustrated with supervisord, tmux is actually a good substitute. Here's my vite-serve command from that repo. Key part, where I kill the process if it's already running, and restart it in the tmux detached process: # create a background tmux session and tell it to run
# our vite script
tmux kill-session -t vite-sess 2>/dev/null
tmux new -s vite-sess -d
tmux send "node node_modules/vite/bin/vite.js --port $VITE_PRIMARY_PORT --host" C-m
echo "Vite now serving $VITE_DIR"Nice thing is it fails gracefully, and if you need to debug it, you reattach the window and see what happened. No smoke and no fire, just heat. |
|
I imagine tmux is a great way to manage separate sessions; supervisord has been working fine for years and I probably won't mess with it. However, it may not be the perfect solution for adding user-daemons. I see you install tmux during your install in that repo. |
|
I install both tmux and pnpm, yeah. tmux is fairly light-weight, and it allows you to run scripts or binaries that don't tolerate what you need to do in order to daemonize a process (disconnecting the process from any terminal and exec'ing away). The process sees a realistic using pseudo terminal, and just runs as if it was in the foreground. It also doesn't require you to save away the PID anywhere, since tmux will let you address a session by name. Both tools have their place. My usage is slightly "off label", but was a good fit for a script that likes working under terminal. |
|
Tests uncovered an interesting chicken-egg problem in TestExtraPortExpose when using mutagen. The test creates a subdirectory "sub" on the host and then starts the app; daemon requires the directory "sub" per configuration. But mutagen can't sync and create that until the container comes up. But oops, the container can't come up until it finds that "sub" dir. Not sure what the solution is to that. The current behavior of supervisord of killing everything off if one thing can't come up may just not be useful. I know I put that in there years ago, probably for a reason. But not sure it's a good thing. I think healthchecks came along after that. |
|
Added a delay between starting daemon and next retry if it fails, and added a little grace period in the script if running mutagen. Seems to sort this out. Also, bash is now used in running the |
|
Actually used bash in some of those attempts, since it supports `disown`. But that didn’t work either.
Some programs just really want a terminal there. Take away the terminal, and they either crash or silently exit. Which was the behavior I had with Vite 2. The program would start, but it would exit almost immediately. `tmux` gave Vite what it needed, and the program stayed up.
Not sure if this was specifically a Vite 2 issue (vs. Vite 3, just out) or if I’m just better at brining the system up under acceptable conditions.
R
… On Jul 30, 2022, at 2:47 PM, Randy Fay ***@***.***> wrote:
Added a delay between starting daemon and next retry if it fails, and added a little grace period in the script if running mutagen. Seems to sort this out. Also, bash is now used in running the command so that would have prevented some of the trouble you had @torenware <https://github.com/torenware> .
—
Reply to this email directly, view it on GitHub <#4057 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACOUZL74YG2T4HXV2YTJC3VWWPGPANCNFSM55CGY2LA>.
You are receiving this because you were mentioned.
|
|
This is passing tests now with mutagen and without. I hope a couple of people can try it out. |
|
Unfortunately, I was able to first test today and for me nginx does not forward the requests properly for my nextjs app. I've opened #4208 for this. |
The Problem/Issue/Bug:
Manual Testing Instructions:
Try out all the things in the doc
Automated Testing Overview:
Added TestExtraPortExpose