Merge lp:~martin-nowack/maas/fix-sticky-ip into lp:~maas-committers/maas/trunk

[Martin Nowack]

Fixes LP: 1423931

[Mike Pontillo (mpontillo)]

Before accepting this patch, I think we would need to verify that there are no side effects involved with updating the DHCP host maps with the STICKY IP address every time. (we may create duplicate leases - though arguably that's better than creating zero leases!)

Second, I'm curious: why did you add the unicode() cast around the MAC addresses in get_static_ip_mappings()? Is there a problem updating the host maps without it? (I can see that is consistent with a few other occurrences in this file either way, but I'm not sure why MAC addresses would need to be expressed in unicode.)

Finally, we would not be able to accept this patch without corresponding unit tests.

review: Needs Fixing

[Martin Nowack (martin-nowack)]

> Before accepting this patch, I think we would need to verify that there are no
> side effects involved with updating the DHCP host maps with the STICKY IP
> address every time. (we may create duplicate leases - though arguably that's
> better than creating zero leases!)
True, that's an issue - is it possible to try to blindly delete it beforehand?
Do you see any consequences with this?

> Second, I'm curious: why did you add the unicode() cast around the MAC
> addresses in get_static_ip_mappings()? Is there a problem updating the host
> maps without it? (I can see that is consistent with a few other occurrences in
> this file either way, but I'm not sure why MAC addresses would need to be
> expressed in unicode.)
I had an issue with Django complaining about this (stack trace from 1.7 version):
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/django/core/handlers/base.py", line 112, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/lib/python2.7/dist-packages/django/views/generic/base.py", line 69, in view
    return self.dispatch(request, *args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/maasserver/views/nodes.py", line 807, in dispatch
    return super(NodeView, self).dispatch(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/django/views/generic/base.py", line 87, in dispatch
    return handler(request, *args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/django/views/generic/edit.py", line 228, in post
    return super(BaseUpdateView, self).post(request, *args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/django/views/generic/edit.py", line 171, in post
    return self.form_valid(form)
  File "/usr/lib/python2.7/dist-packages/django/views/generic/edit.py", line 147, in form_valid
    self.object = form.save()
  File "/usr/lib/python2.7/dist-packages/maasserver/forms.py", line 1030, in save
    message = action.execute(allow_redirect=allow_redirect)
  File "/usr/lib/python2.7/dist-packages/maasserver/node_action.py", line 341, in execute
    self.node.start(self.user)
  File "/usr/lib/python2.7/dist-packages/maasserver/models/node.py", line 1516, in start
    raise update_host_maps_failures[0].raiseException()
  File "<string>", line 2, in raiseException
AttributeError: 'MAC' object has no attribute 'encode'

> Finally, we would not be able to accept this patch without corresponding unit
> tests.
I'll see if I can add them.

[Mike Pontillo (mpontillo)]

Blake fixed this issue with the following merge: https://code.launchpad.net/~blake-rouse/maas/fix-sticky-ip-hostmap/+merge/251810

So I'm closing this one out. Thanks for your contribution.

Unmerged revisions

3583. By Martin Nowack (<email address hidden>) on 2015-03-02

Fix assignment of sticky ip addresses.

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2015-02-19 21:26:21 +0000
+++ src/maasserver/models/node.py	2015-03-02 19:17:04 +0000
@@ -756,7 +756,7 @@
         """
         macs = self.macaddress_set.all().prefetch_related('ip_addresses')
         return [
-            (sip.ip, mac.mac_address)
+            (sip.ip, unicode(mac.mac_address))
             for mac in macs
             for sip in mac.ip_addresses.all()
             ]
@@ -1626,6 +1626,11 @@
         # Claim static IP addresses for the node if it's ALLOCATED.
         if self.status == NODE_STATUS.ALLOCATED:
             claims = self.claim_static_ip_addresses()
+
+            # If claims are empty, check for sticky ip addresses
+            if len(claims) == 0:
+                claims = self.get_static_ip_mappings()
+
             # If the PXE mac is on a managed interface then we can ask
             # the cluster to generate the DHCP host map(s).
             if self.is_pxe_mac_on_managed_interface():