ovirt_nic: Add network_filter_parameters

[mnecas]

Fixes: #622

[erav]

@mnecas Have you run a test that shows that the code works?

[Gounick]

I just tested your commit. This doesn't work immediately:

• On the first execution of the ovirt_nic module, it creates the NIC without the network_filter_parameters parameters.
• On the second run, it correctly adds the network_filter_parameters.

And the module is not idempotent, I don't know if it was already or not.

Here is my playbook

---
- name: oVirt - Ensure admin-prod NIC to all VMs on the cluster !
  hosts: all
  connection: local
  gather_facts: false
  become: false
  vars:
    vm: myvm
    cluster: mycluster
    mandatory_nics:
      - name: admin-prod
        interface: virtio
        linked: true
        network_filter_parameters:
          - name: GATEWAY_MAC
            value: 01:02:03:ab:cd:ef
          - name: GATEWAY_MAC
            value: 01:02:03:ab:cd:eg
          - name: GATEWAY_MAC
            value: 01:02:03:ab:cd:eh
        network: admin-prod
        profile: admin-prod
        state: plugged

  pre_tasks:
    - name: Login to oVirt
      ovirt.ovirt.ovirt_auth:
        hostname: "{{ engine_fqdn }}"
        username: "{{ engine_user }}"
        password: "{{ engine_password }}"
        ca_file: "{{ engine_cafile }}"
        insecure: "{{ engine_insecure | default(true) }}"
      check_mode: false
      tags:
        - always

  tasks:
    - name: "Gather information about {{ vm }} VM on {{ cluster }} cluster"
      ovirt.ovirt.ovirt_vm_info:
        auth: "{{ ovirt_auth }}"
        pattern: "name={{ vm }} and cluster={{ cluster }}"
        follows:
          - nics.network_filter_parameters
      register: result_ovirt_vm_info
      check_mode: false

    - name: Debug result_ovirt_vm_info var
      ansible.builtin.debug:
        var: result_ovirt_vm_info

    - name: "Ensure the mandatory NICs exist"
      ovirt.ovirt.ovirt_nic:
        auth: "{{ ovirt_auth }}"
        fetch_nested: "{{ mandatory.1.fetch_nested | d(true) }}"
        nested_attributes:
          - nic.network_filter_parameters
        interface: "{{ mandatory.1.interface | d('virtio') }}"
        linked: "{{ mandatory.1.linked | d(true) }}"
        name: "{{ mandatory.1.name | mandatory }}"
        network_filter_parameters: "{{ mandatory.1.network_filter_parameters | d(omit) }}"
        network: "{{ mandatory.1.network | d(omit) }}"
        profile: "{{ mandatory.1.profile | d(omit) }}"
        state: "{{ mandatory.1.state | d(omit) }}"
        vm: "{{ mandatory.0.name | d(omit) }}"
      register: result_mandatory_nic_ovirt_nic
      loop: "{{ result_ovirt_vm_info.ovirt_vms
                | product(mandatory_nics | d([]))
                | list }}"
      loop_control:
        loop_var: mandatory
      when:
        - result_ovirt_vm_info.ovirt_vms is defined and
          result_ovirt_vm_info.ovirt_vms | length > 0
        - mandatory_nics is defined and
          mandatory_nics | length > 0

    - name: Debug result_mandatory_nic_ovirt_nic var
      ansible.builtin.debug:
        var: result_mandatory_nic_ovirt_nic.results

    - name: "Gather information about all NICs  for {{ vm }} VM"
      ovirt.ovirt.ovirt_nic_info:
        auth: "{{ ovirt_auth }}"
        vm: "{{ mandatory.0.name | d(omit) }}"
        name: "{{ mandatory.1.name | mandatory }}"
        follow:
          - network_filter_parameters
      loop: "{{ result_ovirt_vm_info.ovirt_vms
                | product(mandatory_nics | d([]))
                | list }}"
      loop_control:
        loop_var: mandatory
      register: result_mandatory_ovirt_nic_info

    - name: Debug result_mandatory_ovirt_nic_info var
      ansible.builtin.debug:
        var: result_mandatory_ovirt_nic_info


  post_tasks:
    - name: Logout from oVirt
      ovirt.ovirt.ovirt_auth:
        state: absent
        ovirt_auth: "{{ ovirt_auth }}"
      check_mode: false
      tags:
        - always

[mnecas]

@Gounick ah thanks for that! try now

[erav]

I would test

• add some filters
• add one filter
• add zero filters
  Just to make sure we covered all the corner cases. And also check engine log to make sure there are no exceptions during the execution.
  Do we support remove filters? Maybe we should. IIUC adding zero filters does not remove existing ones.

[Gounick]

It works on a single run ! :)
On the other hand, it is not idempotent, the ID and the other parameters associated with network_filter_parameters are renewed.

[erav]

It works on a single run ! :) On the other hand, it is not idempotent, the ID and the other parameters associated with network_filter_parameters are renewed.

Since this is an 'add' and not an 'update' IMO it is ok that the ID etc. are renewed. According to the code in this patch, the add first removes the old content and then applies the content in the request.
@mnecas do our roles conventionaly support a separate 'update'? Should we support it here separately or as part of the 'add' or not at all?

[mnecas]

well it is kind of idempotent. You get exactly what you specify and if you run it multiple times it's still the same result. The code is a bit hacky that we remove and add it but it's consistent across all our modules with these specific args.

[erav]

@mnecas so if the tests pass from your point of view I will +1

[mnecas]

I had no problems when testing it and happy that @Gounick was able to also test it so quickly

[Gounick]

In the log file /var/log/ovirt-engine/engine.log, there are only INFO messages.

To remove the filters, I tested an empty network_filter_parameters:[] list but that does not purge the filters on oVirt.

[mnecas]

Fixes

In the log file /var/log/ovirt-engine/engine.log, there are only INFO messages.

To remove the filters, I tested an empty network_filter_parameters:[] list but that does not purge the filters on oVirt.

Thanks! Fixed

[Gounick]

Fixes

In the log file /var/log/ovirt-engine/engine.log, there are only INFO messages.
To remove the filters, I tested an empty network_filter_parameters:[] list but that does not purge the filters on oVirt.

Thanks! Fixed

Perfect, an empty list allows to purge the filters!

[Gounick]

it works

[mwperina]

+1