opendev/glean
Code Issues Proposed changes

redhat-ish platforms: refactor simplification of interface writing

Browse Source 
It was pointed out in reviews of
Iac0760ed1dd33c06b81cdd2ea3885279d2aab878 (thanks clarkb) that a
single network is just a case of a multiple network list len()==1,
thus we can simplify this path.

This is a refactor to implement this, removing _write_rh_interfaces()
and combining it into write_redhat_interfaces().  I was probably a bit
deep into it when I wrote a lot of these comments, rework them a bit.

Change-Id: Iede6054d84cffa28e1365501918264a779e5db76
This commit is contained in:
Ian Wienand 2022-06-01 10:27:36 +10:00
parent cbbad59484
commit 69d724a6f3
1 changed files with 27 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
glean/cmd.py
View File

@ -174,22 +174,6 @@ def _set_rh_vlan(name, interface, distro):
return results return results
def _write_rh_interfaces(name, interfaces, args):
files = {}
# Sort the interfaces by type. ipv4 should come before ipv6. We
# need to process ipv4 first; it builds the base config file.
# _write_rh_v6_interface() then adds ipv6 info. See prior notes
# about this being a hack that should all be refactored.
for interface in sorted(interfaces, key=lambda x: x['type']):
if interface['type'] == 'ipv4':
files = _write_rh_interface(name, interface, args)
elif interface['type'] == 'ipv4_dhcp':
files = _write_rh_dhcp(name, interface, args)
elif interface['type'] in ('ipv6', 'ipv6_slaac'):
files = _write_rh_v6_interface(name, interface, args, files)
return files
def _write_rh_v6_interface(name, interface, args, files): def _write_rh_v6_interface(name, interface, args, files):
config_file = _network_files(args.distro)["ifcfg"] + '-%s' % name config_file = _network_files(args.distro)["ifcfg"] + '-%s' % name
# We should already have this config file key key, we need to # We should already have this config file key key, we need to
@ -370,36 +354,46 @@ def write_redhat_interfaces(interfaces, sys_interfaces, args):
_interfaces_by_sys_name[interface_name].append(interface) _interfaces_by_sys_name[interface_name].append(interface)
for _name, _interfaces in _interfaces_by_sys_name.items(): for _name, _interfaces in _interfaces_by_sys_name.items():
# NOTE(ianw) 2022-05-25 : this is all *terrible* and should
# definitely be refactored. "interfaces" is a really bad term
# used everywhere here. It's actually the "networks" key of
# the configdrive metadata. We stick to the term to avoid
# even more confusion.
# This function handles when a device has more than one # Sort the interfaces by type. ipv4 should come before ipv6.
# interface/network attached. As I have noted, this is a mess # We need to process ipv4 first; it builds the base config
# and should all be refactored. # file (because when written we only dealt with ipv4).
if len(_interfaces) > 1: # _write_rh_v6_interface() then adds ipv6 info, which is why
files_to_write.update( # it gets passed "files_to_write".
_write_rh_interfaces(_name, _interfaces, args)) #
else: # NOTE(ianw) : 2022-06-01 It would definitely be better to
assert(len(_interfaces) == 1) # refactor this in a way that does more like a generic
interface = _interfaces[0] # template that each possible interface type fills in, rather
logging.debug("Processing : %s -> %s" % (_name, interface)) # than relying on ordering like this. The term "interfaces"
# is a bit of a misnomer, and it's really a network being
# setup on what we'd think of as an interface (eth<X>, etc.).
# This could also be cleaned up to be clearer.
for interface in sorted(_interfaces, key=lambda x: x['type']):
logging.debug("Processing interface %s/%s" %
(_name, interface['type']))
if interface['type'] == 'ipv4': if interface['type'] == 'ipv4':
files_to_write.update( files_to_write.update(
_write_rh_interface(_name, interface, args)) _write_rh_interface(_name, interface, args))
if interface['type'] == 'ipv4_dhcp': elif interface['type'] == 'ipv4_dhcp':
files_to_write.update( files_to_write.update(
_write_rh_dhcp(_name, interface, args)) _write_rh_dhcp(_name, interface, args))
if interface['type'] == 'manual': elif interface['type'] == 'manual':
files_to_write.update( files_to_write.update(
_write_rh_manual(_name, interface, args)) _write_rh_manual(_name, interface, args))
elif interface['type'] in ('ipv6', 'ipv6_slaac'):
files_to_write.update(
_write_rh_v6_interface(_name,
interface, args, files_to_write))
else:
logging.error(
"Unhandled interface %s/%s" % (_name, interface['type']))
if args.no_dhcp_fallback: if args.no_dhcp_fallback:
log.debug('DHCP fallback disabled') log.debug('DHCP fallback disabled')
return files_to_write return files_to_write
# Strip out anything that has now been configured, the rest falls
# back to simple DHCP
for mac, iname in sorted( for mac, iname in sorted(
sys_interfaces.items(), key=lambda x: x[1]): sys_interfaces.items(), key=lambda x: x[1]):
if _exists_rh_interface(iname, args.distro): if _exists_rh_interface(iname, args.distro):