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.

Sign up for GitHub

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

Jump to bottom

Extend metadata in details panel for Weave Net nodes #1973

Merged
merged 3 commits into from
Nov 4, 2016
Merged

Extend metadata in details panel for Weave Net nodes #1973

merged 3 commits into from
Nov 4, 2016

Conversation

2opremio
Copy link
Contributor

@2opremio 2opremio commented Nov 3, 2016 •
edited
Loading

Fixes #1938

Here's how it looks:

screen shot 2016-11-03 at 16 04 29

There's still no connections table, that will come later. I don't see it critical for the kubecon deadline, but let me know if I am wrong.

@rade
Copy link
Member

rade commented Nov 3, 2016

There's still no connection table... let me know if I am wrong.

You are wrong.

@2opremio
Copy link
Contributor Author

2opremio commented Nov 3, 2016

You are wrong.

OK, I will work on that next, but let's merge this first

@rade
Copy link
Member

rade commented Nov 3, 2016

I find the "#" notation a bit odd. I don't think we are using that elsewhere.

jml
jml reviewed Nov 3, 2016
View reviewed changes
Copy link
Contributor

@jml jml left a comment

Choose a reason for hiding this comment

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

Please bounce back for follow-up.

probe/overlay/weave.go
WeaveProxyAddress = "weave_proxy_address"
WeavePluginTableID = "weave_plugin_table"
WeavePluginStatus = "weave_plugin_status"
WeavePluginDriver = "weave_plugin_driver"

This comment was marked as abuse.

Sign in to view

probe/overlay/weave.go
weaveMetadata = report.MetadataTemplates{
WeaveVersion: {ID: WeaveVersion, Label: "Version", From: report.FromLatest, Priority: 1},
WeaveProtocol: {ID: WeaveProtocol, Label: "Protocol", From: report.FromLatest, Priority: 2},
WeavePeerName: {ID: WeavePeerName, Label: "Peer Name", From: report.FromLatest, Truncate: 17, Priority: 3},

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

probe/overlay/weave.go

switch err.(type) {
case *docker_client.Error:
// This is really ugly, but there's no way around it :(

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

probe/overlay/weave.go
)
}
return r, nil
}

func (w *Weave) getPeerNode(peer weave.Peer) report.Node {

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

prog/probe.go
var pluginAPIVersion = "1"
var (
pluginAPIVersion = "1"
dockerEndpoint = "unix:///var/run/docker.sock"

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

probe/overlay/weave.go
}
// Peer corresponding to current host
if peer.Name == w.statusCache.Router.Name {
latests[report.HostNodeID] = w.hostID

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@2opremio
Copy link
Contributor Author

2opremio commented Nov 3, 2016

I find the "#" notation a bit odd. I don't think we are using that elsewhere.

We are (in the container count of the container image topology). I tried to be consistent here but I can happily change it.

@rade
Copy link
Member

rade commented Nov 3, 2016 •
edited
Loading

We are (using the "#" notation) in the container count of the container image topology

The notation there is "# CONTAINERS: 2", not "CONTAINERS: #2".

@2opremio
Copy link
Contributor Author

2opremio commented Nov 3, 2016 •
edited
Loading

The notation there is "# CONTAINERS: 2", not "CONTAINERS: #2".

Ops, my bad, will fix

@2opremio
Copy link
Contributor Author

2opremio commented Nov 3, 2016 •
edited
Loading

Everything should now be addressed (and fixed a bug on the way). @jml Thanks for the review, please take another look.

Here's a screenshot after the fixes:

screen shot 2016-11-03 at 23 02 02

@rade
Copy link
Member

rade commented Nov 3, 2016

"DEF. SUBNET"? Why not spell it out? I recommend using exactly the same keys as in the weave status output, except perhaps for replacing "CompoundNoun" with "Compound Noun".

And why do the keys take up so much horizontal space, instead of leaving that to the values? Hence the unfortunate truncation of the proxy address.

rade
rade reviewed Nov 3, 2016
View reviewed changes
common/weave/client.go
}
Targets []string
TrustedSubnets []string
}

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

@2opremio 2opremio merged commit 3ba83dd into master Nov 4, 2016
@2opremio 2opremio deleted the 1938-enrich-weave-details-panel branch November 4, 2016 08:45
@2opremio 2opremio mentioned this pull request Feb 7, 2017
Open
@rndstr rndstr mentioned this pull request Jun 21, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jml jml jml left review comments

@rade rade rade left review comments

Assignees

@jml jml

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@2opremio @rade @jml