From a098ec35f46ff485e6df770c853b7a9ae0859b69 Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Sat, 28 Dec 2024 12:33:11 +1300 Subject: [PATCH 1/2] Bring back deployment device matching when a device connects This fixes a bug which was introduced by accident. --- lib/nerves_hub/deployments.ex | 92 +++++++++++++++++++ lib/nerves_hub/logger.ex | 29 +++++- lib/nerves_hub_web/channels/device_channel.ex | 5 +- 3 files changed, 124 insertions(+), 2 deletions(-) diff --git a/lib/nerves_hub/deployments.ex b/lib/nerves_hub/deployments.ex index 1fb54bec2..84180492a 100644 --- a/lib/nerves_hub/deployments.ex +++ b/lib/nerves_hub/deployments.ex @@ -6,6 +6,7 @@ defmodule NervesHub.Deployments do alias NervesHub.AuditLogs alias NervesHub.Deployments.Deployment alias NervesHub.Deployments.InflightDeploymentCheck + alias NervesHub.Devices alias NervesHub.Devices.Device alias NervesHub.Products.Product alias NervesHub.Repo @@ -342,4 +343,95 @@ defmodule NervesHub.Deployments do end def verify_deployment_membership(device), do: device + + @doc """ + If the device is missing a deployment, find a matching deployment + + Do nothing if a deployment is already set + """ + def set_deployment(%{deployment_id: nil} = device) do + case matching_deployments(device, [true]) do + [] -> + set_deployment_telemetry(:none_found, device) + + %{device | deployment: nil} + + [deployment] -> + set_deployment_telemetry(:one_found, device, deployment) + + device + |> Devices.update_deployment(deployment) + |> preload_with_firmware_and_archive(true) + + [deployment | _] -> + set_deployment_telemetry(:multiple_found, device, deployment) + + device + |> Devices.update_deployment(deployment) + |> preload_with_firmware_and_archive(true) + end + end + + def set_deployment(device) do + preload_with_firmware_and_archive(device) + end + + defp set_deployment_telemetry(result, device, deployment \\ nil) do + metadata = %{device: device} + + metadata = + if deployment do + Map.put(metadata, :deployment, deployment) + else + metadata + end + + :telemetry.execute( + [:nerves_hub, :deployments, :set_deployment, result], + %{count: 1}, + metadata + ) + end + + def preload_with_firmware_and_archive(device, force \\ false) do + Repo.preload(device, [deployment: [:archive, :firmware]], force: force) + end + + @doc """ + Find all potential deployments for a device + + Based on the product, firmware platform, firmware architecture, and device tags + """ + def matching_deployments(device, active \\ [true, false]) + def matching_deployments(%Device{firmware_metadata: nil}, _active), do: [] + + def matching_deployments(device, active) do + Deployment + |> join(:inner, [d], assoc(d, :firmware), as: :firmware) + |> preload([_, firmware: f], firmware: f) + |> where([d], d.product_id == ^device.product_id) + |> where([d], d.is_active in ^active) + |> ignore_same_deployment(device) + |> where([d, firmware: f], f.platform == ^device.firmware_metadata.platform) + |> where([d, firmware: f], f.architecture == ^device.firmware_metadata.architecture) + |> where([d], fragment("?->'tags' <@ to_jsonb(?::text[])", d.conditions, ^device.tags)) + |> Repo.all() + |> Enum.filter(&version_match?(device, &1)) + |> Enum.sort_by( + &{&1.firmware.version, &1.id}, + fn {a_vsn, a_id}, {b_vsn, b_id} -> + case Version.compare(a_vsn, b_vsn) do + :lt -> false + :eq -> a_id <= b_id + :gt -> true + end + end + ) + end + + defp ignore_same_deployment(query, %{deployment_id: nil}), do: query + + defp ignore_same_deployment(query, %{deployment_id: deployment_id}) do + where(query, [d], d.id != ^deployment_id) + end end diff --git a/lib/nerves_hub/logger.ex b/lib/nerves_hub/logger.ex index f6d63ef9c..e133b483a 100644 --- a/lib/nerves_hub/logger.ex +++ b/lib/nerves_hub/logger.ex @@ -31,7 +31,11 @@ defmodule NervesHub.Logger do [:nerves_hub, :devices, :connect], [:nerves_hub, :devices, :disconnect], [:nerves_hub, :devices, :duplicate_connection], - [:nerves_hub, :devices, :update, :automatic] + [:nerves_hub, :devices, :update, :automatic], + [:nerves_hub, :devices, :update, :successful], + [:nerves_hub, :deployments, :set_deployment, :none_found], + [:nerves_hub, :deployments, :set_deployment, :one_found], + [:nerves_hub, :deployments, :set_deployment, :multiple_found] ] Enum.each(events, fn event -> @@ -107,6 +111,29 @@ defmodule NervesHub.Logger do ) end + def log_event([:nerves_hub, :deployments, :set_deployment, :none_found], _, metadata, _) do + Logger.info("No matching deployments", + event: "nerves_hub.deployments.set_deployment.none_found", + identifier: metadata[:device].identifier + ) + end + + def log_event([:nerves_hub, :deployments, :set_deployment, :one_found], _, metadata, _) do + Logger.info("Deployment match found", + event: "nerves_hub.deployments.set_deployment.one_found", + identifier: metadata[:device].identifier, + deployment_id: metadata[:deployment].id + ) + end + + def log_event([:nerves_hub, :deployments, :set_deployment, :multiple_found], _, metadata, _) do + Logger.info("More than one deployment match found, setting to the first", + event: "nerves_hub.deployments.set_deployment.multiple_found", + identifier: metadata[:device].identifier, + deployment_id: metadata[:deployment].id + ) + end + # Helper functions defp ignore_list() do diff --git a/lib/nerves_hub_web/channels/device_channel.ex b/lib/nerves_hub_web/channels/device_channel.ex index 162c1a25b..b677edc29 100644 --- a/lib/nerves_hub_web/channels/device_channel.ex +++ b/lib/nerves_hub_web/channels/device_channel.ex @@ -35,7 +35,10 @@ defmodule NervesHubWeb.DeviceChannel do @decorate with_span("Channels.DeviceChannel.handle_info:after_join") def handle_info({:after_join, params}, %{assigns: %{device: device}} = socket) do - device = Deployments.verify_deployment_membership(device) + device = + device + |> Deployments.verify_deployment_membership() + |> Deployments.set_deployment() maybe_send_public_keys(device, socket, params) From ce3682b59896195611fc73c397f09aea9d1c8d8d Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Thu, 2 Jan 2025 13:41:55 +1300 Subject: [PATCH 2/2] Add tests for `Deployments.matching_deployments` --- test/nerves_hub/deployments_test.exs | 168 +++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) diff --git a/test/nerves_hub/deployments_test.exs b/test/nerves_hub/deployments_test.exs index a8695f5b1..5460413d6 100644 --- a/test/nerves_hub/deployments_test.exs +++ b/test/nerves_hub/deployments_test.exs @@ -3,6 +3,7 @@ defmodule NervesHub.DeploymentsTest do import Phoenix.ChannelTest alias NervesHub.Deployments + alias NervesHub.Devices.Device alias NervesHub.Fixtures alias Ecto.Changeset @@ -124,4 +125,171 @@ defmodule NervesHub.DeploymentsTest do assert_broadcast("deployments/update", %{}, 500) end end + + describe "device's matching deployments" do + test "finds all matching deployments", state do + %{org: org, product: product, firmware: firmware} = state + + %{id: beta_deployment_id} = + Fixtures.deployment_fixture(org, firmware, %{ + name: "beta", + conditions: %{"tags" => ["beta"]} + }) + + %{id: rpi_deployment_id} = + Fixtures.deployment_fixture(org, firmware, %{ + name: "rpi", + conditions: %{"tags" => ["rpi"]} + }) + + Fixtures.deployment_fixture(org, firmware, %{ + name: "rpi0", + conditions: %{"tags" => ["rpi0"]} + }) + + device = Fixtures.device_fixture(org, product, firmware, %{tags: ["beta", "rpi"]}) + + assert [ + %{id: ^beta_deployment_id}, + %{id: ^rpi_deployment_id} + ] = Deployments.matching_deployments(device) + end + + test "finds matching deployments including the platform", state do + %{org: org, org_key: org_key, product: product} = state + + rpi_firmware = Fixtures.firmware_fixture(org_key, product, %{platform: "rpi"}) + rpi0_firmware = Fixtures.firmware_fixture(org_key, product, %{platform: "rpi0"}) + + %{id: rpi_deployment_id} = + Fixtures.deployment_fixture(org, rpi_firmware, %{ + name: "rpi", + conditions: %{"tags" => ["rpi"]} + }) + + Fixtures.deployment_fixture(org, rpi0_firmware, %{ + name: "rpi0", + conditions: %{"tags" => ["rpi"]} + }) + + device = Fixtures.device_fixture(org, product, rpi_firmware, %{tags: ["beta", "rpi"]}) + + assert [%{id: ^rpi_deployment_id}] = Deployments.matching_deployments(device) + end + + test "finds matching deployments including the architecture", state do + %{org: org, org_key: org_key, product: product} = state + + rpi_firmware = Fixtures.firmware_fixture(org_key, product, %{architecture: "rpi"}) + rpi0_firmware = Fixtures.firmware_fixture(org_key, product, %{architecture: "rpi0"}) + + %{id: rpi_deployment_id} = + Fixtures.deployment_fixture(org, rpi_firmware, %{ + name: "rpi", + conditions: %{"tags" => ["rpi"]} + }) + + Fixtures.deployment_fixture(org, rpi0_firmware, %{ + name: "rpi0", + conditions: %{"tags" => ["rpi"]} + }) + + device = Fixtures.device_fixture(org, product, rpi_firmware, %{tags: ["beta", "rpi"]}) + + assert [%{id: ^rpi_deployment_id}] = Deployments.matching_deployments(device) + end + + test "finds matching deployments including the version", state do + %{org: org, product: product, firmware: firmware} = state + + %{id: low_deployment_id} = + Fixtures.deployment_fixture(org, firmware, %{ + name: "rpi", + conditions: %{"tags" => ["rpi"], "version" => "~> 1.0"} + }) + + Fixtures.deployment_fixture(org, firmware, %{ + name: "rpi0", + conditions: %{"tags" => ["rpi"], "version" => "~> 2.0"} + }) + + device = Fixtures.device_fixture(org, product, firmware, %{tags: ["beta", "rpi"]}) + + assert [%{id: ^low_deployment_id}] = Deployments.matching_deployments(device) + end + + test "finds matching deployments including pre versions", state do + %{org: org, org_key: org_key, product: product, firmware: firmware} = state + + %{id: low_deployment_id} = + Fixtures.deployment_fixture(org, firmware, %{ + name: "rpi", + conditions: %{"tags" => ["rpi"], "version" => "~> 1.0"} + }) + + Fixtures.deployment_fixture(org, firmware, %{ + name: "rpi0", + conditions: %{"tags" => ["rpi"], "version" => "~> 2.0"} + }) + + firmware = Fixtures.firmware_fixture(org_key, product, %{version: "1.2.0-pre"}) + + device = Fixtures.device_fixture(org, product, firmware, %{tags: ["beta", "rpi"]}) + + assert [%{id: ^low_deployment_id}] = Deployments.matching_deployments(device) + end + + test "finds the newest firmware version including pre-releases", state do + %{ + org: org, + org_key: org_key, + product: product, + firmware: %{version: "1.0.0"} = v100_firmware + } = state + + v090_fw = Fixtures.firmware_fixture(org_key, product, %{version: "0.9.0"}) + v100rc1_fw = Fixtures.firmware_fixture(org_key, product, %{version: "1.0.0-rc.1"}) + v100rc2_fw = Fixtures.firmware_fixture(org_key, product, %{version: "1.0.0-rc.2"}) + v101_fw = Fixtures.firmware_fixture(org_key, product, %{version: "1.0.1"}) + + %{id: v100_deployment_id} = + Fixtures.deployment_fixture(org, v100_firmware, %{ + name: v100_firmware.version, + conditions: %{"version" => "", "tags" => ["next"]} + }) + + %{id: v100rc1_deployment_id} = + Fixtures.deployment_fixture(org, v100rc1_fw, %{ + name: v100rc1_fw.version, + conditions: %{"version" => "", "tags" => ["next"]} + }) + + %{id: v100rc2_deployment_id} = + Fixtures.deployment_fixture(org, v100rc2_fw, %{ + name: v100rc2_fw.version, + conditions: %{"version" => "", "tags" => ["next"]} + }) + + %{id: v101_deployment_id} = + Fixtures.deployment_fixture(org, v101_fw, %{ + name: v101_fw.version, + conditions: %{"version" => "", "tags" => ["next"]} + }) + + device = Fixtures.device_fixture(org, product, v090_fw, %{tags: ["next"]}) + + assert [ + %{id: ^v101_deployment_id}, + %{id: ^v100_deployment_id}, + %{id: ^v100rc2_deployment_id}, + %{id: ^v100rc1_deployment_id} + ] = Deployments.matching_deployments(device) + end + + test "ignores device without firmware metadata" do + assert [] == Deployments.matching_deployments(%Device{firmware_metadata: nil}) + assert [] == Deployments.matching_deployments(%Device{firmware_metadata: nil}, [true]) + assert [] == Deployments.matching_deployments(%Device{firmware_metadata: nil}, [false]) + end + end end