Show owner of pod, rs (#1747) #2133

gitolicious · 2023-06-15T15:25:50Z

I reused the code from #1564 and used it for navigating "upstream" from pods to replica sets and daemon sets as well as from replica sets to deployments.

This fixes parts of what was discussed in #1747 though I didn't find a generic way that would work for all kinds of resources. Also it is a step-wise approach, not going all the way to the controller. If that is preferred, at least for pods it would be easy to directly jump to the deployment. But quickly hitting o twice also gets you there.

derailed

@gitolicious Nice feature! I think the tests will need some tlc.

gitolicious · 2023-11-12T18:40:42Z

Thanks. Tests fixed and rebase done.

derailed

@gitolicious Thank you for the updates!!

I think this is actually a bit more involved ;(

What if there are more than one controllers?

Also we would need to offer a similar feature for other controllers ie job, cronjob, ds, ... so that behavior is universal on pods no matter what the owing controller is.

I think the best approach here would be to define a view extender to dry up this code and inject the owner action aka OwnerExtender (see ***_ext.go in the view pkg).
On the dao side implement an Owner interface that would return a collection of gvr/fqn tuples for the owning controllers.
Would this make sense?

gitolicious · 2024-01-01T21:45:20Z

Thanks for the feedback. I migrated the logic to a view extender. Handling multiple owners and the gvr/fqn tuples are still missing. Will work on that in the next days probably.
If you find the time, you can let me know if I am on the right track.

gitolicious · 2024-01-02T21:33:35Z

On the dao side implement an Owner interface that would return a collection of gvr/fqn tuples for the owning controllers.

I didn't fully understand that part. Could you explain your idea a bit more? Would the DAOs themselves (e.g. dao/pod.go) provide their owners? Would it actually be simpler than what I have implemented at the moment, or would it result in more redundant code?

derailed

@gitolicious Thank you for the updates. I think this can be greatly simplified if we delegate the owners behavior to the daos. Please take a look at scale or image extenders for examples. I think if we define an Owned interface on the dao with something like GetOwners() map[client.GVR][]string where the value slice in a collection of fqn ie ns/name. This should be enough to power the extender needs.

Would this make sense?

internal/view/owner_extender.go

gitolicious · 2024-01-03T21:46:55Z

internal/dao/registry.go

@@ -117,6 +118,77 @@ func AccessorFor(f Factory, gvr client.GVR) (Accessor, error) {
 	return r, nil
 }

+func GVRForKind(apiVersion string, kind string) (string, bool, error) {


An alternative would be to simply concatenate and hope for the best...

apiVersion + "/" + strings.ToLower(kind) + "s"

Too risky?

Indeed! I think there are only a handful of qualified controllers. I think we could just simply have a map kind->resource. Might be easier to maintain in the future should these go thru an api rev.

Is this what you imagined?

k9s/internal/dao/registry.go

Lines 131 to 143 in 0edc12f

var gvrs = map[string]map[string]gvrInfo{

"apps/v1": {

"ReplicaSet": {client.NewGVR("apps/v1/replicasets"), true},

"DaemonSet": {client.NewGVR("apps/v1/daemonsets"), true},

"StatefulSet": {client.NewGVR("apps/v1/statefulsets"), true},

"Deployment": {client.NewGVR("apps/v1/deployments"), true},

"Jobs": {client.NewGVR("apps/v1/jobs"), true},

"CronJobs": {client.NewGVR("apps/v1/cronjobs"), true},

},

"v1": {

"Node": {client.NewGVR("v1/nodes"), false},

},

}

I think the best way to handle getting to gvr/fqn in the helper is as follows:

mapper := RestMapper{Connection: c} m, err := mapper.ToRESTMapper() rm, err := m.RESTMapping(schema.GroupKind{Group: g, Kind: k}, v) gvr := client.NewGVR(rm.Resource.String()) meta, err := MetaAccess.MetaFor(gvr) if meta.Namespaced {} else {}

I got pretty close to your suggestion. The map is gone now thanks to the RestMapper.

derailed

@gitolicious Very cool! Thank you for these updates!!
A few items up for tlc.

derailed · 2024-01-19T23:23:53Z

internal/dao/registry.go

@@ -117,6 +118,77 @@ func AccessorFor(f Factory, gvr client.GVR) (Accessor, error) {
 	return r, nil
 }

+func GVRForKind(apiVersion string, kind string) (string, bool, error) {


Indeed! I think there are only a handful of qualified controllers. I think we could just simply have a map kind->resource. Might be easier to maintain in the future should these go thru an api rev.

internal/dao/registry.go

internal/dao/rs.go

internal/dao/registry.go

derailed · 2024-01-19T23:40:08Z

internal/dao/rs.go

+	if err != nil {
+		return nil, err
+	}
+


We don't need to have the full hydrated instance to retrieve controllers ie o.GetOwnerReferences()

How would I only get the OwnerReferences? I haven't found existing code to only get specific attributes of a resource.

u, ok := o.(*unstructured.Unstructured) u.GetOwnerReferences()

This was the golden tip for me to finally make the function generic. So I moved GetOwners to generic.go.

@derailed, what do you think about the current implementation? I would be really happy if we could get this feature added to k9s.

internal/dao/types.go

internal/view/owner_extender.go

derailed · 2024-01-19T23:47:37Z

internal/view/owner_extender.go

+		return nil
+	}
+
+	return o.showSelectOwnerDialog(owners)


I think if there are multiple controllers then we should use the same mechanic we currently use for sec/cm aka 'Used By` and show a new view vs a picker to list out controllers fqn/gvr (please see view.reference.go)
The picker is fine but think it might be more consistent.
What do you think?

I wasn't too sure about the picker myself. Its UX is a bit cumbersome for this purpose. But, let's be honest, a resource with multiple owners is rarely seen in the wild. The picker was easy to implement so I went with it. Will look into the view as an alternative if you think it would be required.

Yes you are correct! Most instances will be a straight nav. Extending reference view/model, I think is more elegant and could provide more info/actions.
Given one of the aspect of owner refs is gc, possible this will be more prevalent in crd land.

gitolicious · 2024-01-21T21:35:21Z

The code got much cleaner now thanks to your suggestions and after me getting a better grasp on the structure of the code.

gitolicious · 2024-04-23T19:44:05Z

@derailed, still waiting for your feedback on this. I have rebased the branch after the O hotkey got snagged in the meantime.

docs: add description style: cleanup and error fmt

feat: switch used hotkey to W

gitolicious · 2024-05-24T19:29:46Z

Superseded by the merge of #2700.

gitolicious mentioned this pull request Jun 15, 2023

Open controller shortcut #1747

Closed

derailed requested changes Nov 12, 2023

View reviewed changes

derailed added enhancement New feature or request needs-tlc Pr needs additional updates labels Nov 12, 2023

gitolicious force-pushed the 1747-show-owner branch 2 times, most recently from a83fe75 to 50bfc9f Compare November 12, 2023 18:39

gitolicious requested a review from derailed November 12, 2023 18:41

derailed requested changes Dec 4, 2023

View reviewed changes

gitolicious force-pushed the 1747-show-owner branch from 50bfc9f to c7e47d7 Compare January 1, 2024 21:42

derailed requested changes Jan 3, 2024

View reviewed changes

gitolicious commented Jan 3, 2024

View reviewed changes

gitolicious force-pushed the 1747-show-owner branch from 719a336 to 531d1ac Compare January 19, 2024 20:58

derailed requested changes Jan 19, 2024

View reviewed changes

gitolicious force-pushed the 1747-show-owner branch from 7d4e699 to fb857e8 Compare February 18, 2024 11:57

gitolicious force-pushed the 1747-show-owner branch from fb857e8 to 21369c0 Compare April 22, 2024 19:57

gitolicious added 10 commits May 7, 2024 21:12

feat: show owner (derailed#1747)

9daa501

refactor: reorganize code for maintainability

9c486b1

feat: add owner ref select dialog

ace25ed

refactor: cleanup code

6771826

refactor: cleanup code

db1cf9c

refactor: change logic to Owned interface

f35e15b

chore: rebase

1e01ee1

chore: use client.GVR instead of string

46e59f9

docs: add description style: cleanup and error fmt

chore: use client.GVR instead of string

34cb390

fix: repair tests

f910a0d

gitolicious added 7 commits May 7, 2024 21:12

style: change error fmt

3baf0ee

style: remove pre-check

a914d51

style: move gvrs to static map

0d524a6

refactor: extract AsOwnerInfo

13efa12

refactor: make GetOwners generic

901d65d

refactor: use the RestMapper for the gvr lookup

96a9ba0

chore: rebase

e54c75b

feat: switch used hotkey to W

gitolicious force-pushed the 1747-show-owner branch from 21369c0 to e54c75b Compare May 7, 2024 19:37

gitolicious mentioned this pull request May 7, 2024

Feature: goto owner (ctrl-o) #2640

Open

gitolicious mentioned this pull request May 24, 2024

feat: allow jumping to the owner of the resource #2700

Merged

gitolicious closed this May 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show owner of pod, rs (#1747) #2133

Show owner of pod, rs (#1747) #2133

gitolicious commented Jun 15, 2023

derailed left a comment

gitolicious commented Nov 12, 2023

derailed left a comment •

edited

Loading

gitolicious commented Jan 1, 2024

gitolicious commented Jan 2, 2024

derailed left a comment

gitolicious Jan 3, 2024

derailed Jan 19, 2024

gitolicious Jan 21, 2024

derailed Jan 21, 2024

gitolicious Jan 21, 2024

derailed left a comment

derailed Jan 19, 2024

derailed Jan 19, 2024

gitolicious Jan 21, 2024

derailed Jan 21, 2024

gitolicious Jan 21, 2024 •

edited

Loading

gitolicious Feb 18, 2024

derailed Jan 19, 2024

gitolicious Jan 20, 2024 •

edited

Loading

derailed Jan 21, 2024

gitolicious commented Jan 21, 2024

gitolicious commented Apr 23, 2024

gitolicious commented May 24, 2024

	var gvrs = map[string]map[string]gvrInfo{
	"apps/v1": {
	"ReplicaSet": {client.NewGVR("apps/v1/replicasets"), true},
	"DaemonSet": {client.NewGVR("apps/v1/daemonsets"), true},
	"StatefulSet": {client.NewGVR("apps/v1/statefulsets"), true},
	"Deployment": {client.NewGVR("apps/v1/deployments"), true},
	"Jobs": {client.NewGVR("apps/v1/jobs"), true},
	"CronJobs": {client.NewGVR("apps/v1/cronjobs"), true},
	},
	"v1": {
	"Node": {client.NewGVR("v1/nodes"), false},
	},
	}

Show owner of pod, rs (#1747) #2133

Show owner of pod, rs (#1747) #2133

Conversation

gitolicious commented Jun 15, 2023

derailed left a comment

Choose a reason for hiding this comment

gitolicious commented Nov 12, 2023

derailed left a comment • edited Loading

Choose a reason for hiding this comment

gitolicious commented Jan 1, 2024

gitolicious commented Jan 2, 2024

derailed left a 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

derailed left a 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

gitolicious Jan 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitolicious Jan 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitolicious commented Jan 21, 2024

gitolicious commented Apr 23, 2024

gitolicious commented May 24, 2024

derailed left a comment •

edited

Loading

gitolicious Jan 21, 2024 •

edited

Loading

gitolicious Jan 20, 2024 •

edited

Loading