hubble[/relay]: add version to observer.ServerStatus and add and implement observer.GetNodes #13979

rolinh · 2020-11-10T23:00:59Z

Add version information to observer.ServerStatus.

Additionally, add observer.GetNodes and implement it in Hubble Relay. This new endpoint provides a list of nodes with name, version, address and connection state and TLS information. This is mostly useful for Hubble UI to provide a list of all nodes with version and status on a status page but it could also be leveraged by Hubble CLI (eg hubble nodes list).

For reference, this is the output I get when using grpcurl to issue an observer.GetNodes request against Hubble Relay on my 4 nodes dev cluster with one unavailable node:

$ grpcurl -plaintext -use-reflection localhost:4245 observer.Observer.GetNodes
{
  "nodes": [
    {
      "name": "kind-worker",
      "version": "cilium v1.9.90-gccaaaa966",
      "address": "172.18.0.2:4244",
      "state": "NODE_CONNECTED",
      "tls": {
        "enabled": true,
        "serverName": "kind-worker.default.hubble-grpc.cilium.io"
      },
      "uptimeNs": "60622955603",
      "numFlows": "304",
      "maxFlows": "4095",
      "seenFlows": "304"
    },
    {
      "name": "kind-worker2",
      "address": "172.18.0.5:4244",
      "state": "NODE_UNAVAILABLE",
      "tls": {
        "enabled": true,
        "serverName": "kind-worker2.default.hubble-grpc.cilium.io"
      }
    },
    {
      "name": "kind-control-plane",
      "version": "cilium v1.9.90-gccaaaa966",
      "address": "172.18.0.4:4244",
      "state": "NODE_CONNECTED",
      "tls": {
        "enabled": true,
        "serverName": "kind-control-plane.default.hubble-grpc.cilium.io"
      },
      "uptimeNs": "60888864685",
      "numFlows": "85",
      "maxFlows": "4095",
      "seenFlows": "85"
    },
    {
      "name": "kind-worker3",
      "version": "cilium v1.9.90-gccaaaa966",
      "address": "172.18.0.3:4244",
      "state": "NODE_CONNECTED",
      "tls": {
        "enabled": true,
        "serverName": "kind-worker3.default.hubble-grpc.cilium.io"
      },
      "uptimeNs": "60685807845",
      "numFlows": "84",
      "maxFlows": "4095",
      "seenFlows": "84"
    }
  ]
}

And for completeness, here is the version information in observer.ServerStatus:

$ grpcurl -plaintext -use-reflection localhost:4245 observer.Observer.ServerStatus
{
  "numFlows": "3415",
  "maxFlows": "12285",
  "seenFlows": "3415",
  "uptimeNs": "461108917333",
  "numConnectedNodes": 3,
  "numUnavailableNodes": 1,
  "unavailableNodes": [
    "kind-worker3"
  ],
  "version": "hubble-relay v1.9.90-g59c7b8350"
}

When querying a local hubble instance from a Cilium pod:

$ grpcurl -plaintext -use-reflection -unix /var/run/cilium/hubble.sock observer.Observer.GetNodes    
ERROR:
  Code: Unimplemented
  Message: GetNodes not implemented

And for the status:

$ grpcurl -plaintext -use-reflection -unix /var/run/cilium/hubble.sock observer.Observer.ServerStatus
{
  "numFlows": "2443",
  "maxFlows": "4095",
  "seenFlows": "2443",
  "uptimeNs": "670048540549",
  "version": "cilium v1.9.90-g59c7b8350"
}

Closes: #13935

Hubble: add GetNodes rpc endpoint

rolinh · 2020-11-11T16:05:30Z

Will rework this PR to add a new GetNodes rpc endpoint to the observer service.

rolinh · 2020-11-12T14:00:55Z

Changes implemented. This PR is now ready for review.

pkg/hubble/build/version.go

kaworu

Great patch, tested and it looks good! I have a couple of questions, and also would like to see a trivial test case for the codes.Unimplemented returned by Hubble's GetNodes.

pkg/hubble/relay/observer/server.go

nathanjsweet · 2020-11-18T18:57:29Z

test-me-please

tklauser

Change to blang/semver LGTM, one remark regarding missing test tag (see GH action failure).

pkg/hubble/build/version.go

tklauser

LGTM 🚀

kaworu

Working great, thanks!

Knowing about the running version is useful, notably during a cluster upgrade. Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

This endpoint is intended to be implemented by Hubble Relay to provide information about nodes and their status. Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

The new generated code breaks implementations of observer server because they are missing the new GetNodes method. To ensure that every commit compiles on its own, add stubs to implementations of observer server. Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

rolinh · 2020-11-23T10:52:49Z

test-me-please

rolinh · 2020-11-24T08:37:03Z

retest-4.9

EDIT: bah, I meant to re-test runtime. 4.9 run was ✔️

rolinh · 2020-11-24T08:37:54Z

retest-runtime

This failure is a flake (completely unrelated to this PR), see #14149.

rolinh · 2020-11-24T15:32:14Z

retest-runtime

rolinh · 2020-11-26T08:23:50Z

retest-net-next

glibsm · 2020-11-26T14:54:24Z

pkg/hubble/build/version.go

+	// component is the Hubble component (eg: hubble, hubble-relay).
+	component string
+	// Core represents the core version (eg: 1.9.0).
+	Core string
+	// Revision is the software revision, typically a Git commit SHA.
+	Revision string


why is one unexported?

glibsm · 2020-11-26T14:56:13Z

pkg/hubble/observer/local_observer.go

 		MaxFlows:  s.GetRingBuffer().Cap(),
 		NumFlows:  s.GetRingBuffer().Len(),
 		SeenFlows: atomic.LoadUint64(&s.numObservedFlows),
 		UptimeNs:  uint64(time.Since(s.startTime).Nanoseconds()),
 	}, nil
 }

+// GetNodes implements observerpb.ObserverClient.GetNodes.


does it implement it though :)

glibsm · 2020-11-26T15:01:39Z

pkg/hubble/relay/observer/server.go

+			continue
+		}
+		n.State = relaypb.NodeState_NODE_CONNECTED
+		p := p


i would suggest to at least now shadow the name.

EDIT: after further digging through faqs it seems like shadow is the recommendation directly from Rob Pike. golang/go@0cab7d5

@rolinh perhaps would be good to add a comment to explain the motivation?

@errordeveloper This is common practice and even recommended as the "easier" way of achieving this in the Go FAQ. I could add a comment here but then there are many places in the codebase that would need the same comment.

ah, if it's actually part of the FAQ, comment is unnecessary

rolinh added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/hubble Impacts hubble server or relay labels Nov 10, 2020

rolinh requested a review from a team as a code owner November 10, 2020 23:01

rolinh requested review from a team and ti-mo November 10, 2020 23:01

maintainer-s-little-helper bot added this to In progress in 1.10.0 Nov 10, 2020

rolinh requested a review from glibsm November 10, 2020 23:01

maintainer-s-little-helper bot assigned glibsm and ti-mo Nov 10, 2020

rolinh changed the title ~~hubble[/relay]: add version information and a list of nodes name and version to observer status~~ hubble[/relay]: add version information and a list of nodes with name and version to observer status Nov 10, 2020

rolinh marked this pull request as draft November 11, 2020 10:43

rolinh force-pushed the pr/rolinh/relay-all-nodes-status branch from f77e7e9 to 59c7b83 Compare November 12, 2020 13:44

rolinh changed the title ~~hubble[/relay]: add version information and a list of nodes with name and version to observer status~~ hubble[/relay]: add version to observer.ServerStatus and add and implement observer.GetNodes Nov 12, 2020

rolinh marked this pull request as ready for review November 12, 2020 13:57

rolinh commented Nov 12, 2020

View reviewed changes

pkg/hubble/build/version.go Outdated Show resolved Hide resolved

rolinh force-pushed the pr/rolinh/relay-all-nodes-status branch from 59c7b83 to c7f9f47 Compare November 13, 2020 08:23

kaworu requested a review from yandzee November 13, 2020 10:30

maintainer-s-little-helper bot assigned yandzee Nov 13, 2020

kaworu reviewed Nov 13, 2020

View reviewed changes

pkg/hubble/relay/observer/server.go Outdated Show resolved Hide resolved

pkg/hubble/relay/observer/server.go Show resolved Hide resolved

rolinh force-pushed the pr/rolinh/relay-all-nodes-status branch 2 times, most recently from 751f0f8 to ccaaaa9 Compare November 16, 2020 10:11

rolinh mentioned this pull request Nov 16, 2020

cmd: add node list subcommand to list hubble nodes with status cilium/hubble#427

Merged

ti-mo reviewed Nov 16, 2020

View reviewed changes

pkg/hubble/relay/observer/server.go Outdated Show resolved Hide resolved

ti-mo approved these changes Nov 16, 2020

View reviewed changes

rolinh force-pushed the pr/rolinh/relay-all-nodes-status branch from ccaaaa9 to efaa764 Compare November 19, 2020 13:11

