Multiple device Configlets not correctly validated when they depend on each other

[gusmb]

Issue Summary

cv_validate_v3 does not validate configlets for a device as ONE config, but each configlet individually. This causes false positives when a configlet contains dependencies to other configlets that are applied to the device before in the list.

Which component(s) of AVD impacted

other

How do you run AVD ?

Ansible CLI (with virtual-env or native python)

Input variables

---
CVP_DEVICES:
  - device_name: XXXXXXXXX
    search_type: serialNumber
    local_configlets:
      device1.cfg: "main configlet contents omitted"
      device1-extra.cfg: “custom configlet contents omitted"

  - device_name: YYYYYYYYY
    search_type: serialNumber
    local_configlets:
      device2.cfg: "main configlet contents omitted"
      device2-extra.cfg: “custom configlet contents omitted"

Steps to reproduce

1. Create a device configlet that configures a VRF instance
2. Create a second configlet that configures a static route on that VRF
3. Apply both configlets on CVP for that device in the right order
4. Run `cv_validate_v3` role with the vars above. It will fail validation for the "extra" config, reporting that the VRF instance does not exist.

I would expect the validation of config to be done per-device, not per-configlet, applying all relevant device-level configlets in the right order, as it would be done when validating the config on CVP. Otherwise, the role is just limited to test a valid configlet that contains configuration which don't have any dependencies on other configlets.

Relevant log output

fatal: [cvp1]: FAILED! => changed=false
  msg:
  - configlet_name: device1-extra.cfg
    device: XXXXXXX
    errors:
    - error: '> ipv6 unicast-routing vrf TEST% No such VRF TEST. at line 8'
      lineNo: ' 8'
  - configlet_name: device2-extra.cfg
    device: YYYYYYY
    errors:
    - error: '> ipv6 unicast-routing vrf TEST% No such VRF TEST. at line 8'
      lineNo: ' 8'

vrf instance TEST was defined on device1.cfg and device2.cfg respectively

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[gusmb]

I see that the whole config validation is based on an iterative process, looping through the list of configlets (as dictionary items, probably ordering here is not guaranteed), and running this cvprac API call:

https://github.com/aristanetworks/ansible-cvp/blob/devel/ansible_collections/arista/cvp/plugins/module_utils/validate_tools.py#L185C13-L185C62

resp = self.__cv_client.api.validate_config_for_device(
                        device_mac=system_mac,
                        config=config)

That is OK if the configlet has the full device config or at least config without dependencies, but since it is validated as a standalone configlet, it might fail in certain cases. IMO the solution here would be to combine the config of all device configlets into a single config text in the right order, and then do a single device-level validation, instead of validating each configlet individually

[noredistribution]

Thanks for filing the issue! The main uses case for this module was to use it in tangent with arista-avd in CI pipelines, where you'd have one true single configlet (with full target config) generated by AVD per device and you'd validate the multiline string before the configlet was even uploaded to CloudVision, thus saving a lot of time at scale compared to previously where if there was an error, the user would only find out after the configlet was uploaded and then assigned to the device. This module validates each configlet individually just as the UI does on the Configlets page, where you can validate an individual configlet and doesn't use the validateAndCompareConfiglets.do API that is used on "Manage->Configlets->Validate" page in Network Provisioning

With that said, we could take a look at what would it mean to add a knob to concatenate all the strings, ideally users should create configlets that are independent of each other as that may cause issues, especially if they are not ordered correctly

[gusmb]

Thanks for filing the issue! The main uses case for this module was to use it in tangent with arista-avd in CI pipelines, where you'd have one true single configlet (with full target config) generated by AVD per device and you'd validate the multiline string before the configlet was even uploaded to CloudVision, thus saving a lot of time at scale compared to previously where if there was an error, the user would only find out after the configlet was uploaded and then assigned to the device. This module validates each configlet individually just as the UI does on the Configlets page, where you can validate an individual configlet and doesn't use the validateAndCompareConfiglets.do API that is used on "Manage->Configlets->Validate" page in Network Provisioning

With that said, we could take a look at what would it mean to add a knob to concatenate all the strings, ideally users should create configlets that are independent of each other as that may cause issues, especially if they are not ordered correctly

Thanks for the explanation. I think limiting the use case is confusing. On one side the data structure exposed to the user allows specifying different configlets, but on the other side you only expect a single configlet. I run CI pipelines powered by AVD, but it is typical to have additional configlets mapped to devices for certain use cases, like add-on user temp configs, or a dedicated configlet for another service that runs on top, etc. Usually as users we are interested in the overall config for the device, rather than an individual configlet. I have solved this locally for now by concatenating the config strings on input, but I think this would be better to handle within the module.

To honor the configlet order, I would suggest updating the data structure to a list of dicts instead of a dict of dicts. And probably interesting to expand the module functionality to do device level validation (taking into account all configlets in the order provided), or individual configlet validation. I would think device level should be the default as this is usually what customers are looking for when running manual operations on CVP

[noredistribution]

Thanks for the feedback! Definitely it's better to handle it in the module, we'll have to see if this can be done in a non-breaking manner for customers who are using this already, in which case we could do it in one of the next feature releases, otherwise it would be only in the next major release(4.0.0)

[github-actions[bot]]

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

[gusmb]

Has there been any progress on this? Issue is still relevant and should not be closed automatically.

[ClausHolbechArista]

We do not plan to address this in arista.cvp. Your workaround with concatenating the configlets before running the module will have to hold a bit longer, until you can move to the new arista.avd.deploy_to_cv role.