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

enhancement vcctl features #3495

Open
2 of 7 tasks
googs1025 opened this issue May 27, 2024 · 16 comments · Fixed by #3508 · 4 remaining pull requests
Open
2 of 7 tasks

enhancement vcctl features #3495

googs1025 opened this issue May 27, 2024 · 16 comments · Fixed by #3508 · 4 remaining pull requests
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@googs1025
Copy link
Member

googs1025 commented May 27, 2024

What would you like to be added:

  • Enhance the management capabilities for jobTemplate and jobflow:
    • create delete list get describe
  • Enhance vcctl job list command by adding filtering options:
    • Add "--queue=default" to filter by the default queue.
    • Add "--phase=Running" to filter by the Running phase.
  • Add the ability to display all pods under a vcjob:
    Example usage:
    • vcctl list pod --for vcjobs/<job-name>
    • vcctl list job --for jobflow/<jobflow-name>
    • vcctl get pod --for vcjobs/job-1
    • vcctl describe pod --for vcjobs/job-1

Why is this needed:

enhancement vcctl features

TODO:

Notes:

it is expected that work on this proposal will be carried out after this #3494 PR is merged. Split each todo work item and merge it using multiple PRs.

@googs1025 googs1025 added the kind/feature Categorizes issue or PR as related to a new feature. label May 27, 2024
@googs1025
Copy link
Member Author

/assign

@googs1025
Copy link
Member Author

@lowang-bh @hwdef @Monokaix /PTAL Please help identify whether we need this type of enhancement. thanks!

@lowang-bh
Copy link
Member

I am ok.
vcctl job list need another filter --namespace=ns
Other's detail need to be discussed. You'd better first submit a docs to illustrate the detail display.

@googs1025
Copy link
Member Author

I am ok. vcctl job list need another filter --namespace=ns Other's detail need to be discussed. You'd better first submit a docs to illustrate the detail display.

Okay, I will post a document describing the details in the next few days.

@googs1025
Copy link
Member Author

Enhancement vcctl Proposal:

Summary:

The functionality of vcctl is currently limited. For example, it only supports job and queue operations. However, Volcano itself has other custom resource (CR) types, such as jobflow and jobtemplate, among others.

Motivation

To enhance vcctl functionality

Tasks

  1. Enhance vcctl to support jobflow and jobtemplate CRs:
  • Add create, delete, list, get, describe commands for jobflow and jobtemplate CRs
    example: vcctl jobflow create, vcctl jobflow delete, vcctl jobflow list, vcctl jobflow get, vcctl jobflow describe
    example: vcctl jobtemplate create, vcctl jobtemplate delete, vcctl jobtemplate list, vcctl jobtemplate get, vcctl jobtemplate describe
  1. Add filtering options to vcctl job list command
  • Add "--queue=default" to filter by the specific queue.
  • Add "--phase=Running" to filter by the specific phase.

The following are the original functions

root@VM-0-17-ubuntu:~# vcctl job list -h
list job information

Usage:
  vcctl job list [flags]

Flags:
      --all-namespaces      list jobs in all namespaces
  -h, --help                help for list
  -k, --kubeconfig string   (optional) absolute path to the kubeconfig file (default "/root/.kube/config")
  -s, --master string       the address of apiserver
  -n, --namespace string    the namespace of job (default "default")
  -S, --scheduler string    list job with specified scheduler name
      --selector string     fuzzy matching jobName

Global Flags:
      --log-flush-frequency duration   Maximum number of seconds between log flushes (default 5s)
  -v, --v Level                        number for the log level verbosity
      --vmodule moduleSpec             comma-separated list of pattern=N settings for file-filtered logging (only works for the default text log format)
  1. Add support for vcctl job get <job-name> commands
  • How to use:
root@VM-0-17-ubuntu:~# vcctl job get job-1
Name     Creation       Phase       JobType     Replicas    Min   Pending   Running   Succeeded   Failed    Unknown     RetryCount
job-1    2024-05-27     Completed   Batch       1           1     0         0         1           0         0           0
  1. Add support for vcctl queue describe <queue-name> commands
  • How to use:
root@VM-0-17-ubuntu:~# vcctl queue describe deault
Name:         default
Namespace:
Labels:       <none>
Annotations:  <none>
API Version:  scheduling.volcano.sh/v1beta1
Kind:         Queue
Metadata:
  Creation Timestamp:  2024-05-26T14:27:56Z
  Generation:          1
  Resource Version:    350469
  UID:                 5ecfb13b-5597-4ec6-8811-d80256849884
...
  1. Add support for vcctl pod commands
    Users sometimes pay attention to the pods started by vcjob, so we can add a query function for this
root@VM-0-17-ubuntu:/home/ubuntu# vcctl pod get --for vcjobs/job-1
NAME                               READY   STATUS      RESTARTS   AGE
test-a-default-nginx-0             0/1     Completed   0          23h

root@VM-0-17-ubuntu:/home/ubuntu# vcctl pod list --for vcjobs/test-a
NAME                               READY   STATUS      RESTARTS   AGE
test-a-default-nginx-0             0/1     Completed   0          21h
test-a-default-nginx-1             0/1     Completed   0          21h

@Monokaix
Copy link
Member

Monokaix commented May 29, 2024

Add support for vcctl queue describe commands

So what's the difference between this and kubectl describe queue?

@googs1025
Copy link
Member Author

Add support for vcctl queue describe commands

So what's the difference between this and kubectl describe queue?

Yes, there is no difference in the current design, but I think that since vcctl features are already supported, these basic features should be included. Users should not have to switch back and forth between vcctl kubectl all the time.

@googs1025
Copy link
Member Author

The PR #3494 has been merged, maybe we can submit features in batches.

@Monokaix
Copy link
Member

Monokaix commented Jun 3, 2024

The PR #3494 has been merged, maybe we can submit features in batches.

Yeah you can have a try.

@googs1025
Copy link
Member Author

/reopen

@volcano-sh-bot
Copy link
Contributor

@googs1025: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Monokaix
Copy link
Member

root@VM-0-17-ubuntu:/home/ubuntu# vcctl pod get --for vcjobs/job-1
NAME READY STATUS RESTARTS AGE
test-a-default-nginx-0 0/1 Completed 0 23h

root@VM-0-17-ubuntu:/home/ubuntu# vcctl pod list --for vcjobs/test-a
NAME READY STATUS RESTARTS AGE
test-a-default-nginx-0 0/1 Completed 0 21h
test-a-default-nginx-1 0/1 Completed 0 21h

seems that you wanna use the first command get to get a pod in a job, but didn's specify the pod name, so is there any difference between the two comands?

@Monokaix
Copy link
Member

Another question is that we should unify the format of commands and determine whether resources should be placed first or verbs first: )

  • resources first:vcctl pod/vcctl job
  • verbs first: vcctl list/vcctl get

@googs1025
Copy link
Member Author

googs1025 commented Jun 19, 2024

Another question is that we should unify the format of commands and determine whether resources should be placed first or verbs first: )

  • resources first:vcctl pod/vcctl job
  • verbs first: vcctl list/vcctl get

@Monokaix
Yes, I totally agree with you. I also thought this was a bit strange when I first used it. This method is different from the general use of kubectl. However, since vcctl is an existing feature, I should not modify it arbitrarily. Maybe we should refactor vcctl to make vcctl more consistent with kubectl

@Monokaix
Copy link
Member

Another question is that we should unify the format of commands and determine whether resources should be placed first or verbs first: )

  • resources first:vcctl pod/vcctl job
  • verbs first: vcctl list/vcctl get

@Monokaix Yes, I totally agree with you. I also thought this was a bit strange when I first used it. This method is different from the general use of kubectl. However, since vcctl is an existing feature, I should not modify it arbitrarily. Maybe we should refactor vcctl to make vcctl more consistent with kubectl

If previous using method is resources first, we can keep this way to be consistent with old habit and user-side API: )

@googs1025
Copy link
Member Author

googs1025 commented Jun 19, 2024

I agree that we need to maintain forward compatibility. But I think it feels better to use it in a way that is more similar to kubectl. @william-wang WDYT

@googs1025 googs1025 linked a pull request Jun 24, 2024 that will close this issue
@googs1025 googs1025 linked a pull request Jul 14, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment