Skip to content

Ensure workers use private ip to connect to scheduler#394

Merged
jacobtomlinson merged 4 commits into
dask:mainfrom
nordange:azurevm-workers-connect-privately-to-scheduler
Feb 9, 2023
Merged

Ensure workers use private ip to connect to scheduler#394
jacobtomlinson merged 4 commits into
dask:mainfrom
nordange:azurevm-workers-connect-privately-to-scheduler

Conversation

@nordange

@nordange nordange commented Dec 9, 2022

Copy link
Copy Markdown
Contributor

Intended to address #254

Overview
Let the scheduler and dashboard be publicly adressable with the workers in a vnet. The workers will always connect to the scheduler using the private IP, regardless of whether the scheduler has a public IP or not.

  • self.address is the private IP
  • self.external_address is the public IP on the scheduler object if public_ingress=True

Testing
Confirmed that it works by creating a cluster with public_ingress = True and a vnet allowing traffic on ports 8786-8787 from IP of local laptop only. Without this fix, the workers try to communicate with the scheduler using the public IP.

@jacobtomlinson jacobtomlinson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! This seems like a great enhancement and we should be doing this everywhere. Instead of special casing Azure I'd rather implement this in a way that allows us to implement it for other clouds easily in the future. See my commments.

Comment thread dask_cloudprovider/generic/vmcluster.py Outdated
Comment thread dask_cloudprovider/azure/azurevm.py Outdated

@jacobtomlinson jacobtomlinson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome thanks. Sorry for the delay in coming back here.

@jacobtomlinson jacobtomlinson enabled auto-merge (squash) February 9, 2023 12:06
@jacobtomlinson

Copy link
Copy Markdown
Member

Pulled from main to hopefully fix up linting failures.

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.

2 participants