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

Feat: add flags for podPriorityClassName to support external priorityclasses #2499

Conversation

capuche2412
Copy link
Contributor

Hello,

To solve this issue #2441, I present a possible solution by adding flags to create the PriorityClass (or not) and to set the priority. In case of a use of an external priorityclass, only the name should be set.

@capuche2412 capuche2412 changed the title feat(chart): add flags for podPriorityClassName [Feature Request] add flags for podPriorityClassName Dec 18, 2023
@capuche2412 capuche2412 changed the title [Feature Request] add flags for podPriorityClassName Feat: add flags for podPriorityClassName to support external priorityclasses Dec 18, 2023
@FxKu FxKu added the Helm User label Jan 25, 2024
@FxKu FxKu added this to the 1.11.0 milestone Jan 25, 2024
@capuche2412 capuche2412 force-pushed the feat/allow-external-priorityclassname-for-postgres-pods branch from 97a1415 to 4f01234 Compare January 26, 2024 12:41
@capuche2412
Copy link
Contributor Author

I notice another scenario that is not being taken into account. If someone explicitly does not want to set a priorityClassName, it is not possible to do so here. A priorityClass will still be created. This could be perceived as unnecessary boilerplate, especially in a cluster that does not utilize custom priorityClasses at all.
Should an option like enabled: true be added to address this?

@FxKu
Copy link
Member

FxKu commented Jan 26, 2024

I don get your last comment. The default should be false like before. The priority class should not get created if somebody does not want to use it. You have to enable it manually in the values.yaml file. (It's funny after all those years I still see no use in helm charts haha)

@capuche2412 capuche2412 force-pushed the feat/allow-external-priorityclassname-for-postgres-pods branch from 4f01234 to 30f95d9 Compare January 29, 2024 16:47
@capuche2412 capuche2412 force-pushed the feat/allow-external-priorityclassname-for-postgres-pods branch from 30f95d9 to c1f5946 Compare February 1, 2024 09:55
@capuche2412 capuche2412 requested a review from FxKu February 1, 2024 09:55
@FxKu
Copy link
Member

FxKu commented Feb 1, 2024

👍

1 similar comment
@jopadi
Copy link
Member

jopadi commented Feb 2, 2024

👍

@FxKu FxKu merged commit 8bde04f into zalando:master Feb 2, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants