Merge lp:~newell-jensen/maas/add-ucsm-power-query-bug-1389416 into lp:~maas-committers/maas/trunk

[Newell Jensen]

Add power query capabilities to UCSM hardware.

[Newell Jensen (newell-jensen)]

Setting to Work in Progress because this needs to be tested on UCS Manager hardware and the only place that seems to be is in the OIL lab. Andres, any idea when we could get this tested in there?

[Andres Rodriguez (andreserl)]

Ok, so I have tested this in real environment and it seems to work fine other for the comment bellow. Please fix that and then it should be good to go.

However, also please have someone else look at this branch.

review: Needs Fixing

[Jason Hobbs (jason-hobbs)]

I think there are some improvements that should be made to the template, but the rest looks good. See comments inline below.

[Jason Hobbs (jason-hobbs)]

review: Needs Fixing

[Newell Jensen (newell-jensen)]

Jason,

Concerning the stack trace logging, pserv.log already gets the entire stack trace if there is an error within the try block of the power template (this was tested by raising an exception within the try block and monitoring the logs). Go ahead and have another look as I have changed the other items you commented on.

[Jason Hobbs (jason-hobbs)]

Getting closer, but the template can still be reduced more by moving the error handling introduced here into a method you can import instead of just being directly in the template.

review: Needs Fixing

[Newell Jensen (newell-jensen)]

Ready for another look. Template was reduced and error handling was moved to Python code. Leaving the try/except block in the template due to copious amounts of code in provisioningserver.rpc.power/poweraction, which handle this well.

[Jason Hobbs (jason-hobbs)]

Looks much better, thanks. I don't follow what you mean about leaving the try/except in the template, but it's small enough I won't block on it. Approve, as long as you've tested this latest revision.

review: Approve

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

=== modified file 'etc/maas/templates/power/ucsm.template'
--- etc/maas/templates/power/ucsm.template	2014-07-18 17:05:57 +0000
+++ etc/maas/templates/power/ucsm.template	2014-12-05 21:35:37 +0000
@@ -2,13 +2,63 @@
 #
 # Control a system via Cisco UCS Manager XML API.
 
+# Exit with failure message.
+# Parameters: exit code, and error message.
+fail() {
+    echo "$2" >&2
+    exit $1
+}
+
+issue_ucsm_command() {
 python - << END
+import sys
 from provisioningserver.drivers.hardware.ucsm import power_control_ucsm
-power_control_ucsm(
-    {{escape_py_literal(power_address) | safe}},
-    {{escape_py_literal(power_user) | safe}},
-    {{escape_py_literal(power_pass) | safe}},
-    {{escape_py_literal(uuid) | safe}},
-    {{escape_py_literal(power_change) | safe}},
-)
-END
+try:
+    power_control_ucsm(
+        {{escape_py_literal(power_address) | safe}},
+        {{escape_py_literal(power_user) | safe}},
+        {{escape_py_literal(power_pass) | safe}},
+        {{escape_py_literal(uuid) | 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_state() {
+python - << END
+import sys
+from provisioningserver.drivers.hardware.ucsm import power_state_ucsm
+try:
+    print(power_state_ucsm(
+        {{escape_py_literal(power_address) | safe}},
+        {{escape_py_literal(power_user) | safe}},
+        {{escape_py_literal(power_pass) | safe}},
+        {{escape_py_literal(uuid) | 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
+}
+
+main() {
+    case $1 in
+    'on'|'off')
+        issue_ucsm_command
+        ;;
+    'query')
+        query_state
+        ;;
+    *)
+        fail 2 "Unknown power command: '$1'"
+    esac
+}
+
+main "{{power_change}}"

=== modified file 'src/provisioningserver/drivers/hardware/tests/test_ucsm.py'
--- src/provisioningserver/drivers/hardware/tests/test_ucsm.py	2014-07-18 17:05:57 +0000
+++ src/provisioningserver/drivers/hardware/tests/test_ucsm.py	2014-12-05 21:35:37 +0000
@@ -50,6 +50,7 @@
     make_request_data,
     parse_response,
     power_control_ucsm,
+    power_state_ucsm,
     probe_and_enlist_ucsm,
     probe_servers,
     RO_KEYS,
@@ -60,6 +61,7 @@
     UCSM_XML_API_Error,
     )
 import provisioningserver.utils as utils
+from testtools.matchers import Equals
 
 
 def make_api(url='http://url', user='u', password='p',
@@ -461,7 +463,7 @@
 
     def test_get_power_command_raises_assertion_error_on_bad_power_mode(self):
         bad_power_mode = factory.make_name('unlikely')
-        error = self.assertRaises(AssertionError, get_power_command,
+        error = self.assertRaises(UCSM_XML_API_Error, get_power_command,
                                   bad_power_mode, None)
         self.assertIn(bad_power_mode, error.args[0])
 
@@ -490,6 +492,76 @@
                         MockCalledOnceWith(api, power_control, state))
 
 
+class TestUCSMPowerState(MAASTestCase):
+    """Tests for `power_state_ucsm`."""
+
+    def test_power_state_get_off(self):
+        url = factory.make_name('url')
+        username = factory.make_name('username')
+        password = factory.make_name('password')
+        uuid = factory.make_UUID()
+        api = Mock()
+        self.patch(ucsm, 'UCSM_XML_API').return_value = api
+        get_servers_mock = self.patch(ucsm, 'get_servers')
+        server = make_server()
+        current_state = 'down'
+        power_control = Element('lsPower', {'state': current_state})
+        get_servers_mock.return_value = [server]
+        get_server_power_control_mock = self.patch(
+            ucsm, 'get_server_power_control')
+        get_server_power_control_mock.return_value = power_control
+
+        power_state = power_state_ucsm(url, username, password, uuid)
+        self.expectThat(get_servers_mock, MockCalledOnceWith(api, uuid))
+        self.expectThat(
+            get_server_power_control_mock,
+            MockCalledOnceWith(api, server))
+        self.expectThat(power_state, Equals('off'))
+
+    def test_power_state_get_on(self):
+        url = factory.make_name('url')
+        username = factory.make_name('username')
+        password = factory.make_name('password')
+        uuid = factory.make_UUID()
+        api = Mock()
+        self.patch(ucsm, 'UCSM_XML_API').return_value = api
+        get_servers_mock = self.patch(ucsm, 'get_servers')
+        server = make_server()
+        current_state = 'up'
+        power_control = Element('lsPower', {'state': current_state})
+        get_servers_mock.return_value = [server]
+        get_server_power_control_mock = self.patch(
+            ucsm, 'get_server_power_control')
+        get_server_power_control_mock.return_value = power_control
+
+        power_state = power_state_ucsm(url, username, password, uuid)
+        self.expectThat(get_servers_mock, MockCalledOnceWith(api, uuid))
+        self.expectThat(
+            get_server_power_control_mock,
+            MockCalledOnceWith(api, server))
+        self.expectThat(power_state, Equals('on'))
+
+    def test_power_state_error_on_unknown_state(self):
+        url = factory.make_name('url')
+        username = factory.make_name('username')
+        password = factory.make_name('password')
+        uuid = factory.make_UUID()
+        api = Mock()
+        self.patch(ucsm, 'UCSM_XML_API').return_value = api
+        get_servers_mock = self.patch(ucsm, 'get_servers')
+        server = make_server()
+        current_state = factory.make_name('error')
+        power_control = Element('lsPower', {'state': current_state})
+        get_servers_mock.return_value = [server]
+        get_server_power_control_mock = self.patch(
+            ucsm, 'get_server_power_control')
+        get_server_power_control_mock.return_value = power_control
+
+        self.assertRaises(
+            UCSM_XML_API_Error, power_state_ucsm, url,
+            username, password, uuid)
+
+
 class TestProbeAndEnlistUCSM(MAASTestCase):
     """Tests for ``probe_and_enlist_ucsm``."""
 

=== modified file 'src/provisioningserver/drivers/hardware/ucsm.py'
--- src/provisioningserver/drivers/hardware/ucsm.py	2014-09-10 16:20:31 +0000
+++ src/provisioningserver/drivers/hardware/ucsm.py	2014-12-05 21:35:37 +0000
@@ -83,10 +83,16 @@
 __metaclass__ = type
 __all__ = [
     'power_control_ucsm',
+    'power_state_ucsm',
     'probe_and_enlist_ucsm',
 ]
 
 
+class UCSMState:
+    DOWN = "down"
+    UP = "up"
+
+
 class UCSM_XML_API_Error(Exception):
     """Failure talking to a Cisco UCS Manager."""
 
@@ -375,8 +381,8 @@
     elif maas_power_mode == 'off':
         return 'admin-down'
     else:
-        message = 'Unexpected maas power mode: %s' % (maas_power_mode)
-        raise AssertionError(message)
+        raise UCSM_XML_API_Error(
+            'Unexpected maas power mode: %s' % (maas_power_mode), None)
 
 
 def power_control_ucsm(url, username, password, uuid, maas_power_mode):
@@ -393,6 +399,23 @@
         set_server_power_control(api, power_control, command)
 
 
+def power_state_ucsm(url, username, password, uuid):
+    """Return the power state for the ucsm machine."""
+    with logged_in(url, username, password) as api:
+        # UUIDs are unique per server, so we get either one or zero
+        # servers for a given UUID.
+        [server] = get_servers(api, uuid)
+        power_control = get_server_power_control(api, server)
+        power_state = power_control.get('state')
+
+        if power_state == UCSMState.DOWN:
+            return 'off'
+        elif power_state == UCSMState.UP:
+            return 'on'
+        raise UCSM_XML_API_Error(
+            'Unknown power state: %s' % power_state, None)
+
+
 def probe_and_enlist_ucsm(url, username, password):
     """Probe a UCS Manager and enlist all its servers.
 

=== modified file 'src/provisioningserver/rpc/power.py'
--- src/provisioningserver/rpc/power.py	2014-11-24 22:38:15 +0000
+++ src/provisioningserver/rpc/power.py	2014-12-05 21:35:37 +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', 'ucsm', 'virsh']
 
 
 # Timeout for change_power_state(). We set it to 2 minutes by default,