-
Notifications
You must be signed in to change notification settings - Fork 19
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
The node.js plugin doesn't run npm install with this runner #18
Comments
@knownasilya this is blocked, see Strider-CD/strider-heroku#4 (comment) |
I'm looking into this right now. The problem seems to originate from strider-node: https://github.com/Strider-CD/strider-node/blob/master/worker.js#L35
There are really a lot of confusing things going on here. It's not that the paths are incorrectly set, strider-docker-runner will still let strider-simple-runner do a lot of work. Which in turn creates directories on the host (like the data and cache directories). This should be a big no-no when considering that we're trying to run stuff in isolation. @knownasilya, do you have a deeper understanding how all these components are mixed with each other? Saying this is a huge mess is kind of an understatement. The callback hell is very real with this problem. |
So, digging deeper into the issue. strider-docker-runner makes use of strider-simple-runner, so we will have 2 job queues (strider-simple-runner/lib/jobqueue.js). The job queue is what will invoke our runners, invoke processJob and, thus, cause the incorrect directories in the Docker context. IMHO, strider-docker-runner must have it's own job queue that is not bound strongly to the processJob method of strider-simple-runner. |
Okay, so the problem isn't that the paths are wrong. The paths are kinda right, except that the whole plugin is not running in the Docker context. So even if the paths are set to |
This is really impossible to fix IMHO. The code is such a mess I wouldn't even know where to start. The best that could (and probably should) be done, is a big fat error that tells you that no plugins are compatible with the docker runner. |
Okay, more progress reporting. We're now using an updated docker image: https://github.com/fairmanager/strider-docker-slave This gives us a newer OS, Node and npm. It also has support for private npm through setting NPM_TOKEN in the environment plugin. The custom plugin seems mandatory when using Docker. This way you can run {
"runner": {
"id": "docker"
},
"merge_plugins": true,
"plugins": [
{
"id": "custom",
"enabled": true,
"showStatus": true,
"config": {
"prepare": ".scripts/prepare.sh --<%= ref.branch %>",
"deploy": ".scripts/deploy.sh --<%= ref.branch %>"
}
}
]
} Then you just drop prepare code into #!/bin/sh
self=`readlink -f "$0"`
basedir=`dirname "$self"`
echo "node -v"; node -v
echo "npm -v"; npm -v
cd ${basedir}
npm install Now, because we don't run with the Node plugin, no actual testing is done. That's a task for another day :P |
Glad you could make some head way on this. |
This should work now with HEAD of strider-docker-runner and strider-node, without using strider-custom for |
If I switch from the simple-runner to the docker-runner, the node.js plugin seems to run the
npm install
from the node.js plugin.It works if I
npm install
from the Custom Scripts pluginThe text was updated successfully, but these errors were encountered: