Make orchestrator and resolve_update respect deployment.is_active (#1743

) Solves issue #1742 --------- Co-authored-by: Josh Kalderimis <[email protected]>
nerves-hub · Jan 8, 2025 · e915164 · e915164
1 parent 9f3ed29
commit e915164
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 4 deletions.
diff --git a/lib/nerves_hub/deployments/orchestrator.ex b/lib/nerves_hub/deployments/orchestrator.ex
@@ -15,6 +15,8 @@ defmodule NervesHub.Deployments.Orchestrator do
 
   alias NervesHub.Devices
   alias NervesHub.Devices.Device
+  alias NervesHub.Deployments
+  alias NervesHub.Deployments.Deployment
   alias NervesHub.Repo
   alias Phoenix.PubSub
   alias Phoenix.Socket.Broadcast
@@ -24,7 +26,7 @@ defmodule NervesHub.Deployments.Orchestrator do
   end
 
   def name(deployment_id) when is_integer(deployment_id) do
-    {:via, Registry, {NervesHub.Deployments, deployment_id}}
+    {:via, Registry, {Deployments, deployment_id}}
   end
 
   def name(deployment), do: name(deployment.id)
@@ -49,6 +51,11 @@ defmodule NervesHub.Deployments.Orchestrator do
   As devices update and reconnect, the new orchestrator is told that the update
   was successful, and the process is repeated.
   """
+  @decorate with_span("Deployments.Orchestrator.trigger_update#noop")
+  def trigger_update(%Deployment{is_active: false}) do
+    :ok
+  end
+
   @decorate with_span("Deployments.Orchestrator.trigger_update")
   def trigger_update(deployment) do
     :telemetry.execute([:nerves_hub, :deployment, :trigger_update], %{count: 1})
@@ -67,7 +74,7 @@ defmodule NervesHub.Deployments.Orchestrator do
     }
 
     devices =
-      Registry.select(NervesHub.Devices.Registry, [
+      Registry.select(Devices.Registry, [
         {{:_, :_, :"$1"}, match_conditions, [match_return]}
       ])
 

diff --git a/lib/nerves_hub/devices.ex b/lib/nerves_hub/devices.ex
@@ -833,6 +833,9 @@ defmodule NervesHub.Devices do
           deployment_id: deployment.id
         }
 
+      {:error, :deployment_not_active, _device} ->
+        %UpdatePayload{update_available: false}
+
       {:error, :up_to_date, _device} ->
         %UpdatePayload{update_available: false}
 
@@ -928,6 +931,9 @@ defmodule NervesHub.Devices do
 
   def verify_update_eligibility(device, deployment, now \\ DateTime.utc_now()) do
     cond do
+      not deployment.is_active ->
+        {:error, :deployment_not_active, device}
+
       device_matches_deployment?(device, deployment) ->
         {:error, :up_to_date, device}
 

diff --git a/test/nerves_hub/devices_test.exs b/test/nerves_hub/devices_test.exs
@@ -20,7 +20,7 @@ defmodule NervesHub.DevicesTest do
     product = Fixtures.product_fixture(user, org)
     org_key = Fixtures.org_key_fixture(org, user)
     firmware = Fixtures.firmware_fixture(org_key, product)
-    deployment = Fixtures.deployment_fixture(org, firmware)
+    deployment = Fixtures.deployment_fixture(org, firmware, %{is_active: true})
     device = Fixtures.device_fixture(org, product, firmware)
     device2 = Fixtures.device_fixture(org, product, firmware)
     device3 = Fixtures.device_fixture(org, product, firmware)

diff --git a/test/nerves_hub_web/live/devices/show_test.exs b/test/nerves_hub_web/live/devices/show_test.exs
@@ -374,7 +374,7 @@ defmodule NervesHubWeb.Live.Devices.ShowTest do
       firmware = Fixtures.firmware_fixture(org_key, product, %{dir: tmp_dir})
 
       deployment
-      |> Ecto.Changeset.change(%{firmware_id: firmware.id})
+      |> Ecto.Changeset.change(%{firmware_id: firmware.id, is_active: true})
       |> Repo.update!()
 
       NervesHubWeb.Endpoint.subscribe("device:#{device.id}")
@@ -390,6 +390,36 @@ defmodule NervesHubWeb.Live.Devices.ShowTest do
 
       assert_receive %Phoenix.Socket.Broadcast{event: "deployments/update"}
     end
+
+    test "available update exists but deployment is not active", %{
+      conn: conn,
+      org: org,
+      product: product,
+      device: device,
+      deployment: deployment,
+      org_key: org_key,
+      tmp_dir: tmp_dir
+    } do
+      device =
+        device
+        |> Ecto.Changeset.change(%{deployment_id: deployment.id})
+        |> Repo.update!()
+
+      firmware = Fixtures.firmware_fixture(org_key, product, %{dir: tmp_dir})
+
+      deployment
+      |> Ecto.Changeset.change(%{firmware_id: firmware.id, is_active: false})
+      |> Repo.update!()
+
+      NervesHubWeb.Endpoint.subscribe("device:#{device.id}")
+
+      conn
+      |> visit("/org/#{org.name}/#{product.name}/devices/#{device.identifier}")
+      |> assert_has("h1", text: device.identifier)
+      |> refute_has("span", text: "Update available")
+
+      assert Repo.aggregate(NervesHub.Devices.InflightUpdate, :count) == 0
+    end
   end
 
   describe "support scripts" do

diff --git a/test/support/fixtures.ex b/test/support/fixtures.ex
@@ -213,12 +213,16 @@ defmodule NervesHub.Fixtures do
   end
 
   def deployment_fixture(%Org{} = org, %Firmwares.Firmware{} = firmware, params \\ %{}) do
+    {is_active, params} = Map.pop(params, :is_active, false)
+
     {:ok, deployment} =
       %{org_id: org.id, firmware_id: firmware.id}
       |> Enum.into(params)
       |> Enum.into(@deployment_params)
       |> Deployments.create_deployment()
 
+    {:ok, deployment} = Deployments.update_deployment(deployment, %{is_active: is_active})
+
     deployment
   end