-
Notifications
You must be signed in to change notification settings - Fork 820
RFC: Add integration test for etcd #4113
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
Can you please take a look at failed linter checks? https://github.com/cortexproject/cortex/pull/4113/checks?check_run_id=2431082044 Thank you! |
One irrelevant test is failing; do we need to kick the CI again? |
@pstibrany A gentle ping; A test that seems irrelevant fails. Thanks! |
I don't seem to be able to restart checks here, not sure why. Can you try to push empty commit to your branch instead? That will trigger new run of checks. Thanks! |
Signed-off-by: ChinYing-Li <chinying.li@mail.utoronto.ca>
|
Signed-off-by: ChinYing-Li <chinying.li@mail.utoronto.ca>
PR ready for review, and any suggestion is appreciated! @pstibrany |
This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions. |
Thanks for the PR; sorry it has sat around such a long time. |
Signed-off-by: ChinYing-Li <chinying.li@mail.utoronto.ca>
15826ab
to
7acffaa
Compare
Hi, sorry for getting back late; was a bit tied down by the recruiting season. I have introduced the |
@bboreham Just a ping to let you know I've updated the PR to use block storage. Thank you! |
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 might have packaged up store RingStore, address string
into a struct to reduce the boilerplate at all call-sites, but it's fine as-is.
Thanks!
What this PR does:
Adds one integration test for etcd.
Signatures of six functions used in the integration tests had been changed.
I believe we should also test compactor, store gateway and ruler that use
etcd
as ring backend (hence the signature ofNewRuler
,NewCompactor
etc. should also be modified), but I'd like to have some early feedback before moving forward.Questions:
consul
is used. Shall backward compatibility test cases foretcd
be added as well?Which issue(s) this PR fixes:
Fixes #2137
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]