Package ClusterRunner using Docker container #331

yamaszone · 2017-03-14T07:40:47Z

@josephharrington 👀
This addresses 1st and 2nd steps for my initial pull request. Please see the TODO notes and let me know your thoughts. This pull request adds 6 new items (one directory and five Docker related files) to the project root and I hope that's tolerable for now! :)

boxcla · 2017-03-14T07:40:56Z

Verified that @yamaszone has signed the CLA. Thanks for the pull request!

yamaszone · 2017-03-14T08:12:03Z

@josephharrington While adding bats framework based tests under test folder I was very iffy on mixing non-Python tests with pure Python and was (reluctantly) considering another project root level directories to keep them separate. Now I am seeing another use case by breaking the build above! :) Thoughts?

josephharrington · 2017-03-23T18:40:58Z

Hi @yamaszone, sorry for the slow response -- I was on vacation and just got back. I'll try to look at this within the next day or two.

josephharrington · 2017-04-06T22:45:58Z

docker-compose-bin.yml

@@ -0,0 +1,11 @@
+version: "2.1"
+services:
+    cluserrunner:


typo: cluserrunner :)

@josephharrington Thanks! :) Will fix that...

josephharrington · 2017-04-06T22:49:28Z

Dockerfile.src

@@ -0,0 +1,13 @@
+FROM python:3.4-slim


Just out of curiosity -- what is the naming convention around dockerfiles? I'm assuming you created Dockerfile.src to run clusterrunner from source, and Dockerfile.bin to run it from the binary. Is this more conventional that something like Dockerfile-source/Dockerfile-binary? My main goal here is avoiding confusion around the fact that they look like file extensions.

@josephharrington I have seen Dockerfile names with both dot and dash based separation. In some cases I came across Dockerfile name using both! But I agree more with the name you suggested to reduce confusion and to improve readability. Will fix that...

josephharrington · 2017-04-06T22:50:23Z

docker-compose-bin.yml

@@ -0,0 +1,11 @@
+version: "2.1"


Where does this version number come from?

@josephharrington Docker Compose has a number of docker-compose.yml file formats with particular Docker Engine version dependencies. The following explains compatibility requirement and API versioning with upgrade process in details:

https://docs.docker.com/compose/compose-file/compose-versioning/#compatibility-matrix

https://docs.docker.com/compose/compose-file/compose-versioning/#versioning

At the time of writing compose file for ClusterRunner in this pull request, Docker version in my CoreOS environment supported compose API version 2.1.

josephharrington · 2017-04-06T22:54:58Z

test/bats/run-cr.bats

+	run docker run --rm -v $PWD:/sut -w /sut $CR_SRC build --job-name Simple
+	assert_contains "$output" "NO_FAILURES"
+	cd -
+	# Needs super user privilege as build artefacts are written as root


typo: artefacts

Same typo at least one other place in this change.

@josephharrington I mixed up British spelling with American. I will fix that... :)

josephharrington · 2017-04-06T22:55:41Z

test/bats/tmp/.gitignore

@@ -0,0 +1 @@
+#ignore


Did you mean to include this directory/file? I saw there was one other empty .gitignore as well -- maybe include a comment in the file about why it's there?

@josephharrington tmp, fixtures, etc. directories are part of the BATS framework. I will add comment in the placeholder gitignore files to clarify that.

josephharrington · 2017-04-06T23:03:59Z

test/bats/run-cr.bats

+
+@test "CR-BIN: Help msg displayed by image built from binary." {
+	run ./run-cr bin -h
+	assert_contains "$output" "usage: clusterrunner"


This bats framework is interesting -- I hadn't heard of it before!

Also the run-cr script is great. I'm interested in your thoughts on doing the tests in Python instead. These seem like simple enough tests to write in Python, and it might be better to keep all tests in the same framework/language just so that they're easier/more likely to be maintained.

@josephharrington I have been using BATS for a while for health checks/sanity testing for UNIX environments which almost always come with bash and don't need additional dependency management for this purpose. BATS seems to be a popular in the Docker community (e.g. Docker swarm/machine, Dokku, etc.).

I should be able to convert the sanity tests for CR image using Python but that will mean:

Docker, Docker Compose are test runtime dependencies although they are not app dependencies.

docker-py will be another app dependency to do it properly unless using classic Python subprocess.
Both of the above aspects feel a bit heavy considering the purpose, and that's why I intended to keep BATS tests outside the main suite of CR.

I have been rethinking this. Keeping SRP in mind, I'd like to keep the Dockerized distribution responsibility separate from the CR app repository using another repository (say clusterrunner-docker-image). This way:

Distribution related dependencies will be decoupled from app dependencies and will allow easier maintenance.

CR Docker image can be independently patched if high priority vulnerabilities found with the base images or dependency related libraries.
I have seen this pattern frequently used for many popular Docker images (e.g. Terraform, Docker library images, etc.).

CR Docker image repo can be built with packages related to distribution via CI. I'll add CI hooks to build automatically via Circle/Travis (to run tests) and Docker Hub/Quay.io (to build image). This will also help us finding known vulnerabilities through security scans.

I will create a POC repo for this soon and would like to know your thoughts.

@josephharrington I have created a dedicated repo for ClusterRunner Docker image here. Please let me know your thoughts...

josephharrington · 2017-04-06T23:07:02Z

test/bats/helpers/assertions/LICENSE

@@ -0,0 +1,22 @@
+The MIT License (MIT)


(See my comment below about potentially keeping all tests in Python, but if we did start using bats...)
If possible, I think I'd lean towards not committing the bats library to the repo and instead downloading it (potentially as part of make init) similar to how we download/install the Python dependencies externally.

@josephharrington It's me being lazy in the early days when I adopted BATS and not being sure if these supporting libraries will be well maintained. :) I forked these repo to address my maintenance concern. I have already fixed this for some of my other repos. I will fix that here too... Thanks for bringing this.

josephharrington · 2017-04-07T01:54:01Z

docs/docker/README.md

@@ -0,0 +1,57 @@
+## TODO for Dockerization
+* Add maintainer(s)
+  - Maintainer for CR Dockerfile needs to be added after pull request is merged. Ideally the maintainer should be someone from official team with appropriate Docker Hub permission to tag image as `box/clusterrunner:latest`. I can volunteer if needed but that will require collaborator permission for Docker Hub.


Can you elaborate on what's involved with this? We have a Docker Hub account named boxinc that I can use. I'm assuming this is something we'd have to do for every new version?

@josephharrington Currently I can push CR images to Docker Hub only using my Hub username i.e. as yamaszone/clusterrunner:src. The official CR image should be tagged and pushed as boxinc/clusterrunner:latest, boxinc/clusterrunner:0.5.473, boxinc/clusterrunner:0.5.474, etc. which only users under boxinc and collaborators of a repo have permissions this action. I will include an environment variable CLUSTERRUNNER_VERSION in the Dockerfile so that there is a Docker image for every version of CR.

josephharrington · 2017-04-07T01:55:23Z

docs/docker/README.md

+* Add maintainer(s)
+  - Maintainer for CR Dockerfile needs to be added after pull request is merged. Ideally the maintainer should be someone from official team with appropriate Docker Hub permission to tag image as `box/clusterrunner:latest`. I can volunteer if needed but that will require collaborator permission for Docker Hub.
+* Lean Docker image (consider tiny base e.g. busybox, alpine, etc.)
+  - Currently `Dockerfile.src` uses `python:3.4-slim` as the base. I attempted `python:alpine` as the base but it didn't feel like worth the [trouble](https://github.com/docker/docker/issues/27940). My docker version is `1.12.6` but the problem might have been fixed in `1.13.0+`. I will revisit this in the future.


Does python:3.4-slim include more than what is needed? (I assume it does from your comment, but I'm curious as to what.)

@josephharrington The official python:3.4-slim Dockerfile includes a bunch of dependencies but I am not sure if all of them are needed for CR. I am yet to do a research though...

josephharrington · 2017-04-07T02:01:51Z

docker-compose-src.yml

@@ -0,0 +1,7 @@
+version: "2.1"
+services:
+    cluserrunner:


@josephharrington Thanks. Will fix that...

yamaszone · 2017-04-18T03:35:50Z

@josephharrington Just wanted to let you know that this is in my radar. Thanks for the feedback! 👍 I plan to address them later this month. I will ping you if I need your help on anything.

vpm-bradleyhession · 2017-10-10T11:49:27Z

Any updates to this? Really would like to see it in docker sometime soon.

CLAassistant · 2020-03-03T19:01:28Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

yamaszone and others added 8 commits February 22, 2017 08:02

WIP: Added two quick Dockerfiles to compare bin vs. src packaging

6542123

WIP: Initial commit to Dockerize ClusterRunner

39e69a8

Added docs for users and developers

7449a41

Installed BATS properly instead of adding it to the codebase

bf00999

Added assertion for build success using demo project

73a24d0

Cleaned up redundant mounts

4b4f68e

Updated notes for Dockerization TODO list

ff6e0ee

Fixed typo in docs

7199d90

josephharrington reviewed Apr 7, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Package ClusterRunner using Docker container #331

Package ClusterRunner using Docker container #331

yamaszone commented Mar 14, 2017

boxcla commented Mar 14, 2017

yamaszone commented Mar 14, 2017

josephharrington commented Mar 23, 2017

josephharrington Apr 6, 2017

yamaszone May 1, 2017

josephharrington Apr 6, 2017

yamaszone May 1, 2017

josephharrington Apr 6, 2017

yamaszone May 1, 2017

josephharrington Apr 6, 2017

yamaszone May 1, 2017

josephharrington Apr 6, 2017

yamaszone May 1, 2017

josephharrington Apr 6, 2017

yamaszone May 1, 2017

yamaszone May 1, 2017

josephharrington Apr 6, 2017

yamaszone May 1, 2017

josephharrington Apr 7, 2017 •

edited

Loading

yamaszone May 1, 2017

josephharrington Apr 7, 2017

yamaszone May 1, 2017

josephharrington Apr 7, 2017

yamaszone May 1, 2017

yamaszone commented Apr 18, 2017

vpm-bradleyhession commented Oct 10, 2017

CLAassistant commented Mar 3, 2020

		@@ -0,0 +1 @@
		#ignore

Package ClusterRunner using Docker container #331

Are you sure you want to change the base?

Package ClusterRunner using Docker container #331

Conversation

yamaszone commented Mar 14, 2017

boxcla commented Mar 14, 2017

yamaszone commented Mar 14, 2017

josephharrington commented Mar 23, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephharrington Apr 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamaszone commented Apr 18, 2017

vpm-bradleyhession commented Oct 10, 2017

CLAassistant commented Mar 3, 2020

josephharrington Apr 7, 2017 •

edited

Loading