Merge lp:~julian-edwards/maas/optional-ip-api into lp:~maas-committers/maas/trunk

[Julian Edwards]

Add functionality to the API to request specific IPs for STICKY addresses on nodes.

[Graham Binns (gmb)]

Couple of comments, no blockers. Nice work :)

review: Approve

[Julian Edwards (julian-edwards)]

Thanks for the review! (this and the other)

On Monday 04 Aug 2014 15:21:50 you wrote:
> If claim_static_ip_address is mapped to 'claim_sticky_ip_address' as far as
> the world is concerned, we should do a rename of static_ip -> sticky_ip for
> the sake of not having another Cluster/NodeGroup-ish situation. That can be
> a tech-debt bug for now, of course.

Sticky is a *type* of static IP. See IPADDRESS_TYPE.

The API call is called claim_sticky_ip_address because it's node-specific and
only works with sticky types.

I have no inclination to rename static IP at all and will resist it unless
someone comes up with some better arguments.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2014-08-04 14:29:05 +0000
+++ src/maasserver/api.py	2014-08-04 15:55:26 +0000
@@ -553,6 +553,10 @@
         :param mac_address: Optional MAC address on the node on which to
             assign the sticky IP address.  If not passed, defaults to the
             primary MAC for the node.
+        :param requested_address: Optional IP address to claim.  Must be in
+            the range defined on the cluster interface to which the context
+            MAC is related, or 403 Forbidden is returned.  If the requested
+            address is unavailable for use, 404 Not Found is returned.
 
         A sticky IP is one which stays with the node until the IP is
         disassociated with the node, or the node is deleted.  It allows
@@ -575,9 +579,12 @@
             except MACAddress.DoesNotExist:
                 raise MAASAPIBadRequest(
                     "mac_address %s not found on the node" % raw_mac)
-        sip = mac_address.claim_static_ip(alloc_type=IPADDRESS_TYPE.STICKY)
+        requested_address = request.POST.get('requested_address', None)
+        sticky_ip = mac_address.claim_static_ip(
+            alloc_type=IPADDRESS_TYPE.STICKY,
+            requested_address=requested_address)
         maaslog.info(
-            "%s: Sticky IP address %s allocated", node.hostname, sip.ip)
+            "%s: Sticky IP address %s allocated", node.hostname, sticky_ip.ip)
         return node
 
     @operation(idempotent=False)

=== modified file 'src/maasserver/exceptions.py'
--- src/maasserver/exceptions.py	2014-08-04 15:55:26 +0000
+++ src/maasserver/exceptions.py	2014-08-04 15:55:26 +0000
@@ -127,7 +127,7 @@
     api_error = httplib.NOT_FOUND
 
 
-class StaticIPAddressOutOfRange(MAASException):
+class StaticIPAddressOutOfRange(MAASAPIException):
     """Raised when a requested IP is not in an acceptable range."""
     api_error = httplib.FORBIDDEN
 

=== modified file 'src/maasserver/models/macaddress.py'
--- src/maasserver/models/macaddress.py	2014-07-31 10:56:18 +0000
+++ src/maasserver/models/macaddress.py	2014-08-04 15:55:26 +0000
@@ -89,7 +89,8 @@
         """Return networks to which this MAC is connected, sorted by name."""
         return self.networks.all().order_by('name')
 
-    def claim_static_ip(self, alloc_type=IPADDRESS_TYPE.AUTO):
+    def claim_static_ip(self, alloc_type=IPADDRESS_TYPE.AUTO,
+                        requested_address=None):
         """Assign a static IP to this MAC.
 
         It is the caller's responsibility to create a celery Task that will
@@ -99,6 +100,9 @@
 
         :param alloc_type: See :class:`StaticIPAddress`.alloc_type.
             This parameter musn't be IPADDRESS_TYPE.USER_RESERVED.
+        :param requested_address: Optional IP address to claim.  Must be in
+            the range defined on the cluster interface to which this MACAddress
+            is related.
         :return: A :class:`StaticIPAddress` object. Returns None if
             the cluster_interface is not yet known, or the
             static_ip_range_low/high values values are not set on the
@@ -107,6 +111,10 @@
         :raises: StaticIPAddressExhaustion if there are not enough IPs left.
         :raises: StaticIPAddressTypeClash if an IP already exists with a
             different type.
+        :raises: StaticIPAddressOutOfRange if the requested_address is not in
+            the cluster interface's defined range.
+        :raises: StaticIPAddressUnavailable if the requested_address is already
+            allocated.
         """
         # Avoid circular import.
         from maasserver.models.staticipaddress import StaticIPAddress
