Merge lp:~ltrager/maas/lp1655721 into lp:~maas-committers/maas/trunk

[Lee Trager]

Only reload TGT config when a TGT config file exists.

From the pastebin in the bug it looks like no images were imported so there was no config file to reload. This patches MAAS to only reload TGT when a config is available.

[Gavin Panella (allenap)]

Looks fine to me. One question, one suggestion, no blockers.

review: Approve

=== modified file 'src/provisioningserver/import_images/boot_resources.py'
--- src/provisioningserver/import_images/boot_resources.py	2016-12-07 12:46:14 +0000
+++ src/provisioningserver/import_images/boot_resources.py	2017-01-17 21:00:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __all__ = [
@@ -208,6 +208,12 @@
 
     # Update the tgt config.
     targets_conf = os.path.join(snapshot, 'maas.tgt')
+
+    # The targets_conf may not exist in the event the BootSource is broken
+    # and images havn't been imported yet. This fixes LP:1655721
+    if not os.path.exists(targets_conf):
+        return
+
     try:
         call_and_check(sudo([
             '/usr/sbin/tgt-admin',

=== modified file 'src/provisioningserver/import_images/tests/test_boot_resources.py'
--- src/provisioningserver/import_images/tests/test_boot_resources.py	2016-12-07 12:46:14 +0000
+++ src/provisioningserver/import_images/tests/test_boot_resources.py	2017-01-17 21:00:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the boot_resources module."""
@@ -25,12 +25,14 @@
 )
 
 from maastesting.factory import factory
+from maastesting.fixtures import TempDirectory
 from maastesting.matchers import (
     MockAnyCall,
     MockCalledOnce,
     MockCalledOnceWith,
     MockCalledWith,
     MockCallsMatch,
+    MockNotCalled,
 )
 from maastesting.testcase import MAASTestCase
 from maastesting.utils import age_file
@@ -599,6 +601,7 @@
         mock_try_send_rack_event = self.patch(
             boot_resources, 'try_send_rack_event')
         mock_maaslog = self.patch(boot_resources.maaslog, 'warning')
+        self.patch(boot_resources.os.path, 'exists').return_value = True
         self.patch(boot_resources, 'call_and_check').side_effect = (
             ExternalProcessError(
                 returncode=2, cmd=('tgt-admin',), output='error'))
@@ -606,6 +609,22 @@
         self.assertThat(mock_try_send_rack_event, MockCalledOnce())
         self.assertThat(mock_maaslog, MockCalledOnce())
 
+    def test_update_targets_only_runs_when_conf_exists(self):
+        # Regression test for LP:1655721
+        temp_dir = self.useFixture(TempDirectory()).path
+        self.useFixture(ClusterConfigurationFixture(tftp_root=temp_dir))
+        mock_ensureService = self.patch(
+            boot_resources.service_monitor, "ensureService")
+        mock_call_and_check = self.patch(boot_resources, "call_and_check")
+        mock_path_exists = self.patch(boot_resources.os.path, 'exists')
+        mock_path_exists.return_value = False
+        boot_resources.update_targets_conf(temp_dir)
+        self.assertThat(mock_ensureService, MockCalledOnceWith("tgt"))
+        self.assertThat(
+            mock_path_exists,
+            MockCalledOnceWith(os.path.join(temp_dir, 'maas.tgt')))
+        self.assertThat(mock_call_and_check, MockNotCalled())
+
 
 class TestMetaContains(MAASTestCase):
     """Tests for the `meta_contains` function."""