Merge ~andreserl/maas:snap_rack_delete into maas:master

[Andres Rodriguez]

LP: #1711414 - Fix deleting a rack when it is installed via the snap

[Andres Rodriguez (andreserl)]

Please read https://bugs.launchpad.net/maas/+bug/1711414/comments/1 for the review!

[Mike Pontillo (mpontillo)]

Can this code ever be reached on a rack+region?

review: Needs Information

[Mike Pontillo (mpontillo)]

(Also, this branch is missing updates to the unit tests.)

[Andres Rodriguez (andreserl)]

@Mike, no, this code is only called if the machine is "NODE.RACK_CONTROLLER" so a REGION_AND_RACK won't have this same code path.

[Mike Pontillo (mpontillo)]

Sounds good. I think you need one more test case for ignoring the CalledProcessError (not raising CannotDisableAndShutoffRackd) if running_in_snap() and e.returncode == -15.

[Mike Pontillo (mpontillo)]

LGTM!

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b snap_rack_delete lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/243/consoleText

[MAAS Lander (maas-lander)]

LANDING
-b snap_rack_delete lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/244/consoleText

[Andres Rodriguez (andreserl)]

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b snap_rack_delete lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/246/consoleText

[MAAS Lander (maas-lander)]

LANDING
-b snap_rack_delete lp:~andreserl/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/248/consoleText

diff --git a/src/provisioningserver/rpc/clusterservice.py b/src/provisioningserver/rpc/clusterservice.py
index 7157c81..50019d3 100644
--- a/src/provisioningserver/rpc/clusterservice.py
+++ b/src/provisioningserver/rpc/clusterservice.py
@@ -98,6 +98,10 @@ from provisioningserver.utils.shell import (
     ExternalProcessError,
     select_c_utf8_bytes_locale,
 )
+from provisioningserver.utils.snappy import (
+    get_snap_path,
+    running_in_snap,
+)
 from provisioningserver.utils.twisted import (
     call,
     callOut,
@@ -676,17 +680,30 @@ class Cluster(RPCProtocol):
         Implementation of
         :py:class:`~provisioningserver.rpc.cluster.DisableAndShutoffRackd`.
         """
-        maaslog.info("Rack deleted, disabling rackd service")
+        maaslog.info("Attempting to disable the rackd service.")
         try:
-            # We can't use the --now flag as if the maas-rackd service is on
-            # but not enabled the service won't be stopped
-            call_and_check(
-                ['sudo', 'systemctl', 'disable', 'maas-rackd'])
-            call_and_check(
-                ['sudo', 'systemctl', 'stop', 'maas-rackd'])
+            if running_in_snap():
+                cmd = os.path.join(get_snap_path(), 'command-maas.wrapper')
+                call_and_check(
+                    [cmd, 'config', '--mode', 'none'])
+            else:
+                # We can't use the --now flag as if the maas-rackd service is
+                # on but not enabled the service won't be stopped
+                call_and_check(
+                    ['sudo', 'systemctl', 'disable', 'maas-rackd'])
+                call_and_check(
+                    ['sudo', 'systemctl', 'stop', 'maas-rackd'])
         except ExternalProcessError as e:
-            maaslog.error("Unable to disable and stop maas-rackd service")
-            raise exceptions.CannotDisableAndShutoffRackd(e.output_as_unicode)
+            # Since the snap sends a SIGTERM to terminate the process, python
+            # returns -15 as a return code. This indicates the termination
+            # signal has been performed and the process terminated. However,
+            # This is not a failure. As such, work around the non-zero return
+            # (-15) and do not raise an error.
+            if not (running_in_snap() and e.returncode == -15):
+                maaslog.error("Unable to disable and stop the rackd service")
+                raise exceptions.CannotDisableAndShutoffRackd(
+                    e.output_as_unicode)
+        maaslog.info("Successfully stopped the rackd service.")
         return {}
 
 
diff --git a/src/provisioningserver/rpc/tests/test_clusterservice.py b/src/provisioningserver/rpc/tests/test_clusterservice.py
index 4aea7c2..4b54fc9 100644
--- a/src/provisioningserver/rpc/tests/test_clusterservice.py
+++ b/src/provisioningserver/rpc/tests/test_clusterservice.py
@@ -3127,18 +3127,47 @@ class TestClusterProtocol_DisableAndShutoffRackd(MAASTestCase):
             cluster.DisableAndShutoffRackd.commandName)
         self.assertIsNotNone(responder)
 
-    def test_issues_restart(self):
+    def test_issues_restart_systemd(self):
         mock_call_and_check = self.patch(clusterservice, 'call_and_check')
         response = call_responder(
             Cluster(), cluster.DisableAndShutoffRackd, {})
         self.assertEquals({}, response.result)
         self.assertEquals(2, mock_call_and_check.call_count)
 
+    def test_issues_restart_snap(self):
+        self.patch(clusterservice, 'running_in_snap').return_value = True
+        self.patch(clusterservice, 'get_snap_path').return_value = '/'
+        mock_call_and_check = self.patch(clusterservice, 'call_and_check')
+        response = call_responder(
+            Cluster(), cluster.DisableAndShutoffRackd, {})
+        self.assertEquals({}, response.result)
+        self.assertEquals(1, mock_call_and_check.call_count)
+
     @inlineCallbacks
-    def test_raises_error_on_failure(self):
+    def test_raises_error_on_failure_systemd(self):
         mock_call_and_check = self.patch(clusterservice, 'call_and_check')
         mock_call_and_check.side_effect = ExternalProcessError(
             1, 'systemctl', 'failure')
         with ExpectedException(exceptions.CannotDisableAndShutoffRackd):
             yield call_responder(
                 Cluster(), cluster.DisableAndShutoffRackd, {})
+
+    @inlineCallbacks
+    def test_raises_error_on_failure_snap(self):
+        mock_call_and_check = self.patch(clusterservice, 'call_and_check')
+        mock_call_and_check.side_effect = ExternalProcessError(
+            random.randint(1, 255), 'command-maas.wrapper', 'failure')
+        with ExpectedException(exceptions.CannotDisableAndShutoffRackd):
+            yield call_responder(
+                Cluster(), cluster.DisableAndShutoffRackd, {})
+
+    def test_snap_ignores_signal_error_code_on_restart(self):
+        self.patch(clusterservice, 'running_in_snap').return_value = True
+        self.patch(clusterservice, 'get_snap_path').return_value = '/'
+        mock_call_and_check = self.patch(clusterservice, 'call_and_check')
+        mock_call_and_check.side_effect = ExternalProcessError(
+            -15, 'command-maas.wrapper', 'failure')
+        response = call_responder(
+            Cluster(), cluster.DisableAndShutoffRackd, {})
+        self.assertEquals({}, response.result)
+        self.assertEquals(1, mock_call_and_check.call_count)