Skip to content

Commit c0781b1

Browse files
committed
server: add SucceedsSoon to some CheckRestartSafe tests
These tests are often waiting on internal signals which are asynchronous and hence should not fail immediately. Resolves: #152143 Resolves: #152198 Resolves: #151676 Epic: None Release note: None
1 parent ac39c2e commit c0781b1

File tree

1 file changed

+72
-24
lines changed

1 file changed

+72
-24
lines changed

pkg/server/api_v2_test.go

Lines changed: 72 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -199,18 +199,38 @@ func TestRestartSafetyV2(t *testing.T) {
199199
// Drain the node; then we'll expect success.
200200
require.NoError(t, drain(ctx, ts1, t))
201201

202-
req, err = http.NewRequest("GET", urlStr, nil)
203-
require.NoError(t, err)
204-
resp, err = client.Do(req)
205-
require.NoError(t, err)
206-
require.NotNil(t, resp)
202+
testutils.SucceedsSoon(t, func() error {
203+
req, err := http.NewRequest("GET", urlStr, nil)
204+
if err != nil {
205+
return err
206+
}
207+
resp, err := client.Do(req)
208+
if err != nil {
209+
return err
210+
}
211+
if resp == nil {
212+
return fmt.Errorf("response is nil")
213+
}
207214

208-
bodyBytes, err = io.ReadAll(resp.Body)
209-
require.NoError(t, err)
210-
require.NoError(t, resp.Body.Close())
215+
bodyBytes, err := io.ReadAll(resp.Body)
216+
if err != nil {
217+
return err
218+
}
219+
if err := resp.Body.Close(); err != nil {
220+
return err
221+
}
211222

212-
require.Equal(t, 200, resp.StatusCode, "expected 200: %s", string(bodyBytes))
213-
require.NoError(t, json.Unmarshal(bodyBytes, &response))
223+
if resp.StatusCode != 200 {
224+
return fmt.Errorf("expected 200 but got %d: %s", resp.StatusCode, string(bodyBytes))
225+
}
226+
227+
var response RestartSafetyResponse
228+
if err := json.Unmarshal(bodyBytes, &response); err != nil {
229+
return err
230+
}
231+
232+
return nil
233+
})
214234
}
215235

216236
// TestRulesV2 tests the /api/v2/rules endpoint to ensure it
@@ -392,14 +412,29 @@ func TestCheckRestartSafe_RangeStatus(t *testing.T) {
392412
require.True(t, vitality.GetNodeVitalityFromCache(ts0.NodeID()).IsDraining())
393413
require.False(t, vitality.GetNodeVitalityFromCache(ts1nodeID).IsLive(livenesspb.Metrics))
394414

395-
res, err := checkRestartSafe(ctx, ts0.NodeID(), vitality, ts0.GetStores().(storeVisitor), 3, false)
396-
require.NoError(t, err)
397-
require.False(t, res.IsRestartSafe, "expected unsafe since a different node is down")
415+
testutils.SucceedsSoon(t, func() error {
416+
res, err := checkRestartSafe(ctx, ts0.NodeID(), vitality, ts0.GetStores().(storeVisitor), 3, false)
417+
if err != nil {
418+
return err
419+
}
420+
if res.IsRestartSafe {
421+
return fmt.Errorf("expected unsafe since a different node is down")
422+
}
398423

399-
require.Equal(t, int32(0), res.UnavailableRangeCount)
400-
require.Equal(t, int32(0), res.RaftLeadershipOnNodeCount)
401-
require.Equal(t, int32(0), res.StoreNotDrainingCount)
402-
require.Greater(t, res.UnderreplicatedRangeCount, int32(0))
424+
if res.UnavailableRangeCount != 0 {
425+
return fmt.Errorf("expected UnavailableRangeCount=0, got %d", res.UnavailableRangeCount)
426+
}
427+
if res.RaftLeadershipOnNodeCount != 0 {
428+
return fmt.Errorf("expected RaftLeadershipOnNodeCount=0, got %d", res.RaftLeadershipOnNodeCount)
429+
}
430+
if res.StoreNotDrainingCount != 0 {
431+
return fmt.Errorf("expected StoreNotDrainingCount=0, got %d", res.StoreNotDrainingCount)
432+
}
433+
if res.UnderreplicatedRangeCount <= 0 {
434+
return fmt.Errorf("expected UnderreplicatedRangeCount>0, got %d", res.UnderreplicatedRangeCount)
435+
}
436+
return nil
437+
})
403438
}
404439

405440
func updateAllReplicaCounts(
@@ -455,20 +490,33 @@ func TestCheckRestartSafe_AllowMinimumQuorum_Pass(t *testing.T) {
455490
require.NoError(t, err)
456491

457492
vitality.Draining(ts0.NodeID(), true)
458-
res, err := checkRestartSafe(ctx, ts0.NodeID(), vitality, stores, testCluster.NumServers(), false)
459-
require.NoError(t, err)
460-
require.True(t, res.IsRestartSafe)
493+
testutils.SucceedsSoon(t, func() error {
494+
res, err := checkRestartSafe(ctx, ts0.NodeID(), vitality, stores, testCluster.NumServers(), false)
495+
if err != nil {
496+
return err
497+
}
498+
if !res.IsRestartSafe {
499+
return fmt.Errorf("expected node to be restart safe")
500+
}
501+
return nil
502+
})
461503

462504
ts1nodeID := testCluster.Server(1).NodeID()
463505
vitality.DownNode(ts1nodeID)
464506

465507
require.True(t, vitality.GetNodeVitalityFromCache(ts0.NodeID()).IsDraining())
466508
require.False(t, vitality.GetNodeVitalityFromCache(ts1nodeID).IsLive(livenesspb.Metrics))
467509

468-
res, err = checkRestartSafe(ctx, ts0.NodeID(), vitality, stores, testCluster.NumServers(), true)
469-
require.NoError(t, err)
470-
471-
require.True(t, res.IsRestartSafe)
510+
testutils.SucceedsSoon(t, func() error {
511+
res, err := checkRestartSafe(ctx, ts0.NodeID(), vitality, stores, testCluster.NumServers(), true)
512+
if err != nil {
513+
return err
514+
}
515+
if !res.IsRestartSafe {
516+
return fmt.Errorf("expected node to be restart safe with minimum quorum")
517+
}
518+
return nil
519+
})
472520
}
473521

474522
// TestCheckRestartSafe_AllowMinimumQuorum_Fail verifies that with 2

0 commit comments

Comments
 (0)