na_ontap_broadcast_domain doesnt correctly modifying a broadcast domain's ports using REST

[hashi825]

Summary

When providing a broadcast domain that already contains existing ports and requires adding new ports, using REST will add ALL ports to the REST Patch request due to this statement which prevents actually only returning the ports to add

netapp.ontap/plugins/modules/na_ontap_broadcast_domain.py

Lines 586 to 599 in 78486a1

	def get_ports_rest(self, ports):
	# if desired ports with uuid present then return only the ports to add or move.
	if self.desired_ports:
	return self.ports_to_add_move_from_desired(ports)
	# list of desired ports not present in the node.
	missing_ports = []
	# list of uuid information of each desired port should present in broadcast domain.
	desired_ports = []
	for port in ports:
	current = self.get_net_port_rest(port)
	if current is None:
	missing_ports.append(port)
	else:
	desired_ports.append(current)

netapp.ontap/plugins/modules/na_ontap_broadcast_domain.py

Lines 466 to 474 in 78486a1

	ports_to_add = list(set(expect_ports) - set(current_ports))
	if len(ports_to_add) > 0:
	if self.use_rest:
	ports = self.get_ports_rest(ports_to_add)
	if ports:
	self.add_or_move_broadcast_domain_ports_rest(ports)
	else:
	self.add_broadcast_domain_ports(ports_to_add)

ports_to_add provides the correct list of ports to add get_ports_rest then checks self.desired_ports which contains all ports found from the ports parameter of the module. This causes the module to send a patch request to api/network/ethernet/ports that essentially removes/readds ports that were already in the broadcast domain.

Removing the if statement seems to provide the desired behaviour.

Component Name

na_ontap_broadcast_domain

Ansible Version

$ ansible --version
ansible [core 2.15.0]
  config file = ##/ansible.cfg
  configured module search path = ['/home/##/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = ##/venv/lib64/python3.9/site-packages/ansible
  ansible collection location = ##
  executable location = ##/bin/ansible
  python version = 3.9.18 (main, Jan  4 2024, 00:00:00) [GCC 11.4.1 20230605 (Red Hat 11.4.1-2)] (##/venv/bin/python3.9)
  jinja version = 3.1.2
  libyaml = True

ONTAP Collection Version

Collection            Version
--------------------- -------
ansible.posix         1.5.4  
community.general     7.2.0  
community.hashi_vault 4.2.0  
netapp.ontap          22.11.0
netbox.netbox         3.13.0

ONTAP Version

##::> version
NetApp Release 9.13.1P7: Wed Jan 24 16:17:54 UTC 2024

Playbook

- name: Modify broadcast domain
      gather_facts: false
      # Use local host for delegation
    connection: local
    hosts: all
    vars_prompt:
    - name: ontap_username
      prompt: Enter username for ontap_username
      private: false
    - name: ontap_password
      prompt: Enter password
      private: true
    vars:
        broadcast_domains:
          - name: cluster_mgmt
            mtu: 1500
            ports:
             - srva001ppa1:a0a-111
             - srva001ppa1:a1a-111
             - srva001ppa2:a0a-111
             - srva001ppa2:a1a-111
             - srva001ppa3:a0a-111
             - srva001ppa3:a1a-111
             - srva001ppa4:a0a-111
             - srva001ppa4:a1a-111
             - srva001ppa5:a0a-111
             - srva001ppa5:a1a-111
             - srva001ppa6:a0a-111
             - srva001ppa6:a1a-111
             - srva001ppa7:a0a-111
             - srva001ppa7:a1a-111
             - srva001ppa7:a2a-111
             - srva001ppa8:a0a-111
             - srva001ppa8:a1a-111
             - srva001ppa8:a2a-111

        tasks:
        - name: Create Broadcast Domains
          netapp.ontap.na_ontap_broadcast_domain:
            state: present
            name: "{{ item.name }}"
            mtu: "{{ item.mtu }}"
            ports: "{{ item.ports }}"
            ipspace: Default
            hostname: "{{ inventory_hostname }}"
            username: "{{ ontap_username }}"
            password: "{{ ontap_password }}"
            https: true
            validate_certs: false
          with_items: "{{ broadcast_domains }}"
          when: broadcast_domains != None

Steps to Reproduce

- name: Create Broadcast Domains
  netapp.ontap.na_ontap_broadcast_domain:
    state: present
    name: "{{ item.name }}"
    mtu: "{{ item.mtu }}"
    ports: "{{ item.ports }}"
    ipspace: Default
    hostname: "{{ inventory_hostname }}"
    username: "{{ ontap_username }}"
    password: "{{ ontap_password }}"
    https: true
    validate_certs: "{{ ontap_validate_certs }}"
  with_items: "{{ broadcast_domains }}"
  when: broadcast_domains != None

Expected Results

Expected that module will only add MISSING broadcast domain ports and not try and PATCH existing ports.

Actual Results

msg: 'calling: network/ethernet/ports/fbec63a4-af37-11eb-b786-00a098dc46e4: got {''message'': ''Port "###:a1a-111" cannot be used because it is currently the home port or current port of a LIF.'', ''code'': ''1377608''}.'

[ghost]

hi @hashi825,
Results shown above mentions about the error i.e. the port cannot be used because it is currently the home port or the current port of a LIF.
Can you please refer to the below KB and try running this module again?
https://kb.netapp.com/on-prem/ontap/OHW/OHW-KBs/Unable_to__add_a_port_to_broadcast_domain

[hashi825]

hi @hashi825, Results shown above mentions about the error i.e. the port cannot be used because it is currently the home port or the current port of a LIF. Can you please refer to the below KB and try running this module again? https://kb.netapp.com/on-prem/ontap/OHW/OHW-KBs/Unable_to__add_a_port_to_broadcast_domain

Hey @csahu1 thats not the issue, the module is attempting to remove existing ports in the broadcast domain instead of adding the missing ports. I've pointed the problem out in the code. This doesn't happen when using the module in ZAPI as the ZAPI call in this situation would add ports where as the REST function uses a PATCH request which would run on the existing ports in the list resulting in attempting to remove the ports.

[carchi8py]

@hashi825 sorry this fell through the cracks.

I'll have the team look at this and get back to you

I have DEVOPS-7088 open for this.

[csahu9]

@hashi825
As the module documentation mentions, one needs to pass the desired list of of ports to the module.
As the module na_ontap_broadcast_domain takes care of creation & deletion of broadcast domain along with modification of ports, it is designed to expect all the ports to be mentioned in input that need to exist inside the broadcast domain; it removes the ports that are not mentioned in the input list but are present in current config when state is present.
state: absent is used to manipulate the creation & deletion of the broadcast domain itself.

Could you confirm if this behaviour is not seen in your case? If not, could you share the steps to reproduce the issue?

[hashi825]

@hashi825 As the module documentation mentions, one needs to pass the desired list of of ports to the module. As the module na_ontap_broadcast_domain takes care of creation & deletion of broadcast domain along with modification of ports, it is designed to expect all the ports to be mentioned in input that need to exist inside the broadcast domain; it removes the ports that are not mentioned in the input list but are present in current config when state is present. state: absent is used to manipulate the creation & deletion of the broadcast domain itself.

Could you confirm if this behaviour is not seen in your case? If not, could you share the steps to reproduce the issue?

It's in the initial post and I've highlighted the code issue. The module is taking the existing list and performing a patch requests on ports that exist in the broadcast domain that are in the provided variable. They should not be touched as they are meant to exist when state present is specified, a patch request either removes or adds a port the operation should not be performed at all when the module encounters a port in the input list that already is a member of the requested broadcast domain

[csahu9]

@hashi825 The patch request that is referenced in your query calls API network/ethernet/ports/{port.uuid}.
This PATCH request is called only for the ports that need to be added to the domain; not for the existing ports.
For example, if the broadcast domain has e0a port and if the user passes e0a, e0b as the desired list of ports then this API call would be called only for e0b.

[hashi825]

@hashi825 The patch request that is referenced in your query calls API network/ethernet/ports/{port.uuid}. This PATCH request is called only for the ports that need to be added to the domain; not for the existing ports. For example, if the broadcast domain has e0a port and if the user passes e0a, e0b as the desired list of ports then this API call would be called only for e0b.

Actually looks like I found where the issue lies, this function here returns the ports for the add operation on the broadcast domain

https://github.com/ansible-collections/netapp.ontap/blob/78486a16018f1c3f1d8cf133f6f4b0b39ba4bcb9/plugins/modules/na_ontap_broadcast_domain.py#L586C1-L589C62

The ports_to_add_move_from_desired function is splitting the port name, this then matches on

if port_name == port_to_add_or_move['name']

which causes existing ports to be added.

https://github.com/ansible-collections/netapp.ontap/blob/78486a16018f1c3f1d8cf133f6f4b0b39ba4bcb9/plugins/modules/na_ontap_broadcast_domain.py#L625C1-L632C33

The reason this happening looks like due to the port names, splitting on port name which removes the node reference is a very poor way of figuring out which ports need to be modified in a broadcast domain because ports across nodes are usually the same especially when using interface groups.

[csahu9]

We've applied a fix for this; it would mostly be a part of the next release.