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

Apply tpl() to affinity values to enable reuse of helpers / labels #213

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

frittentheke
Copy link
Contributor

The Helm tpl function allows to use templates in values. This does make sense where values might want reference existing labels of a Helm chart. This is is the case for Pod affinity (or anti-affinity) which needs to match the labels of the Pods created.

This is somewhat inspired by the way this was done for the NGINX Ingress Controller

Resolves: #212

@frittentheke
Copy link
Contributor Author

^^ @timvisee

…labels

The Helm tpl function [1] allows to use templates in values. This does make
sense where values might want reference existing labels of a Helm chart.
This is is the case for Pod affinity (or anti-affinity) which needs to match
the labels of the Pods created.

This is somewhat inspired by the way this was done for the NGINX Ingress
Controller with [2].

[1] https://helm.sh/docs/howto/charts_tips_and_tricks/#using-the-tpl-function
[2] kubernetes/ingress-nginx@af9e524

Resolves: qdrant#212
Signed-off-by: Christian Rohmann <[email protected]>
@elbart
Copy link
Contributor

elbart commented Aug 12, 2024

@frittentheke Thanks a lot for your proactive addition to the qdrant helm chart. We value your participation and are happy you found the chance to support us here.

I got feedback from another colleague for this PR! We would kindly ask you to add a test for the affinities. For illustration, this file might serve as an example: https://github.com/qdrant/qdrant-helm/blob/main/test/qdrant_topology-spread-constraints_test.go

We then can actually approve/merge this one!

Thanks a lot in advance!

@elbart
Copy link
Contributor

elbart commented Aug 12, 2024

Never mind @frittentheke. Thanks a lot for your support. Will provide the unit test myself! PR is here: #217

@elbart elbart merged commit 1d4535c into qdrant:main Aug 12, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants