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

[azure][autoscaler] Fix Azure autoscaler node naming and deletion delays #31645

Merged

Conversation

gramhagen
Copy link
Contributor

Why are these changes needed?

This reverts prior changes to node naming which led to non-unique names, causing constant node refreshing
Currently the Azure autoscaler blocks on node destruction, so that was removed in this change

Related issue number

Closes #31538
Closes #25971

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • 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 :(

Scott Graham added 2 commits January 12, 2023 05:16
…on deallocation to avoid long delays when removing nodes

Signed-off-by: Scott Graham <[email protected]>
@gramhagen gramhagen marked this pull request as ready for review January 25, 2023 20:07
@gramhagen
Copy link
Contributor Author

tested this locally, and it looks good to me.

@wuisawesome
Copy link
Contributor

PR seems pretty reasonable, though just taking a step back @gramhagen do you know if there's an easy way we can test these types of things in CI?

@gramhagen
Copy link
Contributor Author

PR seems pretty reasonable, though just taking a step back @gramhagen do you know if there's an easy way we can test these types of things in CI?

i'd love for that to happen but I'm not sure the best way to do that. the most thorough way would be to deploy a cluster to azure and confirm the head and worker node are healthy, then tear-down the resources.

there are some things you can do to lint the ARM template portion of this, but it wouldn't give you much of guarantee that things are working, just a flag if things are really messed up.

Copy link
Contributor

@wuisawesome wuisawesome left a comment

Choose a reason for hiding this comment

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

lgtm

@wuisawesome
Copy link
Contributor

@gramhagen do you mind rebasing and doing the DCO?

Scott Graham added 2 commits January 26, 2023 19:56
@gramhagen
Copy link
Contributor Author

@gramhagen do you mind rebasing and doing the DCO?

ah, right forgot about that.

@gramhagen gramhagen force-pushed the gramhagen/fix_azure_node_names branch from 5697926 to f7ad5a9 Compare January 27, 2023 17:30
@gramhagen
Copy link
Contributor Author

@wuisawesome anything else needed to close this?

@wuisawesome wuisawesome changed the title Fix Azure autoscaler node naming and deletion delays [azure][autoscaler] Fix Azure autoscaler node naming and deletion delays Feb 28, 2023
@wuisawesome
Copy link
Contributor

@wuisawesome anything else needed to close this?

Nope sorry about the delay

Test failures look unrelated, merging

@wuisawesome wuisawesome merged commit a777a02 into ray-project:master Feb 28, 2023
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request Mar 21, 2023
…ays (ray-project#31645)

This reverts prior changes to node naming which led to non-unique names, causing constant node refreshing
Currently the Azure autoscaler blocks on node destruction, so that was removed in this change

Related issue number
Closes ray-project#31538
Closes ray-project#25971

---------

Signed-off-by: Scott Graham <[email protected]>
Co-authored-by: Scott Graham <[email protected]>
Signed-off-by: Jack He <[email protected]>
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
…ays (ray-project#31645)

This reverts prior changes to node naming which led to non-unique names, causing constant node refreshing
Currently the Azure autoscaler blocks on node destruction, so that was removed in this change

Related issue number
Closes ray-project#31538
Closes ray-project#25971

---------

Signed-off-by: Scott Graham <[email protected]>
Co-authored-by: Scott Graham <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
peytondmurray pushed a commit to peytondmurray/ray that referenced this pull request Mar 22, 2023
…ays (ray-project#31645)


This reverts prior changes to node naming which led to non-unique names, causing constant node refreshing
Currently the Azure autoscaler blocks on node destruction, so that was removed in this change

Related issue number
Closes ray-project#31538
Closes ray-project#25971


---------

Signed-off-by: Scott Graham <[email protected]>
Co-authored-by: Scott Graham <[email protected]>
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
…ays (ray-project#31645)

This reverts prior changes to node naming which led to non-unique names, causing constant node refreshing
Currently the Azure autoscaler blocks on node destruction, so that was removed in this change

Related issue number
Closes ray-project#31538
Closes ray-project#25971

---------

Signed-off-by: Scott Graham <[email protected]>
Co-authored-by: Scott Graham <[email protected]>
Signed-off-by: elliottower <[email protected]>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
…ays (ray-project#31645)

This reverts prior changes to node naming which led to non-unique names, causing constant node refreshing
Currently the Azure autoscaler blocks on node destruction, so that was removed in this change

Related issue number
Closes ray-project#31538
Closes ray-project#25971

---------

Signed-off-by: Scott Graham <[email protected]>
Co-authored-by: Scott Graham <[email protected]>
Signed-off-by: Jack He <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants