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.

Sign up for GitHub

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

Jump to bottom

Use the namespace configuration in kubeconfig #2596

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use the namespace configuration in kubeconfig #2596

wants to merge 1 commit into from
+272 −19

Conversation

wangyang0616
Copy link
Member

@wangyang0616 wangyang0616 commented Dec 8, 2022 •
edited
Loading

``Signed-off-by: wangyang wangyang289@huawei.com

fix: #2594

  • If ns is specified on the vcctl command line, the ns specified on the command line is used as the default value.
  • If ns is not specified in the vcctl command but is specified in kubeconfig, ns in kubeconfig is used as the default value.
  • If ns is not specified in the vcctl command line and kubeconfig, the default value default is used for ns.

test
The job information in the cluster is as follows:

[root@volcano-dev-wangyang bin]# kubectl get vcjob -A
NAMESPACE        NAME                      STATUS       MINAVAILABLE   RUNNINGS   AGE
default          vcjob-ns-default          Running      1              5          10m
test             vcjob-ns-test             Running      1              6          10m
volcano-system   vcjob-ns-volcano-system   Terminated                             9m28s
[root@volcano-dev-wangyang bin]# ./vcctl job list --all-namespaces
Namespace        Name                      Creation       Phase       JobType     Replicas    Min   Pending   Running   Succeeded   Failed    Unknown     RetryCount
default          vcjob-ns-default          2022-12-09     Running     Batch       8           1     3         5         0           0         0           0         
test             vcjob-ns-test             2022-12-09     Running     Batch       8           1     2         6         0           0         0           0         
volcano-system   vcjob-ns-volcano-system   2022-12-09     Terminated  Batch       8           0     0         0         0           0         0           0
  • If no namespace is specified for vcctl and kubeconfig, job information in default is queried by default.
[root@volcano-dev-wangyang bin]# kubectl get vcjob
NAME               STATUS    MINAVAILABLE   RUNNINGS   AGE
vcjob-ns-default   Running   1              3          23m
[root@volcano-dev-wangyang bin]# ./vcctl job list
Name               Creation       Phase       JobType     Replicas    Min   Pending   Running   Succeeded   Failed    Unknown     RetryCount
vcjob-ns-default   2022-12-09     Running     Batch       8           1     0         3         5           0         0           0
  • If namespace test is specified in kubeconfig, job information in test is queried by default.
- context:
    cluster: kind-my-test
    namespace: test
    user: kind-my-test
  name: kind-my-test
current-context: kind-my-test

[root@volcano-dev-wangyang bin]# kubectl get vcjob
NAME            STATUS    MINAVAILABLE   RUNNINGS   AGE
vcjob-ns-test   Running   1              6          15m
[root@volcano-dev-wangyang bin]# ./vcctl job list
Name            Creation       Phase       JobType     Replicas    Min   Pending   Running   Succeeded   Failed    Unknown     RetryCount
vcjob-ns-test   2022-12-09     Running     Batch       8           1     2         6         0           0         0           0
  • In kubeconfig, namespace test is specified, but namespace volcano-system is specified in the vcctl command. In this case, job information in volcano-system should be displayed.
[root@volcano-dev-wangyang bin]# kubectl get vcjob -n volcano-system
NAME                      STATUS       MINAVAILABLE   RUNNINGS   AGE
vcjob-ns-volcano-system   Terminated                             16m
[root@volcano-dev-wangyang bin]# ./vcctl job list -n volcano-system
Name                      Creation       Phase       JobType     Replicas    Min   Pending   Running   Succeeded   Failed    Unknown     RetryCount
vcjob-ns-volcano-system   2022-12-09     Terminated  Batch       8           0     0         0         0           0         0           0

@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign hzxuzhonghu
You can assign the PR to them by writing /assign @hzxuzhonghu in a comment when ready.

The full list of commands accepted by this bot can be found 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

@volcano-sh-bot volcano-sh-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 8, 2022
@wangyang0616
Use the namespace configuration in kubeconfig
a9b3a69 
Signed-off-by: wangyang <wangyang289@huawei.com>
@william-wang
Copy link
Member

Please add you test result, thanks @wangyang0616

@wangyang0616
Copy link
Member Author

Please add you test result, thanks @wangyang0616

Okay, the test results have been updated to the description of the PR.

hwdef
hwdef reviewed Dec 9, 2022
View reviewed changes
pkg/cli/job/delete.go
var deleteJobFlags = &deleteFlags{}

// InitDeleteFlags init the delete command flags.
func InitDeleteFlags(cmd *cobra.Command) {
initFlags(cmd, &deleteJobFlags.commonFlags)

cmd.Flags().StringVarP(&deleteJobFlags.Namespace, "namespace", "n", "default", "the namespace of job")
cmd.Flags().StringVarP(&deleteJobFlags.Namespace, "namespace", "n", "", "the namespace of job")
Copy link
Member

Choose a reason for hiding this comment

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

why use empty string as default value?

pkg/cli/job/delete.go
Comment on lines +38 to +54
func (dflags *deleteFlags) GetMasterUrl() string {
return dflags.Master
}

func (dflags *deleteFlags) GetKubeconfigPath() string {
return dflags.Kubeconfig
}

func (dflags *deleteFlags) GetNamespace() string {
return dflags.Namespace
}

func (dflags *deleteFlags) SetNamespace(ns string) error {
dflags.Namespace = ns
return nil
}

Copy link
Member

Choose a reason for hiding this comment

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

There are many similar repetitive functions below, I think this kind of scene is very suitable for generics

@wangyang0616 wangyang0616 mentioned this pull request Dec 27, 2022
Closed
@stale
Copy link

stale bot commented Feb 18, 2023

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 18, 2023
@stale stale bot closed this Mar 18, 2023
@lowang-bh
Copy link
Member

/reopen

@volcano-sh-bot volcano-sh-bot reopened this Apr 4, 2024
@volcano-sh-bot
Copy link
Contributor

@lowang-bh: Reopened this PR.

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.

@stale stale bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 4, 2024
@volcano-sh-bot volcano-sh-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 9, 2024
@volcano-sh-bot
Copy link
Contributor

@wangyang0616: PR needs rebase.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hwdef hwdef hwdef left review comments

@jasonliu747 jasonliu747 Awaiting requested review from jasonliu747

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

vcctl not use kubeconfig
5 participants
@wangyang0616 @volcano-sh-bot @william-wang @lowang-bh @hwdef