karmada
karmada copied to clipboard
Cleanup deprecated methods in wait package
What would you like to be added:
- [x] Update
wait.Poll
towait.PollUntilContextTimeout
(withimmediately = false
) (@Affan-7, #3906 ) For example:
--- a/pkg/controllers/certificate/cert_rotation_controller.go
+++ b/pkg/controllers/certificate/cert_rotation_controller.go
@@ -165,7 +165,7 @@ func (c *CertRotationController) syncCertRotation(secret *corev1.Secret) error {
var newCertData []byte
klog.V(1).Infof("Waiting for the client certificate to be issued")
- err = wait.Poll(1*time.Second, 5*time.Minute, func() (done bool, err error) {
+ err = wait.PollUntilContextTimeout(context.TODO(), 1*time.Second, 5*time.Minute, false, func(ctx context.Context) (done bool, err error) {
csr, err := c.KubeClient.CertificatesV1().CertificateSigningRequests().Get(context.TODO(), csr, metav1.GetOptions{})
if err != nil {
return false, fmt.Errorf("failed to get the cluster csr %s. err: %v", clusterName, err)
- [x] Update
wait.PollImmediate
towait.PollUntilContextTimeout
(withimmediately = true
) (@liangyuanpeng, #3921) For example:
diff --git a/pkg/controllers/cluster/cluster_controller.go b/pkg/controllers/cluster/cluster_controller.go
index ae123a86e..678bf7188 100644
--- a/pkg/controllers/cluster/cluster_controller.go
+++ b/pkg/controllers/cluster/cluster_controller.go
@@ -394,7 +394,7 @@ func (c *Controller) monitorClusterHealth(ctx context.Context) (err error) {
for i := range clusters {
cluster := &clusters[i]
var observedReadyCondition, currentReadyCondition *metav1.Condition
- if err = wait.PollImmediate(MonitorRetrySleepTime, MonitorRetrySleepTime*HealthUpdateRetry, func() (bool, error) {
+ if err = wait.PollUntilContextTimeout(ctx, MonitorRetrySleepTime, MonitorRetrySleepTime*HealthUpdateRetry, true, func(ctx context.Context) (bool, error) {
// Cluster object may be changed in this function.
observedReadyCondition, currentReadyCondition, err = c.tryUpdateClusterHealth(ctx, cluster)
if err == nil {
- [x] Update
wait.PollUntil
toPollUntilContextCancel
(@zhzhuang-zju, #4947)
This might be a little bit challenging, we need to transition channel to context. Probably could leverage ContextForChannel.
- [ ] Enable static check by removing these lines
Why is this needed:
The wait.PollImmediate
, wait.Poll
, etc methods were deprecated in Kubernetes v1.27 (#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(2*time.Second, 10*time.Second, func() (done bool, err error) {
So it's pending for https://github.com/karmada-io/karmada/pull/3730 if i'm understand correctly.
/assign
Yes, exactly.
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.
Can I work on this?
@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.
Thanks @RainbowMango.
/assign @Affan-7
@RainbowMango Thanks for tasks list and i'm taking the wait. PollImmediate
.
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.
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.
Can I work on this?
@RainbowMango
@RainbowMango anything I can do?
@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.
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.
@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.
@zhzhuang-zju Would you like to help with this?
You can get started by Update wait.PollUntil to PollUntilContextCancel
.
@zhzhuang-zju Would you like to help with this?
Sure, I would.
seems like i missed the ping, let's keep going :hammer: