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

Reinstate /ingester/ prefix #51

Closed

Conversation

MichelHollands
Copy link
Contributor

@MichelHollands MichelHollands commented Apr 24, 2023

Labels in Kubernetes do not allow the / character. Therefore the /ingester/ prefix needs to be added.
Found this while working on an integration test for the downscale webhook.

Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
@MichelHollands MichelHollands requested a review from a team as a code owner April 24, 2023 14:49
@colega
Copy link
Contributor

colega commented Apr 24, 2023

I don't like having to enforce a given random prefix here. There's no such thing as an ingester in the scope of rollout-operator. For example, this feature is very likely going to be used to scale Mimir's store-gateway's too, and I'd feel weird exposing an /ingester path on them.

Discussed on slack with @MichelHollands, it seems that annotations are a better fit to store this data (port & path).

@MichelHollands
Copy link
Contributor Author

Closing this and I'll add another PR with annotations.

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.

None yet

3 participants