Fix VerifyBiosAttributes command on Systems with multiple entries #9234

sseekamp · 2024-12-09T23:54:11Z

SUMMARY

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

redfish_utils

ADDITIONAL INFORMATION

Previously, this command was working on single system resource systems due to get_multi_bios_attributes() returning a consistent response. When running on a system with multiple system resources the response from get_multi_bios_attributes() includes additional output structure that was not being accounted for in the method.

Due to this command being part of the redfish_command the data_modification=True attribute is set which requires passing in resource_id on nodes with multiple managers/systems/chassis. We can switch to using the singular get_bios_attributes method and eliminate the extra parsing as we will never have multiple system entries worth of data to handle.

Before this change on a system with multiple system entries:

Traceback (most recent call last):
  File "/home/user/.ansible/tmp/ansible-tmp-1733524983.8725114-939343-222145990725532/AnsiballZ_redfish_command.py", line 259, in <module>
    _ansiballz_main()
  File "/home/user/.ansible/tmp/ansible-tmp-1733524983.8725114-939343-222145990725532/AnsiballZ_redfish_command.py", line 246, in _ansiballz_main
    exitcode = debug(sys.argv[1], zipped_mod, ANSIBALLZ_PARAMS)
  File "/home/user/.ansible/tmp/ansible-tmp-1733524983.8725114-939343-222145990725532/AnsiballZ_redfish_command.py", line 213, in debug
    runpy.run_module(mod_name='ansible_collections.community.general.plugins.modules.redfish_command', init_globals=None, run_name='__main__', alter_sys=True)
  File "/usr/lib/python3.10/runpy.py", line 224, in run_module
    return _run_module_code(code, init_globals, run_name, mod_spec)
  File "/usr/lib/python3.10/runpy.py", line 96, in _run_module_code
    _run_code(code, mod_globals, init_globals,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/user/.ansible/tmp/ansible-tmp-1733524983.8725114-939343-222145990725532/debug_dir/ansible_collections/community/general/plugins/modules/redfish_command.py", line 1128, in <module>
    main()
  File "/home/user/.ansible/tmp/ansible-tmp-1733524983.8725114-939343-222145990725532/debug_dir/ansible_collections/community/general/plugins/modules/redfish_command.py", line 1124, in main
    module.fail_json(msg=(result['msg']))
KeyError: 'msg'

Expected failure (missing bios attribute):

fatal: [localhost]: FAILED! => changed=false
  invocation:
    module_args:
      account_properties: {}
      account_types: null
      auth_token: null
      baseuri: x.x.x.x
      bios_attributes:
        ScottFakeAttr: Disable
      boot_next: null
      boot_override_mode: null
      bootdevice: null
      category: Systems
      ciphers: null
      command:
      - VerifyBiosAttributes
      id: null
      new_password: null
      new_username: null
      oem_account_types: null
      password: VALUE_SPECIFIED_IN_NO_LOG_PARAMETER
      reset_to_defaults_mode: null
      resource_id: Self
      roleid: null
      session_uri: null
      strip_etag_quotes: false
      timeout: 60
      uefi_target: null
      update_apply_time: null
      update_creds: null
      update_handle: null
      update_image_file: null
      update_image_uri: null
      update_oem_params: null
      update_protocol: null
      update_targets: []
      update_username: null
      username: admin
      virtual_media: null
      wait: false
      wait_timeout: 120
  msg: 'Wrong parameters are provided: {''ScottFakeAttr'': ''Disable''}'

Successful verification:

ok: [localhost] => changed=false
  invocation:
    module_args:
      account_properties: {}
      account_types: null
      auth_token: null
      baseuri: x.x.x.x
      bios_attributes:
        EagleStream0002: Enable
      boot_next: null
      boot_override_mode: null
      bootdevice: null
      category: Systems
      ciphers: null
      command:
      - VerifyBiosAttributes
      id: null
      new_password: null
      new_username: null
      oem_account_types: null
      password: VALUE_SPECIFIED_IN_NO_LOG_PARAMETER
      reset_to_defaults_mode: null
      resource_id: Self
      roleid: null
      session_uri: null
      strip_etag_quotes: false
      timeout: 60
      uefi_target: null
      update_apply_time: null
      update_creds: null
      update_handle: null
      update_image_file: null
      update_image_uri: null
      update_oem_params: null
      update_protocol: null
      update_targets: []
      update_username: null
      username: admin
      virtual_media: null
      wait: false
      wait_timeout: 120
  msg: Action was successful
  return_values: {}
  session: {}

Expected failure (when missing resource_id):

fatal: [localhost]: FAILED! => changed=false
  invocation:
    module_args:
      account_properties: {}
      account_types: null
      auth_token: null
      baseuri: x.x.x.x
      bios_attributes:
        EagleStream0002: Enable
      boot_next: null
      boot_override_mode: null
      bootdevice: null
      category: Systems
      ciphers: null
      command:
      - VerifyBiosAttributes
      id: null
      new_password: null
      new_username: null
      oem_account_types: null
      password: VALUE_SPECIFIED_IN_NO_LOG_PARAMETER
      reset_to_defaults_mode: null
      resource_id: null
      roleid: null
      session_uri: null
      strip_etag_quotes: false
      timeout: 60
      uefi_target: null
      update_apply_time: null
      update_creds: null
      update_handle: null
      update_image_file: null
      update_image_uri: null
      update_oem_params: null
      update_protocol: null
      update_targets: []
      update_username: null
      username: admin
      virtual_media: null
      wait: false
      wait_timeout: 120
  msg: Issuing a data modification command without specifying the ID of the target System resource when there is more than one System is no longer allowed. Use the `resource_id` option to specify the target System ID.

Running against a single resource system (without specifying resource_id):

fatal: [localhost]: FAILED! => changed=false
  invocation:
    module_args:
      account_properties: {}
      account_types: null
      auth_token: null
      baseuri: x.x.x.x
      bios_attributes:
        PxeDev1Interface: NIC.Slot.31-1
      boot_next: null
      boot_override_mode: null
      bootdevice: null
      category: Systems
      ciphers: null
      command:
      - VerifyBiosAttributes
      id: null
      new_password: null
      new_username: null
      oem_account_types: null
      password: VALUE_SPECIFIED_IN_NO_LOG_PARAMETER
      reset_to_defaults_mode: null
      resource_id: null
      roleid: null
      session_uri: null
      strip_etag_quotes: false
      timeout: 60
      uefi_target: null
      update_apply_time: null
      update_creds: null
      update_custom_oem_header: null
      update_custom_oem_mime_type: null
      update_custom_oem_params: null
      update_handle: null
      update_image_file: null
      update_image_uri: null
      update_oem_params: null
      update_protocol: null
      update_targets: []
      update_username: null
      username: root
      virtual_media: null
      wait: false
      wait_timeout: 120
  msg: 'BIOS parameters are not matching: {''PxeDev1Interface'': ''NIC.Slot.31-1''}'

Successful run on single resource system:

ok: [localhost] => changed=false
  invocation:
    module_args:
      account_properties: {}
      account_types: null
      auth_token: null
      baseuri: x.x.x.x
      bios_attributes:
        BootSeqRetry: Enabled
      boot_next: null
      boot_override_mode: null
      bootdevice: null
      category: Systems
      ciphers: null
      command:
      - VerifyBiosAttributes
      id: null
      new_password: null
      new_username: null
      oem_account_types: null
      password: VALUE_SPECIFIED_IN_NO_LOG_PARAMETER
      reset_to_defaults_mode: null
      resource_id: null
      roleid: null
      session_uri: null
      strip_etag_quotes: false
      timeout: 60
      uefi_target: null
      update_apply_time: null
      update_creds: null
      update_custom_oem_header: null
      update_custom_oem_mime_type: null
      update_custom_oem_params: null
      update_handle: null
      update_image_file: null
      update_image_uri: null
      update_oem_params: null
      update_protocol: null
      update_targets: []
      update_username: null
      username: root
      virtual_media: null
      wait: false
      wait_timeout: 120
  msg: Action was successful
  return_values: {}
  session: {}

ansibullbot · 2024-12-10T00:17:42Z

cc @Bhavya06 @jyundt @mraineri @rajeevkallur @renxulei @tomasg2012 @xmadsen
click here for bot help

mraineri · 2024-12-10T16:22:47Z

plugins/module_utils/redfish_utils.py

@@ -3616,7 +3616,7 @@ def set_session_service(self, sessions_config):

    def verify_bios_attributes(self, bios_attributes):
        # This method verifies BIOS attributes against the provided input
-        server_bios = self.get_multi_bios_attributes()
+        server_bios = self.get_bios_attributes(self.systems_uri)


Does this mean that when a user runs this command, they need to use resource_id to specify an exact system in a multi-system scenario? Did get_multi_bios_attributes ever work properly in the first place?

Correct (that was the case before my change as well):

msg: Issuing a data modification command without specifying the ID of the target System resource when there is more than one System is no longer allowed. Use the `resource_id` option to specify the target System ID.

That threw me off, but because everything in redfish_command uses data_modification=True:

community.general/plugins/modules/redfish_command.py

Line 958 in baef858

resource_id=resource_id, data_modification=True, strip_etag_quotes=strip_etag_quotes,

it hits the gate:

community.general/plugins/module_utils/redfish_utils.py

Lines 518 to 519 in a1c39cc

elif len(self.systems_uris) > 1:

self.module.fail_json(msg=FAIL_MSG % {'resource': 'System'})

Okay, just checking. I don't think it was ever reasonable in the first place to have a "multi-system" BIOS check; I think it would be very likely there'd be differences. I prefer having this check a specific system.

Agreed - I’m not sure of any system types where you’d have that, but I suppose it’s possible.

mraineri · 2024-12-10T16:40:41Z

Thanks for fixing this!

shipit

russoz

LGTM other than a little ~~cosmetic~~ formatting thing in the changelog

changelogs/fragments/9234-fix-verify-bios-attributes-multi-system.yml

…tem.yml Improve fragment Co-authored-by: Alexei Znamensky <[email protected]>

russoz

Thanks for the adjustment. LGTM

patchback · 2024-12-14T09:44:36Z

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/34e8e8e5d18ab339556c922c28967d77faa0084c/pr-9234

Backported as #9248

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

) * Fix verify_bios_attributes command * Add changelog fragment * Update changelogs/fragments/9234-fix-verify-bios-attributes-multi-system.yml Improve fragment Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Alexei Znamensky <[email protected]> (cherry picked from commit 34e8e8e)

