Skip to content

Add flagz endpoint for apiserver#127581

Merged
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
richabanker:flagz-apiserver
Nov 8, 2024
Merged

Add flagz endpoint for apiserver#127581
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
richabanker:flagz-apiserver

Conversation

@richabanker
Copy link
Copy Markdown
Contributor

@richabanker richabanker commented Sep 24, 2024 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds a /flagz endpoint for kube-apiserver

Which issue(s) this PR fixes:

kubernetes/enhancements#4828

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Adds a /flagz endpoint for kube-apiserver endpoint

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

Snippet of response returned by

curl -k --cert /etc/kubernetes/pki/apiserver-kubelet-client.crt --key /etc/kubernetes/pki/apiserver-kubelet-client.key https://localhost:6443/flagz 

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

advertise-address=192.168.8.2
contention-profiling=false
enable-priority-and-fairness=true
profiling=true
authorization-mode=[Node,RBAC]
authorization-webhook-cache-authorized-ttl=5m0s
authorization-webhook-cache-unauthorized-ttl=30s
authorization-webhook-version=v1beta1
default-watch-cache-size=100
delete-collection-workers=1
enable-garbage-collector=true
encryption-provider-config-automatic-reload=false
etcd-cafile=/etc/kubernetes/pki/etcd/ca.crt
etcd-certfile=/etc/kubernetes/pki/apiserver-etcd-client.crt
etcd-compaction-interval=5m0s
etcd-count-metric-poll-period=1m0s
etcd-db-metric-poll-interval=30s
etcd-healthcheck-timeout=2s
etcd-keyfile=/etc/kubernetes/pki/apiserver-etcd-client.key
etcd-prefix=/registry
etcd-readycheck-timeout=2s
etcd-servers=[https://127.0.0.1:2379]
lease-reuse-duration-seconds=60
storage-media-type=application/vnd.kubernetes.protobuf
watch-cache=true

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Sep 24, 2024
@k8s-ci-robot k8s-ci-robot added area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 24, 2024
@richabanker richabanker force-pushed the flagz-apiserver branch 4 times, most recently from 1de51ac to e67c926 Compare September 24, 2024 01:45
@k8s-ci-robot k8s-ci-robot added area/dependency Issues or PRs related to dependency changes area/kube-proxy area/kubectl area/kubelet sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 24, 2024
@richabanker
Copy link
Copy Markdown
Contributor Author

richabanker commented Sep 24, 2024

/remove-sig cli
/remove-sig network
/remove-sig node

/remove-area kubelet
/remove-area kube-proxy
/remove-area kubectl

@k8s-ci-robot k8s-ci-robot removed sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. labels Sep 24, 2024
dgrisonnet
dgrisonnet approved these changes Nov 6, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@dgrisonnet dgrisonnet left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Nov 6, 2024

LGTM label has been added.

DetailsGit tree hash: 63d6e2cc66c8dca81fc6c826e462e63c726c9fb9

@dgrisonnet
Copy link
Copy Markdown
Member

dgrisonnet commented Nov 6, 2024

Tests failures seem to be unrelated.

@dgrisonnet
Copy link
Copy Markdown
Member

dgrisonnet commented Nov 6, 2024

/lgtm

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Nov 6, 2024

LGTM label has been added.

DetailsGit tree hash: 1becdbcd8b8a5fadb89b13a677c9602236591669

@sttts
Copy link
Copy Markdown
Contributor

sttts commented Nov 7, 2024

/lgtm
/approve

Thanks @richabanker for the updates. Looks great now!

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Nov 7, 2024

LGTM label has been added.

DetailsGit tree hash: a01f3e8246e37f844e091ac9f7687c3745c00e17

@sttts
Copy link
Copy Markdown
Contributor

sttts commented Nov 7, 2024

/retest

1 similar comment
@richabanker
Copy link
Copy Markdown
Contributor Author

richabanker commented Nov 7, 2024

/retest

@liggitt
Copy link
Copy Markdown
Member

liggitt commented Nov 7, 2024

the integration failure looks related to this PR... I can't reproduce on master and I can reproduce the failure 100% on this PR

go test ./test/integration/apiserver/oidc -run 'TestOIDC/ID_token_is_ok$'

@liggitt
Copy link
Copy Markdown
Member

liggitt commented Nov 7, 2024

looks like that's been failing on this PR since 10/16 - https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/127581/pull-kubernetes-integration/1846632627503632384

@sttts
Copy link
Copy Markdown
Contributor

sttts commented Nov 7, 2024

It's very likely o.OIDC.areFlagsConfigured. That func is bound to the fs argument of the AddFlags method. And because this PR calls opts.Flags() another time, areFlagsConfigured is wrong. So it is not this PR that is wrong, but it just uncovers the hack in AddFlags, mutating the receiver, which is should not do.

@richabanker richabanker mentioned this pull request Nov 7, 2024
Merged
@richabanker
Copy link
Copy Markdown
Contributor Author

richabanker commented Nov 7, 2024 •
edited
Loading

It's very likely o.OIDC.areFlagsConfigured. That func is bound to the fs argument of the AddFlags method. And because this PR calls opts.Flags() another time, areFlagsConfigured is wrong. So it is not this PR that is wrong, but it just uncovers the hack in AddFlags, mutating the receiver, which is should not do.

Created #128674 for a partial fix for the OIDC flags. With this, the TestOIDC test was passing with the flagz changes.

@liggitt
Copy link
Copy Markdown
Member

liggitt commented Nov 7, 2024

/approve

@richabanker
Copy link
Copy Markdown
Contributor Author

richabanker commented Nov 7, 2024

/test pull-kubernetes-integration

@dgrisonnet
Copy link
Copy Markdown
Member

dgrisonnet commented Nov 7, 2024

/lgtm

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Nov 7, 2024

LGTM label has been added.

DetailsGit tree hash: 6e30f852399771f58bb2d52a175af52a481e9759

@richabanker
Copy link
Copy Markdown
Contributor Author

richabanker commented Nov 8, 2024

/hold
until #125577 merges

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Nov 8, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgrisonnet, liggitt, richabanker, sttts

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:

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

@dgrisonnet
Copy link
Copy Markdown
Member

dgrisonnet commented Nov 8, 2024

/lgtm

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Nov 8, 2024

LGTM label has been added.

DetailsGit tree hash: aae9d3dcdd951de2a40b516d01d8c7621e5c6f2c

@richabanker
Copy link
Copy Markdown
Contributor Author

richabanker commented Nov 8, 2024

/unhold
since #125577 is now merged

@wendy-ha18
Copy link
Copy Markdown
Member

wendy-ha18 commented Nov 8, 2024

Hi @richabanker @dgrisonnet
Appreciate all of your efforts with this PR!!
A gentle reminder that the code freeze has started 02:00 UTC Friday November 8th 2024 . Please make sure this has both lgtm and approved labels ASAP, and file an Exception.

@richabanker
Copy link
Copy Markdown
Contributor Author

richabanker commented Nov 8, 2024 •
edited
Loading

@wendy-ha18 lgtm label was added an hour ago, and the approve tag was added much before. Do I still need to file an exception?

@wendy-ha18
Copy link
Copy Markdown
Member

wendy-ha18 commented Nov 8, 2024

Hi @richabanker, IOM no you don't need to file Exception. I saw this PR in merging queue are now with a milestone. PR can be merged with milestone after code freeze.

@richabanker richabanker mentioned this pull request Nov 29, 2024
Merged
@richabanker richabanker mentioned this pull request Feb 11, 2025
Closed
@yongruilin yongruilin mentioned this pull request Feb 12, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@liggitt liggitt liggitt left review comments

@enj enj Awaiting requested review from enj

@johnbelamaric johnbelamaric Awaiting requested review from johnbelamaric

@caesarxuchao caesarxuchao Awaiting requested review from caesarxuchao

@dashpole dashpole Awaiting requested review from dashpole

@dgrisonnet dgrisonnet Awaiting requested review from dgrisonnet

@sttts sttts Awaiting requested review from sttts

+2 more reviewers

@jpbetz jpbetz jpbetz left review comments

@Jefftree Jefftree Jefftree left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@sttts sttts

@dgrisonnet dgrisonnet

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

SIG Apps
Archived in project
SIG Auth
Archived in project
SIG CLI
Archived in project
SIG Node CI/Test Board
Archived in project
SIG Node: code and documentation PRs
Archived in project
[sig-release] Bug Triage
Archived in project

Milestone

v1.32

Development

Successfully merging this pull request may close these issues.

10 participants

@richabanker @dgrisonnet @k8s-triage-robot @liggitt @sttts @k8s-ci-robot @wendy-ha18 @jpbetz @Jefftree @benjaminapetersen