ENH: generate Dockerfiles with neurodocker + migrate to CircleCI 2.0 #2202

kaczmarj · 2017-10-02T23:42:01Z

Changes proposed in this pull request:

Addresses (or will address) these items from the original issue:

generate dockerfiles using neurodocker

check if dockerfile hash has changed or force update is set (need to determine this from PR)

if force update, regenerate dockerfile

build docker image if needed or reload minimized image

run all tests via neurodocker reprozip (in parallel)

combine all reprozip packs into a single pack

generate minimized docker image using reprounzip and cache this image

Other changes:

Migrate to CircleCI 2.0.

This is a work-in-progress. It would be great to have some feedback on what I have so far, namely dockerfile generation and the migration to circleci 2.0.

Parts of the code refer to my personal dockerhub repo (kaczmarj/nipype) but those will be changed when this PR is closer to complete. Also, the master neurodocker docker image is used now, but I will change this to v0.3.2 once I release that version.

I am in the process of reworking neurodocker-reprozip's container-minimization functionality (VIDA-NYU/reprozip#274). I hope to finish that soon for this PR. I have already added a command to merge reprozip pack files (ReproNim/neurodocker#69).

Just to note, I propose using neurodebian:stretch-non-free as the base image, instead of ubuntu:xenial-20161213.

Todo

~~generate dockerfiles using neurodocker~~
check if dockerfile hash has changed or force update is set (need to determine this from PR)
- checking is implemented, but forcing update is not.
if force update, regenerate dockerfile
~~build docker image if needed or reload minimized image~~
run all tests via neurodocker reprozip (in parallel)
combine all reprozip packs into a single pack
generate minimized docker image using reprounzip and cache this image

Rename base.Dockerfile to Dockerfile.base to conform to moby style. For now, the master Neurodocker Docker image is used, but a versioned image will be used once a newer Neurodocker version is released.

The specification for CircleCI 2.0 is stored in `.circleci/config.yml` instead of in `circle.yml`. Todo: - Run tests. For now, images are built and pushed, but no tests are run. - Minimize containers with neurodocker reprozip. This functionality will be updated soon. See discussion in VIDA-NYU/reprozip#274.

kaczmarj · 2017-10-02T23:46:09Z

Please see https://circleci.com/gh/kaczmarj/nipype/182 for an example of the CircleCI run on my fork.

satra · 2017-10-03T00:48:17Z

docker/generate_dockerfiles.sh

+            fusefat g++ git graphviz make ruby unzip xvfb \
+  --add-to-entrypoint "source /etc/fsl/fsl.sh" \
+  --env ANTSPATH='/usr/lib/ants' PATH='/usr/lib/ants:$PATH' \
+  --c3d version=1.0.0 \


the latest version of convert3d might be available in neurodebian

satra · 2017-10-30T15:22:55Z

@kaczmarj - please make sure to merge with master, when sending an update.

kaczmarj · 2017-10-30T15:26:11Z

@satra - i'll be sure to do that. i am waiting on VIDA-NYU/reprozip#281. At the moment, reprozip trace returns status code 0 even if a traced command failed.

…odocker

- install dependency codecov - update nipype/info.py if CIRCLE_TAG is set - retry docker image builds - download test data - fix workdir permission denied issue - save all docker images to single tarball - pipe dockerhub password to `docker login` - push all docker images to docker hub on success

Use this file to prune Dockerfiles before getting their hash. - remove empty lines, comments, and timestamp

kaczmarj · 2017-11-06T21:54:31Z

In an effort to complete this PR in time for the release of version 0.14, minimizing containers with ReproZip will be done in a separate PR. The tests might have to be re-balanced in another PR.

Does it seem reasonable to use CircleCI's option to rebuild without cache to force a rebuild of the base Docker image?

Notes

The Dockerfile ADD/COPY instructions do not honor the current user, so the owner of the directories has to be manually changed to user neuro (Dockerfile: ADD does not honor USER: files always owned by root moby/moby#6119). Docker plans on changing this behavior by added a --chown flag to the ADD/COPY commands (Add --chown flag to ADD/COPY commands moby/moby#34263 and Add docs for Dockerfile ADD/COPY --chown flag docker/cli#467 for documentation). Looks like this is available in Docker v17.09.0-ce-rc1.
CircleCI 2.0 does not have the environment variable CIRCLE_TEST_REPORTS. Use store_artifacts in config.yml instead (https://circleci.com/docs/2.0/configuration-reference/#store_artifacts).

mgxd

looks pretty nice - thanks!

mgxd · 2017-11-07T20:50:47Z

.circleci/config.yml

+
+  compare_base_dockerfiles:
+    docker:
+      - image: docker:17.09.0-ce-git


could we update this to 17.10.0-ce for the chown flag

that's a good point, although the change would be in the machine executor of build_and_test. i can change that to circleci/classic:201710-02 (more info on machine executors).

mgxd · 2017-11-07T20:52:45Z

.circleci/config.yml

+            ash prune_dockerfile.sh Dockerfile.base > /tmp/docker/Dockerfile.base-pruned
+      - restore_cache:
+          # TODO: change this to 'master' after we are sure this works.
+          key: dftest-v5-enh/circleci-neurodocker-{{ checksum "/tmp/docker/Dockerfile.base-pruned" }}


does this need to be changed now?

yes i will change this

mgxd · 2017-11-07T20:55:19Z

.circleci/config.yml

+          # TODO: remove `docker pull` once once caching works.
+          command: |
+            # bash /tmp/docker/get_base_image.sh
+            docker pull kaczmarj/nipype:base


also has to be changed. this was in place to cut down on testing time when base docker image didn't change but the cache wasn't set yet

mgxd · 2017-11-07T21:00:48Z

.circleci/config.yml

+            e=1 && for i in {1..5}; do
+              docker build \
+                --rm=false \
+                --tag kaczmarj/nipype:latest \


nipype/nipype

mgxd · 2017-11-07T21:01:06Z

.circleci/config.yml

+            e=1 && for i in {1..5}; do
+              docker build \
+                --rm=false \
+                --tag kaczmarj/nipype:py27 \


nipype/nipype

mgxd · 2017-11-07T21:01:51Z

.circleci/config.yml

+          no_output_timeout: 120m
+          command: |
+            echo "$DOCKER_PASS" | docker login -u "$DOCKER_USER" --password-stdin
+            docker push kaczmarj/nipype:base


nipype/nipype

mgxd · 2017-11-07T21:02:13Z

.circleci/config.yml

+          filters:
+            branches:
+              # TODO: change this to master after we are sure this works.
+              only: enh/circleci-neurodocker


mgxd · 2017-11-07T21:02:29Z

.circleci/tests.sh

+fi
+
+# TODO: change this image name
+DOCKER_IMAGE="kaczmarj/nipype"


mgxd · 2017-11-07T21:05:28Z

Dockerfile


+COPY ["docker/files/run_builddocs.sh", "docker/files/run_examples.sh", "docker/files/run_pytests.sh", "nipype/external/fsl_imglob.py", "/usr/bin/"]


i don't feel too strongly about changing this, but maybe we can take advantage of --chown flag now

yes we can do that. although this would require an update to neurodocker. could this be done in a future pr?

mgxd · 2017-11-07T21:07:27Z

docker/generate_dockerfiles.sh

+# neurodebian:stretch-non-free pulled on November 3, 2017
+BASE_IMAGE="neurodebian@sha256:7590552afd0e7a481a33314724ae27f76ccedd05ffd7ac06ec38638872427b9b"
+
+NIPYPE_BASE_IMAGE="kaczmarj/nipype:base"


nipype/nipype

- use master branch for caching - replace kaczmarj/nipype with nipype/nipype - reset cache prefixes - use latest docker (v17.10.0-ce) containers and machine - use environment variable to determine whether to pull or build base image - save docker images to tar.gz if on master branch (deploying) - use fastest gzip compression (gives good ratio of speed/compression)

…odocker

satra · 2017-11-14T16:41:49Z

i think this looks good - one question: since we are generating the dockerfile do we need to store it?

kaczmarj · 2017-11-14T16:45:27Z

The Dockerfiles are not generated during the testing. I was thinking that someone would generate the Dockerfiles if needed, commit the changes, and push. The testing would use those Dockerfiles.

Do you want CircleCI to generate the Dockerfiles?

satra · 2017-11-14T17:38:24Z

@kaczmarj - since the testing uses docker and has a script that generates the dockerfile, can't that be generated during testing rather than being in the repo?

kaczmarj · 2017-11-14T18:05:56Z

That makes sense. Should both the base and main Dockerfiles be generated in CircleCI? Or would you like to have the main Dockerfile in the repo?

satra · 2017-11-14T20:04:38Z

i think anything that can be generated should be generated otherwise it can go out of sync.

- do not store dockerfiles in repo - cache base dockerfile in deploy step that was generated in `compare_base_dockerfiles` step - use global `working_directory`

kaczmarj · 2017-11-15T02:44:42Z

@satra - dockerfiles are generated in CircleCI now, and I removed them from the repo.

I should also mention that If the base Dockerfile is ever reverted to a previous cached version, the CircleCI cache prefix (i.e., 'dockerfile-cache-v1') should be changed (perhaps incremented to 'dockerfile-cache-v2'). Here are the two relevant lines 1 and 2.

If the base Dockerfile is reverted to be the same (identical, not considering comments and empty lines) as a previous, cached base Dockerfile, the old cached Dockerfile will be found and the current base Docker image will be pulled. This is not desirable, but it happens because CircleCI's caches are immutable.

mgxd · 2017-11-15T15:11:07Z

@kaczmarj don't forget to add yourself to the .zenodo file as well

jakubk added 2 commits October 2, 2017 18:57

enh: generate dockerfiles with neurodocker + rename base dockerfile

7b3c404

Rename base.Dockerfile to Dockerfile.base to conform to moby style. For now, the master Neurodocker Docker image is used, but a versioned image will be used once a newer Neurodocker version is released.

satra reviewed Oct 3, 2017

View reviewed changes

satra added this to the 0.14.0 milestone Oct 20, 2017

satra mentioned this pull request Oct 24, 2017

Problem with singularity import: bad variable name #2064

Closed

jakubk added 8 commits November 6, 2017 14:49

Merge remote-tracking branch 'upstream/master' into enh/circleci-neur…

4770217

…odocker

add: cli to generate dockerfiles

222b15f

Change default TESTPATH to /src/nipype/nipype from /src/nipype

3b81bcd

enh: use svg travis shield + rm whitespace

86d70f5

add: file to prune dockerfiles

70cf230

Use this file to prune Dockerfiles before getting their hash. - remove empty lines, comments, and timestamp

regenerate dockerfiles

1a2a3c4

enh: update codecov call + reformat

9e0571c

fix path when updating dockerfile cache

97b4048

mgxd reviewed Nov 7, 2017

View reviewed changes

mgxd added the needs-review label Nov 7, 2017

jakubk added 4 commits November 13, 2017 11:27

regenerate dockerfiles with base nipype/nipype:base

637f033

use nipype/nipype repo instead of kaczmarj/nipype

399fe16

Merge remote-tracking branch 'upstream/master' into enh/circleci-neur…

126831e

…odocker

kaczmarj changed the title ~~WIP+ENH: use neurodocker to generate and minimize containers for testing~~ ENH: generate Dockerfiles with neurodocker + migrate to CircleCI 2.0 Nov 14, 2017

mgxd requested a review from satra November 14, 2017 16:25

kaczmarj force-pushed the enh/circleci-neurodocker branch from 9f6544f to a189fb5 Compare November 15, 2017 02:15

generate dockerfiles in ci + rm dockerfiles

28547cc

- do not store dockerfiles in repo - cache base dockerfile in deploy step that was generated in `compare_base_dockerfiles` step - use global `working_directory`

kaczmarj force-pushed the enh/circleci-neurodocker branch from a189fb5 to 28547cc Compare November 15, 2017 02:26

add jakub kaczmarzyk (mit)

56838b9

mgxd merged commit edaf222 into nipy:master Nov 15, 2017

mgxd removed the needs-review label Nov 16, 2017

effigies mentioned this pull request Jan 20, 2018

TST/CI: Split Circle workflow #2386

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: generate Dockerfiles with neurodocker + migrate to CircleCI 2.0 #2202

ENH: generate Dockerfiles with neurodocker + migrate to CircleCI 2.0 #2202

kaczmarj commented Oct 2, 2017 •

edited

Loading

kaczmarj commented Oct 2, 2017

satra Oct 3, 2017

satra commented Oct 30, 2017

kaczmarj commented Oct 30, 2017

kaczmarj commented Nov 6, 2017

mgxd left a comment

mgxd Nov 7, 2017

kaczmarj Nov 7, 2017

mgxd Nov 7, 2017

kaczmarj Nov 7, 2017

mgxd Nov 7, 2017

kaczmarj Nov 7, 2017

mgxd Nov 7, 2017

mgxd Nov 7, 2017

mgxd Nov 7, 2017

mgxd Nov 7, 2017

mgxd Nov 7, 2017

mgxd Nov 7, 2017 •

edited

Loading

kaczmarj Nov 7, 2017

mgxd Nov 13, 2017

mgxd Nov 7, 2017

satra commented Nov 14, 2017

kaczmarj commented Nov 14, 2017

satra commented Nov 14, 2017

kaczmarj commented Nov 14, 2017

satra commented Nov 14, 2017

kaczmarj commented Nov 15, 2017

mgxd commented Nov 15, 2017


		COPY ["docker/files/run_builddocs.sh", "docker/files/run_examples.sh", "docker/files/run_pytests.sh", "nipype/external/fsl_imglob.py", "/usr/bin/"]

ENH: generate Dockerfiles with neurodocker + migrate to CircleCI 2.0 #2202

ENH: generate Dockerfiles with neurodocker + migrate to CircleCI 2.0 #2202

Conversation

kaczmarj commented Oct 2, 2017 • edited Loading

Todo

kaczmarj commented Oct 2, 2017

Choose a reason for hiding this comment

satra commented Oct 30, 2017

kaczmarj commented Oct 30, 2017

kaczmarj commented Nov 6, 2017

Notes

mgxd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgxd Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

satra commented Nov 14, 2017

kaczmarj commented Nov 14, 2017

satra commented Nov 14, 2017

kaczmarj commented Nov 14, 2017

satra commented Nov 14, 2017

kaczmarj commented Nov 15, 2017

mgxd commented Nov 15, 2017

kaczmarj commented Oct 2, 2017 •

edited

Loading

mgxd Nov 7, 2017 •

edited

Loading