Feat(cv_device_v3): Dotted hostname support #680

sugetha24 · 2023-10-24T19:30:46Z

Change Summary

Adding support for hostname with dots(s1-leaf.1.aristanetworks.com) to cv_device_v3 module.

Related Issue(s)

Fixes #654

Component(s) name

arista.cvp.cv_device_v3

Proposed changes

Previously, get_device_by_name() cvprac function was used to get info on a device.
This was called for each device. Now, get_inventory() is called once to get the entire CVP inventory and cache it.
get_inventory() reports all information, especially fqdn and hostname as opposed to get_device_by_name() only returning fqdn.
The module no longer derivese hostname from fqdn field. If the module is not provided hostname as input, then we get it from the inventory.
The schema now supports hostname knob.

How to test

NOTE: This PR is dependent on some changes to get_inventory() function on cvprac. While we wait for that change to merge into cvprac, please make sure to comment line #600-626 in the latest cvprac: https://github.com/aristanetworks/cvprac/blob/b5306b22770848a32d5927620e804bd1482932c1/cvprac/cvp_api.py#L600
and run the following playbook

- name: Playbook to demonstrate cvp modules.
  hosts: CVP
  connection: local
  gather_facts: no
  vars:
    # Container definition
    cvp_devices:
      - hostname: s1-leaf.1
      # - fqdn: s1-leaf.1.aristanetworks.com
        parentContainerName: SITE1_LEAFS
        configlets:
          - Add-VLAN-50
  tasks:
    - name: "Configure devices on {{inventory_hostname}}"
      arista.cvp.cv_device_v3:
        devices: '{{cvp_devices}}'
        state: present
        # search_key: fqdn
        search_key: hostname
      register: CVP_DEVICES_RESULTS
    - name: debug
      debug:
        msg: "{{CVP_DEVICES_RESULTS}}"

Use search_key knob to specify if you want to serach the inventory by hostname or fqdn.
Note: if search_key: hostname, then hostname should be specified in cvp_device variable.
Similarly, if search_key: fqdn, then fqdn should be specified in cvp_device variable

Checklist

User Checklist

N/A

Repository Checklist

My code has been rebased from devel before I start
I have read the CONTRIBUTING document.
My change requires a change to the documentation and documentation have been updated accordingly. (check the box if not applicable)
I have updated molecule CI testing accordingly. (check the box if not applicable)

ClausHolbechArista

I am worried about performance on a scaled setup. If you have many devices (hundreds or more), but only run an operation for a single device, you end up parsing the full inventory many times across the code.

ClausHolbechArista · 2023-10-25T04:55:44Z

ansible_collections/arista/cvp/plugins/module_utils/device_tools.py

        if inventory_mode == ModuleOptionValues.INVENTORY_MODE_LOOSE:
            # Remove missing devices on CV from inventory (ignore missing)
            user_inventory = self.__remove_missing_devices(
                user_inventory=user_inventory
            )
        else:
            # Check if all devices are present on CV (fail on missing)
-            self.__check_devices_exist(user_inventory=user_inventory)
+            self.__check_devices_exist(cvp_inventory, user_inventory=user_inventory)


Instead of getting cvp_inventory in every function, just to pass it along to another method of self, you could just let those functions look up the inventory directly in self.cvp_inventory().

Good idea. I will make that change.

ansible_collections/arista/cvp/plugins/module_utils/device_tools.py

ClausHolbechArista · 2023-10-25T05:01:33Z

ansible_collections/arista/cvp/plugins/module_utils/device_tools.py

+        MODULE_LOGGER.warning("Update list is: %s", str(user_result))
+        return DeviceInventory(data=user_result)
+
+    def refresh_hostname(self, cvp_inventory: list, user_inventory: DeviceInventory):


Why do you still need all these "refresh_xxx" when you have all the information already? It would be more efficient to have one function (__refresh_user_inventory) that updates all missing info on user_inventory from cvp_inventory instead of looping through the entire inventory several times.

Makes sense. Will make that change.

ClausHolbechArista · 2023-10-25T05:02:22Z

ansible_collections/arista/cvp/plugins/module_utils/device_tools.py

+            Data from CloudVision
+        """
+        return self.__cv_client.api.get_inventory(start=0, end=0)
+
    # Updated as per issue #365 to set default search with hostname field
    @lru_cache
    def __get_device(self, search_value: str, search_by: str = Api.device.HOSTNAME):


Is this function still needed?

Yes. It is being called by a few other functions.

ClausHolbechArista · 2023-10-25T05:04:13Z

ansible_collections/arista/cvp/plugins/module_utils/resources/schemas/v3.py

@@ -128,6 +134,16 @@
                            "CV-ANSIBLE-EOS01"
                        ]
                    },
+                    "hostname": {
+                        "$id": "#/items/anyOf/0/properties/fqdn",


Suggested change

"$id": "#/items/anyOf/0/properties/fqdn",

"$id": "#/items/anyOf/0/properties/hostname",

ClausHolbechArista · 2023-10-25T05:05:00Z

ansible_collections/arista/cvp/plugins/module_utils/resources/schemas/v3.py

+                        "$id": "#/items/anyOf/0/properties/fqdn",
+                        "type": "string",
+                        "title": "The hostname schema",
+                        "description": "An explanation about the purpose of this instance.",


If we have no good description, just remove the field.

github-actions · 2024-01-31T00:22:32Z

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 15 days

sonarcloud · 2024-05-21T17:47:25Z

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

sugetha24 · 2024-07-16T18:06:33Z

Not fixing!

sugetha24 requested review from carlbuchmann, ClausHolbechArista and noredistribution October 24, 2023 19:30

github-actions bot requested a review from guillaumeVilar October 24, 2023 19:31

github-actions bot added module: cv_device_v3 Issue related to cv_device module in v3 module_utils: cv_client cv_client implementation issue labels Oct 24, 2023

github-actions bot assigned sugetha24 Oct 24, 2023

ClausHolbechArista reviewed Oct 25, 2023

View reviewed changes

sugetha24 marked this pull request as draft November 1, 2023 18:33

github-actions bot added the state: stale Staled issues waiting for closure label Jan 31, 2024

noredistribution removed the state: stale Staled issues waiting for closure label Jan 31, 2024

sugetha24 added 2 commits February 27, 2024 15:05

Feat(cv_device_v3): Dotted hostname support

d1438d3

pep8 fix

a68b70a

sugetha24 force-pushed the dotted-hostname branch from d85b764 to a68b70a Compare February 27, 2024 23:05

sugetha24 closed this Jul 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat(cv_device_v3): Dotted hostname support #680

Feat(cv_device_v3): Dotted hostname support #680

sugetha24 commented Oct 24, 2023 •

edited

Loading

ClausHolbechArista left a comment

ClausHolbechArista Oct 25, 2023

sugetha24 Oct 25, 2023

ClausHolbechArista Oct 25, 2023

sugetha24 Oct 25, 2023

ClausHolbechArista Oct 25, 2023

sugetha24 Oct 25, 2023

ClausHolbechArista Oct 25, 2023

ClausHolbechArista Oct 25, 2023

github-actions bot commented Jan 31, 2024

sonarcloud bot commented May 21, 2024

sugetha24 commented Jul 16, 2024

	"$id": "#/items/anyOf/0/properties/fqdn",
	"$id": "#/items/anyOf/0/properties/hostname",

Feat(cv_device_v3): Dotted hostname support #680

Feat(cv_device_v3): Dotted hostname support #680

Conversation

sugetha24 commented Oct 24, 2023 • edited Loading

Change Summary

Related Issue(s)

Component(s) name

Proposed changes

How to test

Checklist

User Checklist

Repository Checklist

ClausHolbechArista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 31, 2024

sonarcloud bot commented May 21, 2024

Quality Gate passed

sugetha24 commented Jul 16, 2024

sugetha24 commented Oct 24, 2023 •

edited

Loading