Skip to content

Commit 21b5c2c

Browse files
spesnova717vanou
authored andcommitted
Fixes #3073 - Fix mismatched and missing Reboot/Reset power actions
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 This PR fixes first item. For backward compatibility, power_action_v2 capability will be introduced to smart proxy at PRs related to #38498. And this commit introduces logic to determine if smart proxy supports that capability. Co-Authored-By: Vanou Ishii <ishii.vanou@fujitsu.com>
1 parent e6cf46a commit 21b5c2c

File tree

5 files changed

+75
-5
lines changed

5 files changed

+75
-5
lines changed

app/controllers/api/v2/hosts_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ def disassociate
242242

243243
api :PUT, "/hosts/:id/power", N_("Run a power operation on host")
244244
param :id, :identifier_dottable, :required => true
245-
param :power_action, String, :required => true, :desc => N_("power action, valid actions are (on/start), (off/stop), (soft/reboot), (cycle/reset), (state/status)")
245+
param :power_action, String, :required => true, :desc => N_("power action, valid actions are (on/start), (off/stop), reboot, reset, soft, cycle, (state/status)")
246246

247247
def power
248248
unless @host.supports_power?

app/services/power_manager/bmc.rb

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,39 @@ def ready?
1313

1414
attr_reader :proxy
1515

16+
def power_action_v2
17+
smart_proxy = host.smart_proxies.with_features(:BMC).first
18+
19+
return false unless smart_proxy
20+
smart_proxy.has_capability?(:BMC, :power_action_v2)
21+
end
22+
1623
# TODO: consider moving this to the proxy code, so we can just delegate like as with Virt.
1724
def action_map
1825
super.deep_merge({
1926
:start => 'on',
2027
:stop => 'off',
2128
:poweroff => 'off',
22-
:reboot => 'soft',
23-
:reset => 'cycle',
29+
:reboot => power_action_v2 ? 'do_reboot' : 'soft',
30+
:reset => power_action_v2 ? 'do_reset' : 'cycle',
2431
:state => 'status',
2532
:ready? => 'ready?',
2633
})
2734
end
2835

36+
# Avoid infinite loop when action_map maps :reboot to 'reboot'.
37+
# This method was introduced as part of foreman #3073 and smart-proxy #38498
38+
# to explicitly handle reset/reboot actions, ensuring backward compatibility
39+
# with older Smart Proxy implementations that do not support the new capabilities.
40+
def do_reboot
41+
default_action(:reboot)
42+
end
43+
44+
# For the same reason as above, this handles the case where :reset maps to 'reset'.
45+
def do_reset
46+
default_action(:reset)
47+
end
48+
2949
def default_action(action)
3050
proxy.power(:action => action.to_s) # proxy.power(:action => 'on')
3151
end

app/services/proxy_api/bmc.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def power(args)
5353
full_path_as_hash = bmc_url_for_get('power', args[:action])
5454
response = parse(get(full_path_as_hash[:path], query: full_path_as_hash[:query]))
5555
response.is_a?(Hash) ? response['result'] : response
56-
when "on", "off", "cycle", "soft"
56+
when "on", "off", "cycle", "soft", "reboot", "reset"
5757
res = parse put(with_provider(args), bmc_url_for('power', args[:action]))
5858
res && (res['result'] == true || res['result'] == "#{@target}: ok\n")
5959
else

test/unit/power_manager_test.rb

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,54 @@ def actions_list(host)
8282
end
8383
actions.uniq
8484
end
85+
86+
test "should call reboot and reset directly when power_action_v2 is supported" do
87+
host = FactoryBot.build_stubbed(:host, :managed)
88+
bmc_proxy_mock = mock('bmc_proxy')
89+
nic_mock = mock('nic_bmc')
90+
relation_mock = mock('smart_proxies_relation')
91+
bmc_caps = mock('bmc_caps')
92+
filtered_relation_mock = mock('filtered_relation_mock')
93+
94+
host.stubs(:bmc_proxy).returns(bmc_proxy_mock)
95+
host.stubs(:bmc_nic).returns(nic_mock)
96+
nic_mock.stubs(:credentials_present?).returns(true)
97+
nic_mock.stubs(:provider).returns('IPMI')
98+
host.stubs(:bmc_available?).returns(true)
99+
host.stubs(:smart_proxies).returns(relation_mock)
100+
relation_mock.stubs(:with_features).with(:BMC).returns(filtered_relation_mock)
101+
filtered_relation_mock.stubs(:first).returns(bmc_caps)
102+
bmc_caps.stubs(:has_capability?).with(:BMC, :power_action_v2).returns(true)
103+
104+
bmc_proxy_mock.expects(:power).with(:action => "reboot").returns(true)
105+
bmc_proxy_mock.expects(:power).with(:action => "reset").returns(true)
106+
107+
assert host.power.reboot
108+
assert host.power.reset
109+
end
110+
111+
test "should fallback to soft and cycle when power_action_v2 is not supported" do
112+
host = FactoryBot.build_stubbed(:host, :managed)
113+
bmc_proxy_mock = mock('bmc_proxy')
114+
nic_mock = mock('nic_bmc')
115+
relation_mock = mock('smart_proxies_relation')
116+
bmc_caps = mock('bmc_caps')
117+
filtered_relation_mock = mock('filtered_relation_mock')
118+
119+
host.stubs(:bmc_proxy).returns(bmc_proxy_mock)
120+
host.stubs(:bmc_nic).returns(nic_mock)
121+
nic_mock.stubs(:credentials_present?).returns(true)
122+
nic_mock.stubs(:provider).returns('IPMI')
123+
host.stubs(:bmc_available?).returns(true)
124+
host.stubs(:smart_proxies).returns(relation_mock)
125+
relation_mock.stubs(:with_features).with(:BMC).returns(filtered_relation_mock)
126+
filtered_relation_mock.stubs(:first).returns(bmc_caps)
127+
bmc_caps.stubs(:has_capability?).with(:BMC, :power_action_v2).returns(false)
128+
129+
bmc_proxy_mock.expects(:power).with(:action => "soft").returns(true)
130+
bmc_proxy_mock.expects(:power).with(:action => "cycle").returns(true)
131+
132+
assert host.power.reboot
133+
assert host.power.reset
134+
end
85135
end

webpack/assets/javascripts/react_app/components/HostDetails/DetailsCard/PowerStatus/constants.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { translate as __ } from '../../../../common/I18n';
33
export const POWER_REQURST_KEY = 'HOST_TOGGLE_POWER';
44
export const POWER_REQUEST_OPTIONS = { key: POWER_REQURST_KEY, params: { timeout: 30 } };
55
export const BASE_POWER_STATES = { off: __('Off'), on: __('On') };
6-
export const BMC_POWER_STATES = { soft: __('Reboot'), cycle: __('Reset') };
6+
export const BMC_POWER_STATES = { reboot: __('Reboot'), reset: __('Reset'), soft: __('Soft'), cycle: __('Cycle') };
77
export const SUPPORTED_POWER_STATES = {
88
...BASE_POWER_STATES,
99
...BMC_POWER_STATES,

0 commit comments

Comments
 (0)