From aa227755935e39a1601aa9e701e2e75218e7ccc2 Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Thu, 18 Apr 2024 11:31:28 +0200 Subject: [PATCH 1/4] Add testing scenarios for host's health transition to unknown when software_updates_discovery_health goes unknown --- test/trento/hosts/host_test.exs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/trento/hosts/host_test.exs b/test/trento/hosts/host_test.exs index 638dfdd1b3..f702e161c4 100644 --- a/test/trento/hosts/host_test.exs +++ b/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() } ] From 0e1005b292b1921da470f8d266fc5eaf7a7a61a5 Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Thu, 18 Apr 2024 14:17:44 +0200 Subject: [PATCH 2/4] Handle software updates discovery errors by dispatching an unknown health --- lib/trento/software_updates/discovery.ex | 54 +++++--- .../software_updates/discovery_test.exs | 131 ++++++++++++++---- 2 files changed, 144 insertions(+), 41 deletions(-) diff --git a/lib/trento/software_updates/discovery.ex b/lib/trento/software_updates/discovery.ex index 874a775115..f56ddfdd3e 100644 --- a/lib/trento/software_updates/discovery.ex +++ b/lib/trento/software_updates/discovery.ex @@ -80,32 +80,54 @@ defmodule Trento.SoftwareUpdates.Discovery do end def discover_host_software_updates(host_id, fully_qualified_domain_name) do - with {:ok, system_id} <- get_system_id(fully_qualified_domain_name), - {:ok, relevant_patches} <- get_relevant_patches(system_id), - :ok <- - host_id - |> build_discovery_completion_command(relevant_patches) - |> commanded().dispatch() do - {:ok, host_id, system_id, relevant_patches} - else - error -> + case discover_relevant_patches(fully_qualified_domain_name) do + {:ok, system_id, relevant_patches} -> + determine_health_and_dispatch_completion_command(host_id, system_id, relevant_patches) + + {:error, discovery_error} = error -> Logger.error( "An error occurred during software updates discovery for host #{host_id}: #{inspect(error)}" ) - {:error, error} + dispatch_completion_command_on_discovery_error(host_id, discovery_error) end end - defp build_discovery_completion_command(host_id, relevant_patches), - do: + defp discover_relevant_patches(fully_qualified_domain_name) do + with {:ok, system_id} <- get_system_id(fully_qualified_domain_name), + {:ok, relevant_patches} <- get_relevant_patches(system_id) do + {:ok, system_id, relevant_patches} + end + end + + defp determine_health_and_dispatch_completion_command(host_id, system_id, relevant_patches) do + with :ok <- + relevant_patches + |> track_relevant_patches + |> compute_software_updates_discovery_health + |> dispatch_completion_command(host_id) do + {:ok, host_id, system_id, relevant_patches} + end + end + + defp dispatch_completion_command(discovered_software_updates_health, host_id) do + commanded().dispatch( CompleteSoftwareUpdatesDiscovery.new!(%{ host_id: host_id, - health: - relevant_patches - |> track_relevant_patches - |> compute_software_updates_discovery_health + health: discovered_software_updates_health }) + ) + end + + defp dispatch_completion_command_on_discovery_error(host_id, discovery_error) do + case dispatch_completion_command(SoftwareUpdatesHealth.unknown(), host_id) do + :ok -> + {:error, discovery_error} + + {:error, dispatching_error} -> + {:error, [discovery_error, dispatching_error]} + end + end defp track_relevant_patches(relevant_patches), do: diff --git a/test/trento/software_updates/discovery_test.exs b/test/trento/software_updates/discovery_test.exs index a85c191792..a641d5ffed 100644 --- a/test/trento/software_updates/discovery_test.exs +++ b/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, @@ -68,6 +73,57 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do Discovery.discover_host_software_updates(host_id, fully_qualified_domain_name) end + test "should handle failure when dispatching discovery completion command on discovery error" do + for discovery_error <- [:failing_get_system_id, :failing_get_relevant_patches] do + host_id = Faker.UUID.v4() + system_id = 100 + fully_qualified_domain_name = Faker.Internet.domain_name() + dispatching_error = :dispatching_error + + expect( + SoftwareUpdatesDiscoveryMock, + :get_system_id, + fn ^fully_qualified_domain_name -> + case discovery_error do + :failing_get_system_id -> {:error, discovery_error} + _ -> {:ok, system_id} + end + end + ) + + expected_get_relevant_patches_calls = + case discovery_error do + :failing_get_system_id -> 0 + _ -> 1 + end + + expect( + SoftwareUpdatesDiscoveryMock, + :get_relevant_patches, + expected_get_relevant_patches_calls, + fn _ -> + if discovery_error == :failing_get_relevant_patches do + {:error, discovery_error} + end + end + ) + + expect( + Trento.Commanded.Mock, + :dispatch, + fn %CompleteSoftwareUpdatesDiscovery{ + host_id: ^host_id, + health: SoftwareUpdatesHealth.unknown() + } -> + {:error, dispatching_error} + end + ) + + {:error, [^discovery_error, ^dispatching_error]} = + Discovery.discover_host_software_updates(host_id, fully_qualified_domain_name) + end + end + test "should complete discovery" do scenarios = [ %{ @@ -160,9 +216,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 +231,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 +248,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 +297,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 +335,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 +355,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 +392,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 +409,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 +433,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 +469,12 @@ 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 ) end end From 0307ce6aaa7c6b515b2336c76d33f95037d781d4 Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Thu, 18 Apr 2024 14:25:44 +0200 Subject: [PATCH 3/4] Adjust software updates event handler tests --- .../software_updates_discovery_event_handler_test.exs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/trento/infrastructure/commanded/event_handlers/software_updates_discovery_event_handler_test.exs b/test/trento/infrastructure/commanded/event_handlers/software_updates_discovery_event_handler_test.exs index fe0ff6864e..00464a785e 100644 --- a/test/trento/infrastructure/commanded/event_handlers/software_updates_discovery_event_handler_test.exs +++ b/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, %{}) From e67c703b0ce741d4cbaeb8c0e59d17fb9f6757fa Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Thu, 18 Apr 2024 16:00:44 +0200 Subject: [PATCH 4/4] Simplify dispatching on discovery error --- lib/trento/software_updates/discovery.ex | 57 +++++++---------- .../software_updates/discovery_test.exs | 62 ++++--------------- 2 files changed, 32 insertions(+), 87 deletions(-) diff --git a/lib/trento/software_updates/discovery.ex b/lib/trento/software_updates/discovery.ex index f56ddfdd3e..3ba77dc0fc 100644 --- a/lib/trento/software_updates/discovery.ex +++ b/lib/trento/software_updates/discovery.ex @@ -80,54 +80,39 @@ defmodule Trento.SoftwareUpdates.Discovery do end def discover_host_software_updates(host_id, fully_qualified_domain_name) do - case discover_relevant_patches(fully_qualified_domain_name) do - {:ok, system_id, relevant_patches} -> - determine_health_and_dispatch_completion_command(host_id, system_id, relevant_patches) - + with {:ok, system_id} <- get_system_id(fully_qualified_domain_name), + {:ok, relevant_patches} <- get_relevant_patches(system_id), + :ok <- + host_id + |> build_discovery_completion_command(relevant_patches) + |> commanded().dispatch() do + {:ok, host_id, system_id, relevant_patches} + else {:error, discovery_error} = error -> Logger.error( "An error occurred during software updates discovery for host #{host_id}: #{inspect(error)}" ) - dispatch_completion_command_on_discovery_error(host_id, discovery_error) - end - end - - defp discover_relevant_patches(fully_qualified_domain_name) do - with {:ok, system_id} <- get_system_id(fully_qualified_domain_name), - {:ok, relevant_patches} <- get_relevant_patches(system_id) do - {:ok, system_id, relevant_patches} - end - end + commanded().dispatch( + CompleteSoftwareUpdatesDiscovery.new!(%{ + host_id: host_id, + health: SoftwareUpdatesHealth.unknown() + }) + ) - defp determine_health_and_dispatch_completion_command(host_id, system_id, relevant_patches) do - with :ok <- - relevant_patches - |> track_relevant_patches - |> compute_software_updates_discovery_health - |> dispatch_completion_command(host_id) do - {:ok, host_id, system_id, relevant_patches} + {:error, discovery_error} end end - defp dispatch_completion_command(discovered_software_updates_health, host_id) do - commanded().dispatch( + defp build_discovery_completion_command(host_id, relevant_patches), + do: CompleteSoftwareUpdatesDiscovery.new!(%{ host_id: host_id, - health: discovered_software_updates_health + health: + relevant_patches + |> track_relevant_patches + |> compute_software_updates_discovery_health }) - ) - end - - defp dispatch_completion_command_on_discovery_error(host_id, discovery_error) do - case dispatch_completion_command(SoftwareUpdatesHealth.unknown(), host_id) do - :ok -> - {:error, discovery_error} - - {:error, dispatching_error} -> - {:error, [discovery_error, dispatching_error]} - end - end defp track_relevant_patches(relevant_patches), do: diff --git a/test/trento/software_updates/discovery_test.exs b/test/trento/software_updates/discovery_test.exs index a641d5ffed..52f70d1f30 100644 --- a/test/trento/software_updates/discovery_test.exs +++ b/test/trento/software_updates/discovery_test.exs @@ -73,57 +73,6 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do Discovery.discover_host_software_updates(host_id, fully_qualified_domain_name) end - test "should handle failure when dispatching discovery completion command on discovery error" do - for discovery_error <- [:failing_get_system_id, :failing_get_relevant_patches] do - host_id = Faker.UUID.v4() - system_id = 100 - fully_qualified_domain_name = Faker.Internet.domain_name() - dispatching_error = :dispatching_error - - expect( - SoftwareUpdatesDiscoveryMock, - :get_system_id, - fn ^fully_qualified_domain_name -> - case discovery_error do - :failing_get_system_id -> {:error, discovery_error} - _ -> {:ok, system_id} - end - end - ) - - expected_get_relevant_patches_calls = - case discovery_error do - :failing_get_system_id -> 0 - _ -> 1 - end - - expect( - SoftwareUpdatesDiscoveryMock, - :get_relevant_patches, - expected_get_relevant_patches_calls, - fn _ -> - if discovery_error == :failing_get_relevant_patches do - {:error, discovery_error} - end - end - ) - - expect( - Trento.Commanded.Mock, - :dispatch, - fn %CompleteSoftwareUpdatesDiscovery{ - host_id: ^host_id, - health: SoftwareUpdatesHealth.unknown() - } -> - {:error, dispatching_error} - end - ) - - {:error, [^discovery_error, ^dispatching_error]} = - Discovery.discover_host_software_updates(host_id, fully_qualified_domain_name) - end - end - test "should complete discovery" do scenarios = [ %{ @@ -476,5 +425,16 @@ defmodule Trento.SoftwareUpdates.DiscoveryTest do {:error, dispatching_error} end ) + + expect( + Trento.Commanded.Mock, + :dispatch, + fn %CompleteSoftwareUpdatesDiscovery{ + host_id: ^host_id, + health: SoftwareUpdatesHealth.unknown() + } -> + :ok + end + ) end end