Skip to content

Conversation

@spesnova717
Copy link

@spesnova717 spesnova717 commented Jun 13, 2025

Bug #3073 is caused by 3 things:

  1. Foreman core doesn't call BMC proxy's API correctly
    • Changing power state to 'Reboot' via Web GUI calls BMC proxy's 'soft'
      power action
    • Changing power state to 'Reset' via Web GUI calls BMC proxy's 'cycle'
      power action
  2. Foreman BMC proxy doesn't support reboot at all BMC provider
  3. Foreman BMC proxy doesn't support reset(powerreset) at Redfish provider

2nd & 3rd item are mentioned in #38498. And this PR fixes 3rd item.

Modifications in this PR

Enable proper handling of "Reset" power action via Redfish

This PR enables the powerreset operation to be processed correctly by the Smart Proxy when using the Redfish provider.
Internally, this maps the "Reset" request from Foreman core to the appropriate Redfish ForceRestart operation, aligning the behavior with user expectations in the Web UI.
To support this, a new powerreset API endpoint is introduced, and the Redfish BMC class is extended to implement the corresponding logic.

@spesnova717 spesnova717 changed the title Refs #3073 - Add support for Reset (powerreset) via Redfish provider [WIP] Refs #3073 - Add support for Reset (powerreset) via Redfish provider Jun 13, 2025
@spesnova717 spesnova717 force-pushed the fix_3073_bmc_reboot_reset_patch3 branch from 82ff063 to a025c08 Compare June 16, 2025 05:50
@spesnova717 spesnova717 changed the title [WIP] Refs #3073 - Add support for Reset (powerreset) via Redfish provider Fixes #38498 - Add support for Reset (powerreset) via Redfish provider Jun 16, 2025
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stejskalleos do we have the capacity to verify this somewhere using actual Redfish hardware?

poweraction('ForceRestart')
end

def powerreset
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks the same as the powercycle implementation. Is there a reason you can't use `cycle?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ekohl san

Thank you for your comment.
Because powercycle and power reset is different power action.

According to the Redfish specification (DSP2046_2025.1, p.513), these actions are different one and should use different reset type:

  • powercycle should use the PowerCycle
  • powerreset should use ForceRestart

The current implementation uses ForceRestart against powercycle and this is wrong implementation.
This is described in ticket #38498.
I plan to submit a separate PR to correct the powercycle behavior.

@spesnova717
Copy link
Author

@ekohl san

Thank you for your comment.
Because powercycle and power reset is different power action.

According to the Redfish specification (DSP2046_2025.1, p.513), these actions are different one and should use different reset type:

  • powercycle should use the PowerCycle
  • powerreset should use ForceRestart

The current implementation uses ForceRestart against powercycle and this is wrong implementation.
This is described in ticket #38498.
I plan to submit a separate PR to correct the powercycle behavior.

@spesnova717 spesnova717 force-pushed the fix_3073_bmc_reboot_reset_patch3 branch from a025c08 to 3a74cff Compare July 1, 2025 11:26
@spesnova717 spesnova717 force-pushed the fix_3073_bmc_reboot_reset_patch3 branch from 3a74cff to 3a45495 Compare July 16, 2025 09:21
@spesnova717 spesnova717 force-pushed the fix_3073_bmc_reboot_reset_patch3 branch from 3a45495 to 46210ab Compare September 1, 2025 09:56
@stejskalleos stejskalleos self-assigned this Sep 3, 2025
@stejskalleos
Copy link
Contributor

@stejskalleos do we have the capacity to verify this somewhere using actual Redfish hardware?

Yes and no. I have the capacity, but don't have the hardware. I think it could be tested with the DMTF Redfish-Mockup-Server or with the sushy-tools. Both are just simulators, but for the review, it might be enough.

@spesnova717 I will do the review. I added it to my backlog, but I can't promise a quick response as I'm quite new to it.

@spesnova717
Copy link
Author

@stejskalleos
Thanks so much for adding it to your backlog. If any context or repro steps would help, I’m happy to share.

@stejskalleos
Copy link
Contributor

@spesnova717, To resolve the merge conflicts, can you please rebase the branch over the latest develop?

@spesnova717 spesnova717 force-pushed the fix_3073_bmc_reboot_reset_patch3 branch from 8900f46 to f1df1ec Compare September 18, 2025 01:17
@spesnova717
Copy link
Author

@stejskalleos
Thanks for review.
I've resolved the merge conflicts and rebased the branch over the latest develop.
I’d appreciate your review to confirm the changes.

Bug #3073 is caused by 3 things:
1. Foreman core doesn't call BMC proxy's API correctly
   - Changing power state to 'Reboot' via Web GUI calls BMC proxy's 'soft'
     power action
   - Changing power state to 'Reset' via Web GUI calls BMC proxy's 'cycle'
     power action
2. Foreman BMC proxy doesn't support reboot at all BMC provider
3. Foreman BMC proxy doesn't support reset(powerreset) at Redfish provider

2nd & 3rd item are mentioned in #38498. And this PR fixes 3rd item.

For backward compatibility, power_action_v2 capability is introduced
to smart proxy at PRs related to #38498. PR related to #3073 adds logic
to determine if smart proxy supports this capability into Foreman core.

Co-Authored-By: Vanou Ishii <[email protected]>
@spesnova717 spesnova717 force-pushed the fix_3073_bmc_reboot_reset_patch3 branch from f1df1ec to e25e3fa Compare September 22, 2025 13:55
Copy link
Contributor

@stejskalleos stejskalleos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Tested together with theforeman/foreman#10561, works as expected.

🍏 code
🍏 tests
🍏 CI

@stejskalleos stejskalleos merged commit f03ed81 into theforeman:develop Sep 23, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants