Skip to content
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

query: add memcached autodiscovery support #4487

Merged
merged 7 commits into from
Aug 3, 2021

Conversation

roystchiang
Copy link
Contributor

@roystchiang roystchiang commented Jul 26, 2021

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

  • Add a flag to enable auto-discovery for memcached
  • Add a new provider/resolver for auto-discovery

Verification

  • Integrated into cortex, and verified that auto-discovery is able to find scaled up nodes without having to restart pods
  • Tested with AWS ElastiCache Auto Discovery, but should work with Google Cloud too, since the protocol seem the same

I also updated the memcached config documentation so that we can close #3879

@roystchiang roystchiang force-pushed the autodiscovery branch 3 times, most recently from 3644367 to f86af27 Compare July 26, 2021 19:50
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase on latest main, that should fix CircleCI

Signed-off-by: Roy Chiang <roychi@amazon.com>
Signed-off-by: Roy Chiang <roychi@amazon.com>
@GiedriusS
Copy link
Member

@roystchiang could you please fix the linter messages? 🤗

Signed-off-by: Roy Chiang <roychi@amazon.com>
@roystchiang
Copy link
Contributor Author

oops. I've fixed the linter messages now.

Signed-off-by: Roy Chiang <roychi@amazon.com>
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits but overall good work 👍

pkg/discovery/memcache/provider.go Show resolved Hide resolved
pkg/discovery/memcache/resolver.go Outdated Show resolved Hide resolved
pkg/cacheutil/memcached_client.go Outdated Show resolved Hide resolved
Signed-off-by: Roy Chiang <roychi@amazon.com>
GiedriusS
GiedriusS previously approved these changes Aug 2, 2021
docs/components/store.md Outdated Show resolved Hide resolved
@GiedriusS
Copy link
Member

@roystchiang please add the DCO and let's merge 🤗

@GiedriusS
Copy link
Member

@roystchiang make docs now fails :/

@roystchiang
Copy link
Contributor Author

hahahahahaha. yeah I'm on it. sorry about that

Co-authored-by: Giedrius Statkevičius <giedriuswork@gmail.com>
Signed-off-by: Roy Chiang <roychi@amazon.com>
GiedriusS
GiedriusS previously approved these changes Aug 2, 2021
pkg/discovery/memcache/provider.go Show resolved Hide resolved
Signed-off-by: Roy Chiang <roychi@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Give guidance that memcached should not be configured on loadbalancer level
2 participants