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

Update Volcano integration doc #1380

Merged

Conversation

annajung
Copy link
Contributor

reviewers @tgaddair @kevin85421

Why are these changes needed?

  • Update the Volcano integration doc to make it easier for users to follow the installation and example steps
  • Move RayCluster yaml examples into /config/samples/
  • Clarify that additional labels ray.io/priority-class-name and volcano.sh/queue-name requires the corresponding resources to be created first before creating a raycluster

Related issue number

Checks

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

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>
@kevin85421 kevin85421 self-assigned this Aug 31, 2023
@kevin85421
Copy link
Member

@Darren221 is very new to both Kubernetes and KubeRay. He will conduct user testing on this document to determine whether it is clear enough for someone not very familiar with Kubernetes and KubeRay.

@Darren221
Copy link
Contributor

I successfully reproduced the results using the provided documentation. While the instructions were clear and allowed me to achieve the expected outcomes, I found it challenging to fully grasp the benefits of Volcano integration from the document alone. Apart from this aspect, the material was quite beginner-friendly.

Copy link
Member

@kevin85421 kevin85421 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for the contribution! This doc is pretty easy to follow. I just left a minor comment and I will merge it after it is addressed.

# transitionID: 72bbf1b3-d501-4528-a59d-479504f3eaf5
# type: Scheduled
# phase: Running
# running: 2
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# running: 2
# running: 3

My output:

apiVersion: scheduling.volcano.sh/v1beta1
kind: PodGroup
metadata:
  creationTimestamp: "2023-09-06T04:54:03Z"
  generation: 5
  name: ray-test-cluster-1-pg
  namespace: default
  ownerReferences:
  - apiVersion: ray.io/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: RayCluster
    name: test-cluster-1
    uid: 87967f54-a16b-4f58-b5d2-a9daf2a77f55
  resourceVersion: "3540"
  uid: 46338fd7-9463-481a-9d83-770727dc1b70
spec:
  minMember: 3
  minResources:
    cpu: "3"
    memory: 4Gi
  queue: kuberay-test-queue
status:
  conditions:
  - lastTransitionTime: "2023-09-06T04:54:04Z"
    message: '3/3 tasks in gang unschedulable: pod group is not ready, 3 Pending,
      3 minAvailable; Pending: 3 Unschedulable'
    reason: NotEnoughResources
    status: "True"
    transitionID: cb24f520-8156-4a62-ade9-1af71f74426c
    type: Unschedulable
  - lastTransitionTime: "2023-09-06T04:57:21Z"
    reason: tasks in gang are ready to be scheduled
    status: "True"
    transitionID: 990c7753-2c93-4c05-bc56-4b8b01c7f396
    type: Scheduled
  phase: Running
  running: 3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching that, updated

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>
@kevin85421
Copy link
Member

This PR only updates docs. The CI failures should be fixed after rebasing with the master branch. Merge.

@kevin85421 kevin85421 merged commit 729c1b7 into ray-project:master Sep 6, 2023
14 of 19 checks passed
z103cb pushed a commit to z103cb/kuberay that referenced this pull request Sep 11, 2023
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
Update Volcano integration doc
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.

None yet

3 participants