[serve] Use soft constraint for pinning controller on head node

[edoakes]

Why are these changes needed?

Un-reverting #24934 which caused test_cluster to become flaky. This was due to an oversight: we need to update the HTTPState logic to account for the controller not necessarily running on the head node.

This will require using the new SchedulingPolicy API, but I'm not quite sure the best way to do it. Context here: #25090.

Related issue number

Checks

• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

[fishbone]

@edoakes are we working on this one? I thought it's merged.

[edoakes]

@iycheng it was merged but got reverted because it caused a test to fail. I will pick it back up.

[edoakes]

@iycheng this is passing tests, please review!

[fishbone]

LGTM. Could you add a test which is a two nodes ray cluster and kill one node and make sure the serve controller will be able to be scheduled again?

[edoakes]

@iycheng I spent a few mins thinking about how to write this test and it's quite messy, I think we should wait until we can kill the head node to add the test (check that it gets placed on a worker node). Otherwise it's hard to force the controller to get placed on a worker node right now.

[fishbone]

@edoakes you only need to kill the raylet (not gcs which I'll cover in my PR) where the controller is deployed to make sure it's working e2e.

To kill the raylet, just remove it from the cluster. You can follow the test here (https://github.com/ray-project/ray/blob/master/python/ray/tests/test_scheduling_2.py#L363)

[edoakes]

@iycheng using remove_node will only work on worker nodes though, right? In this case I need to remove the head node because we prefer to place the controller there when possible

[fishbone]

@iycheng using remove_node will only work on worker nodes though, right? In this case I need to remove the head node because we prefer to place the controller there when possible

Got it. Yeah, I'm ok with this and let me cover the tests in some other places. I'll come back to you if it's not working. :p

[edoakes]

sounds good :)

[simon-mo]

btw @edoakes do you think it's worth it to update the http_state with these strategy as well? (currently they uses node resource key).

[simon-mo]

Ah it's in #25813

[simon-mo]

🤦‍♂️ . it's already done in this PR. ignore me

[DmitriGekhtman]

???
:)