karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.14k stars 813 forks source link

Cleanup deprecated methods in wait package #3835

Open RainbowMango opened 9 months ago

RainbowMango commented 9 months ago

What would you like to be added:


- [x] Update `wait.PollUntil` to `PollUntilContextCancel` (@zhzhuang-zju, [#4947](https://github.com/karmada-io/karmada/pull/4947))

This might be a little bit challenging, we need to transition channel to context. Probably could leverage [ContextForChannel](https://github.com/karmada-io/karmada/blob/e5277b6317ac1a4717f5fac4057caf51a5d248fc/pkg/util/context.go#L11).

- [ ] Enable static check by removing [these lines](https://github.com/karmada-io/karmada/blame/e5277b6317ac1a4717f5fac4057caf51a5d248fc/.golangci.yml#L69-L72)

**Why is this needed**:
The `wait.PollImmediate`, `wait.Poll`, etc methods were deprecated in Kubernetes v1.27 ([#107826](https://github.com/kubernetes/kubernetes/pull/107826)), lint check is falling during update the dependencies of Kubernetes at #3730.
This kind of check has been disabled by #3730 to avoid huge modifications.

test/e2e/framework/cluster.go:320:9: SA1019: wait.PollImmediate is deprecated: This method does not return errors from context, use PollWithContextTimeout. Note that the new method will no longer return ErrWaitTimeout and instead return errors defined by the context package. Will be removed in a future release. (staticcheck) return wait.PollImmediate(2time.Second, 10time.Second, func() (done bool, err error) {

liangyuanpeng commented 9 months ago

So it's pending for https://github.com/karmada-io/karmada/pull/3730 if i'm understand correctly.

/assign

RainbowMango commented 9 months ago

Yes, exactly.

RainbowMango commented 9 months ago

Hi @liangyuanpeng I added some examples to the issue description, considering that the amount of changes may be relatively large, I have split these tasks into some interactive tasks. Hope this may make the review process more easier.

Affan-7 commented 9 months ago

Can I work on this?

RainbowMango commented 9 months ago

@Affan-7 Sure. How about taking the first one as a start? Update wait.Poll to wait.PollUntilContextTimeout (with immediately = false)

Just reserved this for you.

Affan-7 commented 9 months ago

Thanks @RainbowMango.

/assign @Affan-7

liangyuanpeng commented 9 months ago

@RainbowMango Thanks for tasks list and i'm taking the wait. PollImmediate.

liangyuanpeng commented 9 months ago

Please check the https://github.com/kubernetes/kubernetes/pull/119762 before working for Update wait.PollUntil to PollUntilContextCancel

Seems like the PollUntilContextCancel immediately have some problem.

RainbowMango commented 9 months ago

Thanks @liangyuanpeng for the update, While migrating the wait.Poll to wait.PollUntilContextTimeout, we are currently blocked by weird failing tests. I'm not sure if it is the same issue yet.

wlq1212 commented 8 months ago

Can I work on this?

@RainbowMango

Rei1010 commented 8 months ago

@RainbowMango anything I can do?

RainbowMango commented 8 months ago

@wlq1212 @Rei1010 Sure, thank you both in advance. There are two tasks left, please feel free to help with that. But it seems https://github.com/kubernetes/kubernetes/pull/119762 blocks this issue, I guess we need to wait for a while until there is an official Kubernetes release comes out.

I'll let you know once we are able to do it.

liangyuanpeng commented 5 months ago

Have create PR https://github.com/kubernetes/kubernetes/pull/122119 to cherrypick kubernetes#119762 for kubernetes v1.27, once it's merge, we can keeping going.

RainbowMango commented 1 week ago

@Affan-7 @liangyuanpeng We can get back on this now as we already updated the Kubernetes dependencies to v1.29+ which includes the fix we want. Please feel free to let me know if you can work on this.

@wlq1212 @Rei1010 Please let me know(just leave your comments here) if you want help, there are two items without assignments, you can pick anyone you like.

RainbowMango commented 4 days ago

@zhzhuang-zju Would you like to help with this? You can get started by Update wait.PollUntil to PollUntilContextCancel.

zhzhuang-zju commented 4 days ago

@zhzhuang-zju Would you like to help with this?

Sure, I would.