Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Produce containers topology from endpoints via processes #248

Merged
merged 13 commits into from
Jun 17, 2015

Conversation

tomwilkie
Copy link
Contributor

With this change, the process topology is used to produce the containers topology - fixes #228.

Changes:

  • render.Map -> render.LeafMap (Leaf to indicate that its a tree of renderers, and these ones have to be at the leaf of that tree)
  • a new render.Map, which allows for mapping from NMD -> NMDs. Tests for this.
  • add and accumulate some NodeMetadata in the RenderableNode struct, to enable the above remapping
  • Some WIP mapping function for identity mappers and mappers to up-convert (ie from endpoints to processes)

The meat of the review is the new render.Map type, the mapping functions and render/topologies.go (naming suggestions?).

Left TODO:

  • remove some annotations on nodes from the probe
  • preserve pseudo node behaviour with the new mappers
  • filter out empty nodes from the process topology
  • moar testing

Note I won't be going the container image topology in this PR, so "the old way" of producing renderablenodes will be around for a little longer.

@tomwilkie tomwilkie force-pushed the 228-dev branch 2 times, most recently from 8b57567 to 5f57054 Compare June 16, 2015 17:57
major = fmt.Sprintf("%s:%s", m["addr"], m["port"])
minor = fmt.Sprintf("%s (%s)", m[report.HostNodeID], m["pid"])
rank = m["pid"]
)

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@peterbourgon
Copy link
Contributor

From discussion:

  • Every Leaf mapping MUST append the origin node ID to the list of OriginNodes in the destination RenderableNode. This is done in LeafMap.Render.
  • Identity LeafMapFuncs (mapping into the same domain, e.g. process to process) SHALL copy their NodeMetadata into the destination RenderableNode. This is done in NewRenderableNode, which is always invoked by the identity LeafMapFuncs.
  • Non-identity LeafMapFuncs (mapping from one domain into another, e.g. endpoint to process) MUST NOT copy their NodeMetadata into the destination RenderableNode. Absent a sophisticated, per-key metadata merge strategy, that copy would be both lossy and destructive. Further, that metadata isn't necessary for building the topology, and can be retrieved later by following the OriginNode foreign keys. This is all currently true, as non-identity LeafMapFuncs always invoke newDerivedNode, which doesn't touch NodeMetadata.

@tomwilkie tomwilkie changed the title WIP: Produce containers topology from endpoints via processes Produce containers topology from endpoints via processes Jun 17, 2015
@tomwilkie
Copy link
Contributor Author

@peterbourgon I think this is ready to go now. You were right about not-propagating-NMDs for derived nodes.

@peterbourgon
Copy link
Contributor

OK let me do another parse...

// MapFunc is anything which can take an arbitrary RenderableNode and
// return another RenderableNode.
//
// As with LeadMapFunc, if the final output parameter is false, the node

This comment was marked as abuse.

@peterbourgon
Copy link
Contributor

Okay, I follow everything. After those edits, LGTM.

@tomwilkie
Copy link
Contributor Author

Address feedback; thanks!

there are 2 problems with the PR as it stands:
(1) containers by image is broken; I have another PR for that. Will "fix it up" in this branch
(2) we get this weird periodic "shuffling" in the UI - I think its something to do with the process topology getting out of sync with the endpoint topology

tomwilkie added a commit that referenced this pull request Jun 17, 2015
Produce containers topology from endpoints via processes
@tomwilkie tomwilkie merged commit 5d7c860 into weaveworks:master Jun 17, 2015
@tomwilkie tomwilkie deleted the 228-dev branch June 17, 2015 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Produce containers topology from endpoints via processes...
2 participants