The quantum interface isn't implemented as a manager anymore, so this
isn't relevant anymore.
** Changed in: nova
Status: Confirmed => Invalid
--
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/887692
Title:
The QuantumManager could use some refactoring
Status in OpenStack Compute (Nova):
Invalid
Bug description:
These changes were suggested by Aaron Lee during his review:
Patch Set 9: (5 inline comments)
I'm giving this a 0 because all of my comments are about refactorings
that would have made it easier for me to read what this patch is
doing. The functionality looks good, as far as I can tell, but I think
the code needs to be cleaner.
....................................................
File nova/network/linux_net.py
Line 995: # Don't forward traffic unless we were told to be a gateway
I would rather see the actions within the conditional more focused on the
decision of the conditional.
i.e.
guard = "DROP"
if gateway
guard = "ACCEPT"
for direction in ['in', 'out']
iptables_manager.ipv4['filter'].add_rule('FORWARD',
'--$(direction)-interface %(guard)s -j
%(bridge)s' % \
locals())
This will make it much easier to fix errors in the generation of those
commands.
....................................................
File nova/network/quantum/manager.py
Line 213: "label": "quantum-net-%s" % quantum_net_id}
I don't think this get network ref logic is a part of allocating for an
instance. It would be better stored in it's own method.
Line 234: LOG.info("Using DHCP for network: %s" %
network_ref['label'])
This would be much easier to maintain and test in small units if each step
was a method, and enable_dhcp just called each individual method. Refactoring
like that would also make more granular error handling easier.
Line 449: subnet['network_id'], project_id)
It appears the real meat of this function is these three calls to the driver,
it should be separated from populating the network_ref.
This data mutating done on this response from get_subnets_by_net_id is
very similar to the changes made to this data on both other calls to
get_subnets_by_net_id. This is another candidate for refactoring, but
may be out of the scope of this commit.
Line 485: def get_dhcp_hosts_text(self, context, subnet_id,
project_id=None):
These methods share 10 lines, maybe the collection of information and
iteration over that list could be abstracted?
There are a few others I wanted to do as well.
To manage notifications about this bug go to:
https://bugs.launchpad.net/nova/+bug/887692/+subscriptions
--
Mailing list: https://launchpad.net/~yahoo-eng-team
Post to : [email protected]
Unsubscribe : https://launchpad.net/~yahoo-eng-team
More help : https://help.launchpad.net/ListHelp