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

kubeadm: graduate WaitForAllControlPlaneComponents to Beta #129620

Merged
merged 1 commit into from
Feb 5, 2025
Merged

kubeadm: graduate WaitForAllControlPlaneComponents to Beta #129620

merged 1 commit into from
Feb 5, 2025

Conversation

neolit123
Copy link
Member

@neolit123 neolit123 commented Jan 14, 2025 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

  • Set the feature gate to Beta and enabled by default.
  • Make sure that the source of truth for which address/port to use for a component health check comes from the respective component static Pod manifest. That is done to comply with any user --patches that are applied on top
    of the ClusterConfiguration.

Which issue(s) this PR fixes:

xref

Special notes for your reviewer:

first commit fixes circular dependency problems. check the commit message.

Does this PR introduce a user-facing change?

kubeadm: graduated the WaitForAllControlPlaneComponents feature gate to Beta. When checking the health status of a control plane component, make sure that the address and port defined as arguments in the respective component's static Pod manifest are used.

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

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 14, 2025
@neolit123
Copy link
Member Author

/triage accepted
/priority backlog

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 14, 2025
@neolit123
Copy link
Member Author

/hold for review

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 14, 2025
@neolit123 neolit123 mentioned this pull request Jan 14, 2025
Open
@neolit123
Copy link
Member Author

neolit123 commented Jan 14, 2025 •
edited
Loading

running some e2e:
https://github.com/neolit123/kubeadm-test/actions/runs/12772694792
https://github.com/neolit123/kubeadm-test/actions/runs/12772686860
https://github.com/neolit123/kubeadm-test/actions/runs/12788889731

@neolit123
Copy link
Member Author

i believe we do have users that use the scheduler --config option and have no flags in the scheduler manifest.
like so:
https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/control-plane-flags/#scheduler-flags

for those users this new logic will use the default address/port.

as far as i can tell there is no way to set the --secrure-port and --bind-address as fields in the KubeSchedulerConfiguration?
https://kubernetes.io/docs/reference/config-api/kube-scheduler-config.v1
if that is somehow possible then the logic in this PR will not work and the user must also pass these flags to the scheduler.

@neolit123
Copy link
Member Author

neolit123 commented Jan 14, 2025 •
edited
Loading

there are some really messy circular dependencies going on. will try to debug more tomorrow or later this week..
edit - or maybe it gets fixed by this new commit.

@neolit123 neolit123 force-pushed the 1.33-update-all-cp-components-check branch from 1a1451a to 3a74d70 Compare January 14, 2025 19:52
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 14, 2025
@neolit123 neolit123 changed the title kubeadm: graduate WaitForAllControlPlaneComponents to Beta WIP: kubeadm: graduate WaitForAllControlPlaneComponents to Beta Jan 14, 2025
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 14, 2025
@neolit123 neolit123 force-pushed the 1.33-update-all-cp-components-check branch from 3a74d70 to d70fa87 Compare January 15, 2025 12:56
@neolit123 neolit123 force-pushed the 1.33-update-all-cp-components-check branch from d70fa87 to de6fa77 Compare January 15, 2025 12:58
@neolit123
Copy link
Member Author

neolit123 commented Jan 15, 2025 •
edited
Loading

running some e2e:
https://github.com/neolit123/kubeadm-test/actions/runs/12772694792
https://github.com/neolit123/kubeadm-test/actions/runs/12772686860
https://github.com/neolit123/kubeadm-test/actions/runs/12788889731

latest e2e test runs are green.

@neolit123 neolit123 force-pushed the 1.33-update-all-cp-components-check branch 2 times, most recently from 41a4b59 to 8dfaec6 Compare January 15, 2025 18:21
@pacoxu
Copy link
Member

pacoxu commented Jan 16, 2025

/cc @HirazawaUi

@HirazawaUi HirazawaUi mentioned this pull request Jan 16, 2025
Closed
@neolit123 neolit123 force-pushed the 1.33-update-all-cp-components-check branch from 8dfaec6 to 3bcef34 Compare January 28, 2025 10:23
@neolit123
Copy link
Member Author

neolit123 commented Jan 28, 2025 •
edited
Loading

@HirazawaUi updated. ptal, again.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: neolit123

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:

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 k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 29, 2025
@neolit123
kubeadm: graduate WaitForAllControlPlaneComponents to Beta
f310ac0 
- Set the feature gate to Beta and enabled by default.
- Make sure that the source of truth for which address/port
to use for a component health check comes from the respective
component static Pod manifest. That is done to comply
with any user --patches that are applied on top
of the ClusterConfiguration.
@neolit123 neolit123 force-pushed the 1.33-update-all-cp-components-check branch from 3bcef34 to f310ac0 Compare January 29, 2025 14:07
@neolit123 neolit123 requested a review from HirazawaUi January 29, 2025 14:08
HirazawaUi
HirazawaUi reviewed Jan 29, 2025
View reviewed changes
Copy link
Contributor

@HirazawaUi HirazawaUi left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold

wait for other reviewers' input, feel free to cancel it.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 29, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: b90bd44c4bdc8cfba1744d4f1ee4b35829ea1aae

pacoxu
pacoxu reviewed Feb 5, 2025
View reviewed changes
Copy link
Member

@pacoxu pacoxu left a comment

Choose a reason for hiding this comment

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

/lgtm

@pacoxu
Copy link
Member

pacoxu commented Feb 5, 2025

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 5, 2025
@k8s-ci-robot k8s-ci-robot merged commit 569d189 into kubernetes:master Feb 5, 2025
14 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.33 milestone Feb 5, 2025
swatisehgal pushed a commit to swatisehgal/kubernetes that referenced this pull request Feb 6, 2025
@k8s-ci-robot @swatisehgal
Merge pull request kubernetes#129620 from neolit123/1.33-update-all-c…
46994d1 
…p-components-check

kubeadm: graduate WaitForAllControlPlaneComponents to Beta
This was referenced Feb 6, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pacoxu pacoxu pacoxu left review comments

@HirazawaUi HirazawaUi HirazawaUi left review comments

@carlory carlory Awaiting requested review from carlory

@SataQiu SataQiu Awaiting requested review from SataQiu

Assignees

@pacoxu pacoxu

@HirazawaUi HirazawaUi

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubeadm cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. 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
None yet
Milestone
v1.33
Development

Successfully merging this pull request may close these issues.
4 participants
@neolit123 @pacoxu @k8s-ci-robot @HirazawaUi