Add module redfish_config #43470
Add module redfish_config #43470
Conversation
Communicates with Out-Of-Band Controller through Redfish APIs Sends a configuration update to the Controller
ansibot
added
affects_2.7
|
ready_for_review |
ryansb
removed
the
needs_triage
ansibot
added
the
stale_ci
| # execute only if we find a System resource | ||
| result = rf_utils._find_systems_resource(rf_uri) | ||
| if result['ret'] is False: | ||
| module.fail_json(msg=result['msg']) |
There was a problem hiding this comment.
The value in msg should be wrapped in to_native() to ensure it is the correct string type based on the Python version. from ansible.module_utils._text import to_native
ansibot
added
needs_revision
| version_added: "2.7" | ||
| short_description: Manages Out-Of-Band controllers using Redfish APIs | ||
| description: | ||
| - Builds Redfish URIs locally and sends them to remote OOB controllers to |
There was a problem hiding this comment.
Add a description of what this module actually does beyond building commands and sending them to an API.
ansibot
removed
the
stale_ci
|
The test |
ansibot
removed
the
ci_verified
ansibot
added
stale_ci
| for cmd in command_list: | ||
| # Fail if even one command given is invalid | ||
| if cmd not in CATEGORY_COMMANDS_ALL[category]: | ||
| module.fail_json(msg=to_native("Invalid Command: %s" % cmd)) |
There was a problem hiding this comment.
It would be helpful to display valid values here.
There was a problem hiding this comment.
Yup no problem
| result: | ||
| description: different results depending on task | ||
| returned: always | ||
| type: dict |
There was a problem hiding this comment.
Maybe consider changing the type to complex and documenting a few different examples of data returned by the commands. Since there are only three, this should be feasible unless it's just an enormous amount of data that is returned. See docs for examples.
There was a problem hiding this comment.
Need to update this, this was true before I split my original module into 3 modules. In this module, (redfish_config) the return value is either OK (success) or an error message/code.
| 'pswd': module.params['password']} | ||
|
|
||
| # Manager attributes to update | ||
| mgr_attributes = {'mgr_attr_name': module.params['mgr_attr_name'], |
There was a problem hiding this comment.
The parameters mgr_attr_value and bios_attr_name are not marked as required nor do they have a default value in the argument spec. This means they will be None if not passed into the module. Is that ok?
There was a problem hiding this comment.
Good catch. Need to set to a bogus default value, as setting to a valid default value can have unintended consequences. With an 'invalid' value, it will catch the error and exit.
mgr_attr_name=dict(default='invalid'),
mgr_attr_value=dict(default='invalid'),
bios_attr_name=dict(default='invalid'),
bios_attr_value=dict(default='invalid'),
Without a default value, and if not specified, error message is ugly.
ansibot
removed
the
stale_review
ansibot
removed
the
stale_ci
|
Thanks for your review @samdoran |
SUMMARY
Communicates with Out-Of-Band Controller through Redfish APIs
Sends a configuration update to the Controller
ISSUE TYPE
COMPONENT NAME
redfish_config
ANSIBLE VERSION
ADDITIONAL INFORMATION
This PR is part of a group of modules. PR for first module is #41656 (merged).