Merge lp:~jason-hobbs/maas/add-umg-cli-api into lp:~maas-committers/maas/trunk

[Jason Hobbs]

Add basic UMG CLI API support.

This introduces the basic API code that MAAS will use to interact with the UMG. Additional code will be needed to make this useful to MAAS.

[Blake Rouse (blake-rouse)]

Looks good. Great test coverage. Only a couple of small things.

review: Approve

[Jason Hobbs (jason-hobbs)]

Blake,

Thanks for the review. I made most of the changes you suggested - a few
comments/responses inline below.

On Mon, Jun 2, 2014 at 2:52 PM, Blake Rouse <email address hidden>
wrote:

> > + power_control = choice(['off', 'on', 'cycle'])
>
> Enum for power_control, as they are specific.
>

I just made the test more generic. The server side checks for those values,
but the client doesn't so I don't want to introduce a new enum.

>
> > +
> > +
> > +str = None
> > +
> > +__metaclass__ = type
> > +__all__ = []
>
> This is normally at the beginning of file, before any imports. Is this
> because of paramiko?
>

Cool - you're right, just didn't notice that pattern. Fixed.

 > + return output

>
> Do you need any form of encoding here? Or will it always be a supported
> encoding?
>

It's always been ascii so far :) It's not a generic shell so the encoding
shouldn't be changing.

 > +

> > + Targets are SP names. Valid controls are 'cycle', 'on', 'off'.
>
> Should use an enum for control, as they are specific.
>

I don't want to get into enforcing stuff on the client side that the server
can and will enforce on the server side. I don't actually check for the
values in my code, so I'm not going to introduce an unused enum.

Thanks,
Jason

[Jeroen T. Vermeulen (jtv)]

Good branch, good review, as far as I'm concerned! My only remark is about the question of compiling regular expressions: I don't mind either way, but when it comes to deciding, don't ask "which way is faster?" Ask "do I expect this to be a performance problem?" Next ask "which way is clearer?"

Knuth's Law applies. In this case, the standard library caches compiled regexes under the hood anyway, so the worst cases are already covered.

review: Approve

[Jason Hobbs (jason-hobbs)]

jtv, thanks for the review! I think taking the pattern definition out of the middle of the rest of the code does make it clearer.

[MAAS Lander (maas-lander)]

Attempt to merge into lp:maas failed due to conflicts:

missing parent in src/provisioningserver/custom_hardware
unversioned parent in src/provisioningserver/custom_hardware
missing parent in src/provisioningserver/custom_hardware/tests
unversioned parent in src/provisioningserver/custom_hardware/tests

=== modified file 'required-packages/base'
--- required-packages/base	2014-04-25 13:07:56 +0000
+++ required-packages/base	2014-06-10 14:58:15 +0000
@@ -35,6 +35,7 @@
 python-oops-datedir-repo
 python-oops-twisted
 python-oops-wsgi
+python-paramiko
 python-pexpect
 python-psycopg2
 python-pyinotify

=== added file 'src/provisioningserver/drivers/hardware/tests/test_umg.py'
--- src/provisioningserver/drivers/hardware/tests/test_umg.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/drivers/hardware/tests/test_umg.py	2014-06-10 14:58:15 +0000
@@ -0,0 +1,218 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for ``provisioningserver.drivers.hardware.umg``."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = []
+
+from random import (
+    randint,
+    shuffle,
+    )
+from StringIO import StringIO
+
+from maastesting.factory import factory
+from maastesting.matchers import MockCalledOnceWith
+from maastesting.testcase import MAASTestCase
+from mock import Mock
+from provisioningserver.drivers.hardware.umg import (
+    ShowOutput,
+    UMG_CLI_API,
+    )
+
+
+def make_umg_api():
+    """Make a UMG_CLI_API object with randomized parameters."""
+    host = factory.make_hostname('umg')
+    username = factory.make_name('user')
+    password = factory.make_name('password')
+    return UMG_CLI_API(host, username, password)
+
+
+def make_streams(stdin=None, stdout=None, stderr=None):
+    """Make a fake return value for a SSHClient.exec_command."""
+    # stdout.read() is called so stdout can't be None.
+    if stdout is None:
+        stdout = Mock()
+
+    return (stdin, stdout, stderr)
+
+
+class TestRunCliCommand(MAASTestCase):
+    """Tests for ``UMG_CLI_API.run_cli_command``.
+    """
+
+    def test_uses_full_cli_command(self):
+        api = make_umg_api()
+        ssh_mock = self.patch(api, '_ssh')
+        ssh_mock.exec_command = Mock(return_value=make_streams())
+        command = factory.make_name('command')
+        api._run_cli_command(command)
+        full_command = 'cli -s %s' % command
+        self.assertThat(
+            ssh_mock.exec_command, MockCalledOnceWith(full_command))
+
+    def test_returns_output(self):
+        api = make_umg_api()
+        ssh_mock = self.patch(api, '_ssh')
+        expected = factory.make_name('output')
+        stdout = StringIO(expected)
+        streams = make_streams(stdout=stdout)
+        ssh_mock.exec_command = Mock(return_value=streams)
+        output = api._run_cli_command(factory.make_name('command'))
+        self.assertEqual(expected, output)
+
+    def test_connects_and_closes_ssh_client(self):
+        api = make_umg_api()
+        ssh_mock = self.patch(api, '_ssh')
+        ssh_mock.exec_command = Mock(return_value=make_streams())
+        api._run_cli_command(factory.make_name('command'))
+        self.assertThat(
+            ssh_mock.connect,
+            MockCalledOnceWith(
+                api.host, username=api.username, password=api.password))
+        self.assertThat(ssh_mock.close, MockCalledOnceWith())
+
+    def test_closes_when_exception_raised(self):
+        api = make_umg_api()
+        ssh_mock = self.patch(api, '_ssh')
+
+        def fail():
+            raise Exception('fail')
+
+        ssh_mock.exec_command = Mock(side_effect=fail)
+        command = factory.make_name('command')
+        self.assertRaises(Exception, api._run_cli_command, command)
+        self.assertThat(ssh_mock.close, MockCalledOnceWith())
+
+
+def make_directories(count):
+    """Make a fake CLI directory listing.
+
+    Returns a tuple of the list of the directories, and the raw text of
+    the directories (to be parsed).
+    """
+    names = list(factory.make_names(*(['directory'] * count)))
+    directories = ['|__%s\r\n' % name for name in names]
+    return names, ''.join(directories)
+
+
+def make_settings(count):
+    """Make a fake CLI settings listing.
+
+    Returns a tuple of the list of the settings, and the raw text of the
+    settings (to be parsed).
+    """
+    keys = ['key%d' % i for i in range(count)]
+    records = {key: factory.make_name('value') for key in keys}
+    settings = [' * %s=%s\r\n' % (k, v) for k, v in records.iteritems()]
+    return records, ''.join(settings)
+
+
+class TestParseShowOutput(MAASTestCase):
+    """Tests for ``UMG_CLI_API._parse_show_output``."""
+
+    def test_only_directories(self):
+        api = make_umg_api()
+        directories, parser_input = make_directories(randint(1, 10))
+        results = api._parse_show_output(parser_input)
+        self.assertEqual(directories, results.directories)
+
+    def test_only_settings(self):
+        api = make_umg_api()
+        settings, parser_input = make_settings(randint(1, 10))
+        results = api._parse_show_output(parser_input)
+        self.assertEqual(settings, results.settings)
+
+    def test_empty_input(self):
+        api = make_umg_api()
+        results = api._parse_show_output('')
+        self.assertEqual(ShowOutput([], {}), results)
+
+    def test_directories_and_settings_in_mixed_order(self):
+        api = make_umg_api()
+        directories, directories_input = make_directories(randint(1, 10))
+        settings, settings_input = make_settings(randint(1, 10))
+        combined = directories_input + settings_input
+        lines = combined.split('\r\n')
+        shuffle(lines)
+        shuffled = '\r\n'.join(lines) + '\r\n'
+        results = api._parse_show_output(shuffled)
+        self.assertItemsEqual(directories, results.directories)
+        self.assertEqual(settings, results.settings)
+
+
+class TestShowCommand(MAASTestCase):
+    """Tests for ``UMG_CLI_API._show_command``."""
+
+    def test_runs_command_and_returns_parsed_results(self):
+        api = make_umg_api()
+        command = factory.make_name('command')
+        run_cli_command = self.patch(api, '_run_cli_command')
+        run_cli_command.return_value = factory.make_name('output_text')
+        parse_show_output = self.patch(api, '_parse_show_output')
+        parse_show_output.return_value = factory.make_name('parsed_result')
+        result = api._show_command(command)
+        self.assertThat(run_cli_command, MockCalledOnceWith(command))
+        expected_call = MockCalledOnceWith(run_cli_command.return_value)
+        self.assertThat(parse_show_output, expected_call)
+        self.assertEqual(parse_show_output.return_value, result)
+
+
+class TestShowTargets(MAASTestCase):
+    """Tests for ``UMG_CLI_API.show_targets``."""
+
+    def test_uses_correct_command(self):
+        api = make_umg_api()
+        show_command = self.patch(api, '_show_command')
+        api.show_targets()
+        self.assertThat(show_command, MockCalledOnceWith('show /targets/SP'))
+
+    def test_returns_command_output(self):
+        api = make_umg_api()
+        show_command = self.patch(api, '_show_command')
+        show_command.return_value = factory.make_name('result')
+        result = api.show_targets()
+        self.assertEqual(show_command.return_value, result)
+
+
+class TestShowTarget(MAASTestCase):
+    """Tests for ``UMG_CLI_API.show_target``."""
+
+    def test_uses_correct_command(self):
+        api = make_umg_api()
+        show_command = self.patch(api, '_show_command')
+        target = factory.make_name('target')
+        api.show_target(target)
+        expected = 'show /targets/SP/%s' % (target)
+        self.assertThat(show_command, MockCalledOnceWith(expected))
+
+    def test_returns_command_output(self):
+        api = make_umg_api()
+        show_command = self.patch(api, '_show_command')
+        show_command.return_value = factory.make_name('result')
+        result = api.show_target(factory.make_name('target'))
+        self.assertEqual(show_command.return_value, result)
+
+
+class TestPowerControlTarget(MAASTestCase):
+    """"Tests for ``UMG_CLI_API.power_control_target``."""
+
+    def test_uses_correct_command(self):
+        api = make_umg_api()
+        target = factory.make_name('target')
+        power_control = factory.make_name('power_control')
+        run_cli_command = self.patch(api, '_run_cli_command')
+        api.power_control_target(target, power_control)
+        expected = 'set targets/SP/%s/powerControl powerCtrlType=%s' % (
+            target, power_control)
+        self.assertThat(run_cli_command, MockCalledOnceWith(expected))

