Merge lp:~blake-rouse/maas/sm15k-query into lp:~maas-committers/maas/trunk

[Blake Rouse]

Support the ability to query the power status of sm15k using RESTAPI v2.0.

[Andres Rodriguez (andreserl)]

See comments inline!

review: Needs Fixing

=== modified file 'etc/maas/templates/power/sm15k.template'
--- etc/maas/templates/power/sm15k.template	2014-06-10 14:45:14 +0000
+++ etc/maas/templates/power/sm15k.template	2014-11-05 21:52:32 +0000
@@ -3,50 +3,124 @@
 # Control a system via ipmipower, sending the seamicro specific hex codes
 #
 
-# Parameters
-power_change={{power_change}}
-power_address={{power_address}}
-power_user={{power_user}}
-power_pass={{power_pass}}
-power_control={{power_control}}
-system_id={{system_id}}
-ipmitool={{ipmitool}}
-
-# IPMI power mode
-{{py: power_mode = 1 if power_change == 'on' else 6 }}
-power_mode={{power_mode}}
+# Exit with failure message.
+# Parameters: exit code, and error message.
+fail() {
+    echo "$2" >&2
+    exit $1
+}
 
 # Control power using IPMI
 issue_ipmi_command() {
-    ${ipmitool} -I lanplus \
-        -H ${power_address} -U ${power_user}\
-        -P ${power_pass} raw 0x2E 1 0x00 0x7d 0xab \
-        ${power_mode} 0 ${system_id}
+    {{py: power_mode = 1 if power_change == 'on' else 6 }}
+    {{ipmitool}} -I lanplus \
+        -H {{power_address}} -U {{power_user}}\
+        -P {{power_pass}} raw 0x2E 1 0x00 0x7d 0xab \
+        {{power_mode}} 0 {{system_id}}
 }
 
 # Control power using REST v0.9
 issue_rest_v09_command() {
 python - << END
+import sys
 from provisioningserver.drivers.hardware.seamicro import power_control_seamicro15k_v09
-power_control_seamicro15k_v09("${power_address}", "${power_user}", "${power_pass}", "${system_id}", "${power_change}")
+try:
+    power_control_seamicro15k_v09(
+        {{escape_py_literal(power_address) | safe}},
+        {{escape_py_literal(power_user) | safe}},
+        {{escape_py_literal(power_pass) | safe}},
+        {{escape_py_literal(system_id) | safe}},
+        {{escape_py_literal(power_change) | safe}},
+    )
+except Exception as e:
+    # This gets in the node event log: print the exception's message
+    # and not the stacktrace.
+    print(unicode(e))
+    sys.exit(1)
 END
 }
 
 # Control power using REST v2
 issue_rest_v2_command() {
 python - << END
+import sys
 from provisioningserver.drivers.hardware.seamicro import power_control_seamicro15k_v2
-power_control_seamicro15k_v2("${power_address}", "${power_user}", "${power_pass}", "${system_id}", "${power_change}")
-END
-}
-
-if [ "${power_control}" = "ipmi" ]
-then
-    issue_ipmi_command
-elif [ "${power_control}" = "restapi" ]
-then
-    issue_rest_v09_command
-elif [ "${power_control}" = "restapi2" ]
-then
-    issue_rest_v2_command
-fi
+try:
+    power_control_seamicro15k_v2(
+        {{escape_py_literal(power_address) | safe}},
+        {{escape_py_literal(power_user) | safe}},
+        {{escape_py_literal(power_pass) | safe}},
+        {{escape_py_literal(system_id) | safe}},
+        {{escape_py_literal(power_change) | safe}},
+    )
+except Exception as e:
+    # This gets in the node event log: print the exception's message
+    # and not the stacktrace.
+    print(unicode(e))
+    sys.exit(1)
+END
+}
+
+# Query power state using REST v2
+query_state_rest_v2() {
+python - << END
+import sys
+from provisioningserver.drivers.hardware.mscm seamicro power_query_seamicro15k_v2
+try:
+    print(power_query_seamicro15k_v2(
+        {{escape_py_literal(power_address) | safe}},
+        {{escape_py_literal(power_user) | safe}},
+        {{escape_py_literal(power_pass) | safe}},
+        {{escape_py_literal(system_id) | safe}},
+    ))
+except Exception as e:
+    # This gets in the node event log: print the exception's message
+    # and not the stacktrace.
+    print(unicode(e))
+    sys.exit(1)
+END
+}
+
+# Perform power control
+power_control() {
+    if [ "{{power_control}}" = "ipmi" ]
+    then
+        issue_ipmi_command
+    elif [ "{{power_control}}" = "restapi" ]
+    then
+        issue_rest_v09_command
+    elif [ "{{power_control}}" = "restapi2" ]
+    then
+        issue_rest_v2_command
+    fi
+}
+
+# Query the state.
+# Only supported by REST v2.
+query_state() {
+    if [ "{{power_control}}" = "ipmi" ]
+    then
+        echo "unknown"
+    elif [ "{{power_control}}" = "restapi" ]
+    then
+        echo "unknown"
+    elif [ "{{power_control}}" = "restapi2" ]
+    then
+        query_state_rest_v2
+    fi
+}
+
+main() {
+    case $1 in
+    'on'|'off')
+        power_control
+        ;;
+    'query')
+        query_state
+        ;;
+    *)
+        fail 2 "Unknown power command: '$1'"
+    esac
+}
+
+main "{{power_change}}"

