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-802] Supports to assign nodes to non-default partition #293

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chia7712
Copy link
Contributor

What is this PR for?

see comment (https://issues.apache.org/jira/browse/YUNIKORN-22?focusedCommentId=17398860&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17398860)

Currently, all nodes are hardcode to be assigned to "default" partition. That brings two disadvantages.

  1. we can't select specify nodes, which are used to execute spark job only, from a cluster
  2. multi-partitions does not work since non-default partition can't get nodes

What type of PR is it?

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

Todos

  1. support to change partition assignment of existent node (in this PR, the update request will be skipped)
  2. support to remove existent node which had been reassigned (in this PR, removing such node cause error message "Failed to update non existing node ...")

What is the Jira issue?

https://issues.apache.org/jira/browse/YUNIKORN-802

How should this be tested?

Screenshots (if appropriate)

Questions:

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

@yangwwei yangwwei changed the title YUNIKORN-802 Supports to assign nodes to non-default partition [YUNIKORN-802] Supports to assign nodes to non-default partition Aug 17, 2021
@codecov
Copy link

codecov bot commented Aug 18, 2021

Codecov Report

Merging #293 (ce9e427) into master (c47ed51) will increase coverage by 3.51%.
The diff coverage is 78.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #293      +/-   ##
==========================================
+ Coverage   59.75%   63.27%   +3.51%     
==========================================
  Files          35       37       +2     
  Lines        3133     3455     +322     
==========================================
+ Hits         1872     2186     +314     
- Misses       1180     1182       +2     
- Partials       81       87       +6     
Impacted Files Coverage Δ
pkg/appmgmt/appmgmt_recovery.go 67.50% <0.00%> (-8.18%) ⬇️
pkg/cache/amprotocol_mock.go 0.00% <0.00%> (ø)
pkg/cache/context_recovery.go 45.23% <0.00%> (-1.11%) ⬇️
pkg/cache/external/scheduler_cache.go 34.56% <0.00%> (+0.40%) ⬆️
pkg/common/events/recorder_mock.go 0.00% <0.00%> (ø)
pkg/common/events/states.go 0.00% <0.00%> (ø)
pkg/controller/application/app_controller.go 71.05% <ø> (-0.26%) ⬇️
...missioncontrollers/webhook/admission_controller.go 33.74% <0.00%> (+1.00%) ⬆️
pkg/shim/main.go 0.00% <ø> (ø)
pkg/common/events/recorder.go 33.33% <40.00%> (+3.33%) ⬆️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0bd9366...ce9e427. Read the comment docs.

@wilfred-s
Copy link
Contributor

Lots has changed since this was started. I still think this is a good idea to fix.
There are other changes going in for YUNIKORN-1090 and YUNIKORN-1091, I think we need to clean that up before we restart this one.

@wilfred-s wilfred-s marked this pull request as draft February 24, 2022 06:52
@heumsi
Copy link

heumsi commented May 9, 2023

Hi. Is there any further progress by any chance? I'm waiting as this feature is very useful and needed.

@lixmgl
Copy link

lixmgl commented May 9, 2023

Hi. Is there any further progress by any chance? I'm waiting as this feature is very useful and needed.

hi @chia7712 are you still actively working on this ticket? If not, i can help the work.

@chia7712
Copy link
Contributor Author

chia7712 commented May 9, 2023

@lixmgl feel free to take over it. thanks :)

@lixmgl
Copy link

lixmgl commented May 9, 2023

@lixmgl feel free to take over it. thanks :)

Will do :)

@craigcondit
Copy link
Contributor

This is a very stale PR at this point - I also think we need a design doc on this before proceeding. See comments on https://issues.apache.org/jira/browse/YUNIKORN-22.

@heumsi
Copy link

heumsi commented Sep 20, 2023

Any progress?

@craigcondit
Copy link
Contributor

@heumsi multiple partition support needs an extensive design doc before we can proceed as there are many complications, especially with Kubernetes. As far as I know, the design work has not been completed yet.

@FrankYang0529
Copy link
Member

FrankYang0529 commented Oct 24, 2023

Hi @heumsi, in YuniKorn, if we only use labels to group nodes into different partitions and use labels to assign applications to different nodes, I think this can be achieved by nodeSelector. Not sure whether you have other scenarios which must be done with different partitions in YuniKorn. If yes, could you share in the thread? Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants