Merge ~hloeung/content-cache-charm:master into content-cache-charm:master

[Haw Loeung]

Initial charm

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Haw Loeung (hloeung)]

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change cannot be self approved, setting status to needs review.

[Stuart Bishop (stub)]

This all looks good, and nice to see someone chasing complete test coverage.

I think I found two bugs, commented on inline, but neither dificult to fix.

The rest is stylistic stuff. Most of them are just suggestions on alternative unittest.mock asserts; it is a very fat API and the docs take some getting used to.

I do think it is worth adding layer:status to your layer.yaml, as it will make the charm easier to maintain and, since this is a rare example of a tested charm, avoids some false positives in the tests. Particularly, there will be no need to ensure that an important status like 'blocked' was the last status set, and instead we can rely on layer:status ensuring that the important status is presented to the user when the hook finishes.

In general, I think it is its better for tests to focus on particular important results and only test those, rather than that the implementation exactly matches what the test expects. A good test is one where the implementation can be changed, and if the results are the same, the test passes unmodified. Mocks don't make this easy, and it is worth thinking about if you are best off with exact matches (eg. these flags where set, and only these flags, and in this order), or less fragile tests (eg. these flags were set, and maybe some others, and order doesn't matter).

Assuming this is still a work in progress, it is worth creating a new branch. Future merge proposals can use this branch as the dependent branch, and only the changes displayed for review.

review: Approve

[Haw Loeung (hloeung)]

First round of fixes. Will continue with more.

[Haw Loeung (hloeung)]

Second found of fixes done.

[Haw Loeung (hloeung)]

Another round of fixes.

[Haw Loeung (hloeung)]

Last round of fixes to address all of Stuart's points.

[Haw Loeung (hloeung)]

review: Needs Resubmitting

[Stuart Bishop (stub)]

Yup, all good.

You may be able to replace the status reset_mocks with a single one, doing charms.layer.reset_mock() in the TestCase's setUp method.

It would be possible to target the status mock by installing the MagicMock at charms.layer.status instead of charms.layer. However, what you have may be more useful as pretty much anything that comes from charms.layer will need to be mocked.

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 6dc158dda107948ed5c41d42dbfe81a82f59b66d

diff --git a/Makefile b/Makefile
index cbd8b5c..e592bf7 100644
--- a/Makefile
+++ b/Makefile
@@ -23,8 +23,8 @@ functionaltest:
 
 clean:
 	@echo "Cleaning files"
-	@if [ -d ./.tox ] ; then rm -r ./.tox ; fi
-	@if [ -d ./.pytest_cache ] ; then rm -r ./.pytest_cache ; fi
+	@rm -rf ./.tox
+	@rm -rf ./.pytest_cache
 
 # The targets below don't depend on a file
 .PHONY: lint test unittest functionaltest clean help
diff --git a/README.md b/README.md
index e69de29..004a714 100644
--- a/README.md
+++ b/README.md
@@ -0,0 +1,9 @@
+# Overview
+
+Deploy your own content distribution network (CDN).
+
+
+# Usage
+
+
+# TODO
diff --git a/config.yaml b/config.yaml
index 8d4f5d1..36d48c3 100644
--- a/config.yaml
+++ b/config.yaml
@@ -11,3 +11,15 @@ options:
     description: |
       A comma-separated list of nagios servicegroups.
       If left empty, the nagios_context will be used as the servicegroup
+  sites:
+    default: ""
+    type: string
+    description: |
+      YAML-formatted virtual hosts/sites. e.g.
+      site1.local:
+        server:
+          server_name: site1.local
+          location:
+            path: /
+            proxy_pass: http://localhost:8081
+            proxy_set_header: Host "site1.local"
diff --git a/layer.yaml b/layer.yaml
index 3a60daa..253b93a 100644
--- a/layer.yaml
+++ b/layer.yaml
@@ -2,4 +2,10 @@ includes:
   - layer:basic
   - layer:apt
   - layer:nagios
