From: Carl Baldwin <carl.baldwin@hp.com>
Date: Thu, 4 Jun 2015 22:25:44 +0000 (+0000)
Subject: Refactor awkward logic in setup_dhcp_port
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=713ba0e8d7ce59eaff41518360530b2e7831c322;p=openstack-build%2Fneutron-build.git

Refactor awkward logic in setup_dhcp_port

I noticed this logic as I was reviewing another patch set [1].  I
didn't like removing subnet ids from dhcp_enabled_subnet_ids and I
wasn't too keen on the ips_need_removal semantics that were kind of
forced by the existing structure of the code.  I hope you find this
alternative much clearer.  I like straight-forward code with less
indentation that doesn't use awkward booleans like ips_needs_removal.

[1] https://review.openstack.org/#/c/157697/6

Change-Id: I8bd3d6924a855ea08f8096e66bd3bfbb165a4da3
---

diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py
index 70453d265..ae6fd43fa 100644
--- a/neutron/agent/linux/dhcp.py
+++ b/neutron/agent/linux/dhcp.py
@@ -849,33 +849,28 @@ class DeviceManager(object):
         """Create/update DHCP port for the host if needed and return port."""
 
         device_id = self.get_device_id(network)
-        subnets = {}
-        dhcp_enabled_subnet_ids = []
-        for subnet in network.subnets:
-            if subnet.enable_dhcp:
-                dhcp_enabled_subnet_ids.append(subnet.id)
-                subnets[subnet.id] = subnet
+        subnets = {subnet.id: subnet for subnet in network.subnets
+                   if subnet.enable_dhcp}
 
         dhcp_port = None
         for port in network.ports:
             port_device_id = getattr(port, 'device_id', None)
             if port_device_id == device_id:
+                dhcp_enabled_subnet_ids = set(subnets)
                 port_fixed_ips = []
-                ips_needs_removal = False
                 for fixed_ip in port.fixed_ips:
                     if fixed_ip.subnet_id in dhcp_enabled_subnet_ids:
                         port_fixed_ips.append(
                             {'subnet_id': fixed_ip.subnet_id,
                              'ip_address': fixed_ip.ip_address})
-                        dhcp_enabled_subnet_ids.remove(fixed_ip.subnet_id)
-                    else:
-                        ips_needs_removal = True
 
-                # If there are dhcp_enabled_subnet_ids here that means that
-                # we need to add those to the port and call update.
-                if dhcp_enabled_subnet_ids or ips_needs_removal:
+                port_subnet_ids = set(ip.subnet_id for ip in port.fixed_ips)
+                # If there is a new dhcp enabled subnet or a port that is no
+                # longer on a dhcp enabled subnet, we need to call update.
+                if dhcp_enabled_subnet_ids != port_subnet_ids:
                     port_fixed_ips.extend(
-                        [dict(subnet_id=s) for s in dhcp_enabled_subnet_ids])
+                        dict(subnet_id=s)
+                        for s in dhcp_enabled_subnet_ids - port_subnet_ids)
                     dhcp_port = self.plugin.update_dhcp_port(
                         port.id, {'port': {'network_id': network.id,
                                            'fixed_ips': port_fixed_ips}})
@@ -911,7 +906,7 @@ class DeviceManager(object):
                 device_id=device_id,
                 network_id=network.id,
                 tenant_id=network.tenant_id,
-                fixed_ips=[dict(subnet_id=s) for s in dhcp_enabled_subnet_ids])
+                fixed_ips=[dict(subnet_id=s) for s in subnets])
             dhcp_port = self.plugin.create_dhcp_port({'port': port_dict})
 
         if not dhcp_port: