Merge lp:~vishvananda/nova/lp720393 into lp:~hudson-openstack/nova/trunk

Proposed by Vish Ishaya
Status: Merged
Approved by: Devin Carlen
Approved revision: 688
Merged at revision: 700
Proposed branch: lp:~vishvananda/nova/lp720393
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 58 lines (+15/-12)
1 file modified
nova/network/manager.py (+15/-12)
To merge this branch: bzr merge lp:~vishvananda/nova/lp720393
Reviewer Review Type Date Requested Status
Devin Carlen (community) Approve
Soren Hansen (community) Approve
Review via email: mp+50058@code.launchpad.net

Description of the change

Makes FlatDHCPManager clean up old fixed_ips like VlanManager.

To post a comment you must log in.
Revision history for this message
Devin Carlen (devcamcar) wrote :

So what calls periodic_tasks?

review: Needs Information
Revision history for this message
Soren Hansen (soren) wrote :

Devin, nova.service.Service() sets up a LoopingCall that calls the manager's periodic_tasks method every FLAGS.periodic_interval seconds.

Revision history for this message
Soren Hansen (soren) wrote :

I don't like that we have the exact same code in two places (the exact same code is in VlanManager's periodic_tasks).

Maybe you could make it a module level function or move it into the NetworkManager class and make it conditional on a "needs_dhcp_cleanup" class variable that is set appropriately for the various managers?

Revision history for this message
Soren Hansen (soren) :
review: Needs Fixing
Revision history for this message
Vish Ishaya (vishvananda) wrote :

Soren: good point. I used timeout_fixed_ips as the flag.

Revision history for this message
Vish Ishaya (vishvananda) wrote :

oops, had an extra copy of periodic_tasks. Deleted.

lp:~vishvananda/nova/lp720393 updated
688. By Vish Ishaya

remove leftover periodic tasks

Revision history for this message
Soren Hansen (soren) wrote :

Lovely! lgtm

review: Approve
Revision history for this message
Devin Carlen (devcamcar) wrote :

lgtm

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'nova/network/manager.py'
--- nova/network/manager.py 2011-02-17 21:47:02 +0000
+++ nova/network/manager.py 2011-02-17 22:54:16 +0000
@@ -110,6 +110,7 @@
110110
111 This class must be subclassed to support specific topologies.111 This class must be subclassed to support specific topologies.
112 """112 """
113 timeout_fixed_ips = True
113114
114 def __init__(self, network_driver=None, *args, **kwargs):115 def __init__(self, network_driver=None, *args, **kwargs):
115 if not network_driver:116 if not network_driver:
@@ -138,6 +139,19 @@
138 self.driver.ensure_floating_forward(floating_ip['address'],139 self.driver.ensure_floating_forward(floating_ip['address'],
139 fixed_address)140 fixed_address)
140141
142 def periodic_tasks(self, context=None):
143 """Tasks to be run at a periodic interval."""
144 super(NetworkManager, self).periodic_tasks(context)
145 if self.timeout_fixed_ips:
146 now = utils.utcnow()
147 timeout = FLAGS.fixed_ip_disassociate_timeout
148 time = now - datetime.timedelta(seconds=timeout)
149 num = self.db.fixed_ip_disassociate_all_by_timeout(context,
150 self.host,
151 time)
152 if num:
153 LOG.debug(_("Dissassociated %s stale fixed ip(s)"), num)
154
141 def set_network_host(self, context, network_id):155 def set_network_host(self, context, network_id):
142 """Safely sets the host of the network."""156 """Safely sets the host of the network."""
143 LOG.debug(_("setting network host"), context=context)157 LOG.debug(_("setting network host"), context=context)
@@ -306,6 +320,7 @@
306 not do any setup in this mode, it must be done manually. Requests to320 not do any setup in this mode, it must be done manually. Requests to
307 169.254.169.254 port 80 will need to be forwarded to the api server.321 169.254.169.254 port 80 will need to be forwarded to the api server.
308 """322 """
323 timeout_fixed_ips = False
309324
310 def allocate_fixed_ip(self, context, instance_id, *args, **kwargs):325 def allocate_fixed_ip(self, context, instance_id, *args, **kwargs):
311 """Gets a fixed ip from the pool."""326 """Gets a fixed ip from the pool."""
@@ -457,18 +472,6 @@
457 instances in its subnet.472 instances in its subnet.
458 """473 """
459474
460 def periodic_tasks(self, context=None):
461 """Tasks to be run at a periodic interval."""
462 super(VlanManager, self).periodic_tasks(context)
463 now = datetime.datetime.utcnow()
464 timeout = FLAGS.fixed_ip_disassociate_timeout
465 time = now - datetime.timedelta(seconds=timeout)
466 num = self.db.fixed_ip_disassociate_all_by_timeout(context,
467 self.host,
468 time)
469 if num:
470 LOG.debug(_("Dissassociated %s stale fixed ip(s)"), num)
471
472 def init_host(self):475 def init_host(self):
473 """Do any initialization that needs to be run if this is a476 """Do any initialization that needs to be run if this is a
474 standalone service.477 standalone service.