-repo: lp:cdn-charm
+  - layer:status
+repo: lp:content-cache-charm
+options:
+  basic:
+    packages:
+      - haproxy
+      - nginx
diff --git a/lib/nginx.py b/lib/nginx.py
new file mode 100644
index 0000000..ac2f0eb
--- /dev/null
+++ b/lib/nginx.py
@@ -0,0 +1,83 @@
+import os
+
+NGINX_SITE_BASE_PATH = '/etc/nginx'
+INDENT = ' '*4
+
+
+class NginxConf:
+
+    def __init__(self, sites_base_path=NGINX_SITE_BASE_PATH):
+        self._sites_path = os.path.join(sites_base_path, 'sites-available')
+
+    # Expose sites_path as a property to allow mocking in indirect calls to
+    # this class.
+    @property
+    def sites_path(self):
+        return self._sites_path
+
+    def write_site(self, site, new):
+        fname = os.path.join(self.sites_path, site)
+        # Check if contents changed
+        try:
+            with open(fname, 'r', encoding='utf-8') as f:
+                current = f.read()
+        except FileNotFoundError:
+            current = ''
+        if new == current:
+            return False
+        with open(fname, 'w', encoding='utf-8') as f:
+            f.write(new)
+        return True
+
+    def sync_sites(self, sites):
+        changed = False
+        for site in os.listdir(self.sites_path):
+            available = os.path.join(self.sites_path, site)
+            enabled = os.path.join(os.path.dirname(self.sites_path), 'sites-enabled', site)
+            if site not in sites:
+                changed = True
+                try:
+                    os.remove(available)
+                    os.remove(enabled)
+                except FileNotFoundError:
+                    pass
+            elif not os.path.exists(enabled):
+                changed = True
+                os.symlink(available, enabled)
+
+        return changed
+
+    def render(self, conf):
+        output = []
+        for key in conf.keys():
+            if key == 'server':
+                output.append(self._render_server(conf[key]))
+            else:
+                output.append('{key} {value};'
+                              .format(key=key, value=conf[key]))
+        return '\n'.join(output)
+
+    def _render_server(self, conf):
+        output = ['\nserver {']
+        for key in conf.keys():
+            if key == 'location':
+                output.append(self._render_location(conf[key]))
+            else:
+                output.append('{indent}{key} {value};'
+                              .format(indent=INDENT, key=key, value=conf[key]))
+        for log in ['access_log', 'error_log']:
+            if log not in conf:
+                output.append('{indent}{key} /var/log/nginx/{site}-access.log;'
+                              .format(indent=INDENT, key=log, site=conf['server_name']))
+        output.append('}\n')
+        return '\n'.join(output)
+
+    def _render_location(self, conf):
+        output = ['\n{}location {} {{'.format(INDENT, conf['path'])]
+        for key in conf.keys():
+            if key == 'path':
+                continue
+            output.append('{indent}{indent}{key} {value};'
+                          .format(indent=INDENT, key=key, value=conf[key]))
+        output.append('{}}}\n'.format(INDENT))
+        return '\n'.join(output)
diff --git a/metadata.yaml b/metadata.yaml
index 43eba49..ceba5c2 100644
--- a/metadata.yaml
+++ b/metadata.yaml
@@ -1,10 +1,14 @@
-name: cdn
-summary: Content distribution network (CDN)
-maintainer: CDN Charmers <[e-mail]>
+name: content-cache
+summary: Content / Frontend Cache
+maintainer: Content Cache Charmers <[e-mail]>
 description: |
-  Deploy CDN
+  Installs Nginx and HAProxy as a highly available web accelerator
+  with TLS support. Useful for providing local mirrors of HTTP servers
+  and building content delivery networks (CDN).
 tags:
   - cache-proxy
+  - content-cache
+  - web-cache
   - ops
 series:
   - bionic
