From 9e092f1a4a5fdc39a462ca6a7f150a469766951f Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Fri, 22 Jul 2016 14:34:18 +0200 Subject: [PATCH] Switch to LatestMap-style node controls This allows plugins to add controls to nodes that already have some controls set by other plugin. Previously only the last plugin that sets the controls in the node would have its controls visible. That was because of NodeControls' Merge function that actually weren't merging data from two inputs, but rather returning data that was newer and discarding the older one. --- examples/plugins/iowait/main.go | 86 ++++++++++++++------- probe/docker/container.go | 27 +++++-- probe/docker/container_test.go | 18 +++-- probe/host/reporter.go | 2 +- probe/kubernetes/deployment.go | 2 +- probe/kubernetes/pod.go | 2 +- probe/kubernetes/replica_set.go | 2 +- probe/kubernetes/replication_controller.go | 2 +- probe/plugins/registry.go | 10 +-- probe/plugins/registry_internal_test.go | 11 ++- render/detailed/node.go | 19 +++-- report/node.go | 89 ++++++++++++++-------- 12 files changed, 179 insertions(+), 91 deletions(-) diff --git a/examples/plugins/iowait/main.go b/examples/plugins/iowait/main.go index 15fea6ab1..6f1230605 100644 --- a/examples/plugins/iowait/main.go +++ b/examples/plugins/iowait/main.go @@ -89,8 +89,8 @@ type topology struct { } type node struct { - Metrics map[string]metric `json:"metrics"` - Controls nodeControls `json:"controls"` + Metrics map[string]metric `json:"metrics"` + LatestControls map[string]controlEntry `json:"latestControls,omitempty"` } type metric struct { @@ -104,9 +104,13 @@ type sample struct { Value float64 `json:"value"` } -type nodeControls struct { - Timestamp time.Time `json:"timestamp,omitempty"` - Controls []string `json:"controls,omitempty"` +type controlEntry struct { + Timestamp time.Time `json:"timestamp"` + Value controlData `json:"value"` +} + +type controlData struct { + Dead bool `json:"dead"` } type metricTemplate struct { @@ -140,8 +144,8 @@ func (p *Plugin) makeReport() (*report, error) { Host: topology{ Nodes: map[string]node{ p.getTopologyHost(): { - Metrics: metrics, - Controls: p.nodeControls(), + Metrics: metrics, + LatestControls: p.latestControls(), }, }, MetricTemplates: p.metricTemplates(), @@ -181,16 +185,20 @@ func (p *Plugin) metrics() (map[string]metric, error) { return metrics, nil } -// Get the topology controls and node's controls JSON snippet -func (p *Plugin) nodeControls() nodeControls { - id, _, _ := p.controlDetails() - return nodeControls{ - Timestamp: time.Now(), - Controls: []string{id}, +func (p *Plugin) latestControls() map[string]controlEntry { + ts := time.Now() + ctrls := map[string]controlEntry{} + for _, details := range p.allControlDetails() { + ctrls[details.id] = controlEntry{ + Timestamp: ts, + Value: controlData{ + Dead: details.dead, + }, + } } + return ctrls } -// Get the metrics and metric_templates JSON snippets func (p *Plugin) metricTemplates() map[string]metricTemplate { id, name := p.metricIDAndName() return map[string]metricTemplate{ @@ -203,17 +211,17 @@ func (p *Plugin) metricTemplates() map[string]metricTemplate { } } -// Get the topology controls and node's controls JSON snippet func (p *Plugin) controls() map[string]control { - id, human, icon := p.controlDetails() - return map[string]control{ - id: { - ID: id, - Human: human, - Icon: icon, + ctrls := map[string]control{} + for _, details := range p.allControlDetails() { + ctrls[details.id] = control{ + ID: details.id, + Human: details.human, + Icon: details.icon, Rank: 1, - }, + } } + return ctrls } // Report is called by scope when a new report is needed. It is part of the @@ -299,11 +307,37 @@ func (p *Plugin) metricValue() (float64, error) { return idle() } -func (p *Plugin) controlDetails() (string, string, string) { - if p.iowaitMode { - return "switchToIdle", "Switch to idle", "fa-beer" +type controlDetails struct { + id string + human string + icon string + dead bool +} + +func (p *Plugin) allControlDetails() []controlDetails { + return []controlDetails{ + { + id: "switchToIdle", + human: "Switch to idle", + icon: "fa-beer", + dead: !p.iowaitMode, + }, + { + id: "switchToIOWait", + human: "Switch to IO wait", + icon: "fa-hourglass", + dead: p.iowaitMode, + }, } - return "switchToIOWait", "Switch to IO wait", "fa-hourglass" +} + +func (p *Plugin) controlDetails() (string, string, string) { + for _, details := range p.allControlDetails() { + if !details.dead { + return details.id, details.human, details.icon + } + } + return "", "", "" } func iowait() (float64, error) { diff --git a/probe/docker/container.go b/probe/docker/container.go index c88288d7d..33f862e84 100644 --- a/probe/docker/container.go +++ b/probe/docker/container.go @@ -442,6 +442,22 @@ func (c *container) getBaseNode() report.Node { return result } +func (c *container) controlsMap() map[string]report.NodeControlData { + paused := c.container.State.Paused + running := !paused && c.container.State.Running + stopped := !paused && !running + return map[string]report.NodeControlData{ + UnpauseContainer: {Dead: !paused}, + RestartContainer: {Dead: !running}, + StopContainer: {Dead: !running}, + PauseContainer: {Dead: !running}, + AttachContainer: {Dead: !running}, + ExecContainer: {Dead: !running}, + StartContainer: {Dead: !stopped}, + RemoveContainer: {Dead: !stopped}, + } +} + func (c *container) GetNode() report.Node { c.RLock() defer c.RUnlock() @@ -450,11 +466,9 @@ func (c *container) GetNode() report.Node { ContainerState: c.StateString(), ContainerStateHuman: c.State(), } - controls := []string{} + controls := c.controlsMap() - if c.container.State.Paused { - controls = append(controls, UnpauseContainer) - } else if c.container.State.Running { + if !c.container.State.Paused && c.container.State.Running { uptime := (mtime.Now().Sub(c.container.State.StartedAt) / time.Second) * time.Second networkMode := "" if c.container.HostConfig != nil { @@ -463,13 +477,10 @@ func (c *container) GetNode() report.Node { latest[ContainerUptime] = uptime.String() latest[ContainerRestartCount] = strconv.Itoa(c.container.RestartCount) latest[ContainerNetworkMode] = networkMode - controls = append(controls, RestartContainer, StopContainer, PauseContainer, AttachContainer, ExecContainer) - } else { - controls = append(controls, StartContainer, RemoveContainer) } result := c.baseNode.WithLatests(latest) - result = result.WithControls(controls...) + result = result.WithLatestControls(controls) result = result.WithMetrics(c.metrics()) return result } diff --git a/probe/docker/container_test.go b/probe/docker/container_test.go index a8c3b17b5..28c5c54fd 100644 --- a/probe/docker/container_test.go +++ b/probe/docker/container_test.go @@ -76,6 +76,16 @@ func TestContainer(t *testing.T) { // Now see if we go them { uptime := (now.Sub(startTime) / time.Second) * time.Second + controls := map[string]report.NodeControlData{ + docker.UnpauseContainer: {Dead: true}, + docker.RestartContainer: {Dead: false}, + docker.StopContainer: {Dead: false}, + docker.PauseContainer: {Dead: false}, + docker.AttachContainer: {Dead: false}, + docker.ExecContainer: {Dead: false}, + docker.StartContainer: {Dead: true}, + docker.RemoveContainer: {Dead: true}, + } want := report.MakeNodeWith("ping;", map[string]string{ "docker_container_command": " ", "docker_container_created": "01 Jan 01 00:00 UTC", @@ -87,11 +97,9 @@ func TestContainer(t *testing.T) { "docker_container_state": "running", "docker_container_state_human": "Up 6 years", "docker_container_uptime": uptime.String(), - }). - WithControls( - docker.RestartContainer, docker.StopContainer, docker.PauseContainer, - docker.AttachContainer, docker.ExecContainer, - ).WithMetrics(report.Metrics{ + }).WithLatestControls( + controls, + ).WithMetrics(report.Metrics{ "docker_cpu_total_usage": report.MakeMetric(nil), "docker_memory_usage": report.MakeSingletonMetric(now, 12345).WithMax(45678), }).WithParents(report.EmptySets. diff --git a/probe/host/reporter.go b/probe/host/reporter.go index 920860528..7d0a2a5aa 100644 --- a/probe/host/reporter.go +++ b/probe/host/reporter.go @@ -145,7 +145,7 @@ func (r *Reporter) Report() (report.Report, error) { Add(LocalNetworks, report.MakeStringSet(localCIDRs...)), ). WithMetrics(metrics). - WithControls(ExecHost), + WithLatestActiveControls(ExecHost), ) rep.Host.Controls.AddControl(report.Control{ diff --git a/probe/kubernetes/deployment.go b/probe/kubernetes/deployment.go index 965b812f3..77c785022 100644 --- a/probe/kubernetes/deployment.go +++ b/probe/kubernetes/deployment.go @@ -55,5 +55,5 @@ func (d *deployment) GetNode(probeID string) report.Node { UnavailableReplicas: fmt.Sprint(d.Status.UnavailableReplicas), Strategy: string(d.Spec.Strategy.Type), report.ControlProbeID: probeID, - }).WithControls(ScaleUp, ScaleDown) + }).WithLatestActiveControls(ScaleUp, ScaleDown) } diff --git a/probe/kubernetes/pod.go b/probe/kubernetes/pod.go index 52f53d761..c190d9684 100644 --- a/probe/kubernetes/pod.go +++ b/probe/kubernetes/pod.go @@ -63,5 +63,5 @@ func (p *pod) GetNode(probeID string) report.Node { report.ControlProbeID: probeID, }). WithParents(p.parents). - WithControls(GetLogs, DeletePod) + WithLatestActiveControls(GetLogs, DeletePod) } diff --git a/probe/kubernetes/replica_set.go b/probe/kubernetes/replica_set.go index eafbebe69..745c5e992 100644 --- a/probe/kubernetes/replica_set.go +++ b/probe/kubernetes/replica_set.go @@ -59,5 +59,5 @@ func (r *replicaSet) GetNode(probeID string) report.Node { DesiredReplicas: fmt.Sprint(r.Spec.Replicas), FullyLabeledReplicas: fmt.Sprint(r.Status.FullyLabeledReplicas), report.ControlProbeID: probeID, - }).WithParents(r.parents).WithControls(ScaleUp, ScaleDown) + }).WithParents(r.parents).WithLatestActiveControls(ScaleUp, ScaleDown) } diff --git a/probe/kubernetes/replication_controller.go b/probe/kubernetes/replication_controller.go index 14350f5a6..016bd4dde 100644 --- a/probe/kubernetes/replication_controller.go +++ b/probe/kubernetes/replication_controller.go @@ -50,5 +50,5 @@ func (r *replicationController) GetNode(probeID string) report.Node { DesiredReplicas: fmt.Sprint(r.Spec.Replicas), FullyLabeledReplicas: fmt.Sprint(r.Status.FullyLabeledReplicas), report.ControlProbeID: probeID, - }).WithParents(r.parents).WithControls(ScaleUp, ScaleDown) + }).WithParents(r.parents).WithLatestActiveControls(ScaleUp, ScaleDown) } diff --git a/probe/plugins/registry.go b/probe/plugins/registry.go index 34b235bbc..c9915127f 100644 --- a/probe/plugins/registry.go +++ b/probe/plugins/registry.go @@ -271,8 +271,8 @@ func (r *Registry) updateAndGetControlsInTopology(pluginID string, topology *rep for name, node := range topology.Nodes { log.Debugf("plugins: checking node controls in node %s of %s", name, topology.Label) newNode := node.WithID(name) - var nodeControls []string - for _, controlID := range node.Controls.Controls { + newLatestControls := report.MakeNodeControlDataLatestMap() + node.LatestControls.ForEach(func(controlID string, ts time.Time, data report.NodeControlData) { log.Debugf("plugins: got node control %s", controlID) newControlID := "" if _, found := topology.Controls[controlID]; !found { @@ -282,9 +282,9 @@ func (r *Registry) updateAndGetControlsInTopology(pluginID string, topology *rep newControlID = fakeControlID(pluginID, controlID) log.Debugf("plugins: will replace node control %s with %s", controlID, newControlID) } - nodeControls = append(nodeControls, newControlID) - } - newNode.Controls.Controls = report.MakeStringSet(nodeControls...) + newLatestControls = newLatestControls.Set(newControlID, ts, data) + }) + newNode.LatestControls = newLatestControls newNodes[newNode.ID] = newNode } topology.Controls = newControls diff --git a/probe/plugins/registry_internal_test.go b/probe/plugins/registry_internal_test.go index 194cee5fb..a9f1a2262 100644 --- a/probe/plugins/registry_internal_test.go +++ b/probe/plugins/registry_internal_test.go @@ -627,9 +627,14 @@ func checkControls(t *testing.T, topology report.Topology, expectedControls, exp if !found { t.Fatalf("expected a node %s in a topology", nodeID) } + actualNodeControls := []string{} + node.LatestControls.ForEach(func(controlID string, _ time.Time, _ report.NodeControlData) { + actualNodeControls = append(actualNodeControls, controlID) + }) nodeControlsSet := report.MakeStringSet(expectedNodeControls...) - if !reflect.DeepEqual(nodeControlsSet, node.Controls.Controls) { - t.Fatalf("node controls in node %s in topology %s are not equal:\n%s", nodeID, topology.Label, test.Diff(nodeControlsSet, node.Controls.Controls)) + actualNodeControlsSet := report.MakeStringSet(actualNodeControls...) + if !reflect.DeepEqual(nodeControlsSet, actualNodeControlsSet) { + t.Fatalf("node controls in node %s in topology %s are not equal:\n%s", nodeID, topology.Label, test.Diff(nodeControlsSet, actualNodeControlsSet)) } } @@ -680,7 +685,7 @@ func nodeControls(indices []int) []string { func topologyWithControls(label, nodeID string, controlIndices, nodeControlIndices []int) report.Topology { topology := report.MakeTopology().WithLabel(label, "") topology.Controls = topologyControls(controlIndices) - return topology.AddNode(report.MakeNode(nodeID).WithControls(nodeControls(nodeControlIndices)...)) + return topology.AddNode(report.MakeNode(nodeID).WithLatestActiveControls(nodeControls(nodeControlIndices)...)) } func pluginSpec(ID string, interfaces ...string) xfer.PluginSpec { diff --git a/render/detailed/node.go b/render/detailed/node.go index 3824581da..6c821487c 100644 --- a/render/detailed/node.go +++ b/render/detailed/node.go @@ -2,6 +2,7 @@ package detailed import ( "sort" + "time" "github.com/ugorji/go/codec" @@ -98,20 +99,22 @@ func controlsFor(topology report.Topology, nodeID string) []ControlInstance { if !ok { return result } - - for _, id := range node.Controls.Controls { - if control, ok := topology.Controls[id]; ok { - probeID, ok := node.Latest.Lookup(report.ControlProbeID) - if !ok { - continue - } + probeID, ok := node.Latest.Lookup(report.ControlProbeID) + if !ok { + return result + } + node.LatestControls.ForEach(func(controlID string, _ time.Time, data report.NodeControlData) { + if data.Dead { + return + } + if control, ok := topology.Controls[controlID]; ok { result = append(result, ControlInstance{ ProbeID: probeID, NodeID: nodeID, Control: control, }) } - } + }) return result } diff --git a/report/node.go b/report/node.go index 9321f8d10..858eeaabd 100644 --- a/report/node.go +++ b/report/node.go @@ -10,31 +10,33 @@ import ( // given node in a given topology, along with the edges emanating from the // node and metadata about those edges. type Node struct { - ID string `json:"id,omitempty"` - Topology string `json:"topology,omitempty"` - Counters Counters `json:"counters,omitempty"` - Sets Sets `json:"sets,omitempty"` - Adjacency IDList `json:"adjacency"` - Edges EdgeMetadatas `json:"edges,omitempty"` - Controls NodeControls `json:"controls,omitempty"` - Latest StringLatestMap `json:"latest,omitempty"` - Metrics Metrics `json:"metrics,omitempty"` - Parents Sets `json:"parents,omitempty"` - Children NodeSet `json:"children,omitempty"` + ID string `json:"id,omitempty"` + Topology string `json:"topology,omitempty"` + Counters Counters `json:"counters,omitempty"` + Sets Sets `json:"sets,omitempty"` + Adjacency IDList `json:"adjacency"` + Edges EdgeMetadatas `json:"edges,omitempty"` + Controls NodeControls `json:"controls,omitempty"` + LatestControls NodeControlDataLatestMap `json:"latestControls,omitempty"` + Latest StringLatestMap `json:"latest,omitempty"` + Metrics Metrics `json:"metrics,omitempty"` + Parents Sets `json:"parents,omitempty"` + Children NodeSet `json:"children,omitempty"` } // MakeNode creates a new Node with no initial metadata. func MakeNode(id string) Node { return Node{ - ID: id, - Counters: EmptyCounters, - Sets: EmptySets, - Adjacency: EmptyIDList, - Edges: EmptyEdgeMetadatas, - Controls: MakeNodeControls(), - Latest: EmptyStringLatestMap, - Metrics: Metrics{}, - Parents: EmptySets, + ID: id, + Counters: EmptyCounters, + Sets: EmptySets, + Adjacency: EmptyIDList, + Edges: EmptyEdgeMetadatas, + Controls: MakeNodeControls(), + LatestControls: EmptyNodeControlDataLatestMap, + Latest: EmptyStringLatestMap, + Metrics: Metrics{}, + Parents: EmptySets, } } @@ -136,6 +138,30 @@ func (n Node) WithControls(cs ...string) Node { return n } +// WithLatestActiveControls returns a fresh copy of n, with active controls cs added to LatestControls. +func (n Node) WithLatestActiveControls(cs ...string) Node { + lcs := map[string]NodeControlData{} + for _, control := range cs { + lcs[control] = NodeControlData{} + } + return n.WithLatestControls(lcs) +} + +// WithLatestControls returns a fresh copy of n, with lcs added to LatestControls. +func (n Node) WithLatestControls(lcs map[string]NodeControlData) Node { + ts := mtime.Now() + for k, v := range lcs { + n.LatestControls = n.LatestControls.Set(k, ts, v) + } + return n +} + +// WithLatestControl produces a new Node with control added to it +func (n Node) WithLatestControl(control string, ts time.Time, data NodeControlData) Node { + n.LatestControls = n.LatestControls.Set(control, ts, data) + return n +} + // WithParents returns a fresh copy of n, with sets merged in. func (n Node) WithParents(parents Sets) Node { n.Parents = n.Parents.Merge(parents) @@ -174,16 +200,17 @@ func (n Node) Merge(other Node) Node { panic("Cannot merge nodes with different topology types: " + topology + " != " + other.Topology) } return Node{ - ID: id, - Topology: topology, - Counters: n.Counters.Merge(other.Counters), - Sets: n.Sets.Merge(other.Sets), - Adjacency: n.Adjacency.Merge(other.Adjacency), - Edges: n.Edges.Merge(other.Edges), - Controls: n.Controls.Merge(other.Controls), - Latest: n.Latest.Merge(other.Latest), - Metrics: n.Metrics.Merge(other.Metrics), - Parents: n.Parents.Merge(other.Parents), - Children: n.Children.Merge(other.Children), + ID: id, + Topology: topology, + Counters: n.Counters.Merge(other.Counters), + Sets: n.Sets.Merge(other.Sets), + Adjacency: n.Adjacency.Merge(other.Adjacency), + Edges: n.Edges.Merge(other.Edges), + Controls: n.Controls.Merge(other.Controls), + LatestControls: n.LatestControls.Merge(other.LatestControls), + Latest: n.Latest.Merge(other.Latest), + Metrics: n.Metrics.Merge(other.Metrics), + Parents: n.Parents.Merge(other.Parents), + Children: n.Children.Merge(other.Children), } }