Merge lp:~jtv/maas-test/var-log-maas-test into lp:maas-test

[Jeroen T. Vermeulen]

Move logs into /var/log/maas-test.

This is still follow-up from a change pre-imped with Julian earlier today. The logs (including the actual test result files) now go into /var/log/maas-test, no longer /var/cache/maas-test.

The test changes were limited thanks to a factory I had created earlier. I just needed to add one more configurable directory to the factory, and add tests for the timely creation of the log directory. I also noticed that in moving the pidfiles to /run I had missed a place that needed updating, where the proxy was configured. So this branch fixes both items in the proxy configuration.

Now that things are neatly laid out in /var, having proxy configuration in /var/cache does stick out a bit. But not enough to do anything about, I think.

Jeroen

[lp:~jtv/maas-test/var-log-maas-test]

111. By Jeroen T. Vermeulen on 2014-01-09

Copyright.

[Julian Edwards (julian-edwards)]

review: Approve

=== modified file 'docs/man/maas-test.8.rst'
--- docs/man/maas-test.8.rst	2014-01-09 03:51:20 +0000
+++ docs/man/maas-test.8.rst	2014-01-09 07:48:39 +0000
@@ -109,7 +109,7 @@
 attachment.
 
 By default, the results are also written to timestamped log files under
-`/var/cache/maas-test/logs`.
+`/var/log/maas-test`.
 
 
 Options
@@ -199,7 +199,7 @@
 --log-results-only
   Write test results to a file, but don't upload them to Launchpad.
   Results will be written to a timestamped log file under
-  `/var/cache/maas-test/logs`.
+  `/var/log/maas-test`.
 
 
 Reporting bugs
@@ -228,6 +228,8 @@
 
 State and configuration for `maas-test` is stored in `/var/cache/maas-test`.
 This includes an ssh key pair for communicating with the virtual machine.
+Pidfiles are stored in `/run/maas-test`, and logs and test results are written
+to `/var/log/maas-test`.
 
-If you choose to run a local proxy, downloaded data will also be cached there.
-It can quickly grow to gigabyte sizes.
+If you choose to run a local proxy, downloaded data will also be cached in the
+`/var/cache/maas-test`.  It can quickly grow to gigabyte sizes.

=== modified file 'maastest/proxyfixture.py'
--- maastest/proxyfixture.py	2014-01-09 05:56:08 +0000
+++ maastest/proxyfixture.py	2014-01-09 07:48:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2013-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test whether or not a node is compatible with MAAS."""
@@ -29,8 +29,8 @@
 
 PROXY_CONFIG_ITEMS = {
     'daemonise': 'true',
-    'logFile': '%(config_dir)s/proxy.log',
-    'pidFile': '%(config_dir)s/proxy.pid',
+    'logFile': '%(log_file)s',
+    'pidFile': '%(pid_file)s',
     'proxyAddress': "::0",
     'proxyPort': '%(port)d',
     'allowedClients': '127.0.0.1,%(ip_address)s/%(netmask)s',
@@ -58,7 +58,8 @@
 
     #TODO: port should be determined at runtime from the set of
     # available ports rather than being an arbitrary one as it is here.
-    def __init__(self, config_dir=None, port=42676, pidfile_dir=None):
+    def __init__(self, config_dir=None, port=42676, pidfile_dir=None,
+                 log_dir=None):
         """Create a `LocalProxyFixture` object.
 
         :param url: The URL of the proxy to use. If None, LocalProxyFixture
@@ -70,9 +71,14 @@
         :param config_dir: The directory in which to put the proxy's
             configuration and cache.
         :type config_dir: string
-        :param pid_dir: Optional directory in which to write the proxy's
+        :param pidfile_dir: Optional directory in which to write the proxy's
             pidfile.  Will be created if not present.  Defaults to
             `/run/maas-test`.
+        :type pidfile_dir: string
+        :param log_dir: Optional directory in which to write the proxy's log
+            files.  Will be created if not present.  Defaults to
+            `/var/log/maas-test`.
+        :type log_dir: string
         """
         super(LocalProxyFixture, self).__init__()
         self.port = port
@@ -81,7 +87,10 @@
         self.config_dir = config_dir
         if pidfile_dir is None:
             pidfile_dir = utils.DEFAULT_PIDFILE_DIR
+        if log_dir is None:
+            log_dir = utils.DEFAULT_LOG_DIR
         self.pid_file = os.path.join(pidfile_dir, 'proxy.pid')
+        self.log_file = os.path.join(log_dir, 'proxy.log')
         self._network_address = None
 
     def _get_config_arguments(self):
@@ -92,6 +101,8 @@
             'port': self.port,
             'ip_address': ip_address.ip.format(),
             'netmask': ip_address.prefixlen,
+            'pid_file': self.pid_file,
+            'log_file': self.log_file,
             }
         config_args = []
         for key, value in PROXY_CONFIG_ITEMS.items():
@@ -124,10 +135,13 @@
         # From this point, if further setup fails, we'll need to clean up.
         self.addCleanup(self.kill_running_proxy)
 
-        # Create pidfile directory if it did not already exist.
+        # Create pidfile and log directories if they did not already exist.
         pidfile_dir = os.path.dirname(self.pid_file)
         if not os.path.isdir(pidfile_dir):
             os.makedirs(pidfile_dir, 0o755)
+        log_dir = os.path.dirname(self.log_file)
+        if not os.path.isdir(log_dir):
+            os.makedirs(log_dir)
 
         self.start_proxy()
 

=== modified file 'maastest/report.py'
--- maastest/report.py	2014-01-09 03:51:20 +0000
+++ maastest/report.py	2014-01-09 07:48:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2013-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Reporting functions for MAAS test."""
@@ -28,7 +28,7 @@
     )
 
 from apiclient import multipart
-from maastest.utils import DEFAULT_STATE_DIR
+from maastest.utils import DEFAULT_LOG_DIR
 
 
 # This is largely cargo-culted from apiclient.multipart because
@@ -128,7 +128,7 @@
     :param log_dir: The directory under which to create `log_file_name`.
     """
     if log_dir is None:
-        log_dir = DEFAULT_STATE_DIR
+        log_dir = DEFAULT_LOG_DIR
     if not os.path.exists(log_dir):
         os.makedirs(log_dir)
 

=== modified file 'maastest/tests/test_proxyfixture.py'
--- maastest/tests/test_proxyfixture.py	2014-01-09 05:50:54 +0000
+++ maastest/tests/test_proxyfixture.py	2014-01-09 07:48:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2013-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for `maastest.utils`."""
@@ -44,7 +44,8 @@
         """Create a temporary directory for the duration of this test."""
         return self.useFixture(TempDir()).path
 
-    def make_fixture(self, port=None, config_dir=None, pidfile_dir=None):
+    def make_fixture(self, port=None, config_dir=None, pidfile_dir=None,
+                     log_dir=None):
         """Create a `LocalProxyFixture`.
 
         :param port: Optional port.  Defaults to a random choice.
@@ -52,6 +53,8 @@
             temporary directory.
         :param pidfile_dir: Optional pidfile directory.  Defaults to a
             temporary directory.
+        :param log_dir: Optional log directory.  Defaults to a temporary
+            directory.
         """
         if port is None:
             port = self.get_random_port_number()
@@ -59,8 +62,11 @@
             config_dir = self.make_dir()
         if pidfile_dir is None:
             pidfile_dir = self.make_dir()
+        if log_dir is None:
+            log_dir = self.make_dir()
         return LocalProxyFixture(
-            port=port, config_dir=config_dir, pidfile_dir=pidfile_dir)
+            port=port, config_dir=config_dir, pidfile_dir=pidfile_dir,
+            log_dir=log_dir)
 
     def get_random_port_number(self):
         return random.randint(1, 65535)
@@ -82,6 +88,11 @@
         with self.make_fixture(pidfile_dir=pidfile_dir):
             self.assertThat(pidfile_dir, DirExists())
 
+    def test_creates_log_dir(self):
+        log_dir = os.path.join(self.make_dir(), "maas-test-logs")
+        with self.make_fixture(log_dir=log_dir):
+            self.assertThat(log_dir, DirExists())
+
     def test_get_url_returns_proxy_url(self):
         port = self.get_random_port_number()
         proxy_fixture = self.make_fixture(port=port)

=== modified file 'maastest/utils.py'
--- maastest/utils.py	2014-01-09 05:49:14 +0000
+++ maastest/utils.py	2014-01-09 07:48:39 +0000
@@ -54,6 +54,9 @@
 # Default location for pidfiles.
 DEFAULT_PIDFILE_DIR = '/run/maas-test'
 
+# Default location for log files.
+DEFAULT_LOG_DIR = '/var/log/maas-test'
+
 
 def read_file(path):
     """Return a given file's contents, as `bytes`."""

=== modified file 'man/maas-test.8'
--- man/maas-test.8	2014-01-09 03:51:20 +0000
+++ man/maas-test.8	2014-01-09 07:48:39 +0000
@@ -122,7 +122,7 @@
 attachment.
 .sp
 By default, the results are also written to timestamped log files under
-\fI/var/cache/maas\-test/logs\fP\&.
+\fI/var/log/maas\-test\fP\&.
 .SH OPTIONS
 .INDENT 0.0
 .TP
@@ -210,7 +210,7 @@
 .B \-\-log\-results\-only
 Write test results to a file, but don\(aqt upload them to Launchpad.
 Results will be written to a timestamped log file under
-\fI/var/cache/maas\-test/logs\fP\&.
+\fI/var/log/maas\-test\fP\&.
 .UNINDENT
 .SH REPORTING BUGS
 .sp
@@ -230,9 +230,11 @@
 .sp
 State and configuration for \fImaas\-test\fP is stored in \fI/var/cache/maas\-test\fP\&.
 This includes an ssh key pair for communicating with the virtual machine.
+Pidfiles are stored in \fI/run/maas\-test\fP, and logs and test results are written
+to \fI/var/log/maas\-test\fP\&.
 .sp
-If you choose to run a local proxy, downloaded data will also be cached there.
-It can quickly grow to gigabyte sizes.
+If you choose to run a local proxy, downloaded data will also be cached in the
+\fI/var/cache/maas\-test\fP\&.  It can quickly grow to gigabyte sizes.
 .SH AUTHOR
 MAAS engineering team at Canonical, Ltd.
 .SH COPYRIGHT