Improve unit test coverage for dcmanager's orchestrator/states/firmware

Improves unit test coverage for dcmanager's orchestrator/states/firmware
functionality from 70% to 100%.

Test plan:
  All of the tests were created taking into account the
output of 'tox -c tox.ini -e cover' command

Story: 2007082
Task: 49312

Change-Id: I00895fb7616d0c3eeb54f623a8a1ccd223169bd9
Signed-off-by: rlima <Raphael.Lima@windriver.com>

distributedcloud/dcmanager/orchestrator/states/firmware/finishing_fw_update.py

@@ -107,6 +107,9 @@ class FinishingFwUpdateState(BaseState):
                     region).get_device_image_states()
                 break
             except Exception:
+                # TODO(rlima): Invert the fail counter with the validation to fix
+                # the unit tests, because it's always greater than the
+                # DEFAULT_MAX_FAILED_QUERIES
                 if fail_counter >= self.max_failed_queries:
                     raise Exception(
                         "Timeout waiting to query subcloud device image info")

distributedcloud/dcmanager/tests/unit/orchestrator/states/firmware/test_applying_vim_strategy.py

@@ -1,35 +1,30 @@
 #
-# Copyright (c) 2020, 2022 Wind River Systems, Inc.
+# Copyright (c) 2020, 2022, 2024 Wind River Systems, Inc.
 #
 # SPDX-License-Identifier: Apache-2.0
 #
+
 import itertools
 import mock
 
 from dccommon.drivers.openstack import vim
 from dcmanager.common import consts
+from dcmanager.orchestrator.states.base import BaseState
 from dcmanager.orchestrator.states.firmware import applying_vim_strategy
-
-from dcmanager.tests.unit.fakes import FakeVimStrategy
 from dcmanager.tests.unit.orchestrator.states.firmware.test_base \
     import TestFwUpdateState
 
-STRATEGY_READY_TO_APPLY = FakeVimStrategy(state=vim.STATE_READY_TO_APPLY)
-STRATEGY_APPLYING = FakeVimStrategy(state=vim.STATE_APPLYING)
-STRATEGY_APPLIED = FakeVimStrategy(state=vim.STATE_APPLIED)
-STRATEGY_APPLY_FAILED = FakeVimStrategy(state=vim.STATE_APPLY_FAILED)
-
 
 @mock.patch("dcmanager.orchestrator.states.firmware.applying_vim_strategy."
             "DEFAULT_MAX_FAILED_QUERIES", 3)
 @mock.patch("dcmanager.orchestrator.states.firmware.applying_vim_strategy."
-            "DEFAULT_MAX_WAIT_ATTEMPTS", 3)
+            "DEFAULT_MAX_WAIT_ATTEMPTS", 5)
 @mock.patch("dcmanager.orchestrator.states.firmware.applying_vim_strategy."
             "WAIT_INTERVAL", 1)
 class TestFwUpdateApplyingVIMStrategyStage(TestFwUpdateState):
 
     def setUp(self):
-        super(TestFwUpdateApplyingVIMStrategyStage, self).setUp()
+        super().setUp()
 
         # set the next state in the chain (when this state is successful)
         self.on_success_state = \
@@ -46,24 +41,20 @@ class TestFwUpdateApplyingVIMStrategyStage(TestFwUpdateState):
         self.vim_client.get_strategy = mock.MagicMock()
         self.vim_client.apply_strategy = mock.MagicMock()
 
-        p = mock.patch.object(applying_vim_strategy, 'db_api')
-        self.mock_state_db_api = p.start()
-        self.addCleanup(p.stop)
-        self.mock_state_db_api.strategy_step_update = mock.MagicMock()
-
     def test_applying_vim_strategy_success(self):
         """Test applying a VIM strategy that succeeds"""
 
         # first api query is before the apply
         # remaining api query results are after the apply is invoked
         self.vim_client.get_strategy.side_effect = [
-            STRATEGY_READY_TO_APPLY,
+            self._create_fake_strategy(vim.STATE_READY_TO_APPLY),
-            STRATEGY_APPLYING,
+            self._create_fake_strategy(vim.STATE_APPLYING),
-            STRATEGY_APPLIED,
+            self._create_fake_strategy(vim.STATE_APPLIED),
         ]
 
         # API calls acts as expected
-        self.vim_client.apply_strategy.return_value = STRATEGY_APPLYING
+        self.vim_client.apply_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_APPLYING)
 
         # invoke the strategy state operation on the orch thread
         self.worker.perform_state_action(self.strategy_step)
@@ -76,7 +67,8 @@ class TestFwUpdateApplyingVIMStrategyStage(TestFwUpdateState):
         """Test applying a VIM strategy that raises an exception"""
 
         # first api query is before the apply
-        self.vim_client.get_strategy.return_value = STRATEGY_READY_TO_APPLY
+        self.vim_client.get_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_READY_TO_APPLY)
 
         # raise an exception during apply_strategy
         self.vim_client.apply_strategy.side_effect =\
@@ -93,10 +85,12 @@ class TestFwUpdateApplyingVIMStrategyStage(TestFwUpdateState):
         """Test applying a VIM strategy that returns a failed result"""
 
         # first api query is before the apply
-        self.vim_client.get_strategy.return_value = STRATEGY_READY_TO_APPLY
+        self.vim_client.get_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_READY_TO_APPLY)
 
         # return a failed strategy
-        self.vim_client.apply_strategy.return_value = STRATEGY_APPLY_FAILED
+        self.vim_client.apply_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_APPLY_FAILED)
 
         # invoke the strategy state operation on the orch thread
         self.worker.perform_state_action(self.strategy_step)
