Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

Make container port optional #57

Merged
merged 1 commit into from
Mar 27, 2020
Merged

Conversation

Jeffwan
Copy link
Member

@Jeffwan Jeffwan commented Mar 25, 2020

Address #56

  • Instead of using fixed port to communicate with each other, some frameworks randomly pick up port by worker and use auto-discovery to find entire topology.
  • In that case, we only need pod-to-pod communication and don’t need to specify port in headless service.
  • This give the flexibility for framework which doesn’t rely on fixed port.

Signed-off-by: Jiaxin Shan seedjeffwan@gmail.com

I've tested in a controller using this common.

@kubeflow-bot
Copy link

This change is Reviewable

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

/lgtm

Others PTAL

@Jeffwan
Copy link
Member Author

Jeffwan commented Mar 26, 2020

/cc @gaocegege @richardsliu

- Instead of using fixed port to communicate with each other, some frameworks randomly pick up port by worker and use auto-discovery to find entire topology.
- In that case, we only need pod-to-pod communication and don’t need to specify port in headless service.
- This give the flexibility for framework which doesn’t rely on fix port.

Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
@Jeffwan
Copy link
Member Author

Jeffwan commented Mar 26, 2020

Rebase the master to resolve the conflict.

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: terrytangyuan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 0b3a4c3 into kubeflow:master Mar 27, 2020
@Jeffwan Jeffwan deleted the scale_down branch March 27, 2020 00:27
georgkaleido pushed a commit to georgkaleido/common that referenced this pull request Jun 9, 2022
* Create Dockerfile

* Update core/requirements-deploy.txt

* Update Dockerfile

* Add missing jq dependency

* Change python version

Co-authored-by: jerome <jerome@kaleido.ai>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants