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

[YUNIKORN-2773] Update Document - Removal of default queue configuration #513

Closed
wants to merge 5 commits into from

Conversation

kaichiachen
Copy link
Contributor

@kaichiachen kaichiachen commented Feb 2, 2025

What is this PR for?

According to https://issues.apache.org/jira/browse/YUNIKORN-2711 and https://issues.apache.org/jira/browse/YUNIKORN-2703, user cannot config default queue name through admissionController.filtering.defaultQueue or other configuration anymore. Apps will be placed in 'root.default' if no rule is provided.

What type of PR is it?

  • - Bug Fix
  • - Improvement
  • - Feature
  • - Documentation
  • - Hot Fix
  • - Refactoring

Todos

  • - Task

What is the Jira issue?

https://issues.apache.org/jira/projects/YUNIKORN/issues/YUNIKORN-2773

How should this be tested?

Screenshots (if appropriate)

Move this to the ‘Deprecated Settings’ section and add a link directing users to the ‘App Placement Rules’ page for guidance on configuring the desired queue name with placement rules.
new added Deprecated Settings
Screenshot 68
Fixed Rule
image

Questions:

  • - The licenses files need update.
  • - There is breaking changes for older versions.
  • - It needs documentation.

Copy link
Contributor

@chenyulin0719 chenyulin0719 left a comment

Choose a reason for hiding this comment

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

Hi @kaichiachen, thanks for taking over the Jira.

Although we removed the default queue configuration, we can still achieve the same behavior by setting the fixed rule in the end of placement rules. I'm in favor of adding such an example under the deprecated config.

For example, this explanation looks pretty good to me.

@kaichiachen WDYT?

@kaichiachen
Copy link
Contributor Author

Hi @kaichiachen, thanks for taking over the Jira.

Although we removed the default queue configuration, we can still achieve the same behavior by setting the fixed rule in the end of placement rules. I'm in favor of adding such an example under the deprecated config.

For example, this explanation looks pretty good to me.

@kaichiachen WDYT?

Thanks @chenyulin0719 ! I agree that we should provide guidance on configuring the desired queue once this configuration is no longer available. The official documentation includes a section elaborating how to configure the desired queue name: https://yunikorn.apache.org/docs/next/user_guide/placement_rules#fixed-rule
I think we can leverage it and add a hyperlink to the deprecated configuration. You can check my latest screenshot, thanks!

Copy link
Contributor

@chenyulin0719 chenyulin0719 left a comment

Choose a reason for hiding this comment

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

+1 LGTM, adding hyperlink make sense to me.

Copy link
Contributor

@chenyulin0719 chenyulin0719 left a comment

Choose a reason for hiding this comment

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

@kaichiachen The result layout is correct. This is a very nitpicky change request.

Copy link
Contributor

@chenyulin0719 chenyulin0719 left a comment

Choose a reason for hiding this comment

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

+1 LGTM

@chenyulin0719 chenyulin0719 changed the title [Yunicorn-2773] Removal of default queue configuration [YUNIKORN-2773] Removal of default queue configuration Feb 19, 2025
@chenyulin0719 chenyulin0719 changed the title [YUNIKORN-2773] Removal of default queue configuration [YUNIKORN-2773] Update Document - Removal of default queue configuration Feb 19, 2025
github-actions bot pushed a commit that referenced this pull request Feb 19, 2025
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.

3 participants