Refactor(eos_designs)!: Raise AVD error if sFlow is enabled but `sflo…

…w_settings.destinations` is not configured (aristanetworks#4402)
satishd007 · Sep 4, 2024 · 2096a31 · 2096a31
1 parent 9113eb9
commit 2096a31
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 5 deletions.
diff --git a/ansible_collections/arista/avd/docs/porting-guides/5.x.x.md b/ansible_collections/arista/avd/docs/porting-guides/5.x.x.md
@@ -327,6 +327,12 @@ By default an MLAG ID will only be configured on Port-Channel downlinks dual-hom
 + mlag_on_orphan_port_channel_downlink: true
 ```
 
+### An error is raised if sFlow is enabled on an interface but `sflow_settings.destinations` is not set
+
+In AVD 4.0.0, if `sflow` was enabled on an interface but `sflow_settings.destinations` was not defined, the sFlow configuration was still generated for interfaces, although it would not work without "custom structured configuration".
+
+As of AVD 5.0.0, if `sflow` is enabled on an interface but `sflow_settings.destinations` is not set, an error is raised and the execution stops.
+
 ### Default value of `isis_system_id_format` is changed to `underlay_loopback`
 
 In AVD 4.0.0, `node_id` was the default value of `isis_system_id_format`.

diff --git a/...s_designs_negative_unit_tests/inventory/host_vars/sflow-settings-missing-destinations.yml b/...s_designs_negative_unit_tests/inventory/host_vars/sflow-settings-missing-destinations.yml
@@ -0,0 +1,15 @@
+type: l2leaf
+l2leaf:
+  nodes:
+    - name: sflow-settings-missing-destinations
+      id: 1
+
+servers:
+  - name: test
+    adapters:
+      - switches: [sflow-settings-missing-destinations]
+        switch_ports: [Ethernet1]
+        sflow: true
+
+expected_error_message: >-
+  `sflow_settings.destinations` is required to configure `sflow`.
diff --git a/ansible_collections/arista/avd/molecule/eos_designs_negative_unit_tests/inventory/hosts.yml b/ansible_collections/arista/avd/molecule/eos_designs_negative_unit_tests/inventory/hosts.yml
@@ -121,6 +121,7 @@ all:
             source-interfaces-tacacs-missing-inband-mgmt-interface:
             source-interfaces-tacacs-missing-mgmt-ip:
             ul-filter-evpn-default-vrf-services:
+            sflow-settings-missing-destinations:
             sflow-settings-missing-inband-mgmt-interface:
             sflow-settings-missing-mgmt-ip:
             core-interfaces-p2p-links-as:

diff --git a/...llections/arista/avd/roles/eos_designs/docs/tables/management-sflow-settings.md b/...llections/arista/avd/roles/eos_designs/docs/tables/management-sflow-settings.md
diff --git a/python-avd/pyavd/_eos_designs/schema/eos_designs.schema.yml b/python-avd/pyavd/_eos_designs/schema/eos_designs.schema.yml
diff --git a/python-avd/pyavd/_eos_designs/schema/schema_fragments/sflow_settings.schema.yml b/python-avd/pyavd/_eos_designs/schema/schema_fragments/sflow_settings.schema.yml
@@ -29,6 +29,8 @@ keys:
               A rate of 16384 corresponds to an average sample of one per 16384 packets.
       destinations:
         type: list
+        required: true
+        min_length: 1
         items:
           type: dict
           keys:

diff --git a/python-avd/pyavd/_eos_designs/structured_config/flows/__init__.py b/python-avd/pyavd/_eos_designs/structured_config/flows/__init__.py
@@ -37,9 +37,8 @@ def sflow(self) -> dict | None:
 
         destinations = get(self._hostvars, "sflow_settings.destinations")
         if destinations is None:
-            # TODO: AVD5.0.0 raise an error if sflow is enabled on an interface but there are no destinations configured.
-            # This cannot be implemented today since it would be breaking for already released support for sflow on interfaces.
-            return None
+            msg = "`sflow_settings.destinations` is required to configure `sflow`."
+            raise AristaAvdMissingVariableError(msg)
 
         sflow_settings_vrfs = get(self._hostvars, "sflow_settings.vrfs", default=[])