Wire up stats commands for daemonsets (#2006) #2086

zknill · 2019-01-15T09:17:37Z

DaemonSet stats are not currently shown in the cli stat command, web ui
or grafana dashboard. This commit adds daemonset support for stat.

Update stat command's help message to reference daemonsets.
Update the public-api to support stats for daemonsets.
Add tests for stat summary and api.

Add daemonset get/list/watch permissions to the linkerd-controller
cluster role that's created using the install command.
Update golden expectation test files for install command
yaml manifest output.

Update web UI with daemonsets
Update navigation, overview and pages to list daemonsets and the pods
associated to them.
Add daemonset paths to server, and ui apps.

Add grafana dashboard for daemonsets; a clone of the deployment
dashboard.

Fixes of #2006

Signed-off-by: Zak Knill zrjknill@gmail.com

zknill · 2019-01-15T09:18:03Z

Validation

To test, took the example project curl https://run.linkerd.io/emojivoto.yml > emojivoto.yaml , and updated the Deployments for web, voting and emoji to be DaemonSets

cli

linkerd stat all -n emojivoto
linkerd stat daemonset -n emojivoto
linkerd stat ds -n emojivoto

linkerd stat daemonset/emoji -n emojivoto
linkerd stat ds/web -n emojivoto

linkerd stat daemonsets --to ds/emoji -n emojivoto
linkerd stat daemonsets --from ds/web -n emojivoto

$ linkerd stat ds -n emojivoto
NAME     MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   TLS
emoji       1/1   100.00%   1.9rps           1ms           3ms           3ms    0%
voting      1/1    79.66%   1.0rps           1ms           3ms           4ms    0%
web         1/1    89.57%   1.9rps          10ms          37ms          40ms    0%

web UI

Pages checked;

/overview
/daemonsets
/namespaces/:namespace/daemonsets/:daemonset

grafana

Checked dashboard, and links from above pages of the web ui

wmorgan · 2019-01-15T15:09:57Z

Awesome, thanks for submitting this @zknill!

Signed-off-by: Andrew Seigner <siggy@buoyant.io>

siggy · 2019-01-15T18:40:32Z

@zknill awesome! would you mind sharing your modified emojivoto/daemonsets config? bonus points for submitting it as a PR to https://github.com/BuoyantIO/emojivoto ;)

siggy

This is really great work @zknill! Thank you so much for the contribution!

Had a few minor nits but overall this is good to go. 👍 💥

siggy · 2019-01-15T19:35:55Z

controller/k8s/api.go

@@ -33,6 +35,7 @@ type APIResource int
 // These constants enumerate Kubernetes resource types.
 const (
 	CM APIResource = iota
+	Daemonset


mind naming this DS?

siggy · 2019-01-15T19:36:08Z

controller/k8s/api.go

-	sp       spinformers.ServiceProfileInformer
-	svc      coreinformers.ServiceInformer
+	cm        coreinformers.ConfigMapInformer
+	daemonset appv1informers.DaemonSetInformer


siggy · 2019-01-15T19:38:05Z

controller/api/public/stat_summary_test.go

+					err: nil,
+					k8sConfigs: []string{`
+apiVersion: apps/v1
+kind: DaemonSet


siggy · 2019-01-15T19:40:31Z

cli/cmd/stat.go

@@ -59,6 +59,7 @@ func newCmdStat() *cobra.Command {
  Examples:
  * deploy
  * deploy/my-deploy
+  * ds/my-daemonset


a few lines below this, add daemonsets under Valid resource types include:

zknill · 2019-01-16T11:23:40Z

@siggy very happy to open a PR, but not super sure where / how it would fit into that project. It seems more logical for that repo to hold deployments for the services... anyhow, linked below is the daemonset config that I used for testing.

https://gist.github.com/zknill/c2253bad3cc2da0a0b3b10617d2f2bc8

grampelberg · 2019-01-16T17:19:24Z

@zknill just create a secondary yaml named emojivoto-daemonset.yml in the root =) If you want extra brownie points, a PR on linkerd/website putting the same file at run.linkerd.io/public/emojivoto-daemonset.yml would be mind blowing!

klingerf

@zknill Awesome work! So cool to see this wired all the way through.

It looks like your most recent commit doesn't include a DCO signoff. Can I trouble you to rewrite that commit to include it so that we can get this merged?

I also noticed that there's no live tap data showing up on the daemonset detail page in the web UI. This is because we haven't updated the linkerd tap and linkerd top commands to support daemonsets. For instance, on your branch, I get:

$ bin/linkerd tap ds/emoji -n emojivoto
Error: unsupported resource type [daemonset]

And in the web UI, I see:

(Note that for unrelated reasons that error doesn't show up when you run bin/linkerd dashboard (it silently swallows websocket errors), but it does show up when you run bin/web run)

Anyhow, I actually don't think it would be a lot more work to support daemonsets in tap and top, and it would be fine to do that as part of this branch or in a follow up branch -- whatever's easiest for you.

klingerf · 2019-01-16T18:58:53Z

web/app/js/components/Namespace.jsx

@@ -142,6 +142,7 @@ class Namespaces extends React.Component {
              <NetworkGraph namespace={this.state.ns} deployments={metrics.deployment} />
            }
            {this.renderResourceSection("deployment", metrics.deployment)}
+            {this.renderResourceSection("daemonset", metrics.daemonset)}


This is super nit-picky: the renderResourceSection method converts the resource string "daemonset" to title-case, using the friendlyTitle method from web/app/js/components/util/Utils.js. There's a special case inside that function to convert "replicationcontroller" to "Replication Controllers", here:

linkerd2/web/app/js/components/util/Utils.js

Lines 128 to 130 in dacd881

if (resource === "replicationcontroller") {

titleCase = _startCase("replication controller");

}

My preference is to treat daemonsets similarly, by adding an additional special case for "daemonset", so that the headers are displayed as "Daemon Sets" instead of "Daemonsets".

klingerf · 2019-01-16T19:08:36Z

grafana/dashboards/daemonset.json

+    ]
+  },
+  "timezone": "",
+  "title": "Linkerd Daemonset",


Can we call this "Linkerd DaemonSet" to match the Kubernetes casing?

Depends on #2037, #2038, #2063, #2066, #2086, #2087, #2089 Signed-off-by: Andrew Seigner <siggy@buoyant.io>

Signed-off-by: Andrew Seigner <siggy@buoyant.io>

zknill · 2019-01-18T21:44:38Z

Sorry for slow response, on holiday at the moment. I will be able to look into things on Sunday / Monday

zknill · 2019-01-22T16:08:15Z

Hi there @klingerf, I intentionally omitted the top command support because I thought it out of scope of the original ticket.

I can look at adding it as that experience for users of the UI isn't great.

It might be worth updating the other similar tickets for StatefulSets / Jobs to be clear that they will also need top support?

zknill · 2019-01-22T17:08:26Z

I am looking into top and tap commands next

Signed-off-by: Andrew Seigner <siggy@buoyant.io>

siggy

thanks for all the updates @zknill ! one small doc nit (and a rebase from master), and then this is good to go.

siggy · 2019-01-22T21:52:45Z

cli/cmd/top.go

@@ -283,9 +283,11 @@ func newCmdTop() *cobra.Command {
  * deploy
  * deploy/my-deploy
  * deploy my-deploy
+  * ds/my-deploy


ds/my-daemonset ;)

dadjeibaah

This all looks great @zknill! The only thing left to do is just rebase off of master like siggy mentioned

zknill · 2019-01-24T08:01:36Z

Yes ofc, sorry all!

I'm sure you appreciate it's sometimes hard to find the time to do all the things - I've not forgotten!

DaemonSet stats are not currently shown in the cli stat command, web ui or grafana dashboard. This commit adds daemonset support for stat. Update stat command's help message to reference daemonsets. Update the public-api to support stats for daemonsets. Add tests for stat summary and api. Add daemonset get/list/watch permissions to the linkerd-controller cluster role that's created using the install command. Update golden expectation test files for install command yaml manifest output. Update web UI with daemonsets Update navigation, overview and pages to list daemonsets and the pods associated to them. Add daemonset paths to server, and ui apps. Add grafana dashboard for daemonsets; a clone of the deployment dashboard. Update dependencies and dockerfile hashes Fixes of linkerd#2006 Signed-off-by: Zak Knill <zrjknill@gmail.com>

Update daemonset in controller to use short form name Reorder the fields and methods to still be alphabetical List daemonset as one of the supported stat resources in cli help text Signed-off-by: Zak Knill <zrjknill@gmail.com>

Update grafana dashboard name to match kubernetes case; DasemonSet Update friendlyTitle for web UI to be "daemon set" Signed-off-by: Zak Knill <zrjknill@gmail.com>

The web UI support DaemonSets but the Live Calls section will error without support for the tap and top commands. Update tap and top commands with DaemonSet support. Signed-off-by: Zak Knill <zrjknill@gmail.com>

Signed-off-by: Zak Knill <zrjknill@gmail.com>

zknill · 2019-01-24T17:03:50Z

I think I've got everything; rebase and all

Let me know if there are other tweaks to be made! 🙏

Signed-off-by: Andrew Seigner <siggy@buoyant.io>

Relates to #2086 Signed-off-by: Andrew Seigner <siggy@buoyant.io>

siggy

So close @zknill ! I think all the changes in master caused some test failures, and surfaced a new issue, I've taken a crack at addressing things in a commit, feel free to pull this into your branch and verify:
eb5ed7b

Relates to linkerd#2086 Signed-off-by: Andrew Seigner <siggy@buoyant.io>

zknill · 2019-01-24T22:12:24Z

@siggy thank you so much for this! (not quite sure how this slipped through the net)

I've picked the commit, and rechecked the UI and stat, top and tap cli comamnds.

Thanks for all your help, I appreciate your time!

siggy

Awesome work @zknill ! Thanks for sticking with us through all the review!

zknill force-pushed the daemonset-stats branch from cc9917d to c5625f8 Compare January 15, 2019 11:25

wmorgan requested a review from klingerf January 15, 2019 15:10

siggy added a commit that referenced this pull request Jan 15, 2019

integration test branch for #2086

72d6cd4

Signed-off-by: Andrew Seigner <siggy@buoyant.io>

siggy added area/cli area/telemetry labels Jan 15, 2019

siggy self-requested a review January 15, 2019 18:44

siggy reviewed Jan 15, 2019

View reviewed changes

siggy mentioned this pull request Jan 16, 2019

Add daemonset emojivoto yaml manifests BuoyantIO/emojivoto#59

Merged

klingerf reviewed Jan 16, 2019

View reviewed changes

siggy added a commit that referenced this pull request Jan 16, 2019

Add release notes for edge-19.1.2 release

8e1be57

Depends on #2037, #2038, #2063, #2066, #2086, #2087, #2089 Signed-off-by: Andrew Seigner <siggy@buoyant.io>

siggy mentioned this pull request Jan 16, 2019

Add release notes for edge-19.1.2 release #2096

Merged

siggy added a commit that referenced this pull request Jan 16, 2019

Add release notes for edge-19.1.2 release

dbf546f

Depends on #2037, #2038, #2063, #2066, #2086, #2087, #2089 Signed-off-by: Andrew Seigner <siggy@buoyant.io>

siggy added a commit that referenced this pull request Jan 17, 2019

remove mention of #2086

7599be1

Signed-off-by: Andrew Seigner <siggy@buoyant.io>

rmars mentioned this pull request Jan 17, 2019

Allow the resource detail page to display traffic from more than one resource type #2108

Merged

zknill force-pushed the daemonset-stats branch from dfbd77f to 0747ea9 Compare January 22, 2019 16:15

siggy added a commit that referenced this pull request Jan 22, 2019

integration test branch for #2086

1023bf6

Signed-off-by: Andrew Seigner <siggy@buoyant.io>

siggy approved these changes Jan 22, 2019

View reviewed changes

dadjeibaah approved these changes Jan 24, 2019

View reviewed changes

zknill added 4 commits January 24, 2019 16:23

Review changes: Rename daemonset to ds

0f3bdb9

Update daemonset in controller to use short form name Reorder the fields and methods to still be alphabetical List daemonset as one of the supported stat resources in cli help text Signed-off-by: Zak Knill <zrjknill@gmail.com>

Review changes: rename Daemonset and friendlyTitle

7b220b7

Update grafana dashboard name to match kubernetes case; DasemonSet Update friendlyTitle for web UI to be "daemon set" Signed-off-by: Zak Knill <zrjknill@gmail.com>

Add DaemonSet support to tap and top commands

895d29f

The web UI support DaemonSets but the Live Calls section will error without support for the tap and top commands. Update tap and top commands with DaemonSet support. Signed-off-by: Zak Knill <zrjknill@gmail.com>

zknill force-pushed the daemonset-stats branch from d017abc to 895d29f Compare January 24, 2019 17:00

Review changes: Fix typo in top command help text

dbb691a

Signed-off-by: Zak Knill <zrjknill@gmail.com>

siggy added a commit that referenced this pull request Jan 24, 2019

integration branch testing for #2086

1ca2832

Signed-off-by: Andrew Seigner <siggy@buoyant.io>

siggy added a commit that referenced this pull request Jan 24, 2019

DS support in GetNameAndNamespaceOf, fix tests

eb5ed7b

Relates to #2086 Signed-off-by: Andrew Seigner <siggy@buoyant.io>

siggy reviewed Jan 24, 2019

View reviewed changes

DS support in GetNameAndNamespaceOf, fix tests

b269526

Relates to linkerd#2086 Signed-off-by: Andrew Seigner <siggy@buoyant.io>

siggy approved these changes Jan 24, 2019

View reviewed changes

siggy merged commit 8c413ca into linkerd:master Jan 24, 2019

dadjeibaah mentioned this pull request Jan 25, 2019

Wire up stats and dashboards for DaemonSets #2006

Closed

This was referenced Jan 25, 2019

WIP: CNI Plugin #2071

Merged

Dashboard error no longer shows #2151

Closed

klingerf mentioned this pull request Feb 2, 2019

Wire up stats and dashboards for StatefulSets #1983

Closed

dadjeibaah mentioned this pull request Feb 18, 2019

Wire up stats and dashboards for Jobs #2007

Closed

siggy mentioned this pull request Aug 21, 2019

Grafana: non-deployment centric dashboards #706

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wire up stats commands for daemonsets (#2006) #2086

Wire up stats commands for daemonsets (#2006) #2086

zknill commented Jan 15, 2019

zknill commented Jan 15, 2019

wmorgan commented Jan 15, 2019

siggy commented Jan 15, 2019

siggy left a comment

siggy Jan 15, 2019

siggy Jan 15, 2019

siggy Jan 15, 2019

siggy Jan 15, 2019

zknill commented Jan 16, 2019

grampelberg commented Jan 16, 2019

klingerf left a comment

klingerf Jan 16, 2019

klingerf Jan 16, 2019

zknill commented Jan 18, 2019

zknill commented Jan 22, 2019

zknill commented Jan 22, 2019

siggy left a comment

siggy Jan 22, 2019

dadjeibaah left a comment

zknill commented Jan 24, 2019

zknill commented Jan 24, 2019

siggy left a comment

zknill commented Jan 24, 2019

siggy left a comment

	if (resource === "replicationcontroller") {
	titleCase = _startCase("replication controller");
	}

Wire up stats commands for daemonsets (#2006) #2086

Wire up stats commands for daemonsets (#2006) #2086

Conversation

zknill commented Jan 15, 2019

zknill commented Jan 15, 2019

Validation

cli

web UI

grafana

wmorgan commented Jan 15, 2019

siggy commented Jan 15, 2019

siggy left a comment

Choose a reason for hiding this comment

siggy Jan 15, 2019

Choose a reason for hiding this comment

siggy Jan 15, 2019

Choose a reason for hiding this comment

siggy Jan 15, 2019

Choose a reason for hiding this comment

siggy Jan 15, 2019

Choose a reason for hiding this comment

zknill commented Jan 16, 2019

grampelberg commented Jan 16, 2019

klingerf left a comment

Choose a reason for hiding this comment

klingerf Jan 16, 2019

Choose a reason for hiding this comment

klingerf Jan 16, 2019

Choose a reason for hiding this comment

zknill commented Jan 18, 2019

zknill commented Jan 22, 2019

zknill commented Jan 22, 2019

siggy left a comment

Choose a reason for hiding this comment

siggy Jan 22, 2019

Choose a reason for hiding this comment

dadjeibaah left a comment

Choose a reason for hiding this comment

zknill commented Jan 24, 2019

zknill commented Jan 24, 2019

siggy left a comment

Choose a reason for hiding this comment

zknill commented Jan 24, 2019

siggy left a comment

Choose a reason for hiding this comment