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

Dify: add global.labels to values file #91

Merged
merged 4 commits into from
Jun 20, 2024
Merged

Conversation

kaktos
Copy link
Contributor

@kaktos kaktos commented Jun 19, 2024

resolve #90, enable to define global labels.

@LeoQuote
Copy link
Contributor

LeoQuote commented Jun 19, 2024

Thanks for your pr, do you think splitting the config into podLabels and deploymentLabels is better? That would be more clear than just labels.

Would my new proposal cover more use case and more understandable? I don’t use this feature so you opinion also matters.

@kaktos
Copy link
Contributor Author

kaktos commented Jun 20, 2024

Thanks for your pr, do you think splitting the config into podLabels and deploymentLabels is better? That would be more clear than just labels.

Would my new proposal cover more use case and more understandable? I don’t use this feature so you opinion also matters.

Personally, I don't think it's necessary. Most of the Charts I've seen only define an option like .Values.extraLabels. The main purpose is just to use the label to locate the actual payload (such as a Pod).

@LeoQuote
Copy link
Contributor

如果只是用来定位pod,那helm 自带的label app.kubernetes.io/name label如何呢?能否满足需求?

如果不能,确实需要额外的label,还请在命名和注释上体现这些label是做什么用的,会注入到哪里,以方便后续的使用。

@kaktos
Copy link
Contributor Author

kaktos commented Jun 20, 2024

如果只是用来定位pod,那helm 自带的label app.kubernetes.io/name label如何呢?能否满足需求?

如果不能,确实需要额外的label,还请在命名和注释上体现这些label是做什么用的,会注入到哪里,以方便后续的使用。

比如一场景,在 FinOps 需要按照 Pod 统计成本,那么会被某个 team 或者项目的 pod 打上相应 label,比如 example.com/team: devteam,成本计算模块会按照这样 label(或者 annotation)来统计成本。

@LeoQuote
Copy link
Contributor

谢谢解释, 那现在麻烦就加一下注释, 并且改一下名字叫 extraLabels , 明确说明他的用途和效果就可以了.

charts/dify/values.yaml Outdated Show resolved Hide resolved
@LeoQuote LeoQuote enabled auto-merge (squash) June 20, 2024 03:47
@LeoQuote LeoQuote merged commit 3fa7828 into douban:master Jun 20, 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
Development

Successfully merging this pull request may close these issues.

Dify: add a common label to values file
2 participants