Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 e2e test for upgrading from 1.0.x #2145
Add e2e test for upgrading from 1.0.x #2145
Changes from 3 commits
99038cd
c33e116
ab87d14
f1461c3
c0fa58a
e052df8
a19b72e
c38ab9e
3a5fe07
3b8168c
3a39ae6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
we should wait for pd/tikv pods are not changed in a certain of time, the expected err should be
wait.ErrWaitTimeout
, example:tidb-operator/tests/e2e/tidbcluster/stability.go
Lines 162 to 189 in a19b72e
10*time.Minute
can be changed to5*time.Minute
. 5 minutes is enough to make sure our logic is correct if pd/tikv pods are not affected in this time.before checking pd/tikv pods are not affected, you can wait for tidb pods are affected. in this checking, what you expect is
nil
err (returntrue, nil
when the tidb pods are different).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.
I think the current logic is like what you said. After upgrading Operator, we would expect tidb pods updated yet first, then we expect the tikv and pd pods haven't been changed. The whole process may need 5 or 10 minutes.
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.
you didn't check pd/tikv pods are not affected in a certain time. after tidb pods are changed, the wait function checks pd/pods once and return
true, nil
if they are not changed. what we expect is they are not changed in a certain time (e.g. 5 minutes).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.
you can have two
wait.Poll
functions: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.
good idea, updated.
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.
download these charts in
Upgrading Operator from 1.0.6
testThere 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.
updated, using git clone now.