diff --git a/pytest.ini b/pytest.ini
new file mode 100644
index 0000000..39da980
--- /dev/null
+++ b/pytest.ini
@@ -0,0 +1,6 @@
+[pytest]
+filterwarnings =
+    # https://github.com/juju/charm-helpers/issues/294
+    ignore:.*inspect.getargspec\(\) is deprecated.*:DeprecationWarning
+    # https://github.com/juju/charm-helpers/issues/293
+    ignore:.*dist\(\) and linux_distribution\(\) functions are deprecated:PendingDeprecationWarning
diff --git a/reactive/cdn.py b/reactive/cdn.py
deleted file mode 100644
index e69de29..0000000
--- a/reactive/cdn.py
+++ /dev/null
diff --git a/reactive/content_cache.py b/reactive/content_cache.py
new file mode 100644
index 0000000..d620521
--- /dev/null
+++ b/reactive/content_cache.py
@@ -0,0 +1,86 @@
+import yaml
+
+from charms import reactive
+from charms.layer import status
+from charmhelpers.core import hookenv, host
+from lib import nginx
+
+
[e-mail]('upgrade-charm')
+def upgrade_charm():
+    status.maintenance('forcing reconfiguration on upgrade-charm')
+    reactive.clear_flag('content_cache.active')
+    reactive.clear_flag('content_cache.installed')
+    reactive.clear_flag('content_cache.haproxy.configured')
+    reactive.clear_flag('content_cache.nginx.configured')
+
+
[e-mail]_not('content_cache.installed')
+def install():
+    reactive.clear_flag('content_cache.active')
+
+    reactive.clear_flag('content_cache.haproxy.configured')
+    reactive.clear_flag('content_cache.nginx.configured')
+    reactive.set_flag('content_cache.installed')
+
+
[e-mail]('config.changed')
+def config_changed():
+    reactive.clear_flag('content_cache.haproxy.configured')
+    reactive.clear_flag('content_cache.nginx.configured')
+
+
[e-mail]('content_cache.nginx.configured', 'content_cache.haproxy.configured')
[e-mail]_not('content_cache.active')
+def set_active():
+    # XXX: Add more info such as nginx and haproxy status
+    status.active('ready')
+    reactive.set_flag('content_cache.active')
+
+
+def service_start_or_restart(name):
+    if host.service_running(name):
+        status.maintenance('Restarting {}...'.format(name))
+        host.service_restart(name)
+    else:
+        status.maintenance('Starting {}...'.format(name))
+        host.service_start(name)
+
+
[e-mail]_not('content_cache.nginx.configured')
+def configure_nginx():
+    config = hookenv.config()
+
+    if not config.get('sites'):
+        status.blocked('requires list of sites to configure')
+        reactive.clear_flag('content_cache.active')
+        return
+
+    ngx_conf = nginx.NginxConf()
+    conf = yaml.safe_load(config.get('sites'))
+    changed = False
+    for site in conf.keys():
+        if ngx_conf.write_site(site, ngx_conf.render(conf[site])):
+            hookenv.log('Wrote out new configs for site: {}'.format(site))
+            changed = True
+    if ngx_conf.sync_sites(conf.keys()):
+        hookenv.log('Enabled sites: {}'.format(' '.join(conf.keys())))
+        changed = True
+    if changed:
+        service_start_or_restart('nginx')
+
+    reactive.set_flag('content_cache.nginx.configured')
+
+
[e-mail]_not('content_cache.haproxy.configured')
+def configure_haproxy():
+    config = hookenv.config()
+
+    if not config.get('sites'):
+        status.blocked('requires list of sites to configure')
+        reactive.clear_flag('content_cache.active')
+        return
+
+    # TODO: Configure up and start/restart HAProxy
+
+    reactive.set_flag('content_cache.haproxy.configured')
diff --git a/tests/functional/test_cdn.py b/tests/functional/test_content_cache.py
similarity index 86%
rename from tests/functional/test_cdn.py
rename to tests/functional/test_content_cache.py
index 70a79da..e5660c3 100644
--- a/tests/functional/test_cdn.py
+++ b/tests/functional/test_content_cache.py
@@ -21,7 +21,7 @@ async def model():
 async def apps(model):
     apps = []
     for entry in series:
-        app = model.applications['cdn-{}'.format(entry)]
+        app = model.applications['content_cache-{}'.format(entry)]
         apps.append(app)
     return apps
 
@@ -35,15 +35,15 @@ async def units(apps):
 
 
 @pytest.mark.parametrize('series', series)
-async def test_cdn_deploy(model, series):
+async def test_content_cache_deploy(model, series):
     # Starts a deploy for each series