patchback · 2024-12-14T09:44:45Z

Backport to stable-10: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-10/34e8e8e5d18ab339556c922c28967d77faa0084c/pr-9234

Backported as #9249

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

) * Fix verify_bios_attributes command * Add changelog fragment * Update changelogs/fragments/9234-fix-verify-bios-attributes-multi-system.yml Improve fragment Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Alexei Znamensky <[email protected]> (cherry picked from commit 34e8e8e)

felixfontein · 2024-12-14T09:44:57Z

@sseekamp thanks for your contribution!
@mraineri @russoz thanks for reviewing this!

…and on Systems with multiple entries (#9249) Fix VerifyBiosAttributes command on Systems with multiple entries (#9234) * Fix verify_bios_attributes command * Add changelog fragment * Update changelogs/fragments/9234-fix-verify-bios-attributes-multi-system.yml Improve fragment Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Alexei Znamensky <[email protected]> (cherry picked from commit 34e8e8e) Co-authored-by: Scott Seekamp <[email protected]>

…nd on Systems with multiple entries (#9248) Fix VerifyBiosAttributes command on Systems with multiple entries (#9234) * Fix verify_bios_attributes command * Add changelog fragment * Update changelogs/fragments/9234-fix-verify-bios-attributes-multi-system.yml Improve fragment Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Alexei Znamensky <[email protected]> (cherry picked from commit 34e8e8e) Co-authored-by: Scott Seekamp <[email protected]>

sseekamp added 2 commits December 9, 2024 16:26

Fix verify_bios_attributes command

c06825e

Add changelog fragment

6d29165

sseekamp force-pushed the sseekamp/verify-bios-attrs branch from f0f09dd to 6d29165 Compare December 9, 2024 23:59

sseekamp mentioned this pull request Dec 10, 2024

redfish_command: VerifyBiosAttributes fails on Systems with multiple entries #9230

Closed

1 task

ansibullbot added bug This issue/PR relates to a bug has_issue module_utils module_utils plugins plugin (any type) traceback labels Dec 10, 2024

mraineri reviewed Dec 10, 2024

View reviewed changes

russoz reviewed Dec 12, 2024

View reviewed changes

changelogs/fragments/9234-fix-verify-bios-attributes-multi-system.yml Outdated Show resolved Hide resolved

Update changelogs/fragments/9234-fix-verify-bios-attributes-multi-sys…

f67d95c

…tem.yml Improve fragment Co-authored-by: Alexei Znamensky <[email protected]>

russoz approved these changes Dec 12, 2024

View reviewed changes

This comment was marked as duplicate.

Sign in to view

felixfontein merged commit 34e8e8e into ansible-collections:main Dec 14, 2024
129 checks passed

patchback bot mentioned this pull request Dec 14, 2024

[PR #9234/34e8e8e5 backport][stable-9] Fix VerifyBiosAttributes command on Systems with multiple entries #9248

Merged

patchback bot mentioned this pull request Dec 14, 2024

[PR #9234/34e8e8e5 backport][stable-10] Fix VerifyBiosAttributes command on Systems with multiple entries #9249

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix VerifyBiosAttributes command on Systems with multiple entries #9234

Fix VerifyBiosAttributes command on Systems with multiple entries #9234

sseekamp commented Dec 9, 2024

ansibullbot commented Dec 10, 2024

mraineri Dec 10, 2024

sseekamp Dec 10, 2024

mraineri Dec 10, 2024

sseekamp Dec 10, 2024

mraineri commented Dec 10, 2024

russoz left a comment •

edited

Loading

russoz left a comment

This comment was marked as duplicate.

patchback bot commented Dec 14, 2024 •

edited

Loading

patchback bot commented Dec 14, 2024 •

edited

Loading

felixfontein commented Dec 14, 2024

	elif len(self.systems_uris) > 1:
	self.module.fail_json(msg=FAIL_MSG % {'resource': 'System'})

Fix VerifyBiosAttributes command on Systems with multiple entries #9234

Fix VerifyBiosAttributes command on Systems with multiple entries #9234

Conversation

sseekamp commented Dec 9, 2024

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibullbot commented Dec 10, 2024

mraineri Dec 10, 2024

Choose a reason for hiding this comment

sseekamp Dec 10, 2024

Choose a reason for hiding this comment

mraineri Dec 10, 2024

Choose a reason for hiding this comment

sseekamp Dec 10, 2024

Choose a reason for hiding this comment

mraineri commented Dec 10, 2024

russoz left a comment • edited Loading

Choose a reason for hiding this comment

russoz left a comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

patchback bot commented Dec 14, 2024 • edited Loading

Backport to stable-9: 💚 backport PR created

patchback bot commented Dec 14, 2024 • edited Loading

Backport to stable-10: 💚 backport PR created

felixfontein commented Dec 14, 2024

russoz left a comment •

edited

Loading

patchback bot commented Dec 14, 2024 •

edited

Loading

patchback bot commented Dec 14, 2024 •

edited

Loading