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

🐛 Show workspace name in kubectl kcp ws tree #2719

Merged
merged 2 commits into from
Feb 1, 2023

Conversation

hasheddan
Copy link
Member

Summary

With the changing structure of the URL stored in the Workspace spec, we stopped showing the Workspace name in kubectl kcp ws tree. This updates to use the Workspace name and fixes the output when --full is supplied.

Signed-off-by: hasheddan georgedanielmangum@gmail.com

Previously:

$ k kcp ws use root
Current workspace is "root".
$ k kcp ws tree
.
└── root
    └── 1bav17fzw1njbiv3
        └── 274dgmgvx0btkooe

$ k kcp ws tree --full
.
└── root
    └── 1bav17fzw1njbiv3
        └── 274dgmgvx0btkooe

$ k kcp ws use test
Current workspace is "root:test" (type root:universal).
$ k kcp ws tree --full
.
└── root:test
    └── 274dgmgvx0btkooe

$ k kcp ws tree
.
└── test
    └── 274dgmgvx0btkooe

$ k kcp ws use test-1
Current workspace is "root:test:test-1" (type root:universal).
$ (kxp) k kcp ws tree
.
└── test-1

$ k kcp ws tree --full
.
└── root:test:test-1

Update:

$ go run ./cmd/kubectl-kcp/ ws use root
Current workspace is "root".
$ go run ./cmd/kubectl-kcp/ ws tree
.
└── root
    └── test
        └── test-1

$ go run ./cmd/kubectl-kcp/ ws tree --full
.
└── root
    └── root:test
        └── root:test:test-1

$ go run ./cmd/kubectl-kcp/ ws use test
Current workspace is "root:test" (type root:universal).
$ go run ./cmd/kubectl-kcp/ ws tree --full
.
└── root:test
    └── root:test:test-1

$ go run ./cmd/kubectl-kcp/ ws tree
.
└── test
    └── test-1

$ go run ./cmd/kubectl-kcp/ ws use test-1
Current workspace is "root:test:test-1" (type root:universal).
$ go run ./cmd/kubectl-kcp/ ws tree 
.
└── test-1

$ go run ./cmd/kubectl-kcp/ ws tree --full
.
└── root:test:test-1

Related issue(s)

Fixes #2681

@ncdc
Copy link
Member

ncdc commented Jan 31, 2023

Flake #2713

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 31, 2023
@ncdc
Copy link
Member

ncdc commented Jan 31, 2023

/retest

1 similar comment
@hasheddan
Copy link
Member Author

/retest

With the changing structure of the URL stored in the Workspace spec, we
stopped showing the Workspace name in kubectl kcp ws tree. This updates
to use the Workspace name and fixes the output when --full is supplied.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
Fixes a small typo in the help output for the --full flag.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2023
@hasheddan
Copy link
Member Author

Rebased and force pushed to see if I could get past the test flake here.

@vincepri
Copy link
Contributor

vincepri commented Feb 1, 2023

Would it make sense to show both the friendly name and in parentheses the underlying id?

Example:

$ go run ./cmd/kubectl-kcp/ ws tree
.
└── test
    └── test-1 (274dgmgvx0btkooe)

@hasheddan
Copy link
Member Author

@vincepri that sounds pretty reasonable to me, though perhaps only when --full is supplied?

@hasheddan
Copy link
Member Author

That being said -- I believe all the other ws commands deal in "friendly" names, so it might be slightly out of place here.

@hasheddan hasheddan requested a review from ncdc February 1, 2023 16:23
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 1, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ncdc

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

@openshift-merge-robot openshift-merge-robot merged commit 57e9339 into kcp-dev:main Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: kubectl kcp ws tree uses cluster names instead of workspace names
4 participants