Skip to content

Fix --cache-dir with Docker #145

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

Merged
merged 10 commits into from
Mar 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion lib/docker.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,16 @@ function getBindPath(servicePath) {
throw new Error('Unable to find good bind path format');
};

module.exports = {buildImage, getBindPath};
/**
* Find out what uid the docker machine is using
* @param {string} bindPath
* @return {boolean}
*/
function getDockerUid(bindPath) {
const options = ['run', '--rm', '-v', `${bindPath}:/test`,
'alpine', 'stat', '-c', '%u', '/test/.serverless'];
const ps = dockerCommand(options);
return ps.stdout.trim();
};

module.exports = {buildImage, getBindPath, getDockerUid};
15 changes: 6 additions & 9 deletions lib/pip.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ const path = require('path');
const get = require('lodash.get');
const set = require('lodash.set');
const {spawnSync} = require('child_process');
const {quote} = require('shell-quote');
const values = require('lodash.values');
const {buildImage, getBindPath} = require('./docker');
const {buildImage, getBindPath, getDockerUid} = require('./docker');

/**
* Install requirements described in requirementsPath to targetPath
Expand Down Expand Up @@ -82,19 +81,17 @@ function installRequirements(requirementsPath, targetFolder, serverless, service
cmdOptions.push('-e', 'SSH_AUTH_SOCK=/tmp/ssh_sock');
}
if (process.platform === 'linux') {
// Set the ownership of the .serverless/requirements folder to current user
pipCmd = quote(pipCmd);
const chownCmd = quote([
'chown', '-R', `${process.getuid()}:${process.getgid()}`,
targetRequirementsFolder,
]);
pipCmd = ['/bin/bash', '-c', '"' + pipCmd + ' && ' + chownCmd + '"'];
// Use same user so requirements folder is not root and so --cache-dir works
cmdOptions.push('-u', `${process.getuid()}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cgrimal, AFAICT you added the chown stuff. Could you test this alternative?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW tests passed on Linux with all Docker tests enabled. There was no problem deleting the .serverless folder.

// const stripCmd = quote([
// 'find', targetRequirementsFolder,
// '-name', '"*.so"',
// '-exec', 'strip', '{}', '\;',
// ]);
// pipCmd = ['/bin/bash', '-c', '"' + pipCmd + ' && ' + stripCmd + ' && ' + chownCmd + '"'];
} else {
// Use same user so --cache-dir works
cmdOptions.push('-u', getDockerUid(bindPath));
}
cmdOptions.push(dockerImage);
cmdOptions.push(...pipCmd);
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
"lodash.get": "^4.4.2",
"lodash.set": "^4.3.2",
"lodash.values": "^4.3.0",
"shell-quote": "^1.6.1",
"zip-local": "^0.3.4"
}
}
14 changes: 13 additions & 1 deletion test.bats
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

setup() {
export SLS_DEBUG=t
if ! [ -z "$CI" ]; then
export LC_ALL=C.UTF-8
export LANG=C.UTF-8
fi

cd test

Expand All @@ -11,7 +15,7 @@ setup() {

teardown() {
sls requirements clean
rm -rf puck puck2 puck3 node_modules .serverless
rm -rf puck puck2 puck3 node_modules .serverless .requirements.zip .requirements-cache
if [ -f serverless.yml.bak ]; then mv serverless.yml.bak serverless.yml; fi
}

Expand Down Expand Up @@ -58,6 +62,14 @@ teardown() {
ls puck/flask
}

@test "py3.6 uses cache with dockerizePip option" {
[ -z "$CIRCLE_BRANCH" ] || skip "Volumes are weird in CircleCI https://circleci.com/docs/2.0/building-docker-images/#mounting-folders"
! uname -sm|grep Linux || groups|grep docker || id -u|egrep '^0$' || skip "can't dockerize on linux if not root & not in docker group"
perl -p -i'.bak' -e 's/(pythonRequirements:$)/\1\n pipCmdExtraArgs: ["--cache-dir", ".requirements-cache"]/' serverless.yml
sls --dockerizePip=true package
ls .requirements-cache/http
}

@test "py2.7 can package flask with default options" {
sls --runtime=python2.7 package
unzip .serverless/sls-py-req-test.zip -d puck
Expand Down