feat: Add flagz endpoint for kube-proxy by yongruilin · Pull Request #128985 · kubernetes/kubernetes

yongruilin · 2024-11-26T19:21:04Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #128984 and kubernetes/enhancements#4828

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Added a `/flagz` endpoint for kube-proxy

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Example response:

curl http://localhost:10249/flagz

kube-proxy flags
Warning: This endpoint is not meant to be machine parseable, has no formatting compatibility guarantees and is for debugging purposes only.

bind-address=0.0.0.0
bind-address-hard-fail=false
boot-id-file=/proc/sys/kernel/random/boot_id
cleanup=false
cluster-cidr=
config=/var/lib/kube-proxy/config.conf
config-sync-period=15m0s
conntrack-max-per-core=32768
conntrack-min=131072
conntrack-tcp-be-liberal=false
conntrack-tcp-timeout-close-wait=1h0m0s
conntrack-tcp-timeout-established=24h0m0s
conntrack-udp-timeout=0s
conntrack-udp-timeout-stream=0s

- [KEP] : https://github.com/kubernetes/enhancements/blob/master/keps/sig-instrumentation/4828-component-flagz/README.md

k8s-ci-robot · 2024-11-26T19:21:15Z

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

yongruilin · 2024-11-26T19:21:40Z

/sig instrumentation

aojea · 2024-11-26T20:44:16Z

For context @danwinship I talked offline with the author and in kube-proxy we found three options:

use healthz bind address, but I'm afraid this used for lb healthchecks and we have this dual vs single work that I suggested not to use
use metrics bind address, the metrics endpoint seems a more stable endpoint to get information of the component
bind in a new address, we already have enough problems with two addresses :)

/assign @danwinship
LGTM but waiting for Dan opinion

Henrywu573 · 2024-11-26T23:44:32Z

cc @Henrywu573

danwinship · 2024-12-02T16:39:32Z

Is there not a standard for this in other components?
ISTM that it should use the metrics IP, because the information is similarly low-level / debug-ish / potentially-sensitive, as compared to the healthz info.

yongruilin · 2024-12-02T17:14:31Z

@danwinship Thanks for the comment. There are PRs created for other components. I'm also using metrics' IP there.

sftim · 2024-12-10T12:11:42Z

/release-note-edit

Added a `/flagz` endpoint for kube-proxy

richabanker · 2024-12-11T17:21:32Z

/lgtm

k8s-ci-robot · 2024-12-11T17:21:40Z

LGTM label has been added.

Details

Git tree hash: 3ce5fe93157743d31a0a260a619d4cbac3966813

yongruilin · 2024-12-11T19:36:53Z

/retest

danwinship · 2024-12-11T19:55:37Z

+		if flagzReader != nil {
+			s.flagz = flagzReader
+		}
+	}


You don't need to check the feature gate here; flagzReader can only have been set if the feature gate is enabled.

(So you don't even need this as a separate section; you can just unconditionally set flags: flagzReader in the initial creation of the ProxyServer at the top of the function)

Updated. thanks!

danwinship · 2024-12-11T19:56:01Z


+	if utilfeature.DefaultFeatureGate.Enabled(zpagesfeatures.ComponentFlagz) {
+		if flagzReader != nil {
+			flagz.Install(proxyMux, "kube-proxy", flagzReader)


likewise, you don't need to check the feature gate here (though in this case you do still need the nil check)

danwinship · 2024-12-11T20:20:17Z

/lgtm
/approve
thanks!

k8s-ci-robot · 2024-12-11T20:20:24Z

LGTM label has been added.

Details

Git tree hash: 4e0cdd553b6406635880fefb583b1f80d94fb537

k8s-ci-robot · 2024-12-11T20:20:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, yongruilin

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~cmd/kube-proxy/OWNERS~~ [danwinship]

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

k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/kube-proxy sig/network Categorizes an issue or PR as relevant to SIG Network. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 26, 2024

k8s-ci-robot requested review from bowei and thockin November 26, 2024 19:21

k8s-ci-robot added the sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. label Nov 26, 2024

k8s-ci-robot assigned danwinship Nov 26, 2024

Henrywu573 mentioned this pull request Dec 3, 2024

Add statusz endpoint for kube-proxy #128989

Merged

yongruilin force-pushed the flagz-kube-proxy branch from 94a73be to 3f3c824 Compare December 11, 2024 17:08

k8s-ci-robot assigned richabanker Dec 11, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 11, 2024

danwinship suggested changes Dec 11, 2024

View reviewed changes

yongruilin force-pushed the flagz-kube-proxy branch from 3f3c824 to cfc92c7 Compare December 11, 2024 19:58

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 11, 2024

k8s-ci-robot requested review from danwinship and richabanker December 11, 2024 19:58

feat: Adds flagz page for kube-proxy

ebb02bb

yongruilin force-pushed the flagz-kube-proxy branch from cfc92c7 to ebb02bb Compare December 11, 2024 20:01

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 11, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 11, 2024

k8s-ci-robot merged commit a19c27d into kubernetes:master Dec 12, 2024

k8s-ci-robot added this to the v1.33 milestone Dec 12, 2024

richabanker mentioned this pull request Jan 16, 2025

Flagz for Kubernetes Components kubernetes/enhancements#4828

Open

30 tasks

Conversation

yongruilin commented Nov 26, 2024 • edited by k8s-ci-robot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

k8s-ci-robot commented Nov 26, 2024

Uh oh!

yongruilin commented Nov 26, 2024

Uh oh!

aojea commented Nov 26, 2024

Uh oh!

Henrywu573 commented Nov 26, 2024

Uh oh!

danwinship commented Dec 2, 2024

Uh oh!

yongruilin commented Dec 2, 2024

Uh oh!

sftim commented Dec 10, 2024

Uh oh!

richabanker commented Dec 11, 2024

Uh oh!

k8s-ci-robot commented Dec 11, 2024

Uh oh!

yongruilin commented Dec 11, 2024

Uh oh!

danwinship Dec 11, 2024

Choose a reason for hiding this comment

Uh oh!

yongruilin Dec 11, 2024

Choose a reason for hiding this comment

Uh oh!

danwinship Dec 11, 2024

Choose a reason for hiding this comment

Uh oh!

yongruilin Dec 11, 2024

Choose a reason for hiding this comment

Uh oh!

danwinship commented Dec 11, 2024

Uh oh!

k8s-ci-robot commented Dec 11, 2024

Uh oh!

k8s-ci-robot commented Dec 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

yongruilin commented Nov 26, 2024 •

edited by k8s-ci-robot

Loading