dcnm_fabric: Fix evaluation of controller features (#362)

* Fix determination of controller features

1. Fix dictionary access in Merged.get_need() and Replaced.get_need()

In the above methods, we were trying to retrieve the key "fabric_type" from the dict  self.features. But this dict is keyed on the fabric type (e.g. keys in this dict are "ISN", "VXLAN_EVPN", etc).  The fix is to remove the quotes around "fabric_type" -> fabric_type, which is a var containing the fabric type ("ISN", "VXLAN_EVPN", etc).

2. Add backward-compatible feature verification for ND 4.0.

The NDFC versions (as returned by ControllerVersion) for  ND 3.2.1e and ND 4.0 respectively are:

12.2.2.238 -> ND 3.2.1e
12.3.1.248 -> ND 4.0

The unified architecture for ND 4.0 means that it no longer includes information about controller features relevant to fabrics (e.g. that "vxlan" feature must be enabled for VXLAN_EVPN fabric creation/modification, and "pmn" feature must be enabled for IPFM fabric creation/modification, etc).  Hence, this check should be  run only for NDFC minor versions less than 3.

Added call to ControllerVersion() and updated the check in get_need() such that we skip the check if ControllerVersion().version_minor < 3.

3. Updated docstrings in controller_version.py

Updated accessor property docstrings in ControllerVersion() to explicitely mention that a string is returned for version_major, version_minor, and version_patch.

* Centralize function to distinguish ND 4.0 from ND 3.x

We will probably need a way to tell ND 4.0 from ND 3.x in other modules at some point.

1. Add a property to ControllerVersion() that returns True if ND 4.x and False otherwise.

2. Leverage this property in dcnm_fabric.py Merged.get_need() and Replaced.get_need()

* Add unit tests for ControllerVersion().is_controller_version_4x

Add two unit tests to verify correct values are returned for ND 3.x and ND 4.x NDFC versions.

plugins/module_utils/common/controller_version.py

@@ -229,14 +229,14 @@ def version(self):
     @property
     def version_major(self):
         """
-        Return the controller major version, if it exists.
+        Return the controller major version as a string, if it exists.
         Return None otherwise
 
         We are assuming semantic versioning based on:
         https://semver.org
 
         Possible values:
-            if version is 12.1.2e, return 12
+            if version is 12.1.2e, return "12"
             None
         """
         if self.version is None:
@@ -246,7 +246,7 @@ def version_major(self):
     @property
     def version_minor(self):
         """
-        Return the controller minor version, if it exists.
+        Return the controller minor version as a string, if it exists.
         Return None otherwise
 
         We are assuming semantic versioning based on:
@@ -263,7 +263,7 @@ def version_minor(self):
     @property
     def version_patch(self):
         """
-        Return the controller minor version, if it exists.
+        Return the controller patch version as a string, if it exists.
         Return None otherwise
 
         We are assuming semantic versioning based on:
@@ -276,3 +276,26 @@ def version_patch(self):
         if self.version is None:
             return None
         return (self._get("version").split("."))[2]
+
+    @property
+    def is_controller_version_4x(self) -> bool:
+        """
+        ### Summary
+
+        -   Return True if the controller version implies ND 4.0 or higher.
+        -   Return False otherwise.
+        """
+        method_name = inspect.stack()[0][3]
+
+        result = None
+        if int(self.version_major) == 12 and int(self.version_minor) < 3:
+            result = False
+        else:
+            result = True
+
+        msg = f"{self.class_name}.{method_name}: "
+        msg = f"self.version: {self.version}, "
+        msg += f"Controller is version 4.x: {result}"
+        self.log.debug(msg)
+
+        return result

plugins/modules/dcnm_fabric.py

@@ -2671,7 +2671,9 @@
 import logging
 
 from ansible.module_utils.basic import AnsibleModule
+
 from ..module_utils.common.controller_features import ControllerFeatures
+from ..module_utils.common.controller_version import ControllerVersion
 from ..module_utils.common.exceptions import ControllerResponseError
 from ..module_utils.common.log_v2 import Log
 from ..module_utils.common.properties import Properties
@@ -2712,6 +2714,7 @@ def __init__(self, params):
         method_name = inspect.stack()[0][3]  # pylint: disable=unused-variable
 
         self.controller_features = ControllerFeatures()
+        self.controller_version = ControllerVersion()
         self.features = {}
         self._implemented_states = set()
 
@@ -2898,6 +2901,27 @@ def get_controller_features(self) -> None:
             self.controller_features.filter = self.fabric_types.feature_name
             self.features[fabric_type] = self.controller_features.started
 
+    def get_controller_version(self):
+        """
+        ### Summary
+        Initialize and refresh self.controller_version.
+
+        ### Raises
+
+        -   ``ValueError`` if the controller returns an error when attempting
+            to retrieve the controller version.
+        """
+        method_name = inspect.stack()[0][3]
+        try:
+            self.controller_version.rest_send = self.rest_send
+            self.controller_version.refresh()
+        except (ControllerResponseError, ValueError) as error:
+            msg = f"{self.class_name}.{method_name}: "
+            msg += "Controller returned error when attempting to retrieve "
+            msg += "controller version. "
+            msg += f"Error detail: {error}"
+            raise ValueError(msg) from error
+
 
 class Deleted(Common):
     """
@@ -3032,7 +3056,19 @@ def get_need(self):
             fabric_name = want.get("FABRIC_NAME", None)
             fabric_type = want.get("FABRIC_TYPE", None)
 
-            if self.features.get("fabric_type") is False:
+            msg = f"{self.class_name}.{method_name}: "
+            msg += f"self.features: {self.features}"
+            self.log.debug(msg)
+
+            is_4x = self.controller_version.is_controller_version_4x
+
+            msg = f"{self.class_name}.{method_name}: "
+            msg += f"fabric_type: {fabric_type}, "
+            msg += f"configurable: {self.features.get(fabric_type)}, "
+            msg += f"is_4x: {is_4x}"
+            self.log.debug(msg)
+
+            if self.features.get(fabric_type) is False and is_4x is False:
                 msg = f"{self.class_name}.{method_name}: "
                 msg += f"Features required for fabric {fabric_name} "
                 msg += f"of type {fabric_type} are not running on the "
@@ -3140,6 +3176,8 @@ def commit(self):
         msg = f"{self.class_name}.{method_name}: entered"
         self.log.debug(msg)
 
+        self.get_controller_version()
+
         self.fabric_details.rest_send = self.rest_send
         self.fabric_summary.rest_send = self.rest_send
 
@@ -3361,7 +3399,15 @@ def get_need(self):
                 self.need_create.append(want)
                 continue
 
-            if self.features.get("fabric_type") is False:
+            is_4x = self.controller_version.is_controller_version_4x
+
+            msg = f"{self.class_name}.{method_name}: "
+            msg += f"fabric_type: {fabric_type}, "
+            msg += f"configurable: {self.features.get(fabric_type)}, "
+            msg += f"is_4x: {is_4x}"
+            self.log.debug(msg)
+
+            if self.features.get(fabric_type) is False and is_4x is False:
                 msg = f"{self.class_name}.{method_name}: "
                 msg += f"Features required for fabric {fabric_name} "
                 msg += f"of type {fabric_type} are not running on the "
@@ -3387,6 +3433,8 @@ def commit(self):
         msg = f"{self.class_name}.{method_name}: entered"
         self.log.debug(msg)
 
+        self.get_controller_version()
+
         self.fabric_details.rest_send = self.rest_send
         self.fabric_summary.rest_send = self.rest_send
 

tests/unit/module_utils/common/fixtures/responses_ep_version.json

@@ -449,5 +449,43 @@
             "uuid": "",
             "is_upgrade_inprogress": "false"
         }
+    },
+    "test_controller_version_00300a": {
+        "TEST_NOTES": [
+            "12.2.2.238 implies ND 3.x"
+        ],
+        "RETURN_CODE": 200,
+        "METHOD": "GET",
+        "REQUEST_PATH": "https://172.22.150.244:443/appcenter/cisco/ndfc/api/v1/fm/about/version",
+        "MESSAGE": "OK",
+        "DATA": {
+            "version": "12.2.2.238",
+            "mode": "LAN",
+            "isMediaController": "false",
+            "dev": "false",
+            "isHaEnabled": "false",
+            "install": "EASYFABRIC",
+            "uuid": "",
+            "is_upgrade_inprogress": "false"
+        }
+    },
+    "test_controller_version_00300b": {
+        "TEST_NOTES": [
+            "12.3.1.248 implies ND 4.x"
+        ],
+        "RETURN_CODE": 200,
+        "METHOD": "GET",
+        "REQUEST_PATH": "https://172.22.150.244:443/appcenter/cisco/ndfc/api/v1/fm/about/version",
+        "MESSAGE": "OK",
+        "DATA": {
+            "version": "12.3.1.248",
+            "mode": "LAN",
+            "isMediaController": "false",
+            "dev": "false",
+            "isHaEnabled": "false",
+            "install": "EASYFABRIC",
+            "uuid": "",
+            "is_upgrade_inprogress": "false"
+        }
     }
 }

tests/unit/module_utils/common/test_controller_version.py

@@ -827,3 +827,54 @@ def responses():
         instance.rest_send = rest_send
         instance.refresh()
     assert instance.version_patch == expected
+
+
+@pytest.mark.parametrize(
+    "key, expected",
+    [
+        ("test_controller_version_00300a", False),
+        ("test_controller_version_00300b", True),
+    ],
+)
+def test_controller_version_00300(controller_version, key, expected) -> None:
+    """
+    ### Classes and Methods
+
+    -   ``ControllerVersion()``
+            -   ``refresh``
+            -   ``is_controller_version_4x``
+
+    ### Test
+
+    - is_controller_version_4x returns expected values
+
+    ### Description
+
+    ``is_controller_version_4x`` returns:
+
+    - True, if int(self.version_major) == 12 and int(self.version_minor) >= 3.
+    - False, if int(self.version_major) == 12 and int(self.version_minor) < 3.
+
+    ### Expected result
+
+    1. test_controller_version_00300a is True
+    2. test_controller_version_00300b is False
+    """
+
+    def responses():
+        yield responses_ep_version(key)
+
+    gen_responses = ResponseGenerator(responses())
+
+    sender = Sender()
+    sender.ansible_module = MockAnsibleModule()
+    sender.gen = gen_responses
+    rest_send = RestSend(params)
+    rest_send.response_handler = ResponseHandler()
+    rest_send.sender = sender
+
+    with does_not_raise():
+        instance = controller_version
+        instance.rest_send = rest_send
+        instance.refresh()
+    assert instance.is_controller_version_4x == expected