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

opensearchcluster spec add Job resource config #683

Conversation

wangshulei098
Copy link

add job config to support resourcequota

Description

  1. All init containers can inherit the settings from Spec.InitHelper.Resources.
  2. Add a Job field to Spec and configure all jobs with settings such as Resource, NodeSelector, Label, Affinity

Issues Resolved

Fixes #678

@prudhvigodithi
Copy link
Collaborator

Hey @wangshulei098 thanks for your contribution, we can ship this, is it possible to add some unit tests, also please fix the conflicts.
Thank you
Adding @salyh @pchmielnik @swoehrl-mw @jochenkressin
@bbarani

@wangshulei098 wangshulei098 changed the title opensearchcluster spec add Job resource config [WIP] opensearchcluster spec add Job resource config Feb 28, 2024
@wangshulei098 wangshulei098 force-pushed the feat/resourcequota_support branch 2 times, most recently from 2f29aee to ba46d8f Compare February 29, 2024 02:20
@wangshulei098 wangshulei098 changed the title [WIP] opensearchcluster spec add Job resource config opensearchcluster spec add Job resource config Feb 29, 2024
@wangshulei098
Copy link
Author

Hey @wangshulei098 thanks for your contribution, we can ship this, is it possible to add some unit tests, also please fix the conflicts. Thank you Adding @salyh @pchmielnik @swoehrl-mw @jochenkressin @bbarani

Hey, The conflict has been resolved and the unit tests have been added, please approve to proceed,thanks

@salyh
Copy link
Collaborator

salyh commented Mar 30, 2024

Hi @wangshulei098 thanky you but I still see a confilct in "opensearch-operator/pkg/builders/cluster.go". Please re-base from main.

…ntainer inherit the settings from Spec.InitHelper.Resource

opensearchcluster spec add Job  resource config  and all initcontainer inherit the settings from Spec.InitHelper.Resource

Signed-off-by: wangshulei098 <850732903@qq.com>
@wangshulei098
Copy link
Author

Hi @wangshulei098 thanky you but I still see a confilct in "opensearch-operator/pkg/builders/cluster.go". Please re-base from main.

Hey @salyh ,The conflict resulting from merging new commits has been resolved. Please approve at your earliest convenience,thanks

@swoehrl-mw
Copy link
Collaborator

This PR has overlap with #628, need to make sure they do not conflict and do not solve the same thing

@salyh @prudhvigodithi

@wangshulei098
Copy link
Author

This PR has overlap with #628, need to make sure they do not conflict and do not solve the same thing

@salyh @prudhvigodithi

This PR resolves the issue of being unable to deploy applications, including initcontainers and some jobs, after setting resourcequotas. However, The PR associated with #628 fails to address this issue.
Please compare the related PRs and address this issue promptly. Thanks

add initcontainer resource and job config unit test

Signed-off-by: wangshulei098 <850732903@qq.com>
@prudhvigodithi
Copy link
Collaborator

As mentioned I'm ok to move forward with this unless @salyh @swoehrl-mw has any objections, @wangshulei098 can you update the readme with examples as well so folks understand and know resourcequota exists. We can target this in v2.6.0.
Thanks

@wangshulei098
Copy link
Author

As mentioned I'm ok to move forward with this unless @salyh @swoehrl-mw has any objections, @wangshulei098 can you update the readme with examples as well so folks understand and know resourcequota exists. We can target this in v2.6.0. Thanks

Okay, I have updated the readme and added the feature of supporting namespace ResourceQuota.

@prudhvigodithi
Copy link
Collaborator

Thanks @wangshulei098. @salyh @swoehrl-mw need one more approval can you please check?
Thanks

@prudhvigodithi
Copy link
Collaborator

Adding @salyh @swoehrl-mw @jochenkressin @pchmielnik to please check this PR.
Thanks

@wangshulei098
Copy link
Author

@salyh @swoehrl-mw @jochenkressin @pchmielnik please help to check this PR. thanks

Copy link
Collaborator

@swoehrl-mw swoehrl-mw left a comment

Choose a reason for hiding this comment

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

There is overlap with #628 as both PRs configure resources for jobs.
628 has been merged, so please rebase and rework your PR to fit with what was done there.

Also:

  • The changes to the CRDs have not been copied into the operator helm chart
  • The new functionality has not been documented in the userguide

@wangshulei098
Copy link
Author

another pr has coverd all features of this PR。 close #678

@prudhvigodithi prudhvigodithi mentioned this pull request Apr 18, 2024
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.

[BUG] cluster can not be created when ResourceQuota set
4 participants