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

Add the basics for a Weave-specific topology. #1182

Merged
merged 7 commits into from
Oct 28, 2016
Merged

Conversation

tomwilkie
Copy link
Contributor

@tomwilkie tomwilkie commented Mar 17, 2016

This is very much an early prototype. Fixes #1132.

screen shot 2016-03-17 at 16 03 18

It uses the information from weave report to build a topology of nodes and edges. It honours the edge direction from weave report. Peers from weave report which are not running scope are shown as pseudo nodes (fixes #778).

I'd like to be able to:

  • get metrics about traffic on various connections between weave routers (@awh, @rade, @bboreham?)
  • show more useful info in the details panel (like what?)
  • ensure the weave topology is shown as selected in the UI (@davkals, could you take a look?) (this was a non-issue when @2opremio revived the PR)
  • rename occurrences of overlay to weave (not applicable anymore since the overlay topology includes docker nodes)
  • make it obvious when weave is installed on a host and scope is not (done through pseudo nodes).
  • controls (forget, connect, rmpeer?)

@rade
Copy link
Member

rade commented Mar 17, 2016

Fixes #1132.

In #1132 I wasn't complaining about the lack of a weave view, but that the hosts view shows overlay topology rather than underlay topology.

@rade
Copy link
Member

rade commented Mar 17, 2016

#720 has a few ideas for what scope could show/visualise about weave net.

@pidster
Copy link
Contributor

pidster commented Mar 17, 2016

Why do we want a Weave specific view?

@rade
Copy link
Member

rade commented Mar 17, 2016

Why do we want a Weave specific view?

#720 provides a concrete use case.

However, I don't think providing a weave specific view should be conflated with making the existing Hosts view show underlay connections only, which is what #1132 - the apparent motivation for this PR - is about.

@rade
Copy link
Member

rade commented Mar 17, 2016

btw, I reckon showing the weave topology, which is what the bulk of this PR appears to do, is a reasonable first step on the way to #720.

@2opremio
Copy link
Contributor

Addresses #1938

@2opremio
Copy link
Contributor

2opremio commented Oct 24, 2016

@tomwilkie I don't think we should be renaming the Overlay topology anymore since:

  1. It contains non-weave-specific information (
    result.Overlay = result.Overlay.Merge(r.overlayTopology())
    , which was added after the present PR in Add docker networks to the Overlay Topology #1584 and released )
  2. Backwards compatibility. The overlay topology has been providing useful info for a while (like the weave IPs), for instance (1) was released on 0.16 and there will be probes using it.

@2opremio 2opremio force-pushed the 1132-weave-topology branch from 25144f6 to 8e4dfd7 Compare October 24, 2016 15:34
@2opremio
Copy link
Contributor

2opremio commented Oct 24, 2016

I've reworked the code without the renaming and applied it to current master (things had changed quite a bit). I saved the old code in branch 1132-weave-topology-original-tom just in case.

@2opremio 2opremio force-pushed the 1132-weave-topology branch from e86a9f4 to e80a01a Compare October 24, 2016 17:38
@bboreham
Copy link
Collaborator

Re controls:
connect would be cool - takes an address (e.g. an IP address or DNS name). Somewhat rare to use like this, I think, but nice for a demo.
forget needs to be applied to every peer in the system, so perhaps more work than it's worth.
rmpeer would be valid, if you can scrape the candidates from the weave status ipam data
reset, i.e. "shut down this Weave Net peer and remove it from the network" also conceptually valid, perhaps useful if you are migrating to a different machine

@bboreham
Copy link
Collaborator

Re the details panel:
Subsetted from weave status:

        Version: git-947ef8bb7770 (up to date; next check at 2016/10/26 16:15:53)

        Service: router
       Protocol: weave 1..2
           Name: 9a:90:6c:4a:40:07(dev)
     Encryption: disabled
  PeerDiscovery: enabled
 TrustedSubnets: none

        Service: ipam
         Status: ready
          Range: 10.32.0.0/12
  DefaultSubnet: 10.32.0.0/12

        Service: dns
         Domain: weave.local.
       Upstream: 10.0.2.3
            TTL: 1
        Entries: 1

        Service: plugin
     DriverName: weave

then add weave status connections info, perhaps above all of that except the peer name:

-> 192.168.48.11:6783    established sleeve 46:98:1c:22:6d:b7(host1)
-> 192.168.48.12:6783    established sleeve 5a:e6:43:04:ae:40(host2)
-> 127.0.0.1:6783        failed      cannot connect to ourself, retry: never

The interesting parts are sleeve vs fastdp vs awsvpc, encrypted or not, error message.

@rade
Copy link
Member

rade commented Oct 26, 2016

We should merge this w/o adding more stuff to the details panel. Let's do that in a separate issue/PR.

@2opremio
Copy link
Contributor

Re controls:

How about expose?

@2opremio
Copy link
Contributor

We should merge this w/o adding more stuff to the details panel. Let's do that in a separate issue/PR.

Good point. Will do

@2opremio
Copy link
Contributor

Current state of the PR:

screen shot 2016-10-26 at 14 04 19

screen shot 2016-10-26 at 14 04 34

Report: report.json.gz

(CC @davkal )

@pidster
Copy link
Contributor

pidster commented Oct 26, 2016

Q: Should we have a different icon for Weave routers? (Or, as they are containers, should the hexagon be used?)

@2opremio
Copy link
Contributor

Q: Should we have a different icon for Weave routers? (Or, as they are containers, should the hexagon be used?)

We chose the circle because:

  • Each node represents a host (they are not really containers, since they don't provide container information and each router is mapped to a host).
  • In fact, the topology is a child of the host topology (see were the "WEAVE NET" button is located)

@2opremio
Copy link
Contributor

2opremio commented Oct 27, 2016

That said, maybe we could use a different icon, but I wouldn't recommend an hexagon though since it will be confused with containers.

@mjlodge
Copy link

mjlodge commented Oct 27, 2016

Circles are good because they're hosts...

Copy link
Contributor

@paulbellamy paulbellamy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the questions about node shape and topology rank, code looks fine.

parent: "hosts",
renderer: render.WeaveRenderer,
Name: "Weave Net",
Rank: 3,

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@2opremio 2opremio merged commit b458c4c into master Oct 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants