feat: Add support for endpoint topology routing hints #2090

MarkSRobinson · 2023-06-05T16:22:14Z

What this PR does / why we need it:

This PR adds support for reporting the topology routing hints applied to Endpoint objects in kubernetes. This is useful to check if the underlying network layer has hints enabled or if it's been disabled. This will let users diagnose if they had topology aware routing enabled or disable in the networking configuration.

How does this change affect the cardinality of KSM: Increases, if people have topology aware routing enabled and working.

linux-foundation-easycla · 2023-06-05T16:22:19Z

The committers listed above are authorized under a signed CLA.

✅ login: MarkSRobinson / name: Mark Robinson (5968a5e, b4247ac, 6cb9af1, 7cdf165, 699725b, 2cf37cc, ba3f879, 3ae8a28, ada647e, 1d03e5c, 8215b55, e47e783, 92f8173)
✅ login: dgrisonnet / name: Damien Grisonnet (2362bf2)

k8s-ci-robot · 2023-06-05T16:22:22Z

Welcome @MarkSRobinson!

It looks like this is your first PR to kubernetes/kube-state-metrics 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kube-state-metrics has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

Fix goimports Fix gocritic issue

mrueg · 2023-06-05T20:28:55Z

Thanks for your contribution!
This is a new feature and should target the main branch.

Also please review the CLA and sign it, otherwise we are unable to accept your change.

MarkSRobinson · 2023-06-05T21:13:17Z

@mrueg I've changed the base to main, but I'm hoping for a release on the 2.9 branch too. I'm trying to keep my k8s version and ksm/go-client version reasonably closely in-sync.

I've signed the CLA too.

MarkSRobinson · 2023-06-09T21:48:15Z

@mrueg This PR is ready to go when you're ready.

logicalhan · 2023-06-15T16:52:35Z

/triage accepted
/assign @dgrisonnet @rexagod

mrueg · 2023-07-13T08:42:54Z

internal/store/endpointslice_test.go

@@ -17,6 +17,7 @@ import (
 	"testing"

 	corev1 "k8s.io/api/core/v1"
+


No need to add a blank line here

docs/endpointslice-metrics.md

internal/store/endpointslice.go

Co-authored-by: Manuel Rüger <manuel@rueg.eu>

Dockerfile

mrueg · 2023-08-11T14:37:42Z

@MarkSRobinson if you can remove the Dockerfile changes, we can get this merged. :)

MarkSRobinson · 2023-08-11T17:14:59Z

@mrueg Let me clean that up. I found that building for ARM was was easier if we pulled the Go parameters from the Docker build target environment. This let's us build it in k8s super easily.

dgrisonnet

I would suggest creating a new metric kube_endpointslice_endpoints_hints

dgrisonnet · 2023-08-11T19:51:11Z

internal/store/endpointslice.go

+						newlabelValues = append(newlabelValues, address)
+
+						// Hint is populated when the endpoint is configured to be zone aware and preferentially route requests to its local zone.
+						if ep.Hints != nil && len(ep.Hints.ForZones) > 0 {
+
+							// Because each endpoint can have multiple zones, we need to create a metric for each zone.
+							// and we need to make sure we aren't adding the hint label repeatedly, we need to copy the array
+							for _, zone := range ep.Hints.ForZones {
+								zoneLabelValues := make([]string, len(newlabelValues))
+								copy(zoneLabelValues, newlabelValues)
+								zoneLabelValues = append(zoneLabelValues, zone.Name)
+								m = append(m, &metric.Metric{
+									LabelKeys:   append(labelKeys, "hint"),
+									LabelValues: zoneLabelValues,
+									Value:       1,
+								})
+							}
+						} else {
+							m = append(m, &metric.Metric{
+								LabelKeys:   labelKeys,
+								LabelValues: newlabelValues,
+								Value:       1,
+							})
+						}


This shouldn't need to be in the loop iterating over addresses

@MarkSRobinson this is the comment I was mentioning 🙂

MarkSRobinson · 2023-08-24T19:32:29Z

@mrueg I'm ready to merge this PR if you are.

mrueg · 2023-08-24T19:52:09Z

@mrueg I'm ready to merge this PR if you are.

Can you address @dgrisonnet's comment here please? #2090 (comment)

MarkSRobinson · 2023-08-24T20:31:52Z

@dgrisonnet Can you explain why you want a new metric instead of adding to the existing one?

dgrisonnet · 2023-08-25T19:23:36Z

@MarkSRobinson it is generally a better practice to keep metrics focus on only one purpose as they become more readable and easier to use. So that's why I am suggesting to create a new metric dedicated to the hints rather than adding yet another label to the already very packed endpoints metric.

dgrisonnet · 2023-08-28T17:04:18Z

Hi @MarkSRobinson, we are planning to start cutting a new release of kube-state-metrics Wednesday. Do you think you'll be able to address the comments by then?

MarkSRobinson · 2023-08-29T04:38:32Z

@dgrisonnet yeah, I can get to this tomorrow

MarkSRobinson · 2023-08-29T20:52:46Z

@dgrisonnet It's ready if you want to take a look

docs/endpointslice-metrics.md

k8s-ci-robot · 2023-08-30T08:46:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MarkSRobinson, mrueg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mrueg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

internal/store/endpointslice.go

Remove unneeded label

MarkSRobinson · 2023-08-30T19:45:25Z

If someone can kick off another build, we can get this merged.

docs/endpointslice-metrics.md

dgrisonnet · 2023-08-31T14:22:20Z

Merging manually to squash the commits.

k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jun 5, 2023

k8s-ci-robot requested review from dgrisonnet and logicalhan June 5, 2023 16:22

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 5, 2023

Add support for endpoint topology routing hints

5968a5e

Fix goimports Fix gocritic issue

MarkSRobinson force-pushed the release-2.9 branch from 4ea5a45 to 5968a5e Compare June 5, 2023 20:25

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jun 5, 2023

MarkSRobinson changed the base branch from release-2.9 to main June 5, 2023 21:11

k8s-ci-robot assigned dgrisonnet Jun 15, 2023

k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Jun 15, 2023

k8s-ci-robot assigned rexagod Jun 15, 2023

k8s-ci-robot removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jun 15, 2023

mrueg reviewed Jul 13, 2023

View reviewed changes

docs/endpointslice-metrics.md Outdated Show resolved Hide resolved

mrueg reviewed Jul 13, 2023

View reviewed changes

internal/store/endpointslice.go Outdated Show resolved Hide resolved

MarkSRobinson and others added 2 commits July 17, 2023 09:40

Update docs/endpointslice-metrics.md

b4247ac

Co-authored-by: Manuel Rüger <manuel@rueg.eu>

Update internal/store/endpointslice.go

6cb9af1

Co-authored-by: Manuel Rüger <manuel@rueg.eu>

MarkSRobinson changed the title ~~Add support for endpoint topology routing hints~~ feat: Add support for endpoint topology routing hints Jul 17, 2023

Update Dockerfile

7cdf165

mrueg reviewed Jul 26, 2023

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

MarkSRobinson added 2 commits August 11, 2023 10:15

Update Dockerfile

699725b

Update endpointslice_test.go

2cf37cc

dgrisonnet reviewed Aug 11, 2023

View reviewed changes

MarkSRobinson and others added 2 commits August 29, 2023 08:39

Merge branch 'main' into release-2.9

ba3f879

Redesign metric

3ae8a28

mrueg reviewed Aug 30, 2023

View reviewed changes

docs/endpointslice-metrics.md Outdated Show resolved Hide resolved

mrueg approved these changes Aug 30, 2023

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 30, 2023

dgrisonnet reviewed Aug 30, 2023

View reviewed changes

internal/store/endpointslice.go Show resolved Hide resolved

internal/store/endpointslice.go Outdated Show resolved Hide resolved

internal/store/endpointslice.go Outdated Show resolved Hide resolved

MarkSRobinson and others added 4 commits August 30, 2023 12:20

Update endpointslice.go

ada647e

Remove unneeded label

Update endpointslice.go

1d03e5c

Update endpointslice-metrics.md

8215b55

Fix test

e47e783

Remove hostname

92f8173

dgrisonnet reviewed Aug 31, 2023

View reviewed changes

docs/endpointslice-metrics.md Outdated Show resolved Hide resolved

Update docs/endpointslice-metrics.md

2362bf2

dgrisonnet merged commit f369828 into kubernetes:main Aug 31, 2023
11 of 12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add support for endpoint topology routing hints #2090

feat: Add support for endpoint topology routing hints #2090

MarkSRobinson commented Jun 5, 2023 •

edited

Loading

linux-foundation-easycla bot commented Jun 5, 2023 •

edited

Loading

k8s-ci-robot commented Jun 5, 2023

mrueg commented Jun 5, 2023 •

edited

Loading

MarkSRobinson commented Jun 5, 2023

MarkSRobinson commented Jun 9, 2023

logicalhan commented Jun 15, 2023

mrueg Jul 13, 2023

mrueg commented Aug 11, 2023

MarkSRobinson commented Aug 11, 2023

dgrisonnet left a comment

dgrisonnet Aug 11, 2023

mrueg Aug 24, 2023

MarkSRobinson commented Aug 24, 2023

mrueg commented Aug 24, 2023 •

edited

Loading

MarkSRobinson commented Aug 24, 2023

dgrisonnet commented Aug 25, 2023

dgrisonnet commented Aug 28, 2023

MarkSRobinson commented Aug 29, 2023

MarkSRobinson commented Aug 29, 2023

k8s-ci-robot commented Aug 30, 2023

MarkSRobinson commented Aug 30, 2023

dgrisonnet commented Aug 31, 2023

		@@ -17,6 +17,7 @@ import (
		"testing"

		corev1 "k8s.io/api/core/v1"

feat: Add support for endpoint topology routing hints #2090

feat: Add support for endpoint topology routing hints #2090

Conversation

MarkSRobinson commented Jun 5, 2023 • edited Loading

linux-foundation-easycla bot commented Jun 5, 2023 • edited Loading

k8s-ci-robot commented Jun 5, 2023

mrueg commented Jun 5, 2023 • edited Loading

MarkSRobinson commented Jun 5, 2023

MarkSRobinson commented Jun 9, 2023

logicalhan commented Jun 15, 2023

mrueg Jul 13, 2023

Choose a reason for hiding this comment

mrueg commented Aug 11, 2023

MarkSRobinson commented Aug 11, 2023

dgrisonnet left a comment

Choose a reason for hiding this comment

dgrisonnet Aug 11, 2023

Choose a reason for hiding this comment

mrueg Aug 24, 2023

Choose a reason for hiding this comment

MarkSRobinson commented Aug 24, 2023

mrueg commented Aug 24, 2023 • edited Loading

MarkSRobinson commented Aug 24, 2023

dgrisonnet commented Aug 25, 2023

dgrisonnet commented Aug 28, 2023

MarkSRobinson commented Aug 29, 2023

MarkSRobinson commented Aug 29, 2023

k8s-ci-robot commented Aug 30, 2023

MarkSRobinson commented Aug 30, 2023

dgrisonnet commented Aug 31, 2023

MarkSRobinson commented Jun 5, 2023 •

edited

Loading

linux-foundation-easycla bot commented Jun 5, 2023 •

edited

Loading

mrueg commented Jun 5, 2023 •

edited

Loading

mrueg commented Aug 24, 2023 •

edited

Loading