diff --git a/modules/bmc/redfish.rb b/modules/bmc/redfish.rb index 628864b4e..34dc2fce5 100644 --- a/modules/bmc/redfish.rb +++ b/modules/bmc/redfish.rb @@ -55,15 +55,19 @@ def test end def identifystatus + location_active = system.LocationIndicatorActive + return location_active ? 'on' : 'off' unless location_active.nil? + + logger.debug('LocationIndicatorActive is nil, falling back to IndicatorLED for identify status') system.IndicatorLED&.downcase end def identifyon - system.patch_if_match({ 'IndicatorLED' => 'Lit' }) + patch_identify(:on) end def identifyoff - system.patch_if_match({ 'IndicatorLED' => 'Off' }) + patch_identify(:off) end def poweroff(soft = false) @@ -218,6 +222,31 @@ def poweraction(action) host.post(path: system.Actions&.[]('#ComputerSystem.Reset')&.[]('target'), payload: { 'ResetType' => action }) end + def patch_identify(state) + location_active = system.LocationIndicatorActive + + payload = case state + when :on + if location_active.nil? + logger.debug('LocationIndicatorActive is nil, falling back to IndicatorLED for identify on') + { 'IndicatorLED' => 'Lit' } + else + { 'LocationIndicatorActive' => true } + end + when :off + if location_active.nil? + logger.debug('LocationIndicatorActive is nil, falling back to IndicatorLED for identify off') + { 'IndicatorLED' => 'Off' } + else + { 'LocationIndicatorActive' => false } + end + else + raise ArgumentError, "Unsupported identify state: #{state.inspect}" + end + + system.patch_if_match(payload) + end + # I haven't yet encountered a system (apart from a blade chassis, which I think # wouldn't be modeled in Foreman as a host?) which has multiple BMCs, managed systems, # or BMC NICs. But it's part of the Redfish design, so it's at least possible that diff --git a/test/bmc/bmc_redfish_test.rb b/test/bmc/bmc_redfish_test.rb index 8377895b9..6650e0c81 100644 --- a/test/bmc/bmc_redfish_test.rb +++ b/test/bmc/bmc_redfish_test.rb @@ -162,24 +162,57 @@ def test_bootcdrom_calls_bootdevice assert_not_nil result end - def test_identifyon_sets_indicator_led_lit + def test_identifystatus_uses_indicator_led_when_location_indicator_absent system_mock = mock('system') - system_mock.expects(:patch_if_match).with({ 'IndicatorLED' => 'Lit' }).returns(true) + system_mock.expects(:LocationIndicatorActive).returns(nil) + system_mock.expects(:IndicatorLED).returns('Lit') + @bmc.expects(:system).twice.returns(system_mock) + assert_equal 'lit', @bmc.identifystatus + end + + def test_identifystatus_uses_location_indicator_active_true + system_mock = mock('system') + system_mock.expects(:LocationIndicatorActive).returns(true) @bmc.expects(:system).returns(system_mock) - @bmc.identifyon + assert_equal 'on', @bmc.identifystatus end - def test_identifyoff_sets_indicator_led_off + def test_identifystatus_uses_location_indicator_active_false system_mock = mock('system') - system_mock.expects(:patch_if_match).with({ 'IndicatorLED' => 'Off' }).returns(true) + system_mock.expects(:LocationIndicatorActive).returns(false) @bmc.expects(:system).returns(system_mock) + assert_equal 'off', @bmc.identifystatus + end + + def test_identifyon_uses_location_indicator_active_when_present + system_mock = mock('system') + system_mock.expects(:LocationIndicatorActive).returns(false) + system_mock.expects(:patch_if_match).with({ 'LocationIndicatorActive' => true }).returns(true) + @bmc.expects(:system).twice.returns(system_mock) + @bmc.identifyon + end + + def test_identifyon_falls_back_to_indicator_led_when_location_indicator_absent + system_mock = mock('system') + system_mock.expects(:LocationIndicatorActive).returns(nil) + system_mock.expects(:patch_if_match).with({ 'IndicatorLED' => 'Lit' }).returns(true) + @bmc.expects(:system).twice.returns(system_mock) + @bmc.identifyon + end + + def test_identifyoff_uses_location_indicator_active_when_present + system_mock = mock('system') + system_mock.expects(:LocationIndicatorActive).returns(true) + system_mock.expects(:patch_if_match).with({ 'LocationIndicatorActive' => false }).returns(true) + @bmc.expects(:system).twice.returns(system_mock) @bmc.identifyoff end - def test_identifystatus_returns_downcased_indicator_led + def test_identifyoff_falls_back_to_indicator_led_when_location_indicator_absent system_mock = mock('system') - system_mock.expects(:IndicatorLED).returns('Lit') - @bmc.expects(:system).returns(system_mock) - assert_equal 'lit', @bmc.identifystatus + system_mock.expects(:LocationIndicatorActive).returns(nil) + system_mock.expects(:patch_if_match).with({ 'IndicatorLED' => 'Off' }).returns(true) + @bmc.expects(:system).twice.returns(system_mock) + @bmc.identifyoff end end diff --git a/test/bmc/redfish_test_helper.rb b/test/bmc/redfish_test_helper.rb index baca618b4..af3135843 100644 --- a/test/bmc/redfish_test_helper.rb +++ b/test/bmc/redfish_test_helper.rb @@ -37,6 +37,11 @@ module RedfishTestHelper }, }.freeze + # Fixture for systems that support the newer LocationIndicatorActive field + SYSTEM_DATA_WITH_LOCATION_INDICATOR = SYSTEM_DATA.merge( + "LocationIndicatorActive" => false + ).freeze + def mask_redfish_acceess(protocol: "https", host: "host") # root stub_request(:get, "#{protocol}://#{host}#{ROOT_DATA['@odata.id']}").