Fixes #39301 - Add LocationIndicatorActive support for identify LED with IndicatorLED fallback

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

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

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']}").