Repository: ansible/ansible
Pull request: add new command SetManagerNic in redfish_config module
##### SUMMARY
add SetManagerNic in redfish_config module to set Manager NIC configuration.

Fixes #64475 

##### ISSUE TYPE
- Feature Pull Request

##### COMPONENT NAME
redfish_config

##### ADDITIONAL INFORMATION
none

Problem statement:
redfish_config - Manager - SetManagerNic
##### SUMMARY
This feature would implement a SetManagerNic command for the Manager category of redfish_config, to update Manager NIC configuration. 

##### ISSUE TYPE
- Feature Idea

##### COMPONENT NAME
redfish_config

##### ADDITIONAL INFORMATION
This command would help user to configure their Manager NIC.

Hints from issue discussion:
Files identified in the description:
* [`lib/ansible/modules/remote_management/redfish/redfish_config.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/remote_management/redfish/redfish_config.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @billdodd @jose-delarosa @mraineri @tomasg2012
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->

Commit message: add new command SetManagerNic in redfish_config module

Patch to review:
diff --git a/lib/ansible/module_utils/redfish_utils.py b/lib/ansible/module_utils/redfish_utils.py
index 97bbf30d7defa1..a3bcb9284acf09 100644
--- a/lib/ansible/module_utils/redfish_utils.py
+++ b/lib/ansible/module_utils/redfish_utils.py
@@ -2263,3 +2263,91 @@ def get_manager_health_report(self, manager_uri):
 
     def get_multi_manager_health_report(self):
         return self.aggregate_managers(self.get_manager_health_report)
+
+    def set_manager_nic(self, nic_addr, nic_config):
+        # Get EthernetInterface collection
+        response = self.get_request(self.root_uri + self.manager_uri)
+        if response['ret'] is False:
+            return response
+        data = response['data']
+        if 'EthernetInterfaces' not in data:
+            return {'ret': False, 'msg': "EthernetInterfaces resource not found"}
+        ethernetinterfaces_uri = data["EthernetInterfaces"]["@odata.id"]
+        response = self.get_request(self.root_uri + ethernetinterfaces_uri)
+        if response['ret'] is False:
+            return response
+        data = response['data']
+        uris = [a.get('@odata.id') for a in data.get('Members', []) if
+                a.get('@odata.id')]
+
+        # Find target EthernetInterface
+        target_ethernet_uri = None
+        target_ethernet_current_setting = None
+        if nic_addr == 'null':
+            # Find root_uri matched EthernetInterface when nic_addr is not specified
+            nic_addr = (self.root_uri).split('/')[-1]
+        for uri in uris:
+            response = self.get_request(self.root_uri + uri)
+            if response['ret'] is False:
+                return response
+            data = response['data']
+            if nic_addr in str(data):
+                target_ethernet_uri = uri
+                target_ethernet_current_setting = data
+                break
+        if target_ethernet_uri is None:
+            return {'ret': False, 'msg': "No matched EthernetInterface found under Manager"}
+
+        # Convert input to payload and check validity
+        payload = {}
+        for property in nic_config.keys():
+            value = nic_config[property]
+            if property not in target_ethernet_current_setting:
+                return {'ret': False, 'msg': "Property %s in nic_config is invalid" % property}
+            if isinstance(value, dict):
+                if isinstance(target_ethernet_current_setting[property], dict):
+                    payload[property] = value
+                elif isinstance(target_ethernet_current_setting[property], list):
+                    payload[property] = list()
+                    payload[property].append(value)
+                else:
+                    return {'ret': False, 'msg': "Value of property %s in nic_config is invalid" % property}
+            else:
+                payload[property] = value
+
+        # If no need change, nothing to do. If error detected, report it
+        need_change = False
+        for property in payload.keys():
+            setValue = payload[property]
+            curValue = target_ethernet_current_setting[property]
+            # type is simple(not dict/list)
+            if not isinstance(setValue, dict) and not isinstance(setValue, list):
+                if setValue != curValue:
+                    need_change = True
+            # type is dict
+            if isinstance(setValue, dict):
+                for subprop in payload[property].keys():
+                    if subprop not in target_ethernet_current_setting[property]:
+                        return {'ret': False, 'msg': "Sub-property %s in nic_config is invalid" % subprop}
+                    sub_setValue = payload[property][subprop]
+                    sub_curValue = target_ethernet_current_setting[property][subprop]
+                    if sub_setValue != sub_curValue:
+                        need_change = True
+            # type is list
+            if isinstance(setValue, list):
+                for i in range(len(setValue)):
+                    for subprop in payload[property][i].keys():
+                        if subprop not in target_ethernet_current_setting[property][i]:
+                            return {'ret': False, 'msg': "Sub-property %s in nic_config is invalid" % subprop}
+                        sub_setValue = payload[property][i][subprop]
+                        sub_curValue = target_ethernet_current_setting[property][i][subprop]
+                        if sub_setValue != sub_curValue:
+                            need_change = True
+
+        if not need_change:
+            return {'ret': True, 'changed': False, 'msg': "Manager NIC already set"}
+
+        response = self.patch_request(self.root_uri + target_ethernet_uri, payload)
+        if response['ret'] is False:
+            return response
+        return {'ret': True, 'changed': True, 'msg': "Modified Manager NIC"}
diff --git a/lib/ansible/modules/remote_management/redfish/redfish_config.py b/lib/ansible/modules/remote_management/redfish/redfish_config.py
index 165741196a793d..e80ed2c09e6ec9 100644
--- a/lib/ansible/modules/remote_management/redfish/redfish_config.py
+++ b/lib/ansible/modules/remote_management/redfish/redfish_config.py
@@ -94,6 +94,19 @@
       - The ID of the System, Manager or Chassis to modify
     type: str
     version_added: "2.10"
+  nic_addr:
+    required: false
+    description:
+      - EthernetInterface Address string on OOB controller
+    default: 'null'
+    type: str
+    version_added: '2.10'
+  nic_config:
+    required: false
+    description:
+      - setting dict of EthernetInterface on OOB controller
+    type: dict
+    version_added: '2.10'
 
 author: "Jose Delarosa (@jose-delarosa)"
 '''
@@ -180,6 +193,21 @@
       baseuri: "{{ baseuri }}"
       username: "{{ username }}"
       password: "{{ password }}"
+
+  - name: Set Manager NIC
+    redfish_config:
+      category: Manager
+      command: SetManagerNic
+      nic_config:
+        DHCPv4:
+          DHCPEnabled: False
+        IPv4StaticAddresses:
+          Address: 192.168.1.3
+          Gateway: 192.168.1.1
+          SubnetMask: 255.255.255.0
+      baseuri: "{{ baseuri }}"
+      username: "{{ username }}"
+      password: "{{ password }}"
 '''
 
 RETURN = '''
@@ -199,7 +227,7 @@
 CATEGORY_COMMANDS_ALL = {
     "Systems": ["SetBiosDefaultSettings", "SetBiosAttributes", "SetBootOrder",
                 "SetDefaultBootOrder"],
-    "Manager": ["SetNetworkProtocols"]
+    "Manager": ["SetNetworkProtocols", "SetManagerNic"]
 }
 
 
@@ -221,7 +249,12 @@ def main():
                 type='dict',
                 default={}
             ),
-            resource_id=dict()
+            resource_id=dict(),
+            nic_addr=dict(default='null'),
+            nic_config=dict(
+                type='dict',
+                default={}
+            )
         ),
         supports_check_mode=False
     )
@@ -251,6 +284,10 @@ def main():
     # System, Manager or Chassis ID to modify
     resource_id = module.params['resource_id']
 
+    # manager nic
+    nic_addr = module.params['nic_addr']
+    nic_config = module.params['nic_config']
+
     # Build root URI
     root_uri = "https://" + module.params['baseuri']
     rf_utils = RedfishUtils(creds, root_uri, timeout, module,
@@ -292,6 +329,8 @@ def main():
         for command in command_list:
             if command == "SetNetworkProtocols":
                 result = rf_utils.set_network_protocols(module.params['network_protocols'])
+            elif command == "SetManagerNic":
+                result = rf_utils.set_manager_nic(nic_addr, nic_config)
 
     # Return data back or fail with proper message
     if result['ret'] is True:


Code review comments:
diff_hunk: @@ -2263,3 +2263,91 @@ def get_manager_health_report(self, manager_uri):
 
     def get_multi_manager_health_report(self):
         return self.aggregate_managers(self.get_manager_health_report)
+
+    def set_manager_nic(self, nic_addr, nic_config):
+        # Get EthernetInterface collection
+        response = self.get_request(self.root_uri + self.manager_uri)
+        if response['ret'] is False:
+            return response
+        data = response['data']
+        if 'EthernetInterfaces' not in data:
+            return {'ret': False, 'msg': "EthernetInterfaces resource not found"}
+        ethernetinterfaces_uri = data["EthernetInterfaces"]["@odata.id"]
+        response = self.get_request(self.root_uri + ethernetinterfaces_uri)
+        if response['ret'] is False:
+            return response
+        data = response['data']
+        uris = [a.get('@odata.id') for a in data.get('Members', []) if
+                a.get('@odata.id')]
+
+        # Find target EthernetInterface
+        target_ethernet_uri = None
+        target_ethernet_current_setting = None
+        if nic_addr == 'null':
+            # Find root_uri matched EthernetInterface when nic_addr is not specified
+            nic_addr = (self.root_uri).split('/')[-1]
+        for uri in uris:
+            response = self.get_request(self.root_uri + uri)
+            if response['ret'] is False:
+                return response
+            data = response['data']
+            if nic_addr in str(data):
+                target_ethernet_uri = uri
+                target_ethernet_current_setting = data
+                break
+        if target_ethernet_uri is None:
+            return {'ret': False, 'msg': "No matched EthernetInterface found under Manager"}
+
+        # Convert input to payload and check validity
+        payload = {}
+        for property in nic_config.keys():
+            value = nic_config[property]
+            if property not in target_ethernet_current_setting:
+                return {'ret': False, 'msg': "Property %s in nic_config is invalid" % property}
+            if isinstance(value, dict):
+                if isinstance(target_ethernet_current_setting[property], dict):
+                    payload[property] = value
+                elif isinstance(target_ethernet_current_setting[property], list):
+                    payload[property] = list()
+                    payload[property].append(value)
+                else:
+                    return {'ret': False, 'msg': "Value of property %s in nic_config is invalid" % property}
+            else:
+                payload[property] = value
+
+        # If no need change, nothing to do. If error detected, report it
+        need_change = False
+        for property in payload.keys():
+            setValue = payload[property]
+            curValue = target_ethernet_current_setting[property]
path: lib/ansible/module_utils/redfish_utils.py
text: @user1:
nit: lower_case_with_underscores preferred over CamelCase per PEP8 (cur_value, set_value).

@author:
Got it, code is updated.

diff_hunk: @@ -2263,3 +2263,91 @@ def get_manager_health_report(self, manager_uri):
 
     def get_multi_manager_health_report(self):
         return self.aggregate_managers(self.get_manager_health_report)
+
+    def set_manager_nic(self, nic_addr, nic_config):
+        # Get EthernetInterface collection
+        response = self.get_request(self.root_uri + self.manager_uri)
+        if response['ret'] is False:
+            return response
+        data = response['data']
+        if 'EthernetInterfaces' not in data:
+            return {'ret': False, 'msg': "EthernetInterfaces resource not found"}
+        ethernetinterfaces_uri = data["EthernetInterfaces"]["@odata.id"]
+        response = self.get_request(self.root_uri + ethernetinterfaces_uri)
+        if response['ret'] is False:
+            return response
+        data = response['data']
+        uris = [a.get('@odata.id') for a in data.get('Members', []) if
+                a.get('@odata.id')]
+
+        # Find target EthernetInterface
+        target_ethernet_uri = None
+        target_ethernet_current_setting = None
+        if nic_addr == 'null':
+            # Find root_uri matched EthernetInterface when nic_addr is not specified
+            nic_addr = (self.root_uri).split('/')[-1]
+        for uri in uris:
+            response = self.get_request(self.root_uri + uri)
+            if response['ret'] is False:
+                return response
+            data = response['data']
+            if nic_addr in str(data):
path: lib/ansible/module_utils/redfish_utils.py
text: @user1:
Let's say that `nic_addr` has a value of "192.168.0.1". And part of one of the `EthernetInterfaces` resource payloads (what is in the `data` var) looks like this:

```
    ...
    "IPv4Addresses": [
        {
            "Address": "192.168.0.10",
            "SubnetMask": "255.255.252.0",
            "AddressOrigin": "DHCP",
            "Gateway": "192.168.0.1",
            "Oem": {}
        }
    ],
    ...
```
If I'm interpreting this correctIy, I think the expression `if nic_addr in str(data)` is going to return True.
(e.g.: `if '192.168.0.1' in '... "Address": "192.168.0.10" ...'`)

Maybe we need a more explicit/rigorous way to select the EthernetInterface?



@author:
You are right. I have updated code to match '"192.168.0.1"' or "'192.168.0.1'" avoid partial match.