Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce v1alpha2 kube proxy configuration #121830

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Introduce v1alpha2 kube proxy configuration #121830

wants to merge 3 commits into from

Conversation

aroradaman
Copy link
Member

@aroradaman aroradaman commented Nov 9, 2023 •
edited

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces v1alpha2 configuration for kube-proxy. Major difference between both configuration:

Which issue(s) this PR fixes:

Fixes #117909

Special notes for your reviewer:

Does this PR introduce a user-facing change?

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

- [KEP]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-network/784-kube-proxy-component-config

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot
Copy link
Contributor

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ 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. labels Nov 9, 2023
@k8s-ci-robot
Copy link
Contributor

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.

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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Nov 9, 2023
@k8s-ci-robot k8s-ci-robot added area/code-generation area/kube-proxy kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. and removed 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. labels Nov 9, 2023
@aroradaman
Copy link
Member Author

/test pull-kubernetes-e2e-kind

@aroradaman
Copy link
Member Author

/test pull-kubernetes-e2e-gce

@aroradaman
Copy link
Member Author

/milestone v1.30

@k8s-ci-robot
Copy link
Contributor

@aroradaman: You must be a member of the kubernetes/milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Milestone Maintainers Team and have them propose you as an additional delegate for this responsibility.

In response to this:

/milestone v1.30

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/test-infra repository.

@aroradaman aroradaman marked this pull request as ready for review November 9, 2023 17:58
@aroradaman
kube-proxy: refactor api internal configuration
efdc2c0 
Refactor the internal configuration of kube-proxy to adhere to the
v1alpha2 version specifications as detailed in https://kep.k8s.io/784.

Signed-off-by: Daman Arora <aroradaman@gmail.com>
@aroradaman
kube-proxy: add v1alpha2 config
c5ba625 
Add v1alpha2 configuration of kube-proxy with the specifications
mentioned in https://kep.k8s.io/784.

Signed-off-by: Daman Arora <aroradaman@gmail.com>
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Mar 3, 2024 •
edited

@aroradaman: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubernetes-e2e-ubuntu-gce-network-policies adf3f49 link false /test pull-kubernetes-e2e-ubuntu-gce-network-policies
pull-kubernetes-e2e-gce-cos-alpha-features adf3f49 link false /test pull-kubernetes-e2e-gce-cos-alpha-features
pull-kubernetes-node-e2e-crio-dra adf3f49 link false /test pull-kubernetes-node-e2e-crio-dra
pull-kubernetes-conformance-kind-ipv6-parallel adf3f49 link false /test pull-kubernetes-conformance-kind-ipv6-parallel
pull-kubernetes-e2e-gci-gce-ingress adf3f49 link false /test pull-kubernetes-e2e-gci-gce-ingress
pull-kubernetes-e2e-gce-providerless adf3f49 link false /test pull-kubernetes-e2e-gce-providerless
pull-kubernetes-conformance-image-test adf3f49 link false /test pull-kubernetes-conformance-image-test
pull-kubernetes-e2e-gce-storage-slow adf3f49 link false /test pull-kubernetes-e2e-gce-storage-slow
pull-kubernetes-e2e-kind-nftables adf3f49 link false /test pull-kubernetes-e2e-kind-nftables
pull-kubernetes-e2e-gce-storage-snapshot adf3f49 link false /test pull-kubernetes-e2e-gce-storage-snapshot
pull-kubernetes-kind-dra adf3f49 link false /test pull-kubernetes-kind-dra
pull-kubernetes-e2e-gce-csi-serial adf3f49 link false /test pull-kubernetes-e2e-gce-csi-serial
pull-kubernetes-node-e2e-containerd-1-7-dra adf3f49 link false /test pull-kubernetes-node-e2e-containerd-1-7-dra
pull-kubernetes-e2e-kind-kms adf3f49 link false /test pull-kubernetes-e2e-kind-kms
pull-kubernetes-e2e-gci-gce-ipvs adf3f49 link false /test pull-kubernetes-e2e-gci-gce-ipvs
check-dependency-stats adf3f49 link false /test check-dependency-stats
pull-publishing-bot-validate adf3f49 link false /test pull-publishing-bot-validate
pull-kubernetes-e2e-storage-kind-disruptive adf3f49 link false /test pull-kubernetes-e2e-storage-kind-disruptive
pull-kubernetes-local-e2e adf3f49 link false /test pull-kubernetes-local-e2e
pull-kubernetes-cross adf3f49 link false /test pull-kubernetes-cross
pull-kubernetes-unit c5ba625 link true /test pull-kubernetes-unit

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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/test-infra repository. I understand the commands that are listed here.

@bart0sh bart0sh added this to Triage in SIG Node PR Triage Mar 4, 2024
@bart0sh
Copy link
Contributor

bart0sh commented Mar 22, 2024

/remove-sig node

@k8s-ci-robot k8s-ci-robot removed the sig/node Categorizes an issue or PR as relevant to SIG Node. label Mar 22, 2024
@bart0sh bart0sh moved this from Triage to Done in SIG Node PR Triage Mar 22, 2024
@leilajal
Copy link
Contributor

/remove-sig api-machinery

@k8s-ci-robot k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Mar 26, 2024
@SergeyKanzhelev SergeyKanzhelev moved this from Triage to Archive-it in SIG Node CI/Test Board Mar 27, 2024
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 19, 2024
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

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/test-infra repository.

danwinship
danwinship reviewed Apr 21, 2024
View reviewed changes
cmd/kube-proxy/app/options.go
// hostnameOverride, if set from the command line flag, takes precedence over the `HostnameOverride` value from the config file
hostnameOverride string
// nodeIPOverride, if set from the command line flag, takes precedence over the `NodeIPOverride` value from the config file
nodeIPOverride []string
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I should have said before; you should split out options.go (and options_test.go) without changing any functionality first (in particular, making as close to 0 changes to the unit test inputs+results as possible), then add/change things in a second commit. This is one of those things where as a developer you're expected to do some extra work to make things easier for reviewers / future developers.

cmd/kube-proxy/app/options_test.go
expectedHealthzBindAddresses: []string{"1.2.3.4/32"},
expectedHealthzPort: int32(12345),
expectedMetricsBindAddresses: []string{"2.3.4.5/32"},
expectedMetricsPort: int32(23456),
Copy link
Contributor

Choose a reason for hiding this comment

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

So the expected values here are new... to the extent that it's possible, it would be good to add those as another separate commit (either before or after splitting out options), so that we can see that their values don't change in the following commits. (Or else, we can see that they do change, and are able to figure out why.)

@danwinship danwinship mentioned this pull request Apr 22, 2024
Merged
@aroradaman
Copy link
Member Author

/remove-sig apps
/remove-sig architecture
/remove-sig auth
/remove-sig cli
/remove-sig cloud-provider
/remove-sig cluster-lifecycle
/remove-sig instrumentation
/remove-sig network
/remove-sig release
/remove-sig scalability
/remove-sig scheduling
/remove-sig storage
/remove-sig testing

@k8s-ci-robot k8s-ci-robot removed sig/apps Categorizes an issue or PR as relevant to SIG Apps. 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/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danwinship danwinship danwinship left review comments

@neolit123 neolit123 neolit123 left review comments

@andrewsykim andrewsykim Awaiting requested review from andrewsykim

@aojea aojea Awaiting requested review from aojea

@mborsz mborsz Awaiting requested review from mborsz

@mwielgus mwielgus Awaiting requested review from mwielgus

@caseydavenport caseydavenport Awaiting requested review from caseydavenport

Assignees
No one assigned
Labels
area/apiserver area/cloudprovider area/code-generation area/conformance Issues or PRs related to kubernetes conformance tests area/dependency Issues or PRs related to dependency changes area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework area/kube-proxy area/kubeadm area/kubectl area/kubelet area/provider/gcp Issues or PRs related to gcp provider area/release-eng Issues or PRs related to the Release Engineering subproject area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/windows Categorizes an issue or PR as relevant to SIG Windows. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
SIG Apps
Status: Needs Triage
SIG Auth
Status: !SIG Auth
SIG CLI
Status: Needs Triage
SIG Node CI/Test Board
Archive-it
SIG Node PR Triage
Done
[sig-windows] PR tracking
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

generic vs platform/backend-specific options in kube-proxy
7 participants
@aroradaman @k8s-ci-robot @k8s-triage-robot @bart0sh @leilajal @danwinship @neolit123