From 4dc9f7834eea8c2179cee63aecbd79bf9edd07f7 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Thu, 25 Feb 2016 23:05:35 +0000 Subject: [PATCH 1/3] Return response errors --- probe/docker/controls.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/probe/docker/controls.go b/probe/docker/controls.go index 357e3a094e..cb3303eb4d 100644 --- a/probe/docker/controls.go +++ b/probe/docker/controls.go @@ -57,7 +57,7 @@ func (r *registry) attachContainer(containerID string, req xfer.Request) xfer.Re hasTTY := c.HasTTY() id, pipe, err := controls.NewPipe(r.pipes, req.AppID) if err != nil { - xfer.ResponseError(err) + return xfer.ResponseError(err) } local, _ := pipe.Ends() cw, err := r.client.AttachToContainerNonBlocking(docker_client.AttachToContainerOptions{ @@ -103,12 +103,12 @@ func (r *registry) execContainer(containerID string, req xfer.Request) xfer.Resp Container: containerID, }) if err != nil { - xfer.ResponseError(err) + return xfer.ResponseError(err) } id, pipe, err := controls.NewPipe(r.pipes, req.AppID) if err != nil { - xfer.ResponseError(err) + return xfer.ResponseError(err) } local, _ := pipe.Ends() cw, err := r.client.StartExecNonBlocking(exec.ID, docker_client.StartExecOptions{ From a559a23dbadbf0a8b8d04fe63f6c6de1892afcff Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Fri, 26 Feb 2016 01:46:18 +0000 Subject: [PATCH 2/3] Distinguish between reporting probes and controlling probes --- probe/docker/reporter.go | 8 ++++++-- probe/docker/reporter_test.go | 2 +- prog/probe.go | 2 +- render/detailed/node.go | 5 ++++- report/report.go | 2 ++ 5 files changed, 14 insertions(+), 5 deletions(-) diff --git a/probe/docker/reporter.go b/probe/docker/reporter.go index 6cab0728df..d266aa59b4 100644 --- a/probe/docker/reporter.go +++ b/probe/docker/reporter.go @@ -20,14 +20,16 @@ const ( type Reporter struct { registry Registry hostID string + probeID string probe *probe.Probe } // NewReporter makes a new Reporter -func NewReporter(registry Registry, hostID string, probe *probe.Probe) *Reporter { +func NewReporter(registry Registry, hostID string, probeID string, probe *probe.Probe) *Reporter { reporter := &Reporter{ registry: registry, hostID: hostID, + probeID: probeID, probe: probe, } registry.WatchContainerUpdates(reporter.ContainerUpdated) @@ -103,9 +105,11 @@ func (r *Reporter) containerTopology(localAddrs []net.IP) report.Topology { Icon: "fa-terminal", }) + metadata := map[string]string{report.ControlProbeID: r.probeID} + r.registry.WalkContainers(func(c Container) { nodeID := report.MakeContainerNodeID(c.ID()) - result.AddNode(nodeID, c.GetNode(r.hostID, localAddrs)) + result.AddNode(nodeID, c.GetNode(r.hostID, localAddrs).WithLatests(metadata)) }) return result diff --git a/probe/docker/reporter_test.go b/probe/docker/reporter_test.go index c4439ed696..639d2b32d0 100644 --- a/probe/docker/reporter_test.go +++ b/probe/docker/reporter_test.go @@ -51,7 +51,7 @@ var ( func TestReporter(t *testing.T) { containerImageNodeID := report.MakeContainerImageNodeID("baz") - rpt, err := docker.NewReporter(mockRegistryInstance, "host1", nil).Report() + rpt, err := docker.NewReporter(mockRegistryInstance, "host1", "probeID", nil).Report() if err != nil { t.Fatal(err) } diff --git a/prog/probe.go b/prog/probe.go index f143019b56..f170ab5df9 100644 --- a/prog/probe.go +++ b/prog/probe.go @@ -155,7 +155,7 @@ func probeMain() { if registry, err := docker.NewRegistry(*dockerInterval, clients); err == nil { defer registry.Stop() p.AddTagger(docker.NewTagger(registry, processCache)) - p.AddReporter(docker.NewReporter(registry, hostID, p)) + p.AddReporter(docker.NewReporter(registry, hostID, probeID, p)) } else { log.Errorf("Docker: failed to start registry: %v", err) } diff --git a/render/detailed/node.go b/render/detailed/node.go index 8846f910b1..38da896eb3 100644 --- a/render/detailed/node.go +++ b/render/detailed/node.go @@ -101,7 +101,10 @@ func controlsFor(topology report.Topology, nodeID string) []ControlInstance { for _, id := range node.Controls.Controls { if control, ok := topology.Controls[id]; ok { - probeID, _ := node.Latest.Lookup(report.ProbeID) + probeID, ok := node.Latest.Lookup(report.ControlProbeID) + if !ok { + continue + } result = append(result, ControlInstance{ ProbeID: probeID, NodeID: nodeID, diff --git a/report/report.go b/report/report.go index e33c8e85b6..be5662ecc1 100644 --- a/report/report.go +++ b/report/report.go @@ -209,4 +209,6 @@ const ( HostNodeID = "host_node_id" // ProbeID is the random ID of the probe which generated the specific node. ProbeID = "probe_id" + // ControlProbeID is the random ID of the probe which controls the specific node. + ControlProbeID = "control_probe_id" ) From b33c5167588cbb574aea89631dbd402c609208cf Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Fri, 26 Feb 2016 12:58:07 +0000 Subject: [PATCH 3/3] Review feedback+test --- probe/docker/reporter_test.go | 11 +++++++---- probe/host/tagger.go | 11 +++-------- probe/host/tagger_test.go | 8 +------- prog/probe.go | 2 +- report/report.go | 2 -- 5 files changed, 12 insertions(+), 22 deletions(-) diff --git a/probe/docker/reporter_test.go b/probe/docker/reporter_test.go index 639d2b32d0..2be0f698fd 100644 --- a/probe/docker/reporter_test.go +++ b/probe/docker/reporter_test.go @@ -50,8 +50,10 @@ var ( ) func TestReporter(t *testing.T) { + var controlProbeID = "a1b2c3d4" + containerImageNodeID := report.MakeContainerImageNodeID("baz") - rpt, err := docker.NewReporter(mockRegistryInstance, "host1", "probeID", nil).Report() + rpt, err := docker.NewReporter(mockRegistryInstance, "host1", controlProbeID, nil).Report() if err != nil { t.Fatal(err) } @@ -65,9 +67,10 @@ func TestReporter(t *testing.T) { } for k, want := range map[string]string{ - docker.ContainerID: "ping", - docker.ContainerName: "pong", - docker.ImageID: "baz", + docker.ContainerID: "ping", + docker.ContainerName: "pong", + docker.ImageID: "baz", + report.ControlProbeID: controlProbeID, } { if have, ok := node.Latest.Lookup(k); !ok || have != want { t.Errorf("Expected container %s latest %q: %q, got %q", containerNodeID, k, want, have) diff --git a/probe/host/tagger.go b/probe/host/tagger.go index 7f999d9743..105104b82b 100644 --- a/probe/host/tagger.go +++ b/probe/host/tagger.go @@ -9,15 +9,13 @@ import ( // in every topology to an origin host node in the host topology. type Tagger struct { hostNodeID string - probeID string } // NewTagger tags each node with a foreign key linking it to its origin host // in the host topology. -func NewTagger(hostID, probeID string) Tagger { +func NewTagger(hostID string) Tagger { return Tagger{ hostNodeID: report.MakeHostNodeID(hostID), - probeID: probeID, } } @@ -27,11 +25,8 @@ func (Tagger) Name() string { return "Host" } // Tag implements Tagger. func (t Tagger) Tag(r report.Report) (report.Report, error) { var ( - metadata = map[string]string{ - report.HostNodeID: t.hostNodeID, - report.ProbeID: t.probeID, - } - parents = report.EmptySets.Add(report.Host, report.MakeStringSet(t.hostNodeID)) + metadata = map[string]string{report.HostNodeID: t.hostNodeID} + parents = report.EmptySets.Add(report.Host, report.MakeStringSet(t.hostNodeID)) ) // Explicity don't tag Endpoints and Addresses - These topologies include pseudo nodes, diff --git a/probe/host/tagger_test.go b/probe/host/tagger_test.go index 4bf0cb1714..bc35b31e2c 100644 --- a/probe/host/tagger_test.go +++ b/probe/host/tagger_test.go @@ -10,14 +10,13 @@ import ( func TestTagger(t *testing.T) { var ( hostID = "foo" - probeID = "a1b2c3d4" endpointNodeID = report.MakeEndpointNodeID(hostID, "1.2.3.4", "56789") // hostID ignored node = report.MakeNodeWith(map[string]string{"foo": "bar"}) ) r := report.MakeReport() r.Process.AddNode(endpointNodeID, node) - rpt, _ := host.NewTagger(hostID, probeID).Tag(r) + rpt, _ := host.NewTagger(hostID).Tag(r) have := rpt.Process.Nodes[endpointNodeID].Copy() // It should now have the host ID @@ -26,11 +25,6 @@ func TestTagger(t *testing.T) { t.Errorf("Expected %q got %q", wantHostID, report.MakeHostNodeID(hostID)) } - // It should now have the probe ID - if haveProbeID, ok := have.Latest.Lookup(report.ProbeID); !ok || haveProbeID != probeID { - t.Errorf("Expected %q got %q", probeID, haveProbeID) - } - // It should still have the other keys want := "bar" if have, ok := have.Latest.Lookup("foo"); !ok || have != want { diff --git a/prog/probe.go b/prog/probe.go index f170ab5df9..fadbc0529f 100644 --- a/prog/probe.go +++ b/prog/probe.go @@ -146,7 +146,7 @@ func probeMain() { host.NewReporter(hostID, hostName), process.NewReporter(processCache, hostID, process.GetDeltaTotalJiffies), ) - p.AddTagger(probe.NewTopologyTagger(), host.NewTagger(hostID, probeID)) + p.AddTagger(probe.NewTopologyTagger(), host.NewTagger(hostID)) if *dockerEnabled { if err := report.AddLocalBridge(*dockerBridge); err != nil { diff --git a/report/report.go b/report/report.go index be5662ecc1..5c0ad3e5bf 100644 --- a/report/report.go +++ b/report/report.go @@ -207,8 +207,6 @@ const ( // a node in the host topology. That host node is the origin host, where // the node was originally detected. HostNodeID = "host_node_id" - // ProbeID is the random ID of the probe which generated the specific node. - ProbeID = "probe_id" // ControlProbeID is the random ID of the probe which controls the specific node. ControlProbeID = "control_probe_id" )