Merge pull request #26 from wasmCloud/fix/iid_dedupe

Removes dependence on instance ID for all monitoring logic
wasmCloud · Feb 2, 2023 · c4c0860 · c4c0860
2 parents 9b33cc2 + 8cba7ca
commit c4c0860
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 167 deletions.
diff --git a/lib/lattice_observer/observed/event_processor.ex b/lib/lattice_observer/observed/event_processor.ex
@@ -63,7 +63,7 @@ defmodule LatticeObserver.Observed.EventProcessor do
     }
   end
 
-  def put_actor_instance(l = %Lattice{}, host_id, pk, instance_id, spec, stamp, claims)
+  def put_actor_instance(l = %Lattice{}, host_id, pk, instance_id, spec, _stamp, claims)
       when is_binary(pk) and is_binary(instance_id) and is_binary(spec) do
     actor = Map.get(l.actors, pk, Actor.new(pk, "unavailable"))
     actor = merge_actor(actor, l, claims)
@@ -76,18 +76,11 @@ defmodule LatticeObserver.Observed.EventProcessor do
       revision: Map.get(claims, "revision", get_claim(l, :rev, pk, 0))
     }
 
-    actor =
-      if actor.instances |> Enum.find(fn i -> i.id == instance_id end) == nil do
-        %{actor | instances: [instance | actor.instances]}
-      else
-        actor
-      end
+    actor = %Actor{actor | instances: [instance | actor.instances]}
 
     %Lattice{
       l
-      | actors: Map.put(l.actors, pk, actor),
-        instance_tracking:
-          Map.put(l.instance_tracking, instance.id, timestamp_from_iso8601(stamp))
+      | actors: Map.put(l.actors, pk, actor)
     }
   end
 
@@ -116,12 +109,16 @@ defmodule LatticeObserver.Observed.EventProcessor do
       revision: Map.get(claims, "revision", get_claim(l, :rev, pk, "0") |> parse_revision())
     }
 
-    provider =
-      if provider.instances |> Enum.find(fn i -> i.id == instance_id end) == nil do
-        %Provider{provider | instances: [instance | provider.instances]}
-      else
-        provider
-      end
+    # Remove old provider instance for this host and link name
+    provider = %Provider{
+      provider
+      | instances:
+          provider.instances
+          |> Enum.reject(fn i -> i.host_id == source_host end)
+    }
+
+    # Add this instance back to the list
+    provider = %Provider{provider | instances: [instance | provider.instances]}
 
     %Lattice{
       l
@@ -131,19 +128,20 @@ defmodule LatticeObserver.Observed.EventProcessor do
     }
   end
 
-  def remove_provider_instance(l, _source_host, pk, link_name, instance_id, _spec) do
+  def remove_provider_instance(l, source_host, pk, link_name, _spec) do
     provider = l.providers[{pk, link_name}]
 
+    # only one provider+link name can exist per host, so this is guaranteed to
+    # remove that provider since the key is already pk+link.
     if provider != nil do
       provider = %Provider{
         provider
-        | instances: provider.instances |> Enum.reject(fn i -> i.id == instance_id end)
+        | instances: provider.instances |> Enum.reject(fn i -> i.host_id == source_host end)
       }
 
       %Lattice{
         l
-        | providers: Map.put(l.providers, {pk, link_name}, provider),
-          instance_tracking: l.instance_tracking |> Map.delete(instance_id)
+        | providers: Map.put(l.providers, {pk, link_name}, provider)
       }
       |> strip_instanceless_entities()
     else
@@ -378,19 +376,22 @@ defmodule LatticeObserver.Observed.EventProcessor do
     }
   end
 
-  def remove_actor_instance(l = %Lattice{}, _host_id, pk, instance_id, _spec) do
+  def remove_actor_instance(l = %Lattice{}, host_id, pk, instance_id, _spec) do
     actor = l.actors[pk]
 
     if actor != nil do
+      # Since there's no longer any unique distinction between actor instances on
+      # the host, we can simply remove any one we like
+      instances_on_host = Enum.filter(actor.instances, fn i -> i.host_id == host_id end)
+
       actor = %Actor{
         actor
-        | instances: actor.instances |> Enum.reject(fn i -> i.id == instance_id end)
+        | instances: Enum.drop(instances_on_host, 1)
       }
 
       %Lattice{
         l
-        | actors: Map.put(l.actors, pk, actor),
-          instance_tracking: l.instance_tracking |> Map.delete(instance_id)
+        | actors: Map.put(l.actors, pk, actor)
       }
       |> strip_instanceless_entities()
     else

diff --git a/lib/lattice_observer/observed/lattice.ex b/lib/lattice_observer/observed/lattice.ex
@@ -311,8 +311,7 @@ defmodule LatticeObserver.Observed.Lattice do
           data:
             %{
               "link_name" => link_name,
-              "public_key" => pk,
-              "instance_id" => instance_id
+              "public_key" => pk
             } = d,
           datacontenttype: "application/json",
           source: source_host,
@@ -322,7 +321,7 @@ defmodule LatticeObserver.Observed.Lattice do
       ) do
     annotations = Map.get(d, "annotations", %{})
     spec = Map.get(annotations, @annotation_app_spec, "")
-    EventProcessor.remove_provider_instance(l, source_host, pk, link_name, instance_id, spec)
+    EventProcessor.remove_provider_instance(l, source_host, pk, link_name, spec)
   end
 
   def apply_event(

diff --git a/test/observed/actors_test.exs b/test/observed/actors_test.exs
@@ -1,20 +1,16 @@
 defmodule LatticeObserverTest.Observed.ActorsTest do
   use ExUnit.Case
-  alias LatticeObserver.Observed.{Lattice, Instance, Actor, EventProcessor}
+  alias LatticeObserver.Observed.{Lattice, Instance, Actor}
   alias TestSupport.CloudEvents
 
   @test_spec "testapp"
-  @test_spec_2 "othertestapp"
   @test_host "Nxxx"
 
   describe "Observed Lattice Monitors Actor Events" do
     test "Adds and Removes actors" do
       start = CloudEvents.actor_started("Mxxx", "abc123", @test_spec, @test_host)
       l = Lattice.new()
       l = Lattice.apply_event(l, start)
-      stamp1 = EventProcessor.timestamp_from_iso8601(start.time)
-      # ensure idempotence
-      l = Lattice.apply_event(l, start)
 
       assert l == %Lattice{
                Lattice.new()
@@ -49,9 +45,7 @@ defmodule LatticeObserverTest.Observed.ActorsTest do
                      version: "1.0"
                    }
                  },
-                 instance_tracking: %{
-                   "abc123" => stamp1
-                 }
+                 instance_tracking: %{}
              }
 
       stop = CloudEvents.actor_stopped("Mxxx", "abc123", @test_spec, @test_host)
@@ -76,106 +70,5 @@ defmodule LatticeObserverTest.Observed.ActorsTest do
                  }
              }
     end
-
-    test "Stores the same actor belonging to multiple specs" do
-      start = CloudEvents.actor_started("Mxxx", "abc123", @test_spec, @test_host)
-      l = Lattice.new()
-      l = Lattice.apply_event(l, start)
-      start2 = CloudEvents.actor_started("Mxxx", "abc345", @test_spec_2, @test_host)
-      l = Lattice.apply_event(l, start2)
-      stamp1 = EventProcessor.timestamp_from_iso8601(start.time)
-      stamp2 = EventProcessor.timestamp_from_iso8601(start2.time)
-
-      assert l == %Lattice{
-               Lattice.new()
-               | actors: %{
-                   "Mxxx" => %Actor{
-                     call_alias: "",
-                     capabilities: ["test", "test2"],
-                     id: "Mxxx",
-                     instances: [
-                       %Instance{
-                         host_id: "Nxxx",
-                         id: "abc345",
-                         revision: 0,
-                         spec_id: "othertestapp",
-                         version: "1.0"
-                       },
-                       %Instance{
-                         host_id: "Nxxx",
-                         id: "abc123",
-                         revision: 0,
-                         spec_id: "testapp",
-                         version: "1.0"
-                       }
-                     ],
-                     issuer: "ATESTxxx",
-                     name: "Test Actor",
-                     tags: ""
-                   }
-                 },
-                 claims: %{
-                   "Mxxx" => %LatticeObserver.Observed.Claims{
-                     call_alias: "",
-                     caps: "test,test2",
-                     iss: "ATESTxxx",
-                     name: "Test Actor",
-                     rev: 0,
-                     sub: "Mxxx",
-                     tags: "",
-                     version: "1.0"
-                   }
-                 },
-                 instance_tracking: %{
-                   "abc123" => stamp1,
-                   "abc345" => stamp2
-                 }
-             }
-
-      assert Lattice.actors_in_appspec(l, "testapp") == [
-               %{actor_id: "Mxxx", host_id: "Nxxx", instance_id: "abc123"}
-             ]
-
-      stop = CloudEvents.actor_stopped("Mxxx", "abc123", @test_spec, @test_host)
-      l = Lattice.apply_event(l, stop)
-
-      assert l == %LatticeObserver.Observed.Lattice{
-               Lattice.new()
-               | actors: %{
-                   "Mxxx" => %Actor{
-                     call_alias: "",
-                     capabilities: ["test", "test2"],
-                     id: "Mxxx",
-                     instances: [
-                       %Instance{
-                         host_id: "Nxxx",
-                         id: "abc345",
-                         revision: 0,
-                         spec_id: "othertestapp",
-                         version: "1.0"
-                       }
-                     ],
-                     issuer: "ATESTxxx",
-                     name: "Test Actor",
-                     tags: ""
-                   }
-                 },
-                 claims: %{
-                   "Mxxx" => %LatticeObserver.Observed.Claims{
-                     call_alias: "",
-                     caps: "test,test2",
-                     iss: "ATESTxxx",
-                     name: "Test Actor",
-                     rev: 0,
-                     sub: "Mxxx",
-                     tags: "",
-                     version: "1.0"
-                   }
-                 },
-                 instance_tracking: %{
-                   "abc345" => stamp2
-                 }
-             }
-    end
   end
 end
diff --git a/test/observed/claims_test.exs b/test/observed/claims_test.exs
@@ -1,6 +1,6 @@
 defmodule LatticeObserverTest.Observed.ClaimsTest do
   use ExUnit.Case
-  alias LatticeObserver.Observed.{Lattice, Instance, Claims}
+  alias LatticeObserver.Observed.{Lattice, Claims}
   alias TestSupport.CloudEvents
 
   describe "Claims cache works appropriately" do

diff --git a/test/observed/hosts_test.exs b/test/observed/hosts_test.exs
@@ -48,7 +48,6 @@ defmodule LatticeObserverTest.Observed.HostsTest do
 
       assert Map.keys(l.hosts) == ["Nxxxy"]
       assert (l.hosts |> Map.values() |> List.first()).friendly_name == "yellow-cat-6"
-      assert Map.keys(l.instance_tracking) == ["abc123", "abc456", "abc789"]
       assert l.providers == %{}
       assert l.actors == %{}
     end
@@ -83,7 +82,6 @@ defmodule LatticeObserverTest.Observed.HostsTest do
 
       assert Map.keys(l.hosts) == ["Nxxx"]
       assert (l.hosts |> Map.values() |> List.first()).friendly_name == "orange-button-5"
-      assert Map.keys(l.instance_tracking) == ["abc123", "abc456", "abc789"]
 
       # Host is killed forcefully and starts again using the same seed private key
       l =