-    await model.deploy('{}/builds/cdn'.format(juju_repository),
+    await model.deploy('{}/builds/content_cache'.format(juju_repository),
                        series=series,
-                       application_name='cdn-{}'.format(series))
+                       application_name='content_cache-{}'.format(series))
     assert True
 
 
-async def test_cdn_status(apps, model):
+async def test_content_cache_status(apps, model):
     # Verifies status for all deployed series of the charm
     for app in apps:
         await model.block_until(lambda: app.status == 'active')
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site1.local.txt b/tests/unit/files/nginx_config_rendered_test_output-site1.local.txt
new file mode 100644
index 0000000..f5b318d
--- /dev/null
+++ b/tests/unit/files/nginx_config_rendered_test_output-site1.local.txt
@@ -0,0 +1,19 @@
+proxy_cache_path /var/cache/nginx/site1.local use_temp_path=off levels=1:2 keys_zone=site1-cache:10m max_size=1g;
+
+server {
+    server_name site1.local;
+    listen 6081;
+
+    location / {
+        proxy_pass http://localhost:8081;
+        proxy_set_header Host "site1.local";
+        proxy_cache valid 200 1d;
+        proxy_cache_lock on;
+        proxy_cache_min_uses 5;
+        proxy_cache_revalidate on;
+        proxy_cache_use_stale error timeout updating http_500 http_502 http_503 http_504;
+    }
+
+    access_log /var/log/nginx/site1.local-access.log;
+    error_log /var/log/nginx/site1.local-access.log;
+}
diff --git a/tests/unit/files/nginx_config_rendered_test_output-site2.local.txt b/tests/unit/files/nginx_config_rendered_test_output-site2.local.txt
new file mode 100644
index 0000000..a76e005
--- /dev/null
+++ b/tests/unit/files/nginx_config_rendered_test_output-site2.local.txt
@@ -0,0 +1,14 @@
+proxy_cache_path /var/cache/nginx/site2.local use_temp_path=off levels=1:2 keys_zone=site2-cache:10m max_size=1g;
+
+server {
+    server_name site2.local;
+    listen 6082;
+    access_log /var/log/nginx/site2.local-access.log;
+    error_log /var/log/nginx/site2.local-error.log;;
+
+    location / {
+        proxy_pass http://localhost:8082;
+        proxy_set_header Host "site2.local";
+    }
+
+}
diff --git a/tests/unit/files/nginx_config_test_config.txt b/tests/unit/files/nginx_config_test_config.txt
new file mode 100644
index 0000000..9087fae
--- /dev/null
+++ b/tests/unit/files/nginx_config_test_config.txt
@@ -0,0 +1,25 @@
+site1.local:
+    proxy_cache_path: /var/cache/nginx/site1.local use_temp_path=off levels=1:2 keys_zone=site1-cache:10m max_size=1g
+    server:
+        server_name: site1.local
+        listen: 6081
+        location:
+            path: /
+            proxy_pass: http://localhost:8081
+            proxy_set_header: Host "site1.local"
+            proxy_cache: valid 200 1d
+            proxy_cache_lock: 'on'
+            proxy_cache_min_uses: 5
+            proxy_cache_revalidate: 'on'
+            proxy_cache_use_stale: error timeout updating http_500 http_502 http_503 http_504
+site2.local:
+    proxy_cache_path: /var/cache/nginx/site2.local use_temp_path=off levels=1:2 keys_zone=site2-cache:10m max_size=1g
+    server:
+        server_name: site2.local
+        listen: 6082
+        access_log: /var/log/nginx/site2.local-access.log
+        error_log: /var/log/nginx/site2.local-error.log;
+        location:
+            path: /
+            proxy_pass: http://localhost:8082
+            proxy_set_header: Host "site2.local"
diff --git a/tests/unit/test_cdn.py b/tests/unit/test_cdn.py
deleted file mode 100644
index 6342df8..0000000
--- a/tests/unit/test_cdn.py
+++ /dev/null
@@ -1,18 +0,0 @@
-import shutil
-import tempfile
-import unittest
-
-
-class TestCDN(unittest.TestCase):
-    def setUp(self):
-        self.tmpdir = tempfile.mkdtemp(prefix='charm-unittests-')
-
-    def tearDown(self):
-        shutil.rmtree(self.tmpdir)
-
-    def test_test(self):
-        self.assertTrue(True)
-
-
-if __name__ == '__main__':
-    unittest.main()
diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
new file mode 100644
index 0000000..2366df8
--- /dev/null
+++ b/tests/unit/test_content_cache.py
@@ -0,0 +1,152 @@
+import os
+import shutil
+import sys
+import tempfile
+import unittest
+from unittest import mock
+
+# Add path to where our reactive layer lives and import.
+sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))))
+# We also need to mock up charms.layer so we can run unit tests without having
+# to build the charm and pull in layers such as layer-status.
+sys.modules['charms.layer'] = mock.MagicMock()
+from charms.layer import status  # NOQA: E402
+from reactive import content_cache  # NOQA: E402
+
+
+class TestCharm(unittest.TestCase):
+    def setUp(self):
+        self.tmpdir = tempfile.mkdtemp(prefix='charm-unittests-')
+        self.addCleanup(shutil.rmtree, self.tmpdir)
+
+        self.charm_dir = os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__))))
+
+        patcher = mock.patch('charmhelpers.core.hookenv.log')
+        self.mock_log = patcher.start()
+        self.addCleanup(patcher.stop)
+        self.mock_log.return_value = ''
+
+        patcher = mock.patch('charmhelpers.core.hookenv.charm_dir')
+        self.mock_charm_dir = patcher.start()
+        self.addCleanup(patcher.stop)
+        self.mock_charm_dir.return_value = self.charm_dir
+
+        patcher = mock.patch('charmhelpers.core.hookenv.local_unit')
+        self.mock_local_unit = patcher.start()
+        self.addCleanup(patcher.stop)
+        self.mock_local_unit.return_value = 'mock-content-cache/0'
+
+        patcher = mock.patch('charmhelpers.core.hookenv.config')
+        self.mock_config = patcher.start()
+        self.addCleanup(patcher.stop)
+        self.mock_config.return_value = {}
+
+    @mock.patch('charms.reactive.clear_flag')
+    def test_hook_upgrade_charm_flags(self, clear_flag):
+        '''Test correct flags set via upgrade-charm hook'''
+        status.maintenance.reset_mock()
+        content_cache.upgrade_charm()
+        self.assertFalse(status.maintenance.assert_called())
+        expected = [mock.call('content_cache.active'),
+                    mock.call('content_cache.installed'),
+                    mock.call('content_cache.haproxy.configured'),
+                    mock.call('content_cache.nginx.configured')]
+        self.assertFalse(clear_flag.assert_has_calls(expected, any_order=True))
+
+    @mock.patch('charms.reactive.clear_flag')
+    @mock.patch('charms.reactive.set_flag')
+    def test_hook_install_flags(self, set_flag, clear_flag):
+        '''Test correct flags are set via install charm hook'''
+        content_cache.install()
+        expected = [mock.call('content_cache.installed')]
+        self.assertFalse(set_flag.assert_has_calls(expected, any_order=True))
+
+        expected = [mock.call('content_cache.active'),
+                    mock.call('content_cache.haproxy.configured'),
+                    mock.call('content_cache.nginx.configured')]
+        self.assertFalse(clear_flag.assert_has_calls(expected, any_order=True))
+
+    @mock.patch('charms.reactive.clear_flag')
+    def test_hook_config_changed_flags(self, clear_flag):
+        '''Test correct flags are set via config-changed charm hook'''
+        content_cache.config_changed()
+        expected = [mock.call('content_cache.haproxy.configured'),
+                    mock.call('content_cache.nginx.configured')]
+        self.assertFalse(clear_flag.assert_has_calls(expected, any_order=True))
+
+    @mock.patch('charms.reactive.set_flag')
+    def test_hook_set_active(self, set_flag):
+        status.active.reset_mock()
+        content_cache.set_active()
+        self.assertFalse(status.active.assert_called())
+        self.assertFalse(set_flag.assert_called_once_with('content_cache.active'))
+
+    @mock.patch('charmhelpers.core.host.service_running')
+    @mock.patch('charmhelpers.core.host.service_restart')
+    @mock.patch('charmhelpers.core.host.service_start')
+    def test_service_start_or_restart_running(self, service_start, service_restart, service_running):
+        '''Test service restarted when already running'''
+        service_running.return_value = True
+        status.active.reset_mock()
+        content_cache.service_start_or_restart('someservice')
+        self.assertFalse(status.maintenance.assert_called())
+        self.assertFalse(service_start.assert_not_called())
+        self.assertFalse(service_restart.assert_called_once_with('someservice'))
+
+    @mock.patch('charmhelpers.core.host.service_running')
+    @mock.patch('charmhelpers.core.host.service_restart')
+    @mock.patch('charmhelpers.core.host.service_start')
+    def test_service_start_or_restart_stopped(self, service_start, service_restart, service_running):
+        '''Test service started up when not running/stopped'''
+        service_running.return_value = False
+        status.active.reset_mock()
+        content_cache.service_start_or_restart('someservice')
+        self.assertFalse(status.maintenance.assert_called())
+        self.assertFalse(service_start.assert_called_once_with('someservice'))
+        self.assertFalse(service_restart.assert_not_called())
+
+    @mock.patch('charms.reactive.clear_flag')
+    def test_configure_nginx_no_sites(self, clear_flag):
+        '''Test correct flags are set when no sites defined to configure Nginx'''
+        status.blocked.reset_mock()
+        content_cache.configure_nginx()
+        self.assertFalse(status.blocked.assert_called())
+        self.assertFalse(clear_flag.assert_called_once_with('content_cache.active'))
+
+    @mock.patch('reactive.content_cache.service_start_or_restart')
+    def test_configure_nginx_sites(self, service_start_or_restart):
+        '''Test configuration of Nginx sites'''
+        with open('tests/unit/files/nginx_config_test_config.txt', 'r', encoding='utf-8') as f:
+            ngx_config = f.read()
+        self.mock_config.return_value = {'sites': ngx_config}
+
+        with mock.patch('lib.nginx.NginxConf.sites_path', new_callable=mock.PropertyMock) as mock_site_path:
+            mock_site_path.return_value = os.path.join(self.tmpdir, 'sites-available')
+            # sites-available and sites-enabled won't exist in our temp dir
+            os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
+            os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
+            content_cache.configure_nginx()
+            self.assertFalse(service_start_or_restart.assert_called_once_with('nginx'))
+
+            # Re-run with same set of sites, no change so shouldn't need to restart Nginx
+            service_start_or_restart.reset_mock()
+            content_cache.configure_nginx()
+            self.assertFalse(service_start_or_restart.assert_not_called())
+
+    @mock.patch('charms.reactive.clear_flag')
+    def test_configure_haproxy_no_sites(self, clear_flag):
+        status.blocked.reset_mock()
+        content_cache.configure_haproxy()
+        self.assertFalse(status.blocked.assert_called())
+        self.assertFalse(clear_flag.assert_called_once_with('content_cache.active'))
+
+    @mock.patch('reactive.content_cache.service_start_or_restart')
+    def test_configure_haproxy_sites(self, service_start_or_restart):
+        with open('tests/unit/files/nginx_config_test_config.txt', 'r', encoding='utf-8') as f:
+            ngx_config = f.read()
+        self.mock_config.return_value = {'sites': ngx_config}
+        content_cache.configure_haproxy()
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/unit/test_nginx.py b/tests/unit/test_nginx.py
new file mode 100644
index 0000000..7cec4e6
--- /dev/null
+++ b/tests/unit/test_nginx.py
@@ -0,0 +1,83 @@
+import os
+import shutil
+import sys
+import tempfile
+import unittest
+import yaml
+
+sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))))
+from lib import nginx  # NOQA: E402
+
+
+class TestLibNginx(unittest.TestCase):
+    def setUp(self):
+        self.tmpdir = tempfile.mkdtemp(prefix='charm-unittests-')
+        self.addCleanup(shutil.rmtree, self.tmpdir)
+        self.charm_dir = os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__))))
+
+    def test_nginx_config_sites_path(self):
+        sites_path = '/etc/nginx/sites-available'
+        ngx_conf = nginx.NginxConf()
+        self.assertEqual(ngx_conf.sites_path, sites_path)
+
+    def test_nginx_config_render(self):
+        '''Test parsing a YAML-formatted list of sites'''
+
+        ngx_conf = nginx.NginxConf()
+
+        with open('tests/unit/files/nginx_config_test_config.txt', 'r', encoding='utf-8') as f:
+            conf = yaml.safe_load(f.read())
+
+        # From the given YAML-formatted list of sites, check that each individual
+        # Nginx config rendered matches what's in tests/unit/files.
+        for site in conf.keys():
+            output_file = 'tests/unit/files/nginx_config_rendered_test_output-{}.txt'.format(site)
+            with open(output_file, 'r', encoding='utf-8') as f:
+                output = f.read()
+            self.assertEqual(output, ngx_conf.render(conf[site]))
+
+    def test_nginx_config_write_sites(self):
+        '''Test writing out sites to individual Nginx site config files'''
+        ngx_conf = nginx.NginxConf(self.tmpdir)
+        os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
+        os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
+
+        with open('tests/unit/files/nginx_config_rendered_test_output-site1.local.txt', 'r', encoding='utf-8') as f:
+            conf = f.read()
+
+        self.assertTrue(ngx_conf.write_site('site1.local', conf))
+        # Write again with same contents, this time it should return 'False'
+        # as there's no change, thus no need to restart/reload Nginx.
+        self.assertFalse(ngx_conf.write_site('site1.local', conf))
+
+        # Compare what's been written out matches what's in tests/unit/files.
+        with open(os.path.join(self.tmpdir, 'sites-available', 'site1.local'), 'r', encoding='utf-8') as f:
+            output = f.read()
+        self.assertEqual(conf, output)
+
+    def test_nginx_config_sync_sites(self):
+        '''Test cleanup of stale sites and that sites are enabled'''
+        ngx_conf = nginx.NginxConf(self.tmpdir)
+        os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
+        os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
+
+        with open('tests/unit/files/nginx_config_rendered_test_output-site1.local.txt', 'r', encoding='utf-8') as f:
+            conf = f.read()
+
+        # Write out an extra site config to test cleaning it up.
+        for site in ['site1.local', 'site2.local']:
+            ngx_conf.write_site(site, conf)
+        ngx_conf.write_site('site3.local', conf)
+
+        # Clean up anything that's not site1 and site2.
+        self.assertTrue(ngx_conf.sync_sites(['site1.local', 'site2.local']))
+        # Check to make sure site1 still exists and is symlinked in site-senabled.
+        self.assertTrue(os.path.exists(os.path.join(self.tmpdir, 'sites-available', 'site1.local')))
+        self.assertTrue(os.path.islink(os.path.join(self.tmpdir, 'sites-enabled', 'site1.local')))
+        # Only two sites, site3.local shouldn't exist.
+        self.assertFalse(os.path.exists(os.path.join(self.tmpdir, 'sites-available', 'site3.local')))
+        self.assertFalse(os.path.exists(os.path.join(self.tmpdir, 'sites-enabled', 'site3.local')))
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tox.ini b/tox.ini
index ee5dbeb..d6b7675 100644
--- a/tox.ini
+++ b/tox.ini
@@ -9,7 +9,7 @@ setenv =
   PYTHONPATH = .
 
 [testenv:unit]
-commands = pytest -v --ignore {toxinidir}/tests/functional --cov=lib --cov=reactive --cov=actions --cov-report=term
+commands = pytest -v --ignore {toxinidir}/tests/functional --cov=lib --cov=reactive --cov=actions --cov-report=term-missing --cov-branch
 deps = -r{toxinidir}/tests/unit/requirements.txt
        -r{toxinidir}/requirements.txt
 setenv = PYTHONPATH={toxinidir}/lib
@@ -32,6 +32,5 @@ exclude =
     .git,
     __pycache__,
     .tox,
-    hooks/install.d/,
 max-line-length = 120
 max-complexity = 10