diff --git a/render/mapping.go b/render/mapping.go index cbbc4fa0d..66da21d6d 100644 --- a/render/mapping.go +++ b/render/mapping.go @@ -46,33 +46,47 @@ type PseudoFunc func(srcNodeID string, srcNode RenderableNode, dstNodeID string, // shall be omitted from the rendered topology. type MapFunc func(RenderableNode) (RenderableNode, bool) -// MapEndpointIdentity maps a endpoint topology node to endpoint RenderableNode -// node. As it is only ever run on endpoint topology nodes, we can safely -// assume the presence of certain keys. +// MapEndpointIdentity maps an endpoint topology node to an endpoint +// renderable node. As it is only ever run on endpoint topology nodes, we +// expect that certain keys are present. func MapEndpointIdentity(m report.NodeMetadata) (RenderableNode, bool) { + addr, ok := m.Metadata[endpoint.Addr] + if !ok { + return RenderableNode{}, false + } + + port, ok := m.Metadata[endpoint.Port] + if !ok { + return RenderableNode{}, false + } + var ( - id = MakeEndpointID(report.ExtractHostID(m), m.Metadata[endpoint.Addr], m.Metadata[endpoint.Port]) - major = fmt.Sprintf("%s:%s", m.Metadata[endpoint.Addr], m.Metadata[endpoint.Port]) - pid, ok = m.Metadata[process.PID] - minor = report.ExtractHostID(m) - rank = major + id = MakeEndpointID(report.ExtractHostID(m), addr, port) + major = fmt.Sprintf("%s:%s", addr, port) + minor = report.ExtractHostID(m) + rank = major ) - if ok { - minor = fmt.Sprintf("%s (%s)", report.ExtractHostID(m), pid) + if pid, ok := m.Metadata[process.PID]; ok { + minor = fmt.Sprintf("%s (%s)", minor, pid) } return NewRenderableNode(id, major, minor, rank, m), true } -// MapProcessIdentity maps a process topology node to process RenderableNode node. -// As it is only ever run on process topology nodes, we can safely assume the -// presence of certain keys. +// MapProcessIdentity maps a process topology node to a process renderable +// node. As it is only ever run on process topology nodes, we expect that +// certain keys are present. func MapProcessIdentity(m report.NodeMetadata) (RenderableNode, bool) { + pid, ok := m.Metadata[process.PID] + if !ok { + return RenderableNode{}, false + } + var ( - id = MakeProcessID(report.ExtractHostID(m), m.Metadata[process.PID]) + id = MakeProcessID(report.ExtractHostID(m), pid) major = m.Metadata["comm"] - minor = fmt.Sprintf("%s (%s)", report.ExtractHostID(m), m.Metadata[process.PID]) + minor = fmt.Sprintf("%s (%s)", report.ExtractHostID(m), pid) rank = m.Metadata["comm"] ) @@ -80,11 +94,15 @@ func MapProcessIdentity(m report.NodeMetadata) (RenderableNode, bool) { } // MapContainerIdentity maps a container topology node to a container -// RenderableNode node. As it is only ever run on container topology -// nodes, we can safely assume the presences of certain keys. +// renderable node. As it is only ever run on container topology nodes, we +// expect that certain keys are present. func MapContainerIdentity(m report.NodeMetadata) (RenderableNode, bool) { + id, ok := m.Metadata[docker.ContainerID] + if !ok { + return RenderableNode{}, false + } + var ( - id = m.Metadata[docker.ContainerID] major = m.Metadata[docker.ContainerName] minor = report.ExtractHostID(m) rank = m.Metadata[docker.ImageID] @@ -94,11 +112,15 @@ func MapContainerIdentity(m report.NodeMetadata) (RenderableNode, bool) { } // MapContainerImageIdentity maps a container image topology node to container -// image RenderableNode node. As it is only ever run on container image -// topology nodes, we can safely assume the presences of certain keys. +// image renderable node. As it is only ever run on container image topology +// nodes, we expect that certain keys are present. func MapContainerImageIdentity(m report.NodeMetadata) (RenderableNode, bool) { + id, ok := m.Metadata[docker.ImageID] + if !ok { + return RenderableNode{}, false + } + var ( - id = m.Metadata[docker.ImageID] major = m.Metadata[docker.ImageName] rank = m.Metadata[docker.ImageID] ) @@ -106,13 +128,18 @@ func MapContainerImageIdentity(m report.NodeMetadata) (RenderableNode, bool) { return NewRenderableNode(id, major, "", rank, m), true } -// MapAddressIdentity maps a address topology node to address RenderableNode -// node. As it is only ever run on address topology nodes, we can safely -// assume the presence of certain keys. +// MapAddressIdentity maps an address topology node to an address renderable +// node. As it is only ever run on address topology nodes, we expect that +// certain keys are present. func MapAddressIdentity(m report.NodeMetadata) (RenderableNode, bool) { + addr, ok := m.Metadata[endpoint.Addr] + if !ok { + return RenderableNode{}, false + } + var ( - id = MakeAddressID(report.ExtractHostID(m), m.Metadata[endpoint.Addr]) - major = m.Metadata[endpoint.Addr] + id = MakeAddressID(report.ExtractHostID(m), addr) + major = addr minor = report.ExtractHostID(m) rank = major ) @@ -120,9 +147,9 @@ func MapAddressIdentity(m report.NodeMetadata) (RenderableNode, bool) { return NewRenderableNode(id, major, minor, rank, m), true } -// MapHostIdentity maps a host topology node to host RenderableNode -// node. As it is only ever run on host topology nodes, we can safely -// assume the presence of certain keys. +// MapHostIdentity maps a host topology node to a host renderable node. As it +// is only ever run on host topology nodes, we expect that certain keys are +// present. func MapHostIdentity(m report.NodeMetadata) (RenderableNode, bool) { var ( id = MakeHostID(report.ExtractHostID(m)) diff --git a/render/mapping_test.go b/render/mapping_test.go new file mode 100644 index 000000000..6efbf98d8 --- /dev/null +++ b/render/mapping_test.go @@ -0,0 +1,78 @@ +package render_test + +import ( + "testing" + + "github.com/weaveworks/scope/probe/docker" + "github.com/weaveworks/scope/probe/endpoint" + "github.com/weaveworks/scope/probe/process" + "github.com/weaveworks/scope/render" + "github.com/weaveworks/scope/report" +) + +func TestMapEndpointIdentity(t *testing.T) { + for _, input := range []testcase{ + {report.MakeNodeMetadata(), false}, + {report.MakeNodeMetadataWith(map[string]string{endpoint.Addr: "1.2.3.4"}), false}, + {report.MakeNodeMetadataWith(map[string]string{endpoint.Port: "1234"}), false}, + {report.MakeNodeMetadataWith(map[string]string{endpoint.Addr: "1.2.3.4", endpoint.Port: "1234"}), true}, + {report.MakeNodeMetadataWith(map[string]string{report.HostNodeID: report.MakeHostNodeID("foo"), endpoint.Addr: "10.0.0.1", endpoint.Port: "20001"}), true}, + } { + testMap(t, render.MapEndpointIdentity, input) + } +} + +func TestMapProcessIdentity(t *testing.T) { + for _, input := range []testcase{ + {report.MakeNodeMetadata(), false}, + {report.MakeNodeMetadataWith(map[string]string{process.PID: "201"}), true}, + } { + testMap(t, render.MapProcessIdentity, input) + } +} + +func TestMapContainerIdentity(t *testing.T) { + for _, input := range []testcase{ + {report.MakeNodeMetadata(), false}, + {report.MakeNodeMetadataWith(map[string]string{docker.ContainerID: "a1b2c3"}), true}, + } { + testMap(t, render.MapContainerIdentity, input) + } +} + +func TestMapContainerImageIdentity(t *testing.T) { + for _, input := range []testcase{ + {report.MakeNodeMetadata(), false}, + {report.MakeNodeMetadataWith(map[string]string{docker.ImageID: "a1b2c3"}), true}, + } { + testMap(t, render.MapContainerImageIdentity, input) + } +} + +func TestMapAddressIdentity(t *testing.T) { + for _, input := range []testcase{ + {report.MakeNodeMetadata(), false}, + {report.MakeNodeMetadataWith(map[string]string{endpoint.Addr: "192.168.1.1"}), true}, + } { + testMap(t, render.MapAddressIdentity, input) + } +} + +func TestMapHostIdentity(t *testing.T) { + for _, input := range []testcase{ + {report.MakeNodeMetadata(), true}, // TODO it's questionable if this is actually correct + } { + testMap(t, render.MapHostIdentity, input) + } +} + +type testcase struct { + md report.NodeMetadata + ok bool +} + +func testMap(t *testing.T, f render.LeafMapFunc, input testcase) { + if _, have := f(input.md); input.ok != have { + t.Errorf("%v: want %v, have %v", input.md, input.ok, have) + } +} diff --git a/report/topology.go b/report/topology.go index d3b4d1d20..ce9055997 100644 --- a/report/topology.go +++ b/report/topology.go @@ -89,7 +89,6 @@ func (t Topology) Validate() error { } if _, ok := t.NodeMetadatas[srcNodeID]; !ok { errs = append(errs, fmt.Sprintf("node metadata missing for source node ID %q (from edge %q)", srcNodeID, edgeID)) - continue } dstNodeIDs, ok := t.Adjacency[MakeAdjacencyID(srcNodeID)] if !ok { @@ -98,7 +97,6 @@ func (t Topology) Validate() error { } if !dstNodeIDs.Contains(dstNodeID) { errs = append(errs, fmt.Sprintf("adjacency destination missing for destination node ID %q (from edge %q)", dstNodeID, edgeID)) - continue } } @@ -111,20 +109,22 @@ func (t Topology) Validate() error { } if _, ok := t.NodeMetadatas[nodeID]; !ok { errs = append(errs, fmt.Sprintf("node metadata missing for source node %q (from adjacency %q)", nodeID, adjacencyID)) - continue } } - // Check all node metadata keys are parse-able (i.e. contain a scope) + // Check all node metadatas are valid, and the keys are parseable, i.e. + // contain a scope. for nodeID := range t.NodeMetadatas { + if t.NodeMetadatas[nodeID].Metadata == nil { + errs = append(errs, fmt.Sprintf("node ID %q has nil metadata", nodeID)) + } if _, _, ok := ParseNodeID(nodeID); !ok { errs = append(errs, fmt.Sprintf("invalid node ID %q", nodeID)) - continue } } if len(errs) > 0 { - return fmt.Errorf(strings.Join(errs, "; ")) + return fmt.Errorf("%d error(s): %s", len(errs), strings.Join(errs, "; ")) } return nil