@@ -110,13 +104,14 @@ class TestFwUpdateApplyingVIMStrategyStage(TestFwUpdateState):
 
         # first api query is before the apply
         self.vim_client.get_strategy.side_effect = [
-            STRATEGY_READY_TO_APPLY,
+            self._create_fake_strategy(vim.STATE_READY_TO_APPLY),
-            STRATEGY_APPLYING,
+            self._create_fake_strategy(vim.STATE_APPLYING),
-            STRATEGY_APPLY_FAILED,
+            self._create_fake_strategy(vim.STATE_APPLY_FAILED),
         ]
 
         # API calls acts as expected
-        self.vim_client.apply_strategy.return_value = STRATEGY_APPLYING
+        self.vim_client.apply_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_APPLYING)
 
         # invoke the strategy state operation on the orch thread
         self.worker.perform_state_action(self.strategy_step)
@@ -131,10 +126,12 @@ class TestFwUpdateApplyingVIMStrategyStage(TestFwUpdateState):
         # first api query is before the apply
         # test where it never progresses past 'applying'
         self.vim_client.get_strategy.side_effect = itertools.chain(
-            [STRATEGY_READY_TO_APPLY, ], itertools.repeat(STRATEGY_APPLYING))
+            [self._create_fake_strategy(vim.STATE_READY_TO_APPLY), ],
+            itertools.repeat(self._create_fake_strategy(vim.STATE_APPLYING)))
 
         # API calls acts as expected
-        self.vim_client.apply_strategy.return_value = STRATEGY_APPLYING
+        self.vim_client.apply_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_APPLYING)
 
         # invoke the strategy state operation on the orch thread
         self.worker.perform_state_action(self.strategy_step)
@@ -153,8 +150,8 @@ class TestFwUpdateApplyingVIMStrategyStage(TestFwUpdateState):
         # first api query is what already exists in applying state
         # remainder are during the loop
         self.vim_client.get_strategy.side_effect = [
-            STRATEGY_APPLYING,
+            self._create_fake_strategy(vim.STATE_APPLYING),
-            STRATEGY_APPLIED,
+            self._create_fake_strategy(vim.STATE_APPLIED),
         ]
 
         # invoke the strategy state operation on the orch thread
@@ -173,7 +170,7 @@ class TestFwUpdateApplyingVIMStrategyStage(TestFwUpdateState):
         # first api query is what already exists
         # remainder are during the loop
         self.vim_client.get_strategy.side_effect = [
-            STRATEGY_APPLY_FAILED,
+            self._create_fake_strategy(vim.STATE_APPLY_FAILED),
         ]
 
         # invoke the strategy state operation on the orch thread
@@ -185,3 +182,92 @@ class TestFwUpdateApplyingVIMStrategyStage(TestFwUpdateState):
         # Failure case
         self.assert_step_updated(self.strategy_step.subcloud_id,
                                  consts.STRATEGY_STATE_FAILED)
+
+    def test_applying_vim_strategy_skips_without_subcloud_strategy(self):
+        """Test applying a VIM strategy skips when there isn't a strategy to apply"""
+
+        self.vim_client.get_strategy.return_value = None
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.vim_client.apply_strategy.assert_not_called()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id,
+            consts.STRATEGY_STATE_FINISHING_FW_UPDATE
+        )
+
+    @mock.patch.object(BaseState, 'stopped', return_value=True)
+    def test_applying_vim_strategy_fails_when_strategy_stops(self, _):
+        """Test applying a VIM strategy fails when strategy stops"""
+
+        self.vim_client.get_strategy.side_effect = [
+            self._create_fake_strategy(vim.STATE_READY_TO_APPLY)
+        ]
+
+        self.vim_client.apply_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_APPLYING)
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )
+
+    def test_applying_vim_strategy_fails_on_max_failed_queries(self):
+        """Test applying a VIM strategy fails when max_failed_queries is reached
+
+        In this case, the DEFAULT_MAX_WAIT_ATTEMPTS must be greater than
+        DEFAULT_MAX_FAILED_QUERIES in order to throw the correct exception
+        """
+
+        self.vim_client.get_strategy.side_effect = [
+            self._create_fake_strategy(vim.STATE_READY_TO_APPLY),
+            self._create_fake_strategy(vim.STATE_APPLYING),
+            Exception(),
+            Exception(),
+            Exception()
+        ]
+
+        self.vim_client.apply_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_APPLYING)
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )
+
+    def test_applying_vim_strategy_fails_when_second_subcloud_strategy_is_none(self):
+        """Test applying a VIM strategy fails without second subcloud strategy"""
+
+        self.vim_client.get_strategy.side_effect = [
+            self._create_fake_strategy(vim.STATE_READY_TO_APPLY),
+            None
+        ]
+
+        self.vim_client.apply_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_APPLYING)
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )
+
+    def test_applying_vim_strategy_fails_with_invalid_strategy(self):
+        """Test applying a VIM strategy fails with an invalid strategy"""
+
+        self.vim_client.get_strategy.side_effect = [
+            self._create_fake_strategy(vim.STATE_READY_TO_APPLY),
+            self._create_fake_strategy(vim.STATE_ABORTED),
+        ]
+
+        self.vim_client.apply_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_APPLYING)
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )

distributedcloud/dcmanager/tests/unit/orchestrator/states/firmware/test_base.py

@@ -1,9 +1,17 @@
 #
-# Copyright (c) 2020 Wind River Systems, Inc.
+# Copyright (c) 2020, 2024 Wind River Systems, Inc.
 #
 # SPDX-License-Identifier: Apache-2.0
 #
+
+import uuid
+
 from dcmanager.common import consts
+from dcmanager.tests.unit.audit.test_firmware_audit_manager import DeviceImageState
+from dcmanager.tests.unit.fakes import FakeVimStrategy
+from dcmanager.tests.unit.orchestrator.states.fakes import FakeDevice
+from dcmanager.tests.unit.orchestrator.states.fakes import FakeDeviceImage
+from dcmanager.tests.unit.orchestrator.states.fakes import FakeDeviceLabel
 from dcmanager.tests.unit.orchestrator.test_base import TestSwUpdate
 
 
@@ -14,4 +22,40 @@ class TestFwUpdateState(TestSwUpdate):
     DEFAULT_STRATEGY_TYPE = consts.SW_UPDATE_TYPE_FIRMWARE
 
     def setUp(self):
-        super(TestFwUpdateState, self).setUp()
+        super().setUp()
+
+    def _create_fake_strategy(self, state):
+        return FakeVimStrategy(state=state)
+
+    def _create_fake_device(self, pvendor_id, pdevice_id, enabled=True):
+        return FakeDevice(
+            str(uuid.uuid4()),
+            pvendor_id=pvendor_id,
+            pdevice_id=pdevice_id,
+            enabled=enabled
+        )
+
+    def _create_fake_device_label(self, label_key, label_value, pcidevice_uuid):
+        return FakeDeviceLabel(
+            label_key=label_key,
+            label_value=label_value,
+            pcidevice_uuid=pcidevice_uuid
+        )
+
+    def _create_fake_device_image(
+        self, pci_vendor, pci_device, applied, applied_labels
+    ):
+        return FakeDeviceImage(
+            str(uuid.uuid4()),
+            pci_vendor=pci_vendor,
+            pci_device=pci_device,
+            applied=applied,
+            applied_labels=applied_labels
+        )
+
+    def _create_fake_device_image_state(self, pcidevice_uuid, image_uuid, status):
+        return DeviceImageState(
+            pcidevice_uuid,
+            image_uuid,
+            status
+        )

distributedcloud/dcmanager/tests/unit/orchestrator/states/firmware/test_creating_vim_strategy.py

@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2020, 2022 Wind River Systems, Inc.
+# Copyright (c) 2020, 2022, 2024 Wind River Systems, Inc.
 #
 # SPDX-License-Identifier: Apache-2.0
 #
@@ -9,16 +9,11 @@ import mock
 
 from dccommon.drivers.openstack import vim
 from dcmanager.common import consts
+from dcmanager.orchestrator.states.base import BaseState
 from dcmanager.orchestrator.states.firmware import creating_vim_strategy
-
-from dcmanager.tests.unit.fakes import FakeVimStrategy
 from dcmanager.tests.unit.orchestrator.states.firmware.test_base \
     import TestFwUpdateState
 
-STRATEGY_BUILDING = FakeVimStrategy(state=vim.STATE_BUILDING)
-STRATEGY_DONE_BUILDING = FakeVimStrategy(state=vim.STATE_READY_TO_APPLY)
-STRATEGY_FAILED_BUILDING = FakeVimStrategy(state=vim.STATE_BUILD_FAILED)
-
 
 @mock.patch("dcmanager.orchestrator.states.firmware.creating_vim_strategy."
             "DEFAULT_MAX_QUERIES", 3)
@@ -52,12 +47,13 @@ class TestFwUpdateCreatingVIMStrategyStage(TestFwUpdateState):
         # remaining api query results are waiting for the strategy to build
         self.vim_client.get_strategy.side_effect = [
             None,
-            STRATEGY_BUILDING,
+            self._create_fake_strategy(vim.STATE_BUILDING),
-            STRATEGY_DONE_BUILDING,
+            self._create_fake_strategy(vim.STATE_READY_TO_APPLY),
         ]
 
         # API calls acts as expected
-        self.vim_client.create_strategy.return_value = STRATEGY_BUILDING
+        self.vim_client.create_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_BUILDING)
 
         # invoke the strategy state operation on the orch thread
         self.worker.perform_state_action(self.strategy_step)
@@ -90,7 +86,8 @@ class TestFwUpdateCreatingVIMStrategyStage(TestFwUpdateState):
         self.vim_client.get_strategy.return_value = None
 
         # return a failed strategy
-        self.vim_client.create_strategy.return_value = STRATEGY_FAILED_BUILDING
+        self.vim_client.create_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_BUILD_FAILED)
 
         # invoke the strategy state operation on the orch thread
         self.worker.perform_state_action(self.strategy_step)
@@ -105,12 +102,13 @@ class TestFwUpdateCreatingVIMStrategyStage(TestFwUpdateState):
         # first api query is before the create
         self.vim_client.get_strategy.side_effect = [
             None,
-            STRATEGY_BUILDING,
+            self._create_fake_strategy(vim.STATE_BUILDING),
-            STRATEGY_FAILED_BUILDING,
+            self._create_fake_strategy(vim.STATE_BUILD_FAILED),
         ]
 
         # API calls acts as expected
-        self.vim_client.create_strategy.return_value = STRATEGY_BUILDING
+        self.vim_client.create_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_BUILDING)
 
         # invoke the strategy state operation on the orch thread
         self.worker.perform_state_action(self.strategy_step)
@@ -124,10 +122,13 @@ class TestFwUpdateCreatingVIMStrategyStage(TestFwUpdateState):
 
         # first api query is before the create
         self.vim_client.get_strategy.side_effect = itertools.chain(
-            [None, ], itertools.repeat(STRATEGY_BUILDING))
+            [None, ],
+            itertools.repeat(self._create_fake_strategy(vim.STATE_BUILDING))
+        )
 
         # API calls acts as expected
-        self.vim_client.create_strategy.return_value = STRATEGY_BUILDING
+        self.vim_client.create_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_BUILDING)
 
         # invoke the strategy state operation on the orch thread
         self.worker.perform_state_action(self.strategy_step)
