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

[Feature] Set default appProtocol for Ray head service to tcp #668

Merged
merged 2 commits into from
Nov 1, 2022

Conversation

kevin85421
Copy link
Member

@kevin85421 kevin85421 commented Oct 29, 2022

Why are these changes needed?

#625 indicates that the appProtocol field is required to integrate with Open Service Mesh. We discussed how to expose head service for users in the community sync Oct. 25. We did not reach a conclusion in that meeting. Setting the default appProtocol to tcp will not change any existing behaviors because the default value of Protocol is "TCP" (Link). This PR enables users to integrate KubeRay with Open Service Mesh.

Related issue number

#625

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

For manually test, we need to use kubectl edit $HEAD_SVC. The kubectl describe command will not show the information of appProtocol. The following screenshot shows the values of appProtocol.

Screen Shot 2022-10-28 at 6 09 02 PM

@kevin85421
Copy link
Member Author

cc @sigmundv

@kevin85421 kevin85421 marked this pull request as ready for review October 29, 2022 01:19
@Jeffwan
Copy link
Collaborator

Jeffwan commented Oct 31, 2022

Let's add some notes on the compatibility. This won't be available in earlier kubernetes versions

@kevin85421
Copy link
Member Author

Let's add some notes on the compatibility. This won't be available in earlier kubernetes versions

Thank @Jeffwan for the review! appProtocol was first introduced in Kubernetes v1.18 (alpha release), and became stable in Kubernetes v1.20 (Link).

Do you mean updating "Suggested Kubernetes Version" from v1.19 - v1.24 to v1.20 - v1.24 in README.md? Thank you!

截圖 2022-10-31 上午12 03 08

@DmitriGekhtman
Copy link
Collaborator

@Jeffwan given that this feature was introduced in K8s 1.18, which is before our currently stated lower bound of 1.19, do we need to add additional compatibility info?

@DmitriGekhtman
Copy link
Collaborator

We can follow up on compatibility requirements later.

@DmitriGekhtman DmitriGekhtman merged commit fd06b5b into ray-project:master Nov 1, 2022
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
…oject#668)

ray-project#625 indicates that the appProtocol field is required to integrate with Open Service Mesh. We discussed how to expose head service for users in the community sync Oct. 25. We did not reach a conclusion in that meeting. Setting the default appProtocol to tcp will not change any existing behaviors because the default value of Protocol is "TCP" (Link). This PR enables users to integrate KubeRay with Open Service Mesh.
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.

3 participants