New Charm: flannel-docker

[Charles Butler]

I have written a new charm that I feel is a good submission for a reference charm utilizing flannel for the purpose of networking docker containers

What this charm provides:

A flannel runtime for X86 based hosts to establish private networking for cross-host container networking. This allows containers to be deployed in HA situations, as well as extending the 'farm' to run on multiple hosts, and communicate amongst themselves.

This charm has not been tested with any of the other docker charms, and has a hard dependency on the relationship structure of the proposed docker charm in bug# 1413775 - and that charm should be treated as a predependency to this merge.

The tests are scoped strictly to flannel, and it comes with full documentation on the development focus repository on Github here: http://chuckbutler.github.io/flannel-docker-charm/

I look forward to any feedback you might have

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-10972-results

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-10994-results

[Cory Johns (johnsca)]

Charles,

Thank you for this contribution, and this is an exciting step toward getting better Docker integration with Juju. I had a couple of minor notes, but overall this seems good (Amulet test passes, icon & README look good, etc). +1

* The lint check fails with: "ERROR: Unable to find an inventory file, specify one with -i ?" This is reminiscent of an error I saw on the docker charm when using an out-dated Makefile, so I'm guessing there is a fix from there that needs to be applied here

* The Amulet tests contain `amulet.raise_status()` statements, which don't play well with the unittest.TestCase pattern. Those should be changed to `self.assertEqual()` or at least `self.fail()`.

* The start hook symlink is missing, which could cause issues if a unit is restarted

[Matt Bruzek (mbruzek)]

I took a look at this charm as well and had the same error when running make lint. Looking at the Makefile, I see the lint target looking for the file docs/faux-inventory.conf but I see no file like that in the charm.

Both python tests also fail.

# 10-deploy.py

The test calls out a local:trusty/docker charm that was not on my system when I ran the tests. I would recommend changing the charm to deploy the personal namespaced charm cs:~lazypower/trusty/docker so the test can pass in automation.

The setUpClass method is decorated as a @classmethod, but uses an argument named "self". According to the docs (https://docs.python.org/2/library/functions.html#classmethod) a class method receives the class as the first argument. I would recommend renaming the variable "cls" to follow convention here. This may seem like a nit, but I recently discovered this difference when debugging a nasty error in the setUpClass() method of my own tests, and got better results when I realized the agument was *not* an instance of the class but the class object itself. I have also been writing this tests using the argument "self" incorrectly, so you might have seen one of my tests use it incorrectly.

# 99-autogen

This test also fails in bundletester and since you have a real test (10-deploy.py) I would recommend removing the file. Before you delete the file please notice that the setUpClass method correctly uses the cls variable.

Let me know when you make the updates and I will be happy to review this charm again. Thanks!

[Matt Bruzek (mbruzek)]

I ran this branch in our automated testing infrastructure and the code passes for all environments!

+1 LGTM

Thanks for the work here Charles!

[Charles Butler (lazypower)]

a quick update note re: missing start hook

The start hook is actually a no-op in this case, as it has an upstart job that controls the flannel service. The missing start hook works as expected - flannel is only configured to become part of the system services after the network relationship is joined.

Good catch though!