@@ -141,6 +149,7 @@
             MACStaticIPAddressLink,
             StaticIPAddress,
             )
-        sip = StaticIPAddress.objects.allocate_new(low, high, alloc_type)
+        sip = StaticIPAddress.objects.allocate_new(
+            low, high, alloc_type, requested_address=requested_address)
         MACStaticIPAddressLink(mac_address=self, ip_address=sip).save()
         return sip

=== modified file 'src/maasserver/models/tests/test_macaddress.py'
--- src/maasserver/models/tests/test_macaddress.py	2014-07-16 12:33:31 +0000
+++ src/maasserver/models/tests/test_macaddress.py	2014-08-04 15:55:26 +0000
@@ -131,3 +131,9 @@
         ip = mac.claim_static_ip(alloc_type=iptype)
         self.assertEqual(
             ip, mac.claim_static_ip(alloc_type=iptype))
+
+    def test_passes_requested_ip(self):
+        node = factory.make_node_with_mac_attached_to_nodegroupinterface()
+        mac = node.get_primary_mac()
+        ip = node.get_primary_mac().cluster_interface.static_ip_range_high
+        self.assertEqual(ip, mac.claim_static_ip(requested_address=ip).ip)

=== modified file 'src/maasserver/tests/test_api_node.py'
--- src/maasserver/tests/test_api_node.py	2014-07-31 11:47:10 +0000
+++ src/maasserver/tests/test_api_node.py	2014-08-04 15:55:26 +0000
@@ -18,6 +18,7 @@
 from cStringIO import StringIO
 import httplib
 import json
+from netaddr import IPAddress
 import sys
 
 import bson
@@ -1040,6 +1041,63 @@
             "mac_address %s not found on the node" % random_mac,
             response.content)
 
+    def test_claim_sticky_ip_allows_requested_ip(self):
+        self.become_admin()
+        node = factory.make_node_with_mac_attached_to_nodegroupinterface()
+        ngi = node.get_primary_mac().cluster_interface
+        requested_address = ngi.static_ip_range_low
+
+        response = self.client.post(
+            self.get_node_uri(node),
+            {
+                'op': 'claim_sticky_ip_address',
+                'requested_address': requested_address,
+            })
+        self.assertEqual(httplib.OK, response.status_code, response.content)
+        [observed_static_ip] = StaticIPAddress.objects.all()
+        self.assertEqual(observed_static_ip.ip, requested_address)
+
+    def test_claim_sticky_ip_address_detects_out_of_range_requested_ip(self):
+        self.become_admin()
+        node = factory.make_node_with_mac_attached_to_nodegroupinterface()
+        ngi = node.get_primary_mac().cluster_interface
+        requested_address = IPAddress(ngi.static_ip_range_low) - 1
+
+        response = self.client.post(
+            self.get_node_uri(node),
+            {
+                'op': 'claim_sticky_ip_address',
+                'requested_address': requested_address.format(),
+            })
+        self.assertEqual(
+            httplib.FORBIDDEN, response.status_code, response.content)
+
+    def test_claim_sticky_ip_address_detects_unavailable_requested_ip(self):
+        self.become_admin()
+        # Create 2 nodes on the same nodegroup and interface.
+        node = factory.make_node_with_mac_attached_to_nodegroupinterface()
+        ngi = node.get_primary_mac().cluster_interface
+        other_node = factory.make_node(mac=True, nodegroup=ngi.nodegroup)
+        other_mac = other_node.get_primary_mac()
+        other_mac.cluster_interface = ngi
+        other_mac.save()
+
+        # Allocate an IP to one of the nodes.
+        requested_address = IPAddress(ngi.static_ip_range_low) + 1
+        requested_address = requested_address.format()
+        other_node.get_primary_mac().claim_static_ip(
+            requested_address=requested_address)
+
+        # Use the API to try to duplicate the same IP on the other node.
+        response = self.client.post(
+            self.get_node_uri(node),
+            {
+                'op': 'claim_sticky_ip_address',
+                'requested_address': requested_address,
+            })
+        self.assertEqual(
+            httplib.NOT_FOUND, response.status_code, response.content)
+
 
 class TestGetDetails(APITestCase):
     """Tests for /api/1.0/nodes/<node>/?op=details."""