kubernetes / enhancements

Enhancements tracking repo for Kubernetes
Apache License 2.0
3.33k stars 1.44k forks source link

Remove transient node predicates from KCCM's service controller #3458

Open alexanderConstantinescu opened 1 year ago

alexanderConstantinescu commented 1 year ago

Enhancement Description

Please keep this description up to date. This will help the Enhancement Team to track the evolution of the enhancement efficiently.

alexanderConstantinescu commented 1 year ago

/sig network /sig cloud-provider

thockin commented 1 year ago

Punting to 1.27

k8s-triage-robot commented 1 year ago

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

This bot triages issues and PRs according to the following rules:

You can:

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

/lifecycle stale

thockin commented 1 year ago

/label lead-opted-in

alexanderConstantinescu commented 1 year ago

/retitle Remove transient node predicates from KCCM's service controller

marosset commented 1 year ago

@thockin @alexanderConstantinescu can one of you confirm that this targeting beta for v1.27. Thanks!

alexanderConstantinescu commented 1 year ago

Yes, that is correct and what we've discussed / agreed on the KEP. Maybe @thockin wants to give the final confirmation?

thockin commented 1 year ago

yes please - this one doesn't have APi so doesn't need an alpha

marosset commented 1 year ago

Thanks @thockin @alexanderConstantinescu

marosset commented 1 year ago

Once https://github.com/kubernetes/enhancements/pull/3835 merges this enhancement will be ready for inclusion in v1.27

shatoboar commented 1 year ago

With https://github.com/kubernetes/enhancements/pull/3835 merged this enhancement is ready to be tracked for 1.27. Thanks!

shatoboar commented 1 year ago

Hi @thockin πŸ‘‹, Checking in as we approach 1.27 code freeze at 17:00 PDT on Tuesday 14th March 2023. Please ensure the following items are completed:

Please let me know what PRs in k/k I should be tracking for this KEP. As always, we are here to help should questions come up. Thanks!

mickeyboxell commented 1 year ago

@alexanderConstantinescu was there a Docs PR opened against dev-1.27 branch in the k/website repo?

If not, please take a look at Documenting for a release - PR Ready for Review to get your PR ready for review as soon as possible. 01:00 UTC Wednesday 22nd March 2023 / 17:00 PDT Tuesday 21st March 2023 is the official deadline.

This PR will need a doc review by Tuesday 4th April 2023 to get this into the release. Please reach out to required SIGs to get their review. Thank you!

alexanderConstantinescu commented 1 year ago

Hi @mickeyboxell!

I am not sure what features qualify for a doc PR. My PR introduces no user-facing changes, would that still require a doc PR? I am asking because the link provided states:

Do your best to describe your feature and how to use it.

But there is no "explicit way to use", I guess one can see it as a "performance improvement" of sorts.

thockin commented 1 year ago

I think a release note is all that should be needed here.

On Tue, Mar 21, 2023 at 3:48β€―AM Alexander Constantinescu < @.***> wrote:

Hi @mickeyboxell https://github.com/mickeyboxell!

I am not sure what features qualify for a doc PR. My PR introduces no user-facing changes, would that still require a doc PR? I am asking because the link provided states:

Do your best to describe your feature and how to use it.

But there is no "explicit way to use", I guess one can see it as a "performance improvement" of sorts.

β€” Reply to this email directly, view it on GitHub https://github.com/kubernetes/enhancements/issues/3458#issuecomment-1477620297, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKWAVEZZ2EJCH65G3XXFBTW5GBPHANCNFSM552KGYSQ . You are receiving this because you were mentioned.Message ID: @.***>

thockin commented 1 year ago

NEXT: Aiming for GA in 1.28

thockin commented 1 year ago

Are we on-target for GA iin 1.28?

salehsedghpour commented 1 year ago

Hello @alexanderConstantinescu πŸ‘‹, Enhancements team here.

Just checking in as we approach enhancements freeze on 01:00 UTC Friday, 16th June 2023.

This enhancement is targeting for stage stable for 1.28 (correct me, if otherwise)

Here's where this enhancement currently stands:

For this KEP, we would just need to update the following:

The status of this enhancement is marked as at risk. Please keep the issue description up-to-date with appropriate stages as well. Thank you!

thockin commented 1 year ago

@alexanderConstantinescu https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3458-remove-transient-node-predicates-from-service-controller/kep.yaml still says "beta" and needs final PRR signoff (thanks, @wojtek-t for the poke)

alexanderConstantinescu commented 1 year ago

Are we on-target for GA iin 1.28?

Sorry for the late reply to this: the KEP says GA in 1.29 that isn't advanced to 1.28 just because the release went well, right?

alexanderConstantinescu commented 1 year ago

@alexanderConstantinescu https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3458-remove-transient-node-predicates-from-service-controller/kep.yaml still says "beta" and needs final PRR signoff (thanks, @wojtek-t for the poke)

Thanks for the reminder! Here's the PR: https://github.com/kubernetes/enhancements/pull/4088

I didn't update the stable milestone because of I mentioned above

thockin commented 1 year ago

Sorry, somehow I thought it was GA this release. Milestone bumped

Rishit-dagli commented 1 year ago

Hello @thockin @alexanderConstantinescu :wave:, 1.28 Docs Lead here.

Does this enhancement work planned for 1.28 require any new docs or modification to existing docs?

If so, please follows the steps here to open a PR against dev-1.28 branch in the k/website repo. This PR can be just a placeholder at this time and must be created before Thursday 20th July 2023.

Also, take a look at Documenting for a release to get yourself familiarize with the docs requirement for the release.

Thank you!

alexanderConstantinescu commented 1 year ago

@Rishit-dagli : sorry for the late reply

Does this enhancement work planned for 1.28 require any new docs or modification to existing docs?

No, it does not.

salehsedghpour commented 11 months ago

Hey again @alexanderConstantinescu :wave: Just checking in as we approach Code freeze at 01:00 UTC Friday, 19th July 2023 . Here’s the enhancement’s state for the upcoming code freeze:

Please keep the issue description up-to-date with all the PR/s that are associated with this KEP and let me know if there are other PR/s in k/k we should be tracking for this KEP. As always, we are here to help if any questions come up. Thanks!

Atharva-Shinde commented 11 months ago

Hey @alexanderConstantinescu πŸ‘‹ Enhancements Lead here, With https://github.com/kubernetes/kubernetes/pull/115204 merged as per the issue description, this enhancement is now tracked for v1.28 Code Freeze. Thanks!

thockin commented 10 months ago

I am tracking this for GA in 1.29 - is that correct?

alexanderConstantinescu commented 10 months ago

I am tracking this for GA in 1.29 - is that correct?

Yes, that is correct.

salehsedghpour commented 9 months ago

Hello @alexanderConstantinescu πŸ‘‹, Enhancements team here.

Just checking in as we approach enhancements freeze on Friday, 6th October 2023.

This enhancement is targeting for stage stable for 1.29 (correct me, if otherwise)

Here's where this enhancement currently stands:

For this KEP, we would just need to add/update the following:

The status of this enhancement is marked as at risk for enhancement freeze. Please keep the issue description up-to-date with appropriate stages as well. Thank you!

BenTheElder commented 9 months ago

I think this PR needs a metadata bump like https://github.com/kubernetes/enhancements/pull/4088/files

salehsedghpour commented 9 months ago

Hello @alexanderConstantinescu πŸ‘‹,

checking in once more as we approach the 1.29 enhancement freeze deadline on 01:00 UTC, Friday, 6th October, 2023. The status of this enhancement is marked as at risk. But It looks like https://github.com/kubernetes/enhancements/pull/4257 addresses most of the requirements. With this the status will be updated to tracked for the enhancement freeze.

Please let me know if I missed anything.

I would like to also ask to keep the issue description up-to-date with proper information such as the link to the merged PR.

alexanderConstantinescu commented 9 months ago

But It looks like #4257 addresses most of the requirements. With this the status will be updated to tracked for the enhancement freeze.

Hi @salehsedghpour ! That PR has merged and should fulfill all necessary requirements for progressing this KEP to the next phase. Let me know if anything is missing, but this KEP should be ready now for the next Kube release.

salehsedghpour commented 9 months ago

Hi @alexanderConstantinescu, thanks for the update. the status is now tracked for the enhancement freeze.

harshitasao commented 9 months ago

Hi @alexanderConstantinescu πŸ‘‹, v1.29 Docs Shadow here Does this enhancement work planned for v1.29 require any new docs or modification to existing docs? If so, please follows the steps here to open a PR against dev-1.29 branch in the k/website repo. This PR can be just a placeholder at this time and must be created before Thursday, 19 October 2023. Also, take a look at Documenting for a release to get yourself familiarize with the docs requirement for the release. Thank you!

salehsedghpour commented 8 months ago

Hey again @alexanderConstantinescu πŸ‘‹ Enhancements team here,