=== added file 'src/provisioningserver/drivers/hardware/umg.py'
--- src/provisioningserver/drivers/hardware/umg.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/drivers/hardware/umg.py	2014-06-10 14:58:15 +0000
@@ -0,0 +1,149 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Support for managing nodes via a Emerson Network Power Universal
+Management Gatway (UMG).
+
+The UMG is an appliance for managing servers and other devices remotely.
+It can interact with managed systems via IPMI and other network
+protocols, providing services like remote console, power control, and
+sensor aggregration.
+
+This module provides support for interacting UMG's SSH based CLI, and
+for using that support to allow MAAS to manage systems via UMG.
+"""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+str = None
+
+__metaclass__ = type
+__all__ = []
+
+from collections import namedtuple
+import re
+
+import paramiko
+
+
+ShowOutput = namedtuple('ShowOutput', ('directories', 'settings'))
+
+directories_pattern = r'''
+    ^\|__       # Start of line followed by '|__'
+    (.*)        # Capture remaining text on line
+    \r$         # End of line with carriage return.
+'''
+directories_re = re.compile(directories_pattern, re.MULTILINE | re.VERBOSE)
+
+settings_pattern = r'''
+    ^\s\*\s     # Start of line followed by ' * '
+    (\w+)=(.*)  # Capture key and value
+    \r$         # End of line with carriage return.
+'''
+settings_re = re.compile(settings_pattern, re.MULTILINE | re.VERBOSE)
+
+
+class UMG_CLI_API:
+    """An API for interacting with the UMG CLI.
+
+    This API exposes actions that are useful to MAAS, but not
+    necessarily general purpose.
+
+    Some terms:
+    command - input for the CLI.  The CLI will run a command and return
+    some output.
+    SP - Service Processor. A service processor sometimes, but
+    not always, manages a server. There are also service processors for
+    other things (CMM).
+    target - things managed by the UMG. For MAAS's use, these are
+    servers.
+    """
+
+    def __init__(self, host, username, password):
+        self.host = host
+        self.username = username
+        self.password = password
+        self._ssh = paramiko.SSHClient()
+        self._ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
+
+    def _run_cli_command(self, command):
+        """Run a single command and return unparsed text from stdout.
+
+        This method opens and closes an SSH connection each time it's
+        called - the CLI doesn't appear to support multiple commands per
+        connection.
+        """
+        full_command = 'cli -s %s' % (command)
+        self._ssh.connect(
+            self.host, username=self.username, password=self.password)
+        try:
+            _, stdout, _ = self._ssh.exec_command(full_command)
+            output = stdout.read()
+        finally:
+            self._ssh.close()
+
+        return output
+
+    def _parse_show_output(self, text):
+        """Parse the output of a 'show' command.
+
+        The show command shows information about a directory. The
+        directory may contain other directories, and may have settings.
+
+        This method returns a list of the directories and a dictionary
+        containing the settings, encapsulated in a ShowOutput object.
+
+        Each line of output is follwed by '\r\n'.
+
+        Example output - all directories:
+          Cli>show /targets/SP
+          |__1F-C9-DF_CMM-1-1
+          |__1F-C9-DF_CMM-1-2
+          |__1F-C9-DF_MMC-1-1-28
+          |__1F-C9-DF_MMC-1-1-31
+          |__1F-C9-DF_MMC-1-2-28
+          |__1F-C9-DF_MMC-1-2-31
+
+        Example output - directories and text:
+          Cli>show /targets/SP/1F-C9-DF_CMM-1-1
+           * alias=1F-C9-DF_CMM-1-1
+           * internalId=181
+           * ipAddress=10.216.160.113
+          |__powerControl
+        """
+
+        directories = directories_re.findall(text)
+        settings = dict(settings_re.findall(text))
+        return ShowOutput(directories, settings)
+
+    def _show_command(self, command):
+        """Run a show command and return its parsed output."""
+        output_text = self._run_cli_command(command)
+        output = self._parse_show_output(output_text)
+        return output
+
+    def show_targets(self):
+        """Return a list of targets from /targets/SP."""
+        output = self._show_command('show /targets/SP')
+        return output
+
+    def show_target(self, target):
+        """Show the details of a target from /target/SP.
+
+        This provides informations about the SP - like IP
+        address, type, and power status.
+        """
+        output = self._show_command('show /targets/SP/%s' % (target))
+        return output
+
+    def power_control_target(self, target, control):
+        """Issue a power command to a target.
+
+        Targets are SP names.  Valid controls are 'cycle', 'on', 'off'.
+        """
+        command = 'set targets/SP/%s/powerControl powerCtrlType=%s' % (
+            target, control)
+        self._run_cli_command(command)