8000 Include SMI metrics as part of Linkerd install by adleong · Pull Request #4109 · linkerd/linkerd2 · GitHub
[go: up one dir, main page]

Skip to content

Include SMI metrics as part of Linkerd install - #4109

Merged
adleong merged 9 commits into
masterfrom
alex/install-smi-metrics
Mar 2, 2020
Merged

adleong merged 9 commits into
masterfrom
alex/install-smi-metrics

Conversation

@adleong
@adleong adleong commented Feb 27, 2020
Copy link
Copy Markdown
Contributor

Adds the SMI metrics API to the Linkerd install flow. This installs the SMI metrics controller deployment, the SMI metrics ApiService object, and supporting RBAC, and config resources.

This is the first step toward having Linkerd consume the SMI metrics API in the CLI and web dashboard. For a sneak peek at that work, try out the linkerd alpha stat command in the smi-metrics branch.

Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
@Pothulapati
Copy link
Copy Markdown
Contributor

I don't have strong opinions here, but Should this be an add-on i.e optional? like we did for tracing at #3955 :)

@adleong
adleong commented Feb 27, 2020
Copy link
Copy Markdown
Contributor Author

@Pothulapati good question! Since the plan is to eventually switch the CLI and web dashboard over to this API, I think it makes sense to have it as a core component rather than an addon.

@Pothulapati
Copy link
Copy Markdown
Contributor

@adleong Ah! Makes Sense :)

Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
@ihcsim ihcsim left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code change LGTM 👍.

What is the plan on managing the divergence from upstream?

Comment thread cli/cmd/install_test.go Resolved
@kleimkuhler kleimkuhler left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Have a quick question about the install process and if we need to account for multi-stage installs.

Comment thread cli/cmd/install.go Resolved
Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
@adleong
adleong commented Feb 28, 2020
Copy link
Copy Markdown
Contributor Author

@ihcsim This effectively pins our dependency on deislabs/smi-metrics:v0.2.0 and on the Linkerd smi-metrics-config copied from the smi metrics repo. When we want to pick up a new version, we will need to bump the image version and copy the new config over.

Comment thread cli/cmd/install.go Resolved
@ihcsim
ihcsim commented Mar 2, 2020
Copy link
Copy Markdown
Contributor

@adleong While it's not great that we have to manually merge new YAML and maintain the custom changes in these templates down the road, I can't think of a better way for this atm. I was hoping to write a script that can auto-generate all these smi-metrics templates whenever we need to upgrade to a new version, by using helm template to render the smi-metrics original chart, then use kustomize to inject the custom labels, annotations, name prefix and some other fields into the rendered output. The proxy and proxy-init spec remains the main challenge, because we can't just run helm template on the partials chart which consists many install-time configuration. Otherwise, the rendered partial chart can be used by kustomize to inject the proxy spec into the original smi-metrics templates.

Even adding smi-metrics as a subchart to the Linkerd chart in the requirements.yaml isn't great, because it means the unit test won't pass without first running helm dep update.

Signed-off-by: Alex Leong <alex@buoyant.io>
@kleimkuhler kleimkuhler left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

@adleong
adleong merged commit 71d6a00 into master Mar 2, 2020
@adleong
adleong deleted the alex/install-smi-metrics branch March 2, 2020 18:11
adleong added a commit that referenced this pull request Mar 2, 2020
Adds the SMI metrics API to the Linkerd install flow.  This installs the SMI metrics controller deployment, the SMI metrics ApiService object, and supporting RBAC, and config resources.

This is the first step toward having Linkerd consume the SMI metrics API in the CLI and web dashboard.

Signed-off-by: Alex Leong <alex@buoyant.io>
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.

4 participants

0