✨ kubectl support ../../ & ..:..: #3088

mjudeikis · 2024-03-09T21:14:54Z

Summary

Enables to go back more than 1 workspace at the time ws use ../../
Enables to use tree in rootless clusters.

[mjudeikis@unknown2 kcp]$ k ws tree
.
kcp git:(mjudeikis/refactor.ws.use) k ws tree
.
└── kvdk2spgmbix
    └── test
        └── test1
            └── test2

➜  kcp git:(mjudeikis/refactor.ws.use) k ws use :
Current workspace is ':kvdk2spgmbix'.
➜  kcp git:(mjudeikis/refactor.ws.use) k ws use test:test1:test2
Current workspace is ':kvdk2spgmbix:test:test1:test2'.
➜  kcp git:(mjudeikis/refactor.ws.use) k ws use ../..
Current workspace is ':kvdk2spgmbix:test'.

Missing parts:

../../foo multiple actions together does not work
no auto-completetion.

Related issue(s)

Release Notes

kcp ws use support for relative and absolute multi-step navigation

mjudeikis · 2024-03-09T21:16:22Z

We need bigger story to refactor all this function to support syntax like:

ws use ../../foo
ws use ../../foo/bar

basically make it recursive and enable replace : with / for more linux native navigation,
and add alias to use ws cd :)

mjudeikis · 2024-03-11T11:32:26Z

/test pull-kcp-test-e2e-shared

mjudeikis · 2024-03-13T05:11:51Z

@sttts is this missing smth?

embik · 2024-03-14T16:22:50Z

Overall idea as per community meeting:

# absolute path
$ kubectl ws :adct:org-a:team-b
# relative path (called from within adct "root" workspace)
$ kubectl ws org-a:team-b
# relative path (starting from `.` current workspace)
$ kubectl ws .:org-a:team-b
# going from org-a "back" to org-a
$ kubectl ws ..:org-b:..:org-a

# we warn with this case because it's old behaviour
$ kubectl ws root:org-a
warn: did you mean :root:org-a?

sttts · 2024-03-17T10:18:25Z

cli/pkg/workspace/plugin/kubeconfig.go

 defer func() {
 if err == nil {
 _, err = fmt.Fprintf(o.Out, "Note: 'kubectl ws' now matches 'cd' semantics: go to home workspace. 'kubectl ws -' to go back. 'kubectl ws .' to print current workspace.\n")
 }
 }()
 fallthrough

- case "~", home:
- homeWorkspace, err := o.kcpClusterClient.Cluster(core.RootCluster.Path()).TenancyV1alpha1().Workspaces().Get(ctx, "~", metav1.GetOptions{})
+ case o.Name == "~" || o.Name == home:


what about ~:foo ?

added support, just no test as need second run on it :/
~ not being resolved in tests to check if workspace exists gets bit complex in tests :./

cli/pkg/workspace/plugin/kubeconfig.go

sttts · 2024-03-17T10:21:03Z

cli/pkg/workspace/plugin/kubeconfig_test.go

@@ -407,7 +407,7 @@ func TestUse(t *testing.T) {
 AuthInfos: map[string]*clientcmdapi.AuthInfo{"test": {Token: "test"}},
 },
 destination: "root:foo:bar",
- wantStdout: []string{"Current workspace is \"root:foo:bar\""},
+ wantStdout: []string{"Current workspace is ':root:foo:bar'"},


am not convinced we want to print out the : at all. Let's keep the output always foo:bar:abc. The : prefix is only for navigation.

was one the line with this, so this pushes swings to same :) will remove

cli/pkg/workspace/plugin/kubeconfig.go

sttts · 2024-03-21T21:05:14Z

cli/pkg/workspace/plugin/kubeconfig.go

-
- case ".":
- cfg, err := o.ClientConfig.ClientConfig()
+ case strings.Contains(o.Name, ".."):


this one looks fishy. Could see = "..", but anything beyond that needs some more logic, e.g. for ./../foo/../bar.

yes, this is not supported yet. Code code quite convoluted and wanna move it to separate pr. So this is not yet supported

cli/pkg/workspace/plugin/kubeconfig.go

sttts · 2024-03-21T21:07:07Z

cli/pkg/workspace/plugin/kubeconfig.go

+ // system is bit different. Usually when moving to system workspaces you
+ // need to have special access and "if workspace exists" checks fails.
+ // If merged to "absolute path check" code gets convoluted :/
+ case strings.HasPrefix(o.Name, ":system"):


We have a constant for system somewhere I believe.

Also here HasPrefix seems fishy. We have to properly split by ":".

Idea was not to trigger this path is somebody created root:system or foo:bar:system in any ways. So only way to access system from cli is to be explicit - :system.
Basically don't want this code path to be triggered by mistake by users.

My comment was about using string comparison. Convert it to a logical cluster path and operate on that.

Updated but this does not make it prettier in my opinion :D

mjudeikis · 2024-03-23T14:14:49Z

/retest

mjudeikis · 2024-03-26T08:25:16Z

@sttts you ok merge without ../../foo ?

sttts · 2024-03-30T19:39:23Z

cli/pkg/workspace/plugin/kubeconfig.go

-
- return currentWorkspace(o.Out, newServerHost, shortWorkspaceOutput(o.ShortWorkspaceOutput), nil)
+ // LEGACY(mjudeikis): Remove once everybody gets used to this
+ if o.Name == "home" || strings.HasPrefix(o.Name, core.RootCluster.String()+":") {


o.Name == home?

Hmm, that case is below. Did we have special "home" logic?

we did. I just kept it here. Maybe not needed anymore?

Signed-off-by: Dr. Stefan Schimanski <[email protected]>

✨ cli/workspace: implement full relative paths including . and ..

sttts · 2024-03-31T10:45:21Z

/lgtm
/approve

kcp-ci-bot · 2024-03-31T10:45:26Z

LGTM label has been added.

Git tree hash: bc436de3187f91a2036b315af462186fc0202c49

kcp-ci-bot · 2024-03-31T10:45:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sttts

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:

~~OWNERS~~ [sttts]
~~sdk/apis/OWNERS~~ [sttts]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kubectl support ../..

cc3d6e5

kcp-ci-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has signed the DCO. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 9, 2024

mjudeikis added 2 commits March 11, 2024 11:19

nit simplify

111c540

enable rootless cluster navigation

815e270

add support for ..:..

b81b870

mjudeikis changed the title ~~✨ kubectl support ../../~~ ✨ kubectl support ../../ & ..:..: Mar 11, 2024

mjudeikis added 2 commits March 17, 2024 10:24

refactor ws use

4e59254

swap " for ' in output

07fd55f

kcp-ci-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 17, 2024

mjudeikis added 4 commits March 17, 2024 10:43

re-enable test

406ed95

add handler for my root

285f4fe

clean

441b199

add alias cd for use

4f91093

sttts reviewed Mar 17, 2024

View reviewed changes

cli/pkg/workspace/plugin/kubeconfig.go Outdated Show resolved Hide resolved

sttts reviewed Mar 17, 2024

View reviewed changes

mjudeikis added 2 commits March 17, 2024 15:23

add support for ~:foo

0e5fd50

clean

1230514

mjudeikis closed this Mar 18, 2024

mjudeikis force-pushed the mjudeikis/kubectl.nits branch from 918199a to 1230514 Compare March 18, 2024 08:50

kcp-ci-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 18, 2024

sttts reviewed Mar 21, 2024

View reviewed changes

cli/pkg/workspace/plugin/kubeconfig.go Outdated Show resolved Hide resolved

sttts reviewed Mar 21, 2024

View reviewed changes

cli/pkg/workspace/plugin/kubeconfig.go Outdated Show resolved Hide resolved

sttts reviewed Mar 21, 2024

View reviewed changes

cli/pkg/workspace/plugin/kubeconfig.go Outdated Show resolved Hide resolved

sttts reviewed Mar 21, 2024

View reviewed changes

nit updates post review

7877e6e

kcp-ci-bot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Mar 23, 2024

mjudeikis added 2 commits March 30, 2024 11:58

clean how we operate o.Name

667873e

clean debug

5a15edb

sttts reviewed Mar 30, 2024

View reviewed changes

sttts and others added 4 commits March 31, 2024 11:02

cli/workspace: implement full relative paths including . and ..

6f8531d

Signed-off-by: Dr. Stefan Schimanski <[email protected]>

Bump logicalcluster

44d421c

Signed-off-by: Dr. Stefan Schimanski <[email protected]>

cli: split per sub-command

dac6e3c

Signed-off-by: Dr. Stefan Schimanski <[email protected]>

Merge pull request #5 from sttts/sttts-full-relative

8becc5a

✨ cli/workspace: implement full relative paths including . and ..

kcp-ci-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 31, 2024

mjudeikis added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 31, 2024

kcp-ci-bot assigned sttts Mar 31, 2024

kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 31, 2024

kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 31, 2024

mjudeikis merged commit e1c2fd9 into kcp-dev:main Mar 31, 2024
15 of 16 checks passed

mjudeikis removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ kubectl support ../../ & ..:..: #3088

✨ kubectl support ../../ & ..:..: #3088

mjudeikis commented Mar 9, 2024 •

edited

Loading

mjudeikis commented Mar 9, 2024

mjudeikis commented Mar 11, 2024

mjudeikis commented Mar 13, 2024

embik commented Mar 14, 2024 •

edited

Loading

sttts Mar 17, 2024

mjudeikis Mar 17, 2024

sttts Mar 17, 2024

mjudeikis Mar 17, 2024

sttts Mar 21, 2024

mjudeikis Mar 23, 2024

sttts Mar 21, 2024

mjudeikis Mar 23, 2024

sttts Mar 26, 2024

mjudeikis Mar 30, 2024

mjudeikis commented Mar 23, 2024

mjudeikis commented Mar 26, 2024

sttts Mar 30, 2024

sttts Mar 30, 2024

mjudeikis Mar 30, 2024

sttts commented Mar 31, 2024

kcp-ci-bot commented Mar 31, 2024

kcp-ci-bot commented Mar 31, 2024

✨ kubectl support ../../ & ..:..: #3088

✨ kubectl support ../../ & ..:..: #3088

Conversation

mjudeikis commented Mar 9, 2024 • edited Loading

Summary

Related issue(s)

Release Notes

mjudeikis commented Mar 9, 2024

mjudeikis commented Mar 11, 2024

mjudeikis commented Mar 13, 2024

embik commented Mar 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjudeikis commented Mar 23, 2024

mjudeikis commented Mar 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sttts commented Mar 31, 2024

kcp-ci-bot commented Mar 31, 2024

kcp-ci-bot commented Mar 31, 2024

mjudeikis commented Mar 9, 2024 •

edited

Loading

embik commented Mar 14, 2024 •

edited

Loading