fix(exec-script-tmp): uses tmp and mounts volume instead of creating locally #53

ksafranski · 2017-03-18T14:49:46Z

Instead of creating local devlab.sh this will create in the OS's temp dir, mount, and execute.

Why this wasn't done initially is on me; I just defaulted to /tmp but this caused issues on Mac and Circle. Now the temp directory is determined by the /src/tempdir module which will allow either a custom setting via the DEVLAB_TMP environment variable, or will try to determine the best temp directory to use.

Tested both on my linux server and on my local Mac.

Closes #49 by no longer storing this exec script local.

…locally

ksafranski · 2017-03-18T16:26:04Z

Looks like this approach is still causing issues:

docker: Error response from daemon: Mounts denied: ces for more info.
.
folders/3c/3fxzr4l11l353xd26xg4psnm0000gn/T
is not shared from OS X and is not known to Docker.
You can configure shared paths from Docker -> Preferences... -> File Sharing.
See https://docs.docker.com/docker-for-mac/osxfs/#namespa.
ERRO[0000] error getting events from daemon: net/http: request canceled

ksafranski · 2017-03-18T17:19:34Z

Mac's default TMPDIR is /var/folders/... which is not a shared folder with the docker client. I don't like the idea of forcing users to share /var, so the latest fix has multiple approaches to fixing this issue.

It will check for a (user defined) DEVLAB_TMP which will allow a user to explicitly set this path
It will check for TEMPDIR (the Mac designation) and use /tmp
If all else fails it uses os-tmpdir to try to get the temp location

taghost · 2017-03-19T03:33:32Z

@Fluidbyte I pulled this branch and ran npm run test on my local Mac. Two tests failed due to it using /var/folders/... but then I setup export DEVLAB_TMP=/tmp in my ~/.profile and ran npm run test again, and all tests passed!

taghost · 2017-03-19T03:35:06Z

src/index.js

+      // If override set, use that
+      ? process.env.DEVLAB_TMP
+      // If MacOS TEMPDIR, use /tmp, else try to find using os-tmpdir
+      : process.env.TEMPDIR ? '/tmp' : require('os-tmpdir')()


ksafranski · 2017-03-19T13:26:46Z

I'm not really happy with this approach because it seems to be hit or miss. It worked fine for me just letting it default to /tmp, but seems like this could vary on Mac's and having to set an env var (for anything more than customization, fringe cases) seems hacky.

ksafranski · 2017-03-19T14:02:40Z

Latest commit creates a module which will (I think) do a better job. It's process is:

If DEVLAB_TMP is specified, check if writeable, use that
If not 1, Check if /tmp is writable, use that
If not 2, Check if os-tmpdir suggests a writeable dir, use that
If all else fails, error with notification about manually specifying DEVLAB_TMP, exit

Seems to work everywhere I've tested. Would love some local testing both with and without DEVLAB_TMP set.

The best way to check this is with npm run e2e which will run end-to-end tests.

ksafranski · 2017-03-19T15:01:51Z

The latest approach seems to be checking out where I've tested it. If I can get a couple/few people to check it out it would be appreciated.

QA Test Process

Clone the repo and checkout fix/exec-script-tmp, run npm install
cd test/project and run ../../index.js env and ensure it works
export DEVLAB_TMP=/tmpthen run ../../index.js env and ensure that works
Try setting DEVLAB_TMP to another location like /var and test

If the above look good give me a nod and once I'll get it merged and published.

dvdzkwsk · 2017-03-19T15:20:35Z

README.md

@@ -25,6 +25,8 @@ npm install devlab -g

 *Obvious Note: You need to have [Docker](https://www.docker.com/) installed as well.*

+**Important Note**: In order to run the tasks, Devlab creates a temp file (`devlab.sh`). The tool will do it's best to determine the best location (usually `/tmp`), but this can be explicitly set by specifying the environment variable `DEVLAB_TMP`.


it's -> its

changed, just not showing up here...

dvdzkwsk · 2017-03-19T15:21:37Z

src/tempdir.js

+  } else if (isWriteable('/tmp')) {
+    // Use (most common) /tmp
+    return '/tmp'
+  } else if (isWriteable(require('os-tmpdir')())) {


Should this be preferred over /tmp?

You would think that, the problem is it tries to use TMPDIR on Mac which is /var/folders/... and is not shared with Docker on install. In most cases /tmp should work fine and is how you would expect an application to work on *nix systems. This basically is just used when all else fails.

Ah ok. I do remember having odd trouble with os-tmpdir in the past, which seemed silly since the whole point of that module is that it should just work. Thanks for the explanation.

Yeah, this really is the use-case for weird, non-standard setups or Windows.

taghost · 2017-03-20T13:36:54Z

I pulled the latest and ran npm run test which passed tests. I commented out export DEVLAB_TMP=/tmp and ran again, which also passed tests now. So this appears to work for me on my local Mac, yet fwiw I tried the QA Test instructions and couldn't get it working and also npm run e2e didn't all appear to work. Let me know if you want to screenshare and try this together.

ksafranski · 2017-03-20T15:00:29Z

Checked out ^^^. All good.

levithomason · 2017-03-21T00:52:59Z

Sorry for the delay, this is on my list for testing.

levithomason · 2017-03-21T01:37:55Z

2 failures on a fresh pull and deps install. Output trimmed to relevant parts. LMK what I can do to help:

#----------------------------------------------
# basic multiple: Runs multiple tasks
# devlab env test
#----------------------------------------------

> devlab-demo@1.0.0 test /DevLab/test/project
> mocha ./test --recursive

sh: mocha: not found

#----------------------------------------------
# ignore disable: Passes because service is not disabled for all tasks
# devlab env redis:disable
#----------------------------------------------

Error: Cannot find module 'redis'
    at Object.<anonymous> (/DevLab/test/project/scripts/connect.js:1:77)

#----------------------------------------------
# TEST RESULTS: 2 failures
# basic multiple, ignore disable
#----------------------------------------------

Just to be sure, ran it again with fresh deps and got the same results.

ksafranski · 2017-03-21T01:48:56Z

cd test/project/ && node ../../index.js install

That will install department in the test project, then cd ../../ && npm run e2e

levithomason · 2017-03-21T01:49:47Z

hang tight

levithomason · 2017-03-21T01:51:32Z

🎉

#----------------------------------------------
# TEST RESULTS: 0 failures
#----------------------------------------------

ksafranski · 2017-03-21T01:52:44Z

👍

fix(exec-script-tmp): uses tmp and mounts volume instead of creating …

32abfe2

…locally

ksafranski self-assigned this Mar 18, 2017

ksafranski added 2 commits March 18, 2017 10:57

fix(exec-script-tmp): misc cleanup, one-step promisification of fs

1942166

fix(exec-script-tmp): use os-tmpdir

d6f1f80

ksafranski added 2 commits March 18, 2017 13:01

fix(exec-script-tmp): allow override of tmp with env var

e2d80bc

fix(exec-script-tmp): use multiple checks to find workable tmp

58e404d

ksafranski added 2 commits March 18, 2017 13:22

fix(exec-script-tmp): adds docs on DEVLAB_TMP and temp script

99ba243

fix(exec-script-tmp): ensure services.stop is called

b6415b6

taghost reviewed Mar 19, 2017

View reviewed changes

fix(exec-script-tmp): util for determining where to write temp file

29df95b

fix(exec-script-tmp): adds tests to tempdir module

56816bf

dvdzkwsk reviewed Mar 19, 2017

View reviewed changes

fix(exec-script-tmp): fix doc grammatical error

c99d609

ksafranski added the enhancement label Mar 19, 2017

ksafranski merged commit c313d61 into master Mar 21, 2017

ksafranski deleted the fix/exec-script-tmp branch March 21, 2017 13:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(exec-script-tmp): uses tmp and mounts volume instead of creating locally #53

fix(exec-script-tmp): uses tmp and mounts volume instead of creating locally #53

ksafranski commented Mar 18, 2017 •

edited

ksafranski commented Mar 18, 2017

ksafranski commented Mar 18, 2017

taghost commented Mar 19, 2017

taghost Mar 19, 2017

ksafranski commented Mar 19, 2017

ksafranski commented Mar 19, 2017

ksafranski commented Mar 19, 2017 •

edited

dvdzkwsk Mar 19, 2017

ksafranski Mar 19, 2017

dvdzkwsk Mar 19, 2017

ksafranski Mar 19, 2017

dvdzkwsk Mar 19, 2017

ksafranski Mar 19, 2017

taghost commented Mar 20, 2017

ksafranski commented Mar 20, 2017

levithomason commented Mar 21, 2017

levithomason commented Mar 21, 2017 •

edited

ksafranski commented Mar 21, 2017

levithomason commented Mar 21, 2017

levithomason commented Mar 21, 2017

ksafranski commented Mar 21, 2017

		@@ -25,6 +25,8 @@ npm install devlab -g

		Obvious Note: You need to have [Docker](https://www.docker.com/) installed as well.

		Important Note: In order to run the tasks, Devlab creates a temp file (`devlab.sh`). The tool will do it's best to determine the best location (usually `/tmp`), but this can be explicitly set by specifying the environment variable `DEVLAB_TMP`.

fix(exec-script-tmp): uses tmp and mounts volume instead of creating locally #53

fix(exec-script-tmp): uses tmp and mounts volume instead of creating locally #53

Conversation

ksafranski commented Mar 18, 2017 • edited

ksafranski commented Mar 18, 2017

ksafranski commented Mar 18, 2017

taghost commented Mar 19, 2017

taghost Mar 19, 2017

Choose a reason for hiding this comment

ksafranski commented Mar 19, 2017

ksafranski commented Mar 19, 2017

ksafranski commented Mar 19, 2017 • edited

dvdzkwsk Mar 19, 2017

Choose a reason for hiding this comment

ksafranski Mar 19, 2017

Choose a reason for hiding this comment

dvdzkwsk Mar 19, 2017

Choose a reason for hiding this comment

ksafranski Mar 19, 2017

Choose a reason for hiding this comment

dvdzkwsk Mar 19, 2017

Choose a reason for hiding this comment

ksafranski Mar 19, 2017

Choose a reason for hiding this comment

taghost commented Mar 20, 2017

ksafranski commented Mar 20, 2017

levithomason commented Mar 21, 2017

levithomason commented Mar 21, 2017 • edited

ksafranski commented Mar 21, 2017

levithomason commented Mar 21, 2017

levithomason commented Mar 21, 2017

ksafranski commented Mar 21, 2017

ksafranski commented Mar 18, 2017 •

edited

ksafranski commented Mar 19, 2017 •

edited

levithomason commented Mar 21, 2017 •

edited