=== modified file 'src/provisioningserver/drivers/hardware/seamicro.py'
--- src/provisioningserver/drivers/hardware/seamicro.py	2014-09-10 16:20:31 +0000
+++ src/provisioningserver/drivers/hardware/seamicro.py	2014-11-05 21:52:32 +0000
@@ -320,9 +320,22 @@
                                  power_change):
     server_id = '%s/0' % server_id
     api = get_seamicro15k_api('v2.0', ip, username, password)
-    if api:
-        server = api.servers.get(server_id)
-        if power_change == "on":
-            server.power_on(using_pxe=True)
-        elif power_change == "off":
-            server.power_off(force=True)
+    if api is None:
+        raise SeaMicroError('Unable to contact BMC controller.')
+    server = api.servers.get(server_id)
+    if power_change == "on":
+        server.power_on(using_pxe=True)
+    elif power_change == "off":
+        server.power_off(force=True)
+
+
+def power_query_seamicro15k_v2(ip, username, password, server_id,
+                               power_change):
+    server_id = '%s/0' % server_id
+    api = get_seamicro15k_api('v2.0', ip, username, password)
+    if api is None:
+        raise SeaMicroError('Unable to contact BMC controller.')
+    server = api.servers.get(server_id)
+    if server.active:
+        return "on"
+    return "off"

=== modified file 'src/provisioningserver/drivers/hardware/tests/test_seamicro.py'
--- src/provisioningserver/drivers/hardware/tests/test_seamicro.py	2014-09-18 12:44:38 +0000
+++ src/provisioningserver/drivers/hardware/tests/test_seamicro.py	2014-11-05 21:52:32 +0000
@@ -34,6 +34,7 @@
     find_seamicro15k_servers,
     power_control_seamicro15k_v09,
     power_control_seamicro15k_v2,
+    power_query_seamicro15k_v2,
     POWER_STATUS,
     probe_seamicro15k_and_enlist,
     SeaMicroAPIV09,
@@ -466,3 +467,71 @@
 
         power_control_seamicro15k_v2(ip, username, password, '0', 'on')
         mock_power_on.assert_called()
+
+    def test_power_control_seamicro15k_v2_raises_error_when_api_None(self):
+        ip = factory.make_ipv4_address()
+        username = factory.make_string()
+        password = factory.make_string()
+
+        mock_get_api = self.patch(
+            seamicro,
+            'get_seamicro15k_api')
+        mock_get_api.return_value = None
+
+        self.assertRaises(
+            SeaMicroError,
+            power_control_seamicro15k_v2, ip, username, password, '0', 'on')
+
+    def test_power_query_seamicro15k_v2_power_on(self):
+        ip = factory.make_ipv4_address()
+        username = factory.make_string()
+        password = factory.make_string()
+
+        fake_server = FakeServer('0/0')
+        self.patch(fake_server, 'active', True)
+        fake_client = FakeSeaMicroClient()
+        fake_client.servers = FakeSeaMicroServerManager()
+        fake_client.servers.servers.append(fake_server)
+
+        mock_get_api = self.patch(
+            seamicro,
+            'get_seamicro15k_api')
+        mock_get_api.return_value = fake_client
+
+        self.assertEqual(
+            "on",
+            power_query_seamicro15k_v2(ip, username, password, '0', 'on'))
+
+    def test_power_query_seamicro15k_v2_power_off(self):
+        ip = factory.make_ipv4_address()
+        username = factory.make_string()
+        password = factory.make_string()
+
+        fake_server = FakeServer('0/0')
+        self.patch(fake_server, 'active', False)
+        fake_client = FakeSeaMicroClient()
+        fake_client.servers = FakeSeaMicroServerManager()
+        fake_client.servers.servers.append(fake_server)
+
+        mock_get_api = self.patch(
+            seamicro,
+            'get_seamicro15k_api')
+        mock_get_api.return_value = fake_client
+
+        self.assertEqual(
+            "off",
+            power_query_seamicro15k_v2(ip, username, password, '0', 'on'))
+
+    def test_power_query_seamicro15k_v2_raises_error_when_api_None(self):
+        ip = factory.make_ipv4_address()
+        username = factory.make_string()
+        password = factory.make_string()
+
+        mock_get_api = self.patch(
+            seamicro,
+            'get_seamicro15k_api')
+        mock_get_api.return_value = None
+
+        self.assertRaises(
+            SeaMicroError,
+            power_query_seamicro15k_v2, ip, username, password, '0', 'on')

=== modified file 'src/provisioningserver/rpc/power.py'
--- src/provisioningserver/rpc/power.py	2014-10-28 21:48:26 +0000
+++ src/provisioningserver/rpc/power.py	2014-11-05 21:52:32 +0000
@@ -61,7 +61,7 @@
 # state for these power types.
 # This is meant to be temporary until all the power types support
 # querying the power state of a node.
-QUERY_POWER_TYPES = ['amt', 'ipmi', 'mscm', 'virsh']
+QUERY_POWER_TYPES = ['amt', 'ipmi', 'mscm', 'sm15k', 'virsh']
 
 
 # Timeout for change_power_state(). We set it to 2 minutes by default,
@@ -232,7 +232,7 @@
         new_power_state = yield deferToThread(
             perform_power_change, system_id, hostname, power_type,
             'query', context)
-        if new_power_state == power_change:
+        if new_power_state == "unknown" or new_power_state == power_change:
             yield power_change_success(system_id, hostname, power_change)
             return
 
@@ -298,7 +298,7 @@
         try:
             power_state = yield deferToThread(
                 perform_power_query, system_id, hostname, power_type, context)
-            if power_state not in ("on", "off"):
+            if power_state not in ("on", "off", "unknown"):
                 # This is considered an error.
                 raise PowerActionFail(power_state)
         except PowerActionFail as e:

=== modified file 'src/provisioningserver/rpc/tests/test_power.py'
--- src/provisioningserver/rpc/tests/test_power.py	2014-10-27 11:56:50 +0000
+++ src/provisioningserver/rpc/tests/test_power.py	2014-11-05 21:52:32 +0000
@@ -321,6 +321,37 @@
         self.assertThat(markNodeBroken, MockNotCalled())
 
     @inlineCallbacks
+    def test_change_power_state_doesnt_retry_if_query_returns_unknown(self):
+        system_id = factory.make_name('system_id')
+        hostname = factory.make_name('hostname')
+        power_type = random.choice(power.QUERY_POWER_TYPES)
+        power_change = random.choice(['on', 'off'])
+        context = {
+            factory.make_name('context-key'): factory.make_name('context-val')
+        }
+        self.patch(power, 'pause')
+        power.power_action_registry[system_id] = power_change
+        # Patch the power action utility so that it says the node is
+        # in the required power state.
+        power_action, execute = patch_power_action(
+            self, return_value="unknown")
+        markNodeBroken = yield self.patch_rpc_methods()
+
+        yield power.change_power_state(
+            system_id, hostname, power_type, power_change, context)
+        self.assertThat(
+            execute,
+            MockCallsMatch(
+                # One call to change the power state.
+                call(power_change=power_change, **context),
+                # One call to query the power state.
+                call(power_change='query', **context),
+            ),
+        )
+        # The node hasn't been marked broken.
+        self.assertThat(markNodeBroken, MockNotCalled())
+
+    @inlineCallbacks
     def test_change_power_state_marks_the_node_broken_if_failure(self):
         system_id = factory.make_name('system_id')
         hostname = factory.make_name('hostname')
@@ -546,6 +577,29 @@
         self.assertThat(
             power_state_update, MockCalledOnceWith(system_id, power_state))
 
+    def test_get_power_state_changes_power_state_if_unknown(self):
+        system_id = factory.make_name('system_id')
+        hostname = factory.make_name('hostname')
+        power_state = "unknown"
+        power_type = random.choice(power.QUERY_POWER_TYPES)
+        context = {
+            factory.make_name('context-key'): factory.make_name('context-val')
+        }
+        self.patch(power, 'pause')
+        power_state_update = self.patch_autospec(power, 'power_state_update')
+
+        # Simulate success.
+        power_action, execute = patch_power_action(
+            self, return_value=power_state)
+        _, _, io = self.patch_rpc_methods()
+
+        d = power.get_power_state(
+            system_id, hostname, power_type, context)
+        io.flush()
+        self.assertEqual(power_state, extract_result(d))
+        self.assertThat(
+            power_state_update, MockCalledOnceWith(system_id, power_state))
+
     def test_get_power_state_pauses_inbetween_retries(self):
         system_id = factory.make_name('system_id')
         hostname = factory.make_name('hostname')