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

[#837] Documentation and test for setting PriorityClassName #838

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

kevinfrommelt
Copy link
Contributor

@kevinfrommelt kevinfrommelt commented Mar 23, 2024

^

@kevinfrommelt
Copy link
Contributor Author

Looks like the tests don't currently run b/c of this issue kubernetes-sigs/controller-runtime#2720

Copy link
Contributor

@brusdev brusdev left a comment

Choose a reason for hiding this comment

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

@kevinfrommelt thanks for your PR but you could already customize the priorityClassName and other podSpec fields by using a strategic merge patch, for further details see https://github.com/artemiscloud/activemq-artemis-operator/issues/837#issuecomment-2016538868

After #758 new fields to customize the resource managed by the operator are added only for use cases in which a strategic merge patch doesn't work or would be too much complex.

You could change your PR to add a doc and a test to set the priorityClassName by using a strategic merge patch if that works for your use case.

@kevinfrommelt
Copy link
Contributor Author

@brusdev Thanks for the info. I'll update the PR to just include docs and tests like you suggested.

@kevinfrommelt kevinfrommelt changed the title [#837] Allow configuring priorityClassName for broker deployment [#837] Documentation and test for setting PriorityClassName Mar 25, 2024
@kevinfrommelt
Copy link
Contributor Author

@brusdev This is ready for review

Copy link
Contributor

@brusdev brusdev left a comment

Choose a reason for hiding this comment

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

@kevinfrommelt great work!!!

@brusdev brusdev merged commit 18706f4 into arkmq-org:main Mar 25, 2024
2 of 3 checks passed
@brusdev
Copy link
Contributor

brusdev commented Mar 25, 2024

@kevinfrommelt thanks for your contribution and for investigating the CI error, I fixed it in #839

@kevinfrommelt kevinfrommelt deleted the priority-class-name branch March 25, 2024 17:26
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.

2 participants