-
Notifications
You must be signed in to change notification settings - Fork 9.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add livez readyz e2e tests #16835
add livez readyz e2e tests #16835
Conversation
7c9ded2
to
15ae6f0
Compare
15ae6f0
to
d1124f9
Compare
Thanks for the review! Could you please take a second look? @serathius |
Signed-off-by: Chao Chen <[email protected]>
Signed-off-by: Chao Chen <[email protected]>
d1124f9
to
42d9e43
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a minor comment.
thx
time.Sleep(time.Second) | ||
select { | ||
case <-ctx.Done(): | ||
require.NoError(t, err) | ||
default: | ||
} | ||
response, err := etcdctl.AlarmList(ctx) | ||
if err != nil { | ||
continue | ||
} | ||
if len(response.Alarms) == 0 { | ||
continue | ||
} | ||
require.Len(t, response.Alarms, 1) | ||
if response.Alarms[0].Alarm == etcdserverpb.AlarmType_CORRUPT { | ||
break | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: should we have a timeout for this indefinite for loop, and fail the test when timedout?
Replace #16784 since its CI test failed.
Based on #16792 (comment), this test should be merged first and #16792 will be rebased on top of it.
@siyuanfoundation on the second commit.
/cc @serathius
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.