@@ -148,12 +149,16 @@ class TestFwUpdateCreatingVIMStrategyStage(TestFwUpdateState):
         # and a new one recreated
         # remainder are during the loop
         self.vim_client.get_strategy.side_effect = [
-            STRATEGY_FAILED_BUILDING,  # old strategy that gets deleted
+            # old strategy that gets deleted
-            STRATEGY_BUILDING,  # new strategy gets built
+            self._create_fake_strategy(vim.STATE_BUILD_FAILED),
-            STRATEGY_DONE_BUILDING,  # new strategy succeeds during while loop
+            # new strategy gets built
+            self._create_fake_strategy(vim.STATE_BUILDING),
+            # new strategy succeeds during while loop
+            self._create_fake_strategy(vim.STATE_READY_TO_APPLY),
         ]
         # The strategy should be deleted and then created
-        self.vim_client.create_strategy.return_value = STRATEGY_BUILDING
+        self.vim_client.create_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_BUILDING)
 
         # invoke the strategy state operation on the orch thread
         self.worker.perform_state_action(self.strategy_step)
@@ -173,7 +178,7 @@ class TestFwUpdateCreatingVIMStrategyStage(TestFwUpdateState):
         # If it is building,aborting or applying it does not get deleted
         # and the strategy goes to failed state
         self.vim_client.get_strategy.side_effect = [
-            STRATEGY_BUILDING,
+            self._create_fake_strategy(vim.STATE_BUILDING),
         ]
 
         # invoke the strategy state operation on the orch thread
@@ -185,3 +190,54 @@ class TestFwUpdateCreatingVIMStrategyStage(TestFwUpdateState):
         # Failure case
         self.assert_step_updated(self.strategy_step.subcloud_id,
                                  consts.STRATEGY_STATE_FAILED)
+
+    @mock.patch.object(BaseState, 'stopped', return_value=True)
+    def test_creating_vim_strategy_fails_with_strategy_stop(self, _):
+        """Test creating a VIM strategy fails when strategy stops"""
+
+        self.vim_client.get_strategy.side_effect = [None]
+
+        self.vim_client.create_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_BUILDING)
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )
+
+    def test_creating_vim_strategy_fails_with_build_timeout_strategy(self):
+        """Test creating a VIM strategy fails when strategy is build timeout"""
+
+        self.vim_client.get_strategy.side_effect = [
+            None,
+            self._create_fake_strategy(vim.STATE_BUILDING),
+            self._create_fake_strategy(vim.STATE_BUILD_TIMEOUT)
+        ]
+
+        self.vim_client.create_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_BUILDING)
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )
+
+    def test_creating_vim_strategy_fails_with_invalid_strategy(self):
+        """Test creating a VIM strategy fails when strategy is aborted"""
+
+        self.vim_client.get_strategy.side_effect = [
+            None,
+            self._create_fake_strategy(vim.STATE_BUILDING),
+            self._create_fake_strategy(vim.STATE_ABORTED)
+        ]
+
+        self.vim_client.create_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_BUILDING)
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )

distributedcloud/dcmanager/tests/unit/orchestrator/states/firmware/test_finishing_vim_strategy.py

@@ -4,17 +4,19 @@
 # SPDX-License-Identifier: Apache-2.0
 #
 
+
 import mock
 
 from dccommon.drivers.openstack import vim
 from dcmanager.common import consts
+from dcmanager.orchestrator.states.base import BaseState
 from dcmanager.orchestrator.states.firmware import finishing_fw_update
+from dcmanager.tests.unit.orchestrator.states.fakes import FakeController
+from dcmanager.tests.unit.orchestrator.states.firmware.test_base \
+    import TestFwUpdateState
 
-from dcmanager.tests.unit.fakes import FakeVimStrategy
+VENDOR_ID = '1'
-from dcmanager.tests.unit.orchestrator.states.firmware.test_base import \
+DEVICE_ID = '2'
-    TestFwUpdateState
-
-STRATEGY_APPLIED = FakeVimStrategy(state=vim.STATE_APPLIED)
 
 
 @mock.patch("dcmanager.orchestrator.states.firmware."
@@ -26,6 +28,8 @@ class TestFwUpdateFinishingFwUpdateStage(TestFwUpdateState):
     def setUp(self):
         super(TestFwUpdateFinishingFwUpdateStage, self).setUp()
 
+        self._mock_rpc_subcloud_state_client()
+
         # set the next state in the chain (when this state is successful)
         self.on_success_state = consts.STRATEGY_STATE_COMPLETE
 
@@ -34,33 +38,51 @@ class TestFwUpdateFinishingFwUpdateStage(TestFwUpdateState):
 
         # Add the strategy_step state being processed by this unit test
         self.strategy_step = self.setup_strategy_step(
-            self.subcloud.id, consts.STRATEGY_STATE_FINISHING_FW_UPDATE)
+            self.subcloud.id, consts.STRATEGY_STATE_FINISHING_FW_UPDATE
+        )
 
-        # Add mock API endpoints for sysinv client calls invcked by this state
+        # Add mock API endpoints for sysinv client calls invocked by this state
         self.vim_client.get_strategy = mock.MagicMock()
         self.vim_client.delete_strategy = mock.MagicMock()
         self.sysinv_client.get_hosts = mock.MagicMock()
         self.sysinv_client.get_host_device_list = mock.MagicMock()
+        self.sysinv_client.get_device_images = mock.MagicMock()
+        self.sysinv_client.get_device_image_states = mock.MagicMock()
 
-        p = mock.patch.object(finishing_fw_update.FinishingFwUpdateState,
+        # Create fake variables to be used in sysinv_client methods
-                              'align_subcloud_status')
+        self.fake_host = FakeController()
-        self.mock_align = p.start()
+
-        self.addCleanup(p.stop)
+        self.fake_device = self._create_fake_device(VENDOR_ID, DEVICE_ID)
+
+        self.fake_device_image = self._create_fake_device_image(
+            VENDOR_ID, DEVICE_ID, True, {}
+        )
+
+        self.fake_device_image_state = self._create_fake_device_image_state(
+            self.fake_device.uuid,
+            self.fake_device_image.uuid,
+            'completed'
+        )
 
     def test_finishing_vim_strategy_success(self):
-        """Test finishing the firmware update."""
+        """Test finishing the firmware update.
+
+        In this case, there aren't enabled host devices, leaving the execution early
+        """
 
         # this tests successful steps of:
         # - vim strategy exists on subcloud and can be deleted
         # - no device image states on the subcloud are 'failed'
-        self.vim_client.get_strategy.return_value = STRATEGY_APPLIED
+        self.vim_client.get_strategy.return_value = \
+            self._create_fake_strategy(vim.STATE_APPLIED)
 
         # invoke the strategy state operation on the orch thread
         self.worker.perform_state_action(self.strategy_step)
 
         # Successful promotion to next state
-        self.assert_step_updated(self.strategy_step.subcloud_id,
+        self.assert_step_updated(
-                                 self.on_success_state)
+            self.strategy_step.subcloud_id, self.on_success_state
+        )
 
     def test_finishing_vim_strategy_success_no_strategy(self):
         """Test finishing the firmware update.
@@ -80,8 +102,9 @@ class TestFwUpdateFinishingFwUpdateStage(TestFwUpdateState):
         self.vim_client.delete_strategy.assert_not_called()
 
         # Successful promotion to next state
-        self.assert_step_updated(self.strategy_step.subcloud_id,
+        self.assert_step_updated(
-                                 self.on_success_state)
+            self.strategy_step.subcloud_id, self.on_success_state
+        )
 
     def test_finishing_vim_strategy_failure_get_hosts(self):
         """Test finishing firmware update with communication error to subcloud"""
@@ -106,3 +129,151 @@ class TestFwUpdateFinishingFwUpdateStage(TestFwUpdateState):
         # verify that the state moves to the next state
         self.assert_step_updated(self.strategy_step.subcloud_id,
                                  consts.STRATEGY_STATE_FAILED)
+
+    @mock.patch.object(BaseState, 'stopped', return_value=True)
+    def test_finishing_fw_update_fails_when_strategy_stops(self, _):
+        """Test finishing fw update fails when strategy stops before acquiring
+
+        host device
+        """
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )
+
+    def test_finishing_fw_update_succeeds_with_enabled_host_device(self):
+        """Test finishing fw update succeeds with an enabled host device"""
+
+        self.sysinv_client.get_hosts.return_value = [self.fake_host]
+        self.sysinv_client.get_host_device_list.return_value = [self.fake_device]
+        self.sysinv_client.get_device_images.return_value = [self.fake_device_image]
+        self.sysinv_client.get_device_image_states.return_value = [
+            self.fake_device_image_state
+        ]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.get_hosts.assert_called_once()
+        self.sysinv_client.get_host_device_list.assert_called_once()
+        self.sysinv_client.get_device_images.assert_called_once()
+        self.sysinv_client.get_device_image_states.assert_called_once()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, self.on_success_state
+        )
+
+    def test_finishing_fw_update_succeeds_with_host_device_disabled(self):
+        """Test finishing fw update succeeds with a device disabled"""
+        self.fake_device.enabled = False
+
+        self.sysinv_client.get_hosts.return_value = [self.fake_host]
+        self.sysinv_client.get_host_device_list.return_value = [self.fake_device]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.get_hosts.assert_called_once()
+        self.sysinv_client.get_host_device_list.assert_called_once()
+        self.sysinv_client.get_device_images.assert_not_called()
+        self.sysinv_client.get_device_image_states.assert_not_called()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, self.on_success_state
+        )
+
+    @mock.patch.object(BaseState, 'stopped')
+    def test_finishing_fw_update_fails_when_strategy_stops_with_enabled_host_device(
+        self, mock_base_state
+    ):
+        """Test finishing fw update fails when strategy stops after acquiring
+
+        host device
+        """
+
+        mock_base_state.side_effect = [False, True]
+
+        self.sysinv_client.get_hosts.return_value = [self.fake_host]
+        self.sysinv_client.get_host_device_list.return_value = [self.fake_device]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.get_hosts.assert_called_once()
+        self.sysinv_client.get_host_device_list.assert_called_once()
+        self.sysinv_client.get_device_images.assert_not_called()
+        self.sysinv_client.get_device_image_states.assert_not_called()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )
+
+    def test_finishing_fw_update_fails_with_get_device_image_states_exception(self):
+        """Test finishing fw update fails when get_device_image_states raises
+
+        an Exception
+        """
+
+        self.sysinv_client.get_hosts.return_value = [self.fake_host]
+        self.sysinv_client.get_host_device_list.return_value = [self.fake_device]
+        self.sysinv_client.get_device_images.return_value = [self.fake_device_image]
+        self.sysinv_client.get_device_image_states.side_effect = Exception()
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.get_hosts.assert_called_once()
+        self.sysinv_client.get_host_device_list.assert_called_once()
+        self.assertEqual(
+            self.sysinv_client.get_device_images.call_count,
+            finishing_fw_update.DEFAULT_MAX_FAILED_QUERIES + 1
+        )
+        # TODO(rlima): update the code to fix the error where the call_count is
+        # always greater than the DEFAULT_MAX_FAILED_QUERIES
+        self.assertEqual(
+            self.sysinv_client.get_device_image_states.call_count,
+            finishing_fw_update.DEFAULT_MAX_FAILED_QUERIES + 1
+        )
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )
+
+    def test_finishing_fw_update_fails_with_pending_image_state(self):
+        """Test finishing fw update fails with pending image state
+
+        In this scenarion, there are three failed image states in order to cover all
+        possible outcomes in their validation:
+        - The first is complete with the status pending
+        - The second has the same status but its image is None
+        - The third has the same status but rs device is None
+        """
+
+        self.fake_device_image_state.status = 'pending'
+
+        fake_device_image_state_with_image_none = \
+            self._create_fake_device_image_state(
+                self.fake_device.uuid, None, 'pending'
+            )
+        fake_device_image_state_with_device_none = \
+            self._create_fake_device_image_state(
+                None, self.fake_device_image.uuid, 'pending'
+            )
+
+        self.sysinv_client.get_hosts.return_value = [self.fake_host]
+        self.sysinv_client.get_host_device_list.return_value = [self.fake_device]
+        self.sysinv_client.get_device_images.return_value = [self.fake_device_image]
+        self.sysinv_client.get_device_image_states.return_value = [
+            self.fake_device_image_state,
+            fake_device_image_state_with_image_none,
+            fake_device_image_state_with_device_none
+        ]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.get_hosts.assert_called_once()
+        self.sysinv_client.get_host_device_list.assert_called_once()
+        self.sysinv_client.get_device_images.assert_called_once()
+        self.sysinv_client.get_device_image_states.assert_called_once()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )

distributedcloud/dcmanager/tests/unit/orchestrator/states/firmware/test_importing_firmware.py

@@ -4,68 +4,57 @@
 # SPDX-License-Identifier: Apache-2.0
 #
 
-import uuid
-
 import mock
 
 from dcmanager.common import consts
 from dcmanager.tests.unit.orchestrator.states.fakes import FakeController
-from dcmanager.tests.unit.orchestrator.states.fakes import FakeDevice
+from dcmanager.tests.unit.orchestrator.states.firmware.test_base \
-from dcmanager.tests.unit.orchestrator.states.fakes import FakeDeviceImage
+    import TestFwUpdateState
-from dcmanager.tests.unit.orchestrator.states.fakes import FakeDeviceLabel
-from dcmanager.tests.unit.orchestrator.states.firmware.test_base import \
-    TestFwUpdateState
 
 VENDOR_1 = "1001"
 VENDOR_2 = "2002"
 VENDOR_3 = "3003"
 
-VENDOR_DEVICE_1 = "9009"
+VENDOR_DEVICE_1 = '9009'
+VENDOR_DEVICE_2 = '9009'
+VENDOR_DEVICE_3 = '9009'
 
 FAKE_SUBCLOUD_CONTROLLER = FakeController()
-FAKE_SUBCLOUD_DEVICE = FakeDevice(
+FAKE_ALL_LABEL = [{}]
-    str(uuid.uuid4()), pvendor_id=VENDOR_1, pdevice_id=VENDOR_DEVICE_1
-)
-FAKE_SUBCLOUD_LABEL = FakeDeviceLabel(
-    label_key="abc", label_value="123", pcidevice_uuid=FAKE_SUBCLOUD_DEVICE.uuid
-)
-FAKE_ALL_LABEL = [
-    {},
-]
-# These three enabled images are for three different devices
-FAKE_IMAGE_1 = FakeDeviceImage(
-    str(uuid.uuid4()),
-    pci_vendor=VENDOR_1,
-    pci_device=VENDOR_DEVICE_1,
-    applied=True,
-    applied_labels=FAKE_ALL_LABEL,
-)
-FAKE_IMAGE_2 = FakeDeviceImage(
-    str(uuid.uuid4()),
-    pci_vendor=VENDOR_2,
-    applied=True,
-    applied_labels=FAKE_ALL_LABEL,
-)
-FAKE_IMAGE_3 = FakeDeviceImage(
-    str(uuid.uuid4()),
-    pci_vendor=VENDOR_3,
-    applied=True,
-    applied_labels=FAKE_ALL_LABEL,
-)
-
-
-EMPTY_DEVICE_IMAGES = []
-THREE_DEVICE_IMAGES = [
-    FAKE_IMAGE_1,
-    FAKE_IMAGE_2,
-    FAKE_IMAGE_3,
-]
 
 
 class TestFwUpdateImportingFirmwareStage(TestFwUpdateState):
+
     def setUp(self):
         super(TestFwUpdateImportingFirmwareStage, self).setUp()
 
+        # Sets up the necessary variables for mocking
+        self.fake_device = self._create_fake_device(VENDOR_1, VENDOR_DEVICE_1)
+        fake_device_label = self._create_fake_device_label(
+            'fake key', 'fake label', self.fake_device.uuid
+        )
+        fake_device_image_from_vendor_1 = self._create_fake_device_image(
+            VENDOR_1, VENDOR_DEVICE_1, True, FAKE_ALL_LABEL
+        )
+        fake_device_image_from_vendor_2 = self._create_fake_device_image(
+            VENDOR_2, VENDOR_DEVICE_2, True, FAKE_ALL_LABEL
+        )
+        fake_device_image_from_vendor_3 = self._create_fake_device_image(
+            VENDOR_3, VENDOR_DEVICE_3, True, FAKE_ALL_LABEL
+        )
+        self.fake_device_image_list = [
+            fake_device_image_from_vendor_1,
+            fake_device_image_from_vendor_2,
+            fake_device_image_from_vendor_3
+        ]
+        self.empty_fake_device_image_list = []
+
+        self.fake_device_image = self._create_fake_device_image_state(
+            self.fake_device.uuid,
+            fake_device_image_from_vendor_1.uuid,
+            'completed'
+        )
+
         # set the next state in the chain (when this state is successful)
         self.on_success_state = consts.STRATEGY_STATE_CREATING_FW_UPDATE_STRATEGY
 
@@ -83,21 +72,18 @@ class TestFwUpdateImportingFirmwareStage(TestFwUpdateState):
         self.sysinv_client.apply_device_image = mock.MagicMock()
         self.sysinv_client.remove_device_image = mock.MagicMock()
         self.sysinv_client.upload_device_image = mock.MagicMock()
+
         # get_hosts is only called on subcloud
         self.sysinv_client.get_hosts = mock.MagicMock()
-        self.sysinv_client.get_hosts.return_value = [
+        self.sysinv_client.get_hosts.return_value = [FAKE_SUBCLOUD_CONTROLLER]
-            FAKE_SUBCLOUD_CONTROLLER,
+
-        ]
         # get_host_device_list is only called on subcloud
         self.sysinv_client.get_host_device_list = mock.MagicMock()
-        self.sysinv_client.get_host_device_list.return_value = [
+        self.sysinv_client.get_host_device_list.return_value = [self.fake_device]
-            FAKE_SUBCLOUD_DEVICE,
+
-        ]
         # the labels for the device on the subcloud
         self.sysinv_client.get_device_label_list = mock.MagicMock()
-        self.sysinv_client.get_device_label_list.return_value = [
+        self.sysinv_client.get_device_label_list.return_value = [fake_device_label]
-            FAKE_SUBCLOUD_LABEL,
-        ]
 
     def test_importing_firmware_empty_system_controller(self):
         """Test importing firmware step when system controller has no FW"""
@@ -105,8 +91,7 @@ class TestFwUpdateImportingFirmwareStage(TestFwUpdateState):
         # first query is system controller
         # second query is subcloud
         self.sysinv_client.get_device_images.side_effect = [
-            EMPTY_DEVICE_IMAGES,
+            self.empty_fake_device_image_list, self.fake_device_image_list
-            THREE_DEVICE_IMAGES,
         ]
 
         # invoke the strategy state operation on the orch thread
@@ -126,16 +111,14 @@ class TestFwUpdateImportingFirmwareStage(TestFwUpdateState):
             self.strategy_step.subcloud_id, self.on_success_state
         )
 
-    @mock.patch("os.path.isfile")
+    @mock.patch("os.path.isfile", return_value=True)
-    def test_importing_firmware_empty_subcloud(self, mock_isfile):
+    def test_importing_firmware_empty_subcloud(self, _):
         """Test importing firmware step when subcloud has no FW"""
 
-        mock_isfile.return_value = True
         # first query is system controller
         # second query is subcloud
         self.sysinv_client.get_device_images.side_effect = [
-            THREE_DEVICE_IMAGES,
+            self.fake_device_image_list, self.empty_fake_device_image_list
-            EMPTY_DEVICE_IMAGES,
         ]
 
         # invoke the strategy state operation on the orch thread
@@ -164,8 +147,7 @@ class TestFwUpdateImportingFirmwareStage(TestFwUpdateState):
         # second query is subcloud
         # Both are the same
         self.sysinv_client.get_device_images.side_effect = [
-            THREE_DEVICE_IMAGES,
+            self.fake_device_image_list, self.fake_device_image_list
-            THREE_DEVICE_IMAGES,
         ]
 
         # invoke the strategy state operation on the orch thread
@@ -179,3 +161,225 @@ class TestFwUpdateImportingFirmwareStage(TestFwUpdateState):
         self.assert_step_updated(
             self.strategy_step.subcloud_id, self.on_success_state
         )
+
+    def test_importing_firmware_succeeds_without_enabled_host_device_list(self):
+        """Test importing firmware succeeds without enabled host device list"""
+
+        self.sysinv_client.get_host_device_list.return_value = [
+            self._create_fake_device(VENDOR_2, VENDOR_DEVICE_2, False)
+        ]
+
+        self.sysinv_client.get_device_images.side_effect = [
+            self.empty_fake_device_image_list, self.fake_device_image_list
+        ]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.get_device_image_states.assert_not_called()
+        self.sysinv_client.upload_device_image.assert_not_called()
+        self.sysinv_client.apply_device_image.assert_not_called()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, self.on_success_state
+        )
+
+    @mock.patch('os.path.isfile', return_value=False)
+    def test_importing_firmware_fails_when_image_file_is_missing(self, _):
+        """Test importing firmware fails when image file is missing
+
+        The os_path_isfile should raise an Exception
+        """
+
+        self.sysinv_client.get_device_images.side_effect = [
+            self.fake_device_image_list, self.empty_fake_device_image_list
+        ]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.remove_device_image.assert_not_called()
+        self.sysinv_client.upload_device_image.assert_not_called()
+        self.sysinv_client.apply_device_image.assert_not_called()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, consts.STRATEGY_STATE_FAILED
+        )
+
+    @mock.patch('os.path.isfile', return_value=True)
+    def test_importing_firmware_succeeds_with_device_image_state_completed(self, _):
+        """Test importing firmware success with a device image state completed"""
+
+        self.sysinv_client.get_device_images.side_effect = [
+            self.fake_device_image_list, self.empty_fake_device_image_list
+        ]
+
+        self.sysinv_client.get_device_image_states.return_value = [
+            self.fake_device_image
+        ]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.remove_device_image.assert_not_called()
+        self.sysinv_client.apply_device_image.assert_not_called()
+        self.sysinv_client.upload_device_image.assert_called_once()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, self.on_success_state
+        )
+
+    @mock.patch('os.path.isfile', return_value=True)
+    def test_importing_firmware_succeeds_with_device_image_state_pending(self, _):
+        """Test importing firmware success with a device image state pending"""
+
+        self.sysinv_client.get_device_images.side_effect = [
+            self.fake_device_image_list, self.empty_fake_device_image_list
+        ]
+
+        self.fake_device_image.status = 'pending'
+
+        self.sysinv_client.get_device_image_states.return_value = [
+            self.fake_device_image
+        ]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.remove_device_image.assert_not_called()
+        self.sysinv_client.apply_device_image.assert_not_called()
+        self.sysinv_client.upload_device_image.assert_called_once()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, self.on_success_state
+        )
+
+    @mock.patch('os.path.isfile', return_value=True)
+    def test_importing_firmware_succeeds_with_applied_subcloud_images(self, _):
+        """Test importing firmware success with applied subcloudimages"""
+
+        fake_device_image_with_label = self._create_fake_device_image(
+            VENDOR_1, VENDOR_DEVICE_1, True, [{'fake label': 'fake value'}]
+        )
+
+        self.fake_device_image_list.append(fake_device_image_with_label)
+
+        self.sysinv_client.get_device_images.side_effect = [
+            self.empty_fake_device_image_list,
+            self.fake_device_image_list,
+        ]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.assertEqual(
+            self.sysinv_client.remove_device_image.call_count,
+            len(self.fake_device_image_list,)
+        )
+        self.sysinv_client.apply_device_image.assert_not_called()
+        self.sysinv_client.upload_device_image.assert_not_called()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, self.on_success_state
+        )
+
+    @mock.patch('os.path.isfile', return_value=True)
+    def test_importing_firmware_succeeds_without_subcloud_device_image_states(
+        self, _
+    ):
+        """Test importing firmware success without subcloud device image states
+
+        In this scenario, a device image with applied_labels should have them
+        applied to the device image
+        """
+
+        fake_device_image_with_label = self._create_fake_device_image(
+            VENDOR_1, VENDOR_DEVICE_1, True, [{'fake key': 'fake label'}]
+        )
+
+        self.sysinv_client.get_device_images.side_effect = [
+            [fake_device_image_with_label],
+            self.empty_fake_device_image_list
+        ]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.remove_device_image.assert_not_called()
+        self.sysinv_client.apply_device_image.assert_called_once()
+        self.sysinv_client.upload_device_image.assert_called_once()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, self.on_success_state
+        )
+
+    def test_importing_firmware_succeeds_with_device_image_without_label(self):
+        """Test importing firmware succeeds with device image without label
+
+        There are two different validations being done in this test case:
+        - Using a device image without applied labels
+        - Returning an empty device_label_list
+        Both conditions are validated in firmware/utils.py and result in the
+        device being None
+        """
+
+        fake_device_image_with_label = self._create_fake_device_image(
+            VENDOR_1, VENDOR_DEVICE_1, True, None
+        )
+        self.sysinv_client.get_device_label_list.return_value = []
+
+        self.sysinv_client.get_device_images.side_effect = [
+            [fake_device_image_with_label],
+            self.empty_fake_device_image_list
+        ]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.remove_device_image.assert_not_called()
+        self.sysinv_client.upload_device_image.assert_not_called()
+        self.sysinv_client.apply_device_image.assert_not_called()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, self.on_success_state
+        )
+
+    def test_importing_firmware_succeeds_with_device_inelegible(self):
+        """Test importing firmware succeeds with device image inalegible
+
+        When the device image is inelegible, the check_subcloud_device_has_image
+        method from utils returns None, exiting the execution successfully
+        """
+
+        fake_device_image_with_label = self._create_fake_device_image(
+            VENDOR_1, VENDOR_DEVICE_1, True, [{'fake label': 'fake value'}]
+        )
+
+        self.sysinv_client.get_device_images.side_effect = [
+            [fake_device_image_with_label],
+            self.empty_fake_device_image_list
+        ]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.remove_device_image.assert_not_called()
+        self.sysinv_client.upload_device_image.assert_not_called()
+        self.sysinv_client.apply_device_image.assert_not_called()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, self.on_success_state
+        )
+
+    def test_importing_firmware_succeeds_with_device_not_applied(self):
+        """Test importing firmware succeeds with device not applied"""
+
+        fake_device_image_with_label = self._create_fake_device_image(
+            VENDOR_1, VENDOR_DEVICE_1, False, None
+        )
+        self.sysinv_client.get_device_images.side_effect = [
+            [fake_device_image_with_label],
+            self.empty_fake_device_image_list
+        ]
+
+        self.worker.perform_state_action(self.strategy_step)
+
+        self.sysinv_client.remove_device_image.assert_not_called()
+        self.sysinv_client.upload_device_image.assert_not_called()
+        self.sysinv_client.apply_device_image.assert_not_called()
+
+        self.assert_step_updated(
+            self.strategy_step.subcloud_id, self.on_success_state
+        )

distributedcloud/dcmanager/tests/unit/orchestrator/states/test_creating_vim_strategy.py

@@ -1,16 +1,15 @@
 #
-# Copyright (c) 2020-2022 Wind River Systems, Inc.
+# Copyright (c) 2020-2022, 2024 Wind River Systems, Inc.
 #
 # SPDX-License-Identifier: Apache-2.0
 #
+
 import itertools
 import mock
 
-
 from dccommon.drivers.openstack import vim
 from dcmanager.common import consts
 from dcmanager.orchestrator.states import creating_vim_strategy
-
 from dcmanager.tests.unit.fakes import FakeVimStrategy
 from dcmanager.tests.unit.orchestrator.states.upgrade.test_base \
     import TestSwUpgradeState