From 9a21f9b2dbbbe5f51613ebc440ffac7143b768bd Mon Sep 17 00:00:00 2001 From: Ondra Machacek Date: Fri, 30 Mar 2018 13:56:11 +0200 Subject: [PATCH] ovirt_host_networks: Fix of bond assignment (#38054) * ovirt_host_networks: Fix of bond assignment * ovirt_host_networks: Fix bond modes * ovirt_host_networks: Fix incorrect prefix documentation --- .../cloud/ovirt/ovirt_host_networks.py | 110 ++++++++++++------ 1 file changed, 77 insertions(+), 33 deletions(-) diff --git a/lib/ansible/modules/cloud/ovirt/ovirt_host_networks.py b/lib/ansible/modules/cloud/ovirt/ovirt_host_networks.py index eee4fae3402..0275a24959b 100644 --- a/lib/ansible/modules/cloud/ovirt/ovirt_host_networks.py +++ b/lib/ansible/modules/cloud/ovirt/ovirt_host_networks.py @@ -57,7 +57,7 @@ options: - "C(name) - Name of the logical network to be assigned to bond or interface." - "C(boot_protocol) - Boot protocol one of the I(none), I(static) or I(dhcp)." - "C(address) - IP address in case of I(static) boot protocol is used." - - "C(prefix) - Routing prefix in case of I(static) boot protocol is used." + - "C(netmask) - Subnet mask in case of I(static) boot protocol is used." - "C(gateway) - Gateway in case of I(static) boot protocol is used." - "C(version) - IP version. Either v4 or v6. Default is v4." labels: @@ -92,7 +92,7 @@ EXAMPLES = ''' - name: myvlan boot_protocol: static address: 1.2.3.4 - prefix: 24 + netmask: 255.255.255.0 gateway: 1.2.3.4 version: v4 @@ -160,8 +160,56 @@ from ansible.module_utils.ovirt import ( ) +def get_mode_type(mode_number): + """ + Adaptive transmit load balancing (balance-tlb): mode=1 miimon=100 + Dynamic link aggregation (802.3ad): mode=2 miimon=100 + Load balance (balance-xor): mode=3 miimon=100 + Active-Backup: mode=4 miimon=100 xmit_hash_policy=2 + """ + options = [] + if mode_number is None: + return options + + def get_type_name(mode_number): + """ + We need to maintain this type strings, for the __compare_options method, + for easier comparision. + """ + return [ + 'Active-Backup', + 'Load balance (balance-xor)', + None, + 'Dynamic link aggregation (802.3ad)', + ][mode_number] + + try: + mode_number = int(mode_number) + if mode_number >= 1 and mode_number <= 4: + if mode_number == 4: + options.append(otypes.Option(name='xmit_hash_policy', value='2')) + + options.append(otypes.Option(name='miimon', value='100')) + options.append( + otypes.Option( + name='mode', + type=get_type_name(mode_number - 1), + value=str(mode_number) + ) + ) + else: + options.append(otypes.Option(name='mode', value=str(mode_number))) + except ValueError: + raise Exception("Bond mode must be a number.") + + return options + + class HostNetworksModule(BaseModule): + def __compare_options(self, new_options, old_options): + return sorted(get_dict_of_struct(opt) for opt in new_options) != sorted(get_dict_of_struct(opt) for opt in old_options) + def build_entity(self): return otypes.Host() @@ -180,8 +228,8 @@ class HostNetworksModule(BaseModule): if not equal(network.get('gateway'), ip.ip.gateway): ip.ip.gateway = network.get('gateway') changed = True - if not equal(network.get('prefix'), sum([bin(int(x)).count('1') for x in ip.ip.netmask.split('.')]) if ip.ip.netmask else None): - ip.ip.netmask = str(network.get('prefix')) + if not equal(network.get('netmask'), ip.ip.netmask): + ip.ip.netmask = network.get('netmask') changed = True if changed: @@ -202,17 +250,16 @@ class HostNetworksModule(BaseModule): # Check if bond configuration should be updated: if bond: - update = not ( - equal(str(bond.get('mode')), nic.bonding.options[0].value) and - equal( - sorted(bond.get('interfaces')) if bond.get('interfaces') else None, - sorted(get_link_name(self._connection, s) for s in nic.bonding.slaves) - ) + update = self.__compare_options(get_mode_type(bond.get('mode')), getattr(nic.bonding, 'options', [])) + update = update or not equal( + sorted(bond.get('interfaces')) if bond.get('interfaces') else None, + sorted(get_link_name(self._connection, s) for s in nic.bonding.slaves) ) # Check if labels need to be updated on interface/bond: if labels: net_labels = nic_service.network_labels_service().list() + # If any lables which user passed aren't assigned, relabel the interface: if sorted(labels) != sorted([lbl.id for lbl in net_labels]): return True @@ -287,7 +334,7 @@ def main(): nics_service = host_service.nics_service() nic = search_by_name(nics_service, nic_name) - network_names = [network['name'] for network in networks] + network_names = [network['name'] for network in networks or []] state = module.params['state'] if ( state == 'present' and @@ -295,19 +342,21 @@ def main(): ): # Remove networks which are attached to different interface then user want: attachments_service = host_service.network_attachments_service() - remove_network_attachments = [ - a for a in attachments_service.list() - if get_link_name(connection, a.network) in network_names and - get_link_name(connection, a.host_nic) != interface - ] - if remove_network_attachments: - host_networks_module.action( - entity=host, - action='setup_networks', - post_action=host_networks_module._action_save_configuration, - check_connectivity=module.params['check'], - removed_network_attachments=remove_network_attachments, - ) + + # Append attachment ID to network if needs update: + for a in attachments_service.list(): + current_network_name = get_link_name(connection, a.network) + if current_network_name in network_names: + for n in networks: + if n['name'] == current_network_name: + n['id'] = a.id + + # Check if we have to break some bonds: + removed_bonds = [] + if nic is not None: + for host_nic in nics_service.list(): + if host_nic.bonding and nic.id in [slave.id for slave in host_nic.bonding.slaves]: + removed_bonds.append(otypes.HostNic(id=host_nic.id)) # Assign the networks: host_networks_module.action( @@ -315,16 +364,12 @@ def main(): action='setup_networks', post_action=host_networks_module._action_save_configuration, check_connectivity=module.params['check'], + removed_bonds=removed_bonds if removed_bonds else None, modified_bonds=[ otypes.HostNic( name=bond.get('name'), bonding=otypes.Bonding( - options=[ - otypes.Option( - name="mode", - value=str(bond.get('mode')), - ) - ], + options=get_mode_type(bond.get('mode')), slaves=[ otypes.HostNic(name=i) for i in bond.get('interfaces', []) ], @@ -341,6 +386,7 @@ def main(): ] if labels else None, modified_network_attachments=[ otypes.NetworkAttachment( + id=network.get('id'), network=otypes.Network( name=network['name'] ) if network['name'] else None, @@ -362,9 +408,7 @@ def main(): ), ), ], - ) for network in networks if network['name'] not in [ - get_link_name(connection, a.network) for a in attachments_service.list() - ] # Attach only networks which are not yet attached. + ) for network in networks ] if networks else None, ) elif state == 'absent' and nic: