launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/maas/omshell-task into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~julian-edwards/maas/omshell-task/+merge/116789

Pre-imp with Gavin, mostly about how to approach tests.

Add two celery tasks that deal with adding and removing DHCP host maps.  Pretty trivial stuff.
-- 
https://code.launchpad.net/~julian-edwards/maas/omshell-task/+merge/116789
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/maas/omshell-task into lp:maas.

=== modified file 'HACKING.txt'
--- HACKING.txt	2012-07-24 17:47:48 +0000
+++ HACKING.txt	2012-07-26 00:51:19 +0000
@@ -45,7 +45,7 @@
         python-twisted python-oops-wsgi python-oops-twisted \
         python-psycopg2 python-yaml python-convoy python-django-south \
         python-avahi python-dbus python-celery python-tempita distro-info \
-        bind9utils libpq-dev dnsutils bind9 python-netaddr
+        bind9utils libpq-dev dnsutils bind9 python-netaddr isc-dhcp-common
 
 Additionally, you need to install the following python libraries
 for development convenience::

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2012-07-25 13:12:59 +0000
+++ src/provisioningserver/tasks.py	2012-07-26 00:51:19 +0000
@@ -20,6 +20,7 @@
     'write_full_dns_config',
     ]
 
+from subprocess import CalledProcessError
 
 from celery.task import task
 from provisioningserver.dns.config import (
@@ -27,12 +28,18 @@
     execute_rndc_command,
     setup_rndc,
     )
+from provisioningserver.omshell import Omshell
 from provisioningserver.power.poweraction import (
     PowerAction,
     PowerActionFail,
     )
 
 
+# =====================================================================
+# Power-related tasks
+# =====================================================================
+
+
 def issue_power_action(power_type, power_change, **kwargs):
     """Issue a power action to a node.
 
@@ -69,6 +76,11 @@
     issue_power_action(power_type, 'off', **kwargs)
 
 
+# =====================================================================
+# DNS-related tasks
+# =====================================================================
+
+
 @task
 def rndc_command(arguments, callback=None):
     """Use rndc to execute a command.
@@ -145,3 +157,49 @@
     setup_rndc()
     if callback is not None:
         callback.delay()
+
+
+# =====================================================================
+# DHCP-related tasks
+# =====================================================================
+
+
+@task
+def add_new_dhcp_host_map(ip_address, mac_address, server_address, shared_key):
+    """Add a MAC to IP mapping in the DHCP server.
+    
+    :param ip_address: Dotted quad string
+    :param mac_address: Colon-separated hex string, e.g. aa:bb:cc:dd:ee:ff
+    :param server_address: IP or hostname for the DHCP server
+    :param shared_key: The HMAC-MD5 key that the DHCP server uses for access
+        control.
+    """
+    omshell = Omshell(server_address, shared_key)
+    try:
+        omshell.create(ip_address, mac_address)
+    except CalledProcessError:
+        # TODO signal to webapp that the job failed.
+
+        # Re-raise, so the job is marked as failed.  Only currently
+        # useful for tests.
+        raise
+
+
+@task
+def remove_dhcp_host_map(ip_address, server_address, shared_key):
+    """Remove an IP to MAC mapping in the DHCP server.
+
+    :param ip_address: Dotted quad string
+    :param server_address: IP or hostname for the DHCP server
+    :param shared_key: The HMAC-MD5 key that the DHCP server uses for access
+        control.
+    """
+    omshell = Omshell(server_address, shared_key)
+    try:
+        omshell.remove(ip_address)
+    except CalledProcessError:
+        # TODO signal to webapp that the job failed.
+
+        # Re-raise, so the job is marked as failed.  Only currently
+        # useful for tests.
+        raise

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2012-07-25 15:20:42 +0000
+++ src/provisioningserver/tests/test_tasks.py	2012-07-26 00:51:19 +0000
@@ -14,10 +14,12 @@
 
 import os
 import random
+from subprocess import CalledProcessError
 
 from maastesting.celery import CeleryFixture
 from maastesting.factory import factory
 from maastesting.fakemethod import FakeMethod
+from maastesting.matchers import ContainsAll
 from maastesting.testcase import TestCase
 from netaddr import IPNetwork
 from provisioningserver import tasks
@@ -31,9 +33,12 @@
 from provisioningserver.enum import POWER_TYPE
 from provisioningserver.power.poweraction import PowerActionFail
 from provisioningserver.tasks import (
+    add_new_dhcp_host_map,
     power_off,
     power_on,
+    remove_dhcp_host_map,
     rndc_command,
+    Omshell,
     setup_rndc_configuration,
     write_dns_config,
     write_dns_zone_config,
@@ -75,6 +80,71 @@
             POWER_TYPE.WAKE_ON_LAN, mac=arbitrary_mac)
 
 
+class TestDHCPTasks(TestCase):
+
+    resources = (
+        ("celery", FixtureResource(CeleryFixture())),
+        )
+
+    def test_add_new_dhcp_host_map(self):
+        # We don't want to actually run omshell in the task, so we stub
+        # out the wrapper class's _run method and record what it would
+        # do.
+        mac = factory.getRandomMACAddress()
+        ip = factory.getRandomIPAddress()
+        server_address = factory.getRandomString()
+        key = factory.getRandomString()
+        recorder = FakeMethod(result=(0, "hardware-type"))
+        self.patch(Omshell, '_run', recorder)
+        add_new_dhcp_host_map.delay(ip, mac, server_address, key)
+
+        # The extracted args are stdin.  We can just check that all the
+        # parameters that were passed are being used.
+        self.assertThat(
+            recorder.extract_args()[0][0],
+            ContainsAll([ip, mac, server_address, key]))
+
+    def test_add_new_dhcp_host_map_failure(self):
+        # Check that task failures are caught.  Nothing much happens in
+        # the Task code right now though.
+        mac = factory.getRandomMACAddress()
+        ip = factory.getRandomIPAddress()
+        server_address = factory.getRandomString()
+        key = factory.getRandomString()
+        self.patch(Omshell, '_run', FakeMethod(result=(0, "this_will_fail")))
+        self.assertRaises(
+            CalledProcessError, add_new_dhcp_host_map.delay,
+            ip, mac, server_address, key)
+
+    def test_remove_dhcp_host_map(self):
+        # We don't want to actually run omshell in the task, so we stub
+        # out the wrapper class's _run method and record what it would
+        # do.
+        ip = factory.getRandomIPAddress()
+        server_address = factory.getRandomString()
+        key = factory.getRandomString()
+        recorder = FakeMethod(result=(0, "obj: <null>"))
+        self.patch(Omshell, '_run', recorder)
+        remove_dhcp_host_map.delay(ip, server_address, key)
+
+        # The extracted args are stdin.  We can just check that all the
+        # parameters that were passed are being used.
+        self.assertThat(
+            recorder.extract_args()[0][0],
+            ContainsAll([ip, server_address, key]))
+
+    def test_remove_dhcp_host_map_failure(self):
+        # Check that task failures are caught.  Nothing much happens in
+        # the Task code right now though.
+        ip = factory.getRandomIPAddress()
+        server_address = factory.getRandomString()
+        key = factory.getRandomString()
+        self.patch(Omshell, '_run', FakeMethod(result=(0, "this_will_fail")))
+        self.assertRaises(
+            CalledProcessError, remove_dhcp_host_map.delay,
+            ip, server_address, key)
+
+
 class TestDNSTasks(TestCase):
 
     resources = (