Handle software updates discovery errors (#2532)

* Add testing scenarios for host's health transition to unknown when software_updates_discovery_health goes unknown

* Handle software updates discovery errors by dispatching an unknown health

* Adjust software updates event handler tests

* Simplify dispatching on discovery error

lib/trento/software_updates/discovery.ex

@@ -88,12 +88,19 @@ defmodule Trento.SoftwareUpdates.Discovery do
            |> commanded().dispatch() do
       {:ok, host_id, system_id, relevant_patches}
     else
-      error ->
+      {:error, discovery_error} = error ->
         Logger.error(
           "An error occurred during software updates discovery for host #{host_id}:  #{inspect(error)}"
         )
 
-        {:error, error}
+        commanded().dispatch(
+          CompleteSoftwareUpdatesDiscovery.new!(%{
+            host_id: host_id,
+            health: SoftwareUpdatesHealth.unknown()
+          })
+        )
+
+        {:error, discovery_error}
     end
   end
 

test/trento/hosts/host_test.exs

@@ -1736,6 +1736,24 @@ defmodule Trento.Hosts.HostTest do
           initial_host_health: Health.critical(),
           software_updates_discovery_health: SoftwareUpdatesHealth.passing(),
           expected_host_health: Health.passing()
+        },
+        %{
+          initial_host_health: Health.passing(),
+          software_updates_discovery_health: SoftwareUpdatesHealth.unknown(),
+          expected_host_health: Health.unknown()
+        },
+        %{
+          initial_host_health: Health.critical(),
+          initial_heartbeat: :heartbeat_failed,
+          software_updates_discovery_health: SoftwareUpdatesHealth.unknown(),
+          expected_host_health: Health.unknown()
+        },
+        %{
+          initial_host_health: Health.unknown(),
+          initial_heartbeat: :heartbeat_failed,
+          software_updates_discovery_health: SoftwareUpdatesHealth.unknown(),
+          expect_host_health_changed: false,
+          expected_host_health: Health.unknown()
         }
       ]
 

test/trento/infrastructure/commanded/event_handlers/software_updates_discovery_event_handler_test.exs

@@ -11,6 +11,8 @@ defmodule Trento.Infrastructure.Commanded.EventHandlers.SoftwareUpdatesDiscovery
 
   alias Trento.Infrastructure.Commanded.EventHandlers.SoftwareUpdatesDiscoveryEventHandler
 
+  require Trento.SoftwareUpdates.Enums.SoftwareUpdatesHealth, as: SoftwareUpdatesHealth
+
   setup [:set_mox_from_context, :verify_on_exit!]
 
   test "should discover software updates when a SoftwareUpdatesDiscoveryRequested is emitted" do
@@ -63,8 +65,11 @@ defmodule Trento.Infrastructure.Commanded.EventHandlers.SoftwareUpdatesDiscovery
     expect(
       Trento.Commanded.Mock,
       :dispatch,
-      0,
-      fn _ -> :ok end
+      fn %CompleteSoftwareUpdatesDiscovery{
+           health: SoftwareUpdatesHealth.unknown()
+         } ->
+        :ok
+      end
     )
 
     assert :ok = SoftwareUpdatesDiscoveryEventHandler.handle(event, %{})

test/trento/software_updates/discovery_test.exs

@@ -31,9 +31,9 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
       host_id = Faker.UUID.v4()
       fully_qualified_domain_name = Faker.Internet.domain_name()
 
-      discovery_error = {:error, :some_error_while_getting_system_id}
+      discovery_error = :some_error_while_getting_system_id
 
-      fail_on_getting_system_id(fully_qualified_domain_name, discovery_error)
+      fail_on_getting_system_id(host_id, fully_qualified_domain_name, discovery_error)
 
       {:error, ^discovery_error} =
         Discovery.discover_host_software_updates(host_id, fully_qualified_domain_name)
@@ -43,9 +43,14 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
       host_id = Faker.UUID.v4()
       fully_qualified_domain_name = Faker.Internet.domain_name()
       system_id = 100
-      discovery_error = {:error, :some_error_while_getting_relevant_patches}
+      discovery_error = :some_error_while_getting_relevant_patches
 
-      fail_on_getting_relevant_patches(fully_qualified_domain_name, system_id, discovery_error)
+      fail_on_getting_relevant_patches(
+        host_id,
+        fully_qualified_domain_name,
+        system_id,
+        discovery_error
+      )
 
       {:error, ^discovery_error} =
         Discovery.discover_host_software_updates(host_id, fully_qualified_domain_name)
@@ -55,7 +60,7 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
       host_id = Faker.UUID.v4()
       fully_qualified_domain_name = Faker.Internet.domain_name()
       system_id = 100
-      dispatching_error = {:error, :error_while_dispatching_completion_command}
+      dispatching_error = :error_while_dispatching_completion_command
 
       fail_on_dispatching_completion_command(
         host_id,
@@ -160,9 +165,9 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
     test "should handle errors when getting a system id" do
       %{id: host_id, fully_qualified_domain_name: fully_qualified_domain_name} = insert(:host)
 
-      discovery_error = {:error, :some_error_while_getting_system_id}
+      discovery_error = :some_error_while_getting_system_id
 
-      fail_on_getting_system_id(fully_qualified_domain_name, discovery_error)
+      fail_on_getting_system_id(host_id, fully_qualified_domain_name, discovery_error)
 
       {:ok, {[], errored_discoveries}} = Discovery.discover_software_updates()
 
@@ -175,7 +180,12 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
       system_id = 100
       discovery_error = {:error, :some_error_while_getting_relevant_patches}
 
-      fail_on_getting_relevant_patches(fully_qualified_domain_name, system_id, discovery_error)
+      fail_on_getting_relevant_patches(
+        host_id,
+        fully_qualified_domain_name,
+        system_id,
+        discovery_error
+      )
 
       {:ok, {[], errored_discoveries}} = Discovery.discover_software_updates()
 
@@ -187,7 +197,7 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
 
       system_id = 100
 
-      dispatching_error = {:error, :error_while_dispatching_completion_command}
+      dispatching_error = :error_while_dispatching_completion_command
 
       fail_on_dispatching_completion_command(
         host_id,
@@ -236,16 +246,18 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
         system_id3 => {:ok, discovered_relevant_patches}
       }
 
-      expected_software_updates_health = SoftwareUpdatesHealth.critical()
-
       expected_commands = %{
         host_id1 => %CompleteSoftwareUpdatesDiscovery{
           host_id: host_id1,
-          health: expected_software_updates_health
+          health: SoftwareUpdatesHealth.critical()
+        },
+        host_id2 => %CompleteSoftwareUpdatesDiscovery{
+          host_id: host_id2,
+          health: SoftwareUpdatesHealth.unknown()
         },
         host_id3 => %CompleteSoftwareUpdatesDiscovery{
           host_id: host_id3,
-          health: expected_software_updates_health
+          health: SoftwareUpdatesHealth.critical()
         }
       }
 
@@ -272,8 +284,8 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
       expect(
         Trento.Commanded.Mock,
         :dispatch,
-        2,
-        fn %{host_id: host_id} = command ->
+        3,
+        fn %CompleteSoftwareUpdatesDiscovery{host_id: host_id} = command ->
           assert Map.get(expected_commands, host_id) == command
 
           :ok
@@ -292,7 +304,7 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
              ] ==
                successful_discoveries
 
-      assert {:error, host_id2, {:error, :some_error}} in errored_discoveries
+      assert {:error, host_id2, :some_error} in errored_discoveries
       assert {:error, host_id4, :host_without_fqdn} in errored_discoveries
     end
   end
@@ -329,11 +341,11 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
     end
   end
 
-  defp fail_on_getting_system_id(fully_qualified_domain_name, discovery_error) do
+  defp fail_on_getting_system_id(host_id, fully_qualified_domain_name, discovery_error) do
     expect(
       SoftwareUpdatesDiscoveryMock,
       :get_system_id,
-      fn ^fully_qualified_domain_name -> discovery_error end
+      fn ^fully_qualified_domain_name -> {:error, discovery_error} end
     )
 
     expect(
@@ -346,12 +358,21 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
     expect(
       Trento.Commanded.Mock,
       :dispatch,
-      0,
-      fn _ -> :ok end
+      fn %CompleteSoftwareUpdatesDiscovery{
+           host_id: ^host_id,
+           health: SoftwareUpdatesHealth.unknown()
+         } ->
+        :ok
+      end
     )
   end
 
-  defp fail_on_getting_relevant_patches(fully_qualified_domain_name, system_id, discovery_error) do
+  defp fail_on_getting_relevant_patches(
+         host_id,
+         fully_qualified_domain_name,
+         system_id,
+         discovery_error
+       ) do
     expect(
       SoftwareUpdatesDiscoveryMock,
       :get_system_id,
@@ -361,14 +382,18 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
     expect(
       SoftwareUpdatesDiscoveryMock,
       :get_relevant_patches,
-      fn ^system_id -> discovery_error end
+      fn ^system_id -> {:error, discovery_error} end
     )
 
     expect(
       Trento.Commanded.Mock,
       :dispatch,
-      0,
-      fn _ -> :ok end
+      fn %CompleteSoftwareUpdatesDiscovery{
+           host_id: ^host_id,
+           health: SoftwareUpdatesHealth.unknown()
+         } ->
+        :ok
+      end
     )
   end
 
@@ -393,7 +418,23 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do
     expect(
       Trento.Commanded.Mock,
       :dispatch,
-      fn %CompleteSoftwareUpdatesDiscovery{host_id: ^host_id} -> dispatching_error end
+      fn %CompleteSoftwareUpdatesDiscovery{
+           host_id: ^host_id,
+           health: SoftwareUpdatesHealth.critical()
+         } ->
+        {:error, dispatching_error}
+      end
+    )
+
+    expect(
+      Trento.Commanded.Mock,
+      :dispatch,
+      fn %CompleteSoftwareUpdatesDiscovery{
+           host_id: ^host_id,
+           health: SoftwareUpdatesHealth.unknown()
+         } ->
+        :ok
+      end
     )
   end
 end