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

[1.33] promote KEP-3902 to stable #4392

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[1.33] promote KEP-3902 to stable #4392

wants to merge 1 commit into from

Conversation

carlory
Copy link
Member

@carlory carlory commented Jan 8, 2024

  • One-line PR description: promote KEP-3902 to stable
  • Other comments:

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. labels Jan 8, 2024
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 8, 2024
@carlory
Copy link
Member Author

carlory commented Jan 8, 2024

/cc @wojtek-t

@k8s-ci-robot k8s-ci-robot requested a review from wojtek-t January 8, 2024 01:54
@carlory carlory mentioned this pull request Jan 8, 2024
Closed
soltysh
soltysh reviewed Jan 23, 2024
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

You'll need the following to push this forward:

  1. Get a sig-scheduling lead opt-in this feature into 1.30, last time I see it was Aldo, so you should ping him about that
  2. Update the README.md with appopriate changes, from a quick skim you're missing:
    • release checklist updated
    • update tests with links to testgrid, based on tests added in beta
    • describe feedback/bugs you noticed during beta
    • update implementation history

@carlory
Copy link
Member Author

carlory commented Jan 23, 2024

thanks for your review. I'll fill it later.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 4, 2024
@soltysh
Copy link
Contributor

soltysh commented Feb 5, 2024

/hold
based on this comment #3902 (comment) it can't get promoted to beta, yet

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 5, 2024
@carlory
Copy link
Member Author

carlory commented Apr 22, 2024

/hold cancel

1.30 is released.

/cc @sanposhiho @soltysh @yuanchen8911

@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 Apr 22, 2024
@sanposhiho
Copy link
Member

/cc @Huang-Wei @alculquicondor
This KEP reviewers from sig-scheduling

@wojtek-t wojtek-t self-assigned this Apr 24, 2024
@carlory carlory force-pushed the patch-6 branch 2 times, most recently from 690a0e5 to 06daa76 Compare September 19, 2024 06:33
carlory
carlory commented Sep 19, 2024
View reviewed changes
keps/sig-scheduling/3902-decoupled-taint-manager/README.md Outdated
@@ -140,24 +140,29 @@ The creation and startup of the default taint-manager is performed by `kube-cont
Kubernetes already has a good coverage of node `No-Execute` eviction. We will add tests only for the changed code.

##### Unit tests
- `pkg/controller/taintmanager`: 2023-06-03 - 0% (to add)
- `pkg/controller/tainteviction`: 2024-09-18 - 81.8%
- Enable and disable the new `TaintEvictionController`
- The new `TaintEvictionController` acts on node taints properly
- `pkg/controller/apis/config/v1alpha1`: 2023-06-03 - 0%
- Test new configuration
Copy link
Member Author

@carlory carlory Sep 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

kubernetes/kubernetes#119208 did not introduce any new configuration. I want to check if we want to implement this item before promoting it to GA.

/cc @yuanchen8911 @atosatto

Copy link
Contributor

Choose a reason for hiding this comment

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

This is still unresolved, either new configuration was added or not, in either case we need to update the doc to reflect that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed it but still need some feedback from the original author of this KEP
/hold

Copy link
Member Author

Choose a reason for hiding this comment

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

/cc @yuanchen8911 @atosatto
Could you have a look when you have time?

soltysh
soltysh reviewed Sep 30, 2024
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

A few minor prr comments, you're still missing approval from sig-scheduling.

keps/sig-scheduling/3902-decoupled-taint-manager/README.md Outdated
@@ -140,24 +140,29 @@ The creation and startup of the default taint-manager is performed by `kube-cont
Kubernetes already has a good coverage of node `No-Execute` eviction. We will add tests only for the changed code.

##### Unit tests
- `pkg/controller/taintmanager`: 2023-06-03 - 0% (to add)
- `pkg/controller/tainteviction`: 2024-09-18 - 81.8%
- Enable and disable the new `TaintEvictionController`
- The new `TaintEvictionController` acts on node taints properly
- `pkg/controller/apis/config/v1alpha1`: 2023-06-03 - 0%
- Test new configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

This is still unresolved, either new configuration was added or not, in either case we need to update the doc to reflect that.

keps/sig-scheduling/3902-decoupled-taint-manager/README.md
@@ -173,14 +178,18 @@ Since there are no new API changes, we can skip Alpha and go to Beta directly.

#### GA
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe @ravisantoshgudimetla has some use cases for this particular feature. Feel free to sync with him about that.

keps/sig-scheduling/3902-decoupled-taint-manager/README.md Show resolved Hide resolved
keps/sig-scheduling/3902-decoupled-taint-manager/README.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 8, 2024
soltysh
soltysh approved these changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/approve
the PRR portion

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 8, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 8, 2025
@carlory
Copy link
Member Author

carlory commented Jan 8, 2025

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 8, 2025
@carlory carlory changed the title promote KEP-3902 to stable [1.33] promote KEP-3902 to stable Jan 14, 2025
@carlory @alculquicondor
promote KEP-3902 to stable
3959d0c 
Co-authored-by: Aldo Culquicondor <[email protected]>
Signed-off-by: carlory <[email protected]>
@carlory
Copy link
Member Author

carlory commented Feb 6, 2025

/cc @wojtek-t @yuanchen8911 @atosatto
please review it again. thanks!
/hold cancel

@k8s-ci-robot k8s-ci-robot requested a review from wojtek-t February 6, 2025 10:14
@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 6, 2025
@soltysh
Copy link
Contributor

soltysh commented Feb 6, 2025

Now that Wojtek is taking this over
/approve cancel

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: alculquicondor, carlory, yuanchen8911
Once this PR has been reviewed and has the lgtm label, please ask for approval from wojtek-t. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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 removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alculquicondor alculquicondor alculquicondor left review comments

@atiratree atiratree atiratree left review comments

@SergeyKanzhelev SergeyKanzhelev SergeyKanzhelev left review comments

@ravisantoshgudimetla ravisantoshgudimetla ravisantoshgudimetla left review comments

@soltysh soltysh soltysh approved these changes

@ahg-g ahg-g Awaiting requested review from ahg-g

@Huang-Wei Huang-Wei Awaiting requested review from Huang-Wei

@sanposhiho sanposhiho Awaiting requested review from sanposhiho

@yuanchen8911 yuanchen8911 Awaiting requested review from yuanchen8911

@atosatto atosatto Awaiting requested review from atosatto

@wojtek-t wojtek-t Awaiting requested review from wojtek-t

Assignees

@alculquicondor alculquicondor

@wojtek-t wojtek-t

@yuanchen8911 yuanchen8911

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
SIG Scheduling
Status: Needs Approval
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.