fix race conditions in polling functions

Signed-off-by: Omer Aplatony <omerap12@gmail.com>
This commit is contained in:
Omer Aplatony 2024-12-14 16:35:42 +02:00
parent 80ed375e37
commit ed7f7ce92b
6 changed files with 25 additions and 25 deletions

View File

@ -73,9 +73,9 @@ func TestAPIApproval(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
err = wait.PollUntilContextTimeout(context.Background(), 100*time.Millisecond, 30*time.Second, true, func(ctx context.Context) (bool, error) { err = wait.PollUntilContextTimeout(context.Background(), 100*time.Millisecond, 30*time.Second, true, func(ctx context.Context) (bool, error) {
approvedKubeAPI, err = apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, approvedKubeAPI.Name, metav1.GetOptions{}) approvedKubeAPI, getErr := apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, approvedKubeAPI.Name, metav1.GetOptions{})
if err != nil { if getErr != nil {
return false, err return false, getErr
} }
if approvedKubeAPIApproved := findCRDCondition(approvedKubeAPI, apiextensionsv1.KubernetesAPIApprovalPolicyConformant); approvedKubeAPIApproved == nil || approvedKubeAPIApproved.Status != apiextensionsv1.ConditionTrue { if approvedKubeAPIApproved := findCRDCondition(approvedKubeAPI, apiextensionsv1.KubernetesAPIApprovalPolicyConformant); approvedKubeAPIApproved == nil || approvedKubeAPIApproved.Status != apiextensionsv1.ConditionTrue {
t.Log(approvedKubeAPIApproved) t.Log(approvedKubeAPIApproved)

View File

@ -220,12 +220,12 @@ func testWebhookConverter(t *testing.T, watchCache bool) {
// wait until new webhook is called the first time // wait until new webhook is called the first time
if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*100, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) { if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*100, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) {
_, err = ctc.versionedClient(marker.GetNamespace(), "v1alpha1").Get(ctx, marker.GetName(), metav1.GetOptions{}) _, getErr := ctc.versionedClient(marker.GetNamespace(), "v1alpha1").Get(ctx, marker.GetName(), metav1.GetOptions{})
select { select {
case <-upCh: case <-upCh:
return true, nil return true, nil
default: default:
t.Logf("Waiting for webhook to become effective, getting marker object: %v", err) t.Logf("Waiting for webhook to become effective, getting marker object: %v", getErr)
return false, nil return false, nil
} }
}); err != nil { }); err != nil {

View File

@ -65,8 +65,8 @@ func StartConversionWebhookServer(handler http.Handler) (func(), *apiextensionsv
// StartTLS returns immediately, there is a small chance of a race to avoid. // StartTLS returns immediately, there is a small chance of a race to avoid.
if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*100, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) { if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*100, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) {
_, err = webhookServer.Client().Get(webhookServer.URL) // even a 404 is fine _, getErr := webhookServer.Client().Get(webhookServer.URL) // even a 404 is fine
return err == nil, nil return getErr == nil, nil
}); err != nil { }); err != nil {
webhookServer.Close() webhookServer.Close()
return nil, nil, err return nil, nil, err

View File

@ -210,15 +210,15 @@ func TestListTypes(t *testing.T) {
t.Logf("Updating again with invalid values, eventually successfully due to ratcheting logic") t.Logf("Updating again with invalid values, eventually successfully due to ratcheting logic")
err = wait.PollUntilContextTimeout(context.Background(), time.Millisecond*100, wait.ForeverTestTimeout, true, func(ctx context.Context) (bool, error) { err = wait.PollUntilContextTimeout(context.Background(), time.Millisecond*100, wait.ForeverTestTimeout, true, func(ctx context.Context) (bool, error) {
_, err = fooClient.Update(ctx, modifiedInstance, metav1.UpdateOptions{}) _, updateErr := fooClient.Update(ctx, modifiedInstance, metav1.UpdateOptions{})
if err == nil { if updateErr == nil {
return true, err return true, nil
} }
if errors.IsInvalid(err) { if errors.IsInvalid(updateErr) {
// wait until modifiedInstance becomes valid again // wait until modifiedInstance becomes valid again
return false, nil return false, nil
} }
return false, err return false, updateErr
}) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)

View File

@ -779,14 +779,14 @@ func TestCRValidationOnCRDUpdate(t *testing.T) {
// CR is now accepted // CR is now accepted
err = wait.PollUntilContextTimeout(context.Background(), 500*time.Millisecond, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) { err = wait.PollUntilContextTimeout(context.Background(), 500*time.Millisecond, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) {
_, err = noxuResourceClient.Create(ctx, instanceToCreate, metav1.CreateOptions{}) _, createErr := noxuResourceClient.Create(ctx, instanceToCreate, metav1.CreateOptions{})
if _, isStatus := err.(*apierrors.StatusError); isStatus { if _, isStatus := createErr.(*apierrors.StatusError); isStatus {
if apierrors.IsInvalid(err) { if apierrors.IsInvalid(createErr) {
return false, nil return false, nil
} }
} }
if err != nil { if createErr != nil {
return false, err return false, createErr
} }
return true, nil return true, nil
}) })

View File

@ -92,14 +92,14 @@ func TestInternalVersionIsHandlerVersion(t *testing.T) {
patch := []byte(fmt.Sprintf(`{"i": %d}`, i)) patch := []byte(fmt.Sprintf(`{"i": %d}`, i))
i++ i++
_, err = noxuNamespacedResourceClientV1beta1.Patch(ctx, "foo", types.MergePatchType, patch, metav1.PatchOptions{}) _, patchErr := noxuNamespacedResourceClientV1beta1.Patch(ctx, "foo", types.MergePatchType, patch, metav1.PatchOptions{})
if err != nil { if patchErr != nil {
// work around "grpc: the client connection is closing" error // work around "grpc: the client connection is closing" error
// TODO: fix the grpc error // TODO: fix the grpc error
if err, ok := err.(*errors.StatusError); ok && err.Status().Code == http.StatusInternalServerError { if statusErr, ok := patchErr.(*errors.StatusError); ok && statusErr.Status().Code == http.StatusInternalServerError {
return false, nil return false, nil
} }
return false, err return false, patchErr
} }
return true, nil return true, nil
}) })
@ -115,16 +115,16 @@ func TestInternalVersionIsHandlerVersion(t *testing.T) {
patch := []byte(fmt.Sprintf(`{"i": %d}`, i)) patch := []byte(fmt.Sprintf(`{"i": %d}`, i))
i++ i++
_, err = noxuNamespacedResourceClientV1beta2.Patch(ctx, "foo", types.MergePatchType, patch, metav1.PatchOptions{}) _, patchErr := noxuNamespacedResourceClientV1beta2.Patch(ctx, "foo", types.MergePatchType, patch, metav1.PatchOptions{})
assert.Error(t, err) assert.Error(t, patchErr)
// work around "grpc: the client connection is closing" error // work around "grpc: the client connection is closing" error
// TODO: fix the grpc error // TODO: fix the grpc error
if err, ok := err.(*errors.StatusError); ok && err.Status().Code == http.StatusInternalServerError { if statusErr, ok := patchErr.(*errors.StatusError); ok && statusErr.Status().Code == http.StatusInternalServerError {
return false, nil return false, nil
} }
assert.ErrorContains(t, err, "apiVersion") assert.ErrorContains(t, patchErr, "apiVersion")
return true, nil return true, nil
}) })
assert.NoError(t, err) assert.NoError(t, err)