-
Notifications
You must be signed in to change notification settings - Fork 23.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Included ansible_devices for AIX facts/hardware #31546
Conversation
On facts/hardware/aix.py was included devices for AIX. These information is very useful for AIX environment.
The test
|
E123 closing bracket does not match indentation of opening bracket's line
bot_status |
waiting_on: kairoaraujo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to fix the run_command() use to pass multiple args as a list.
field = line.split() | ||
|
||
device_attrs = {} | ||
rc, out_lsattr, err = self.module.run_command("%s -El %s" % (lsattr_cmd, field[0])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use an args list here instead of a string to avoid problems if the first field if lsdev output is unexpected.
for ex:
device_name = field[0]
lsattr_cmd_args = [lsattr_cmd, '-E', '-l', device_name]
rc, out_lsattr, err = self.module.run_command(lsattr_cmd_args)
device_attrs = {} | ||
rc, out_lsattr, err = self.module.run_command("%s -El %s" % (lsattr_cmd, field[0])) | ||
for attr in out_lsattr.splitlines(): | ||
attr_field = attr.split() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since attr_field is a list, attr_fields would be a better name
(and for the 'field' list above as well)
Added arguments list as recommended and clean code for fileds.
@kairoaraujo Could you provide some sample output from the 'lsattr' and 'lsdev' commands for reference and/or tests? |
} | ||
|
||
return device_facts | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me but wouldn't mind seeing some unit tests.
a little refactoring may make it easier to test, something like:
def _parse_lsdev(self, out_lsdev):
devices = []
for line in out_lsdev.splitlines():
field = line.split()
device_name = field[0]
device_state = field[1]
device_type = field[2:]
devices.append({'name': device_name,
'state': device_state,
'type': device_type})
return devices
def _parse_lsattr(self, out_lsattr):
device_attrs = {}
for attr in out_lsattr.splitlines():
attr_fields = attr.split()
attr_name = attr_fields[0]
attr_parameter = attr_fields[1]
# attrs.append({'name': attr_name,
# 'parameter': attr_parameter})
device_attrs[attr_name] = attr_parameter
return device_attrs
def _get_device_attrs(self, lsattr_cmd, device_name):
lsattr_cmd_args = [lsattr_cmd, '-E', '-l', device_name]
rc, out_lsattr, err = self.module.run_command(lsattr_cmd_args)
device_attrs = self._parse_lsattr(out_lsattr)
return device_attrs
def _get_devices(self, lsdev_cmd):
rc, out_lsdev, err = self.module.run_command(lsdev_cmd)
devices = self._parse_lsdev(out_lsdev)
return devices
def get_device_facts(self):
device_facts = {}
device_facts['devices'] = {}
lsdev_cmd = self.module.get_bin_path('lsdev', True)
lsattr_cmd = self.module.get_bin_path('lsattr', True)
rc, out_lsdev, err = self.module.run_command(lsdev_cmd)
devices = self._get_devices(lsdev_cmd, out_lsdev)
for device in devices:
device_attrs = self._get_device_attrs(lsattr_cmd, device['name'])
device_facts['devices'][device['name']] = {
'state': device['state'],
'type': ' '.join(device['type']),
'attributes': device_attrs
}
return device_facts
The _parse_lsattr and _parse_lsdev could be unit tests with sample output from the commands
* Included ansible_devices for AIX
SUMMARY
Included devices for AIX on facts/hardware/aix.py.
These information is very useful for AIX environment.
ISSUE TYPE
COMPONENT NAME
lib/ansible/module_utils/facts/hardware/aix.py
ANSIBLE VERSION
ADDITIONAL INFORMATION
Example of full output of -m setup with ansible_devices for AIX.
https://gist.github.com/kairoaraujo/a9c56b078191842bffb35065e1642d1d
Example of use:
Output