From c0be5943ca524875afd716544f396d7bae04c878 Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Tue, 27 Feb 2018 23:35:07 -0800 Subject: [PATCH 1/8] Fix --cache-dir with Docker Without supplying a user Docker defaulted to root That caused pip to refuse to use any specified --cache-dir because it's owned by the user Run Docker as the user so it has proper access to any -cache-dir --- lib/pip.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/pip.js b/lib/pip.js index ba07670c..02efebc2 100644 --- a/lib/pip.js +++ b/lib/pip.js @@ -3,7 +3,6 @@ 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'); @@ -87,19 +86,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()}`); // const stripCmd = quote([ // 'find', targetRequirementsFolder, // '-name', '"*.so"', // '-exec', 'strip', '{}', '\;', // ]); // pipCmd = ['/bin/bash', '-c', '"' + pipCmd + ' && ' + stripCmd + ' && ' + chownCmd + '"']; + } else if (process.platform === 'win32') { + // Use same user so --cache-dir works + cmdOptions.push('-u', '1000'); // TODO is this always the case? } cmdOptions.push(dockerImage); cmdOptions.push(...pipCmd); From fb15f0a6d47ce57d1652152c3e18be6b12b931e7 Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Wed, 28 Feb 2018 09:19:34 -0800 Subject: [PATCH 2/8] Detect docker-machine uid --- lib/docker.js | 14 +++++++++++++- lib/pip.js | 4 ++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/docker.js b/lib/docker.js index f4d7bdbb..a43f1714 100644 --- a/lib/docker.js +++ b/lib/docker.js @@ -64,4 +64,16 @@ function getBindPath(servicePath) { return bindPath; }; -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}; diff --git a/lib/pip.js b/lib/pip.js index 02efebc2..240a2126 100644 --- a/lib/pip.js +++ b/lib/pip.js @@ -4,7 +4,7 @@ const get = require('lodash.get'); const set = require('lodash.set'); const {spawnSync} = require('child_process'); const values = require('lodash.values'); -const {buildImage, getBindPath} = require('./docker'); +const {buildImage, getBindPath, getDockerUid} = require('./docker'); /** * Install requirements described in requirementsPath to targetPath @@ -96,7 +96,7 @@ function installRequirements(requirementsPath, targetFolder, serverless, service // pipCmd = ['/bin/bash', '-c', '"' + pipCmd + ' && ' + stripCmd + ' && ' + chownCmd + '"']; } else if (process.platform === 'win32') { // Use same user so --cache-dir works - cmdOptions.push('-u', '1000'); // TODO is this always the case? + cmdOptions.push('-u', getDockerUid(bindPath)); } cmdOptions.push(dockerImage); cmdOptions.push(...pipCmd); From f4a5442381eadd6d6ae99a1b0e0d165fdc2b6c91 Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Wed, 28 Feb 2018 18:55:19 -0800 Subject: [PATCH 3/8] Add test to verify --- test.bats | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test.bats b/test.bats index daf944a9..94c3162a 100755 --- a/test.bats +++ b/test.bats @@ -13,7 +13,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 } @@ -58,6 +58,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" + sed -i'.bak' -re '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 From a7be23a89bedc6b39da7bff24549c0e5c2984ab8 Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Fri, 2 Mar 2018 15:41:26 -0800 Subject: [PATCH 4/8] Remove dependency --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 6d1b7c40..429edf28 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,6 @@ "lodash.set": "^4.3.2", "lodash.values": "^4.3.0", "rimraf": "^2.6.2", - "shell-quote": "^1.6.1", "zip-local": "^0.3.4" } } From ba6ec09ad528fe1950eba62906cc982c4b34fe3d Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Mon, 5 Mar 2018 09:27:05 -0800 Subject: [PATCH 5/8] Try to support OS X too --- lib/pip.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pip.js b/lib/pip.js index 240a2126..07faa4c0 100644 --- a/lib/pip.js +++ b/lib/pip.js @@ -94,7 +94,7 @@ function installRequirements(requirementsPath, targetFolder, serverless, service // '-exec', 'strip', '{}', '\;', // ]); // pipCmd = ['/bin/bash', '-c', '"' + pipCmd + ' && ' + stripCmd + ' && ' + chownCmd + '"']; - } else if (process.platform === 'win32') { + } else { // Use same user so --cache-dir works cmdOptions.push('-u', getDockerUid(bindPath)); } From d856a85925c7626ce306c516eb3d6fc777c999c0 Mon Sep 17 00:00:00 2001 From: Daniel Schep Date: Mon, 19 Mar 2018 17:06:49 -0400 Subject: [PATCH 6/8] fix cache test for macs. stupid bsd sed again --- test.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test.bats b/test.bats index 0ca73cc0..cf9cedc4 100755 --- a/test.bats +++ b/test.bats @@ -61,7 +61,7 @@ teardown() { @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" - sed -i'.bak' -re 's/(pythonRequirements:$)/\1\n pipCmdExtraArgs: ["--cache-dir", ".requirements-cache"]/' serverless.yml + perl -p -i'.bak' -e 's/(pythonRequirements:$)/\1\n pipCmdExtraArgs: ["--cache-dir", ".requirements-cache"]/' serverless.yml sls --dockerizePip=true package ls .requirements-cache/http } From 2646bfb5971c3ecb431bf07a83abbaf19fd1cf77 Mon Sep 17 00:00:00 2001 From: Daniel Schep Date: Tue, 20 Mar 2018 09:12:00 -0400 Subject: [PATCH 7/8] need locale stuff for ci --- test.bats | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test.bats b/test.bats index cf9cedc4..70f3be62 100755 --- a/test.bats +++ b/test.bats @@ -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 From ca40274882032b5bb6846064c4eac54f5957f074 Mon Sep 17 00:00:00 2001 From: Daniel Schep Date: Tue, 20 Mar 2018 09:18:58 -0400 Subject: [PATCH 8/8] oops. negate -z test --- test.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test.bats b/test.bats index 70f3be62..0cdda0ee 100755 --- a/test.bats +++ b/test.bats @@ -3,7 +3,7 @@ setup() { export SLS_DEBUG=t - if [ -z "$CI" ]; then + if ! [ -z "$CI" ]; then export LC_ALL=C.UTF-8 export LANG=C.UTF-8 fi