rolinh requested a review from a team as a code owner November 19, 2020 13:11

rolinh requested a review from tklauser November 19, 2020 13:11

rolinh requested a review from kaworu November 19, 2020 13:49

maintainer-s-little-helper bot assigned kaworu Nov 19, 2020

tklauser requested changes Nov 19, 2020

View reviewed changes

pkg/hubble/build/version.go Show resolved Hide resolved

rolinh force-pushed the pr/rolinh/relay-all-nodes-status branch from a8cef44 to c7c6f5d Compare November 19, 2020 14:00

rolinh requested a review from tklauser November 19, 2020 14:00

tklauser approved these changes Nov 19, 2020

View reviewed changes

kaworu added the kind/feature This introduces new functionality. label Nov 19, 2020

kaworu approved these changes Nov 19, 2020

View reviewed changes

rolinh added 8 commits November 23, 2020 11:38

api/observer: add version field to ServerStatusResponse

6db6e93

Knowing about the running version is useful, notably during a cluster upgrade. Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

api/observer: re-generate protobuf code

a130b70

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

hubble: add build package to provide hubble server and relay version

403a123

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

hubble/observer: add version information to status command

225df16

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

hubble/relay: add version information to status command

e03c1f0

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

api/observer: add GetNodes rpc endpoint

f46042c

This endpoint is intended to be implemented by Hubble Relay to provide information about nodes and their status. Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

hubble/relay: implement observer.GetNodes rpc endpoint

ab09bb6

Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>

rolinh force-pushed the pr/rolinh/relay-all-nodes-status branch from c7c6f5d to ab09bb6 Compare November 23, 2020 10:38

rolinh closed this Nov 25, 2020

rolinh reopened this Nov 25, 2020

yandzee approved these changes Nov 26, 2020

View reviewed changes

glibsm reviewed Nov 26, 2020

View reviewed changes

maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 26, 2020

glibsm approved these changes Nov 26, 2020

View reviewed changes

errordeveloper merged commit ae069dc into master Nov 27, 2020

errordeveloper deleted the pr/rolinh/relay-all-nodes-status branch November 27, 2020 12:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hubble[/relay]: add version to observer.ServerStatus and add and implement observer.GetNodes #13979

hubble[/relay]: add version to observer.ServerStatus and add and implement observer.GetNodes #13979

rolinh commented Nov 10, 2020 •

edited

rolinh commented Nov 11, 2020

rolinh commented Nov 12, 2020

kaworu left a comment

nathanjsweet commented Nov 18, 2020

tklauser left a comment

tklauser left a comment

kaworu left a comment

rolinh commented Nov 23, 2020

rolinh commented Nov 24, 2020 •

edited

rolinh commented Nov 24, 2020 •

edited

rolinh commented Nov 24, 2020

rolinh commented Nov 26, 2020

glibsm Nov 26, 2020 •

edited

glibsm Nov 26, 2020

glibsm Nov 26, 2020 •

edited

errordeveloper Nov 27, 2020

rolinh Nov 27, 2020

errordeveloper Nov 27, 2020

hubble[/relay]: add version to observer.ServerStatus and add and implement observer.GetNodes #13979

hubble[/relay]: add version to observer.ServerStatus and add and implement observer.GetNodes #13979

Conversation

rolinh commented Nov 10, 2020 • edited

rolinh commented Nov 11, 2020

rolinh commented Nov 12, 2020

kaworu left a comment

Choose a reason for hiding this comment

nathanjsweet commented Nov 18, 2020

tklauser left a comment

Choose a reason for hiding this comment

tklauser left a comment

Choose a reason for hiding this comment

kaworu left a comment

Choose a reason for hiding this comment

rolinh commented Nov 23, 2020

rolinh commented Nov 24, 2020 • edited

rolinh commented Nov 24, 2020 • edited

rolinh commented Nov 24, 2020

rolinh commented Nov 26, 2020

glibsm Nov 26, 2020 • edited

Choose a reason for hiding this comment

glibsm Nov 26, 2020

Choose a reason for hiding this comment

glibsm Nov 26, 2020 • edited

Choose a reason for hiding this comment

errordeveloper Nov 27, 2020

Choose a reason for hiding this comment

rolinh Nov 27, 2020

Choose a reason for hiding this comment

errordeveloper Nov 27, 2020

Choose a reason for hiding this comment

rolinh commented Nov 10, 2020 •

edited

rolinh commented Nov 24, 2020 •

edited

rolinh commented Nov 24, 2020 •

edited

glibsm Nov 26, 2020 •

edited

glibsm Nov 26, 2020 •

edited