-
Notifications
You must be signed in to change notification settings - Fork 742
test: add etcd key compatibility test #9126
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
Conversation
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
Skipping CI for Draft Pull Request. |
/retest |
/retest |
048a723
to
fbe6877
Compare
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
Signed-off-by: okJiang <819421878@qq.com>
pkg/utils/etcdutil/etcdutil.go
Outdated
func InjectFailToCollectTestEtcdKey(key, op string) { | ||
failpoint.Inject("CollectEtcdKey", func(val failpoint.Value) { | ||
file := val.(string) | ||
fmt.Println("collect etcd key", file, key, op) |
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.
Do we need to output it?
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.
done
@@ -0,0 +1,75 @@ | |||
get |
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.
Are these files necessary? How will we maintain them?
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.
How about adding a script to generate it?
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.
It will be print after execute ./scripts/ci-subtask.sh {group_id}
. So we can see it in the github action
BTW: the code is in https://github.com/okJiang/pd/blob/7025e1f6144f5f6e40546246e97b452e11d1329f/tools/pd-ut/ut.go#L419. If diff exists, it will also print https://github.com/okJiang/pd/blob/7025e1f6144f5f6e40546246e97b452e11d1329f/tools/pd-ut/ut.go#L421-L437
How will we maintain them?
If the ci failed, we can copy the log to the xxxx.etcdkey
Signed-off-by: okJiang <819421878@qq.com>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lhy1024, nolouch The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What problem does this PR solve?
Issue Number: Ref #9127
What is changed and how does it work?
This PR introduces a new etcd key compatibility test,
with its test data based on release 8.5. It is only for reviewing the code of this test, so I have not enabled it. In the next PR, I will fix the changes to the etcd keys from release 8.5 to master and provide source explanations for those changes.I has updated the test data using master code, and enable this test.Code Overview: I used failpoint to collect the operation of keys during the test and saved the results to a file. If the results from the new code do not match those in the file, it indicates that the operations on the keys have changed during testing. This compels us to pay attention to changes in the keys.
Check List
Tests
Please refer to the test results here; they are based on test data generated from the release-8.5 code.
Release note