Just checking in as we approach code freeze at 01:00 UTC Wednesday 1st November.

Here's where this enhancement currently stands:

Please update the Issue description to include all the related PRs (including https://github.com/kubernetes/kubernetes/pull/121091, I guess) of this KEP under a new stable section in the Github Issue description. The status for this KEP is currently at risk for code freeze.

Also, please let me know if there are any other PRs in k/k we should be tracking for this KEP. As always, we are here to help if any questions come up. Thanks!

kcmartin commented 8 months ago

Hi @alexanderConstantinescu ! πŸ‘‹ from the v1.29 Release Team-Communications! We would like to check if you have any plans to publish a blog for this KEP regarding new features, removals, and deprecations for this release.

If so, you need to open a PR placeholder in the website repository. The deadline will be on Tuesday 14th November 2023 (after the Docs deadline PR ready for review)

Here's the 1.29 Calendar

npolshakova commented 8 months ago

Hello @alexanderConstantinescu πŸ‘‹, 1.29 Enhancements team here.

With all the implementation(code related) PRs merged as per the issue description, this enhancement is now marked as tracked for code freeze for the 1.29 Code Freeze! πŸš€

The test freeze is 01:00 UTC Wednesday 15th November 2023 / 18:00 PDT Tuesday 14th November 2023. Please make sure all test PRs are merged in by then. The tracked test PRs for this KEP are:

Please let me know if there are additional test PRs we should track. Thanks!

salehsedghpour commented 6 months ago

/remove-label lead-opted-in

salehsedghpour commented 5 months ago

Hello πŸ‘‹ 1.30 Enhancements Lead here,

I'm closing milestone 1.29 now, If you wish to progress this enhancement in v1.30, please follow the instructions here to opt in the enhancement and make sure the lead-opted-in label is set so it can get added to the tracking board and finally add /milestone v1.30. Thanks!

/milestone clear

thockin commented 5 months ago

Pinging this after sig-net - let's decide if this moves to GA in 1.30

aojea commented 5 months ago

I'll let it soak at least one release more https://github.com/kubernetes/kubernetes/pull/121116, WDYT @alexanderConstantinescu , just to get signal, these things take some time to detect

alexanderConstantinescu commented 5 months ago

I'll let it soak at least one release more https://github.com/kubernetes/kubernetes/pull/121116, WDYT @alexanderConstantinescu , just to get signal, these things take some time to detect

I am fine with this. I'm filing a PR updating the enhancement with a stable milestone set for 1.30

thockin commented 5 months ago

1.30 is next - did you mean 31?

alexanderConstantinescu commented 5 months ago

1.30 is next - did you mean 31?

I meant 1.30. My understanding is: if we want to "soak for one more release" then we extend the current milestone by one release. This feature was marked stable: 1.29, but I forgot to update the feature gate in Kube for this feature in 1.29, so it's still in beta. Extending beta by one release therefore means we go to stable in 1.30...does this makes sense?

@aojea : did you mean that we should go to stable in 1.31?

thockin commented 5 months ago

For context: It's been beta for 1.27, 1.28, 1.29 - majhor providers are all using those versions now.

I'm OK if we want to let it ride one more, but I am not sure it's buying us anything

alexanderConstantinescu commented 5 months ago

I'm OK if we want to let it ride one more, but I am not sure it's buying us anything

+1

I'll let Antonio decide, following how strongly he feels about it

aojea commented 5 months ago

For context: It's been beta for 1.27, 1.28, 1.29 - majhor providers are all using those versions now.

I'm OK if we want to let it ride one more, but I am not sure it's buying us anything

not really, major providers have to revendor and use to take some time to pick up these changes, all cloud provider code moved to external cloud providers, and it was beta but we had a fix in 1.29 with backports https://github.com/kubernetes/kubernetes/pull/121116/commits/2032b742546bd78ea6ac257c6e61fa9a31f67cfe

aojea commented 5 months ago

although, we are going to remove the cloud providers tests this release (most probably), so it will be useless to get it another release more, @alexanderConstantinescu go ahead and GA this then

thockin commented 5 months ago

Also, from the original bug:

""" This PR should have no effect on >= 1.27 since the predicates are already aligned under the feature gate StableLoadBalancerNodeSet, but this should get in so that we guard against the broken behaviour should someone turn that feature gate off. """

It was about a regression on 1.26 or when the gate was disabled.

aojea commented 5 months ago

It was about a regression on 1.26 or when the gate was disabled.

oh, my fault, sorry, I skipped that super important detail πŸ˜“