Add fully qualified resource reference type #351

killianmuldoon · 2022-09-15T11:33:56Z

This PR adds a new type - ResourceReference - and a function to use it which allows logging a fully qualified resource reference that allows logging Name, Namespace, Kind, Version and Group for an object.

Fixes #350

New type - `ResourceReference` which allows logging a fully qualified resource reference that allows logging Name, Namespace, Kind, Version and Group for an object.

k8s-ci-robot · 2022-09-15T11:34:02Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: killianmuldoon
Once this PR has been reviewed and has the lgtm label, please assign dims for approval by writing /assign @dims in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

killianmuldoon

@sbueringer I'd appreciate your review on the API here. I wonder if we could also have KResourceObj that takes an interface that's able to get the GVK as well as the Name and Namespace which might make this easier to consume in some cases.

k8s_references.go

sbueringer · 2022-09-15T13:31:24Z

@sbueringer I'd appreciate your review on the API here. I wonder if we could also have KResourceObj that takes an interface that's able to get the GVK as well as the Name and Namespace which might make this easier to consume in some cases.

Could be nice, the problem is probably to get gvk out of an object without having to handle/return an error (for apiVersion parsing)

klog_test.go

harshanarayana · 2022-09-16T10:42:45Z

k8s_references.go

@@ -156,3 +157,29 @@ func (ks kobjSlice) process() ([]interface{}, error) {
 	}
 	return objectRefs, nil
 }
+
+// ResourceRef is a full Kubernetes resource reference composed of Group, Version, Kind, Namespace and Name.
+type ResourceRef struct {


Can you please check if this can be named as KResourceRef now that there is no func KResourceRef(....) ?

It feels slightly strange that KObj and KRef are functions while this would be a struct, but I'm happy to go either way on this if there's consensus.

Good point. @pohly which one do you think would be more natural to the framework ?

For the sake of clarity and usability, I prefer just the struct (see comment above) But for the sake of consistency we would have to use a function.

@serathius: what do you think?

I agree with @pohly that struct should be preferred.

k8s_references.go

serathius · 2022-09-27T20:59:32Z

I'm not sure about the design the the API change:

Name ResourceRef implies this is a reference to a resource. This doesn't follow K8s naming convention as Resources and Objects are two separate things.
ResourceRef struct just stores reference to K8s object, similarly to ObjectRef with addition of GVK. Should we consider extending ObjectRef to include GVK?
It's good that Cluster API has defined it's own standard for fully qualified object reference, however we need broader discussion to define a standard for whole community. I think we should include api machinery people.

serathius · 2022-09-27T21:18:34Z

Linking in the discussion about fully qualified object references in original structured logging kep kubernetes/enhancements#1367 (comment)

@deads2k @derekwaynecarr @thockin @deads2k Was there any progress made with defining a grammar for K8s object references?

k8s-triage-robot · 2022-12-27T08:28:27Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot · 2023-01-26T09:20:53Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

pohly · 2023-02-13T09:10:35Z

It might be necessary to join a meeting (presumably SIG apimachinery?) and discuss the grammar question there.

pohly · 2023-02-13T09:10:58Z

/remove-lifecycle rotten

thockin · 2023-02-13T17:34:09Z

Was there any progress made with defining a grammar for K8s object references

To the best of my knowledge, nobody has tackled this definition. I don't have bandwidth in the near future but here's where I would start:

Think, grep, ask, look for all the places we use compound strings. Off the top of my head:
a) apiVersion: $group/$version => "apps/v1"
b) kubectl get name: $resource(ish).$group/$name => "deployment.apps/mydeploy"
c) namespacedName: $namespace/$name => "myapp-prod/mysvc"
d) DNS: $name.$namespace => "kubernetes.default"
e) there must be more...
Formalize them (e.g. what is "deployment" in 1b? It's not a kind ("Deployment") nor a resource ("deployments")
Think about where those overlap (e.g. 1b vs 1c) and whether it matters
Think about what a unified format (or formats) could look like, emphasizing compat
Think about how migration could happen (or if it can't)
Make a proposal (ranging from "document the mess" to "a grand unified format")

killianmuldoon · 2023-03-10T16:40:00Z

@pohly - I see you're assigned to this in the issue at #350 - are you planning to work on this?

pohly · 2023-03-10T18:35:06Z

No, I was assigned today and haven't had the time to unassign myself.

k8s-triage-robot · 2023-06-08T18:35:54Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

pohly · 2023-06-12T08:47:04Z

/remove-lifecycle stale

Still seems relevant, but needs someone to drive it.

killianmuldoon · 2023-10-04T15:00:15Z

going to close this PR as a cleanup - but the issue is still open if someone has time to drive the change.

killianmuldoon · 2023-10-04T15:00:22Z

/close

k8s-ci-robot · 2023-10-04T15:00:41Z

@killianmuldoon: Closed this PR.

In response to this:

/close

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.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 15, 2022

k8s-ci-robot requested review from harshanarayana and pohly September 15, 2022 11:34

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 15, 2022

Add fully qualified resource reference type

29e3f73

killianmuldoon force-pushed the add-kresourcereference-type branch from 0a0663d to 29e3f73 Compare September 15, 2022 11:35

killianmuldoon commented Sep 15, 2022

View reviewed changes

pohly reviewed Sep 15, 2022

View reviewed changes

k8s_references.go Outdated Show resolved Hide resolved

sbueringer reviewed Sep 15, 2022

View reviewed changes

k8s_references.go Outdated Show resolved Hide resolved

Remove KResourceRef function

6a9e45a

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 15, 2022

harshanarayana reviewed Sep 16, 2022

View reviewed changes

added tests and cleanups

b730c71

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 27, 2022

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 27, 2022

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 26, 2023

killianmuldoon mentioned this pull request Feb 13, 2023

Add logging reference format with Group, Version, Kind, Namespace and Name #350

Open

k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Feb 13, 2023

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 8, 2023

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 12, 2023

k8s-ci-robot closed this Oct 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fully qualified resource reference type #351

Add fully qualified resource reference type #351

killianmuldoon commented Sep 15, 2022

k8s-ci-robot commented Sep 15, 2022

killianmuldoon left a comment

sbueringer commented Sep 15, 2022 •

edited

harshanarayana Sep 16, 2022

killianmuldoon Sep 27, 2022

harshanarayana Sep 27, 2022

pohly Sep 27, 2022 •

edited

serathius Sep 27, 2022

serathius commented Sep 27, 2022 •

edited

serathius commented Sep 27, 2022 •

edited

k8s-triage-robot commented Dec 27, 2022

k8s-triage-robot commented Jan 26, 2023

pohly commented Feb 13, 2023

pohly commented Feb 13, 2023

thockin commented Feb 13, 2023 •

edited

killianmuldoon commented Mar 10, 2023

pohly commented Mar 10, 2023

k8s-triage-robot commented Jun 8, 2023

pohly commented Jun 12, 2023

killianmuldoon commented Oct 4, 2023

killianmuldoon commented Oct 4, 2023

k8s-ci-robot commented Oct 4, 2023

Add fully qualified resource reference type #351

Add fully qualified resource reference type #351

Conversation

killianmuldoon commented Sep 15, 2022

k8s-ci-robot commented Sep 15, 2022

killianmuldoon left a comment

Choose a reason for hiding this comment

sbueringer commented Sep 15, 2022 • edited

harshanarayana Sep 16, 2022

Choose a reason for hiding this comment

killianmuldoon Sep 27, 2022

Choose a reason for hiding this comment

harshanarayana Sep 27, 2022

Choose a reason for hiding this comment

pohly Sep 27, 2022 • edited

Choose a reason for hiding this comment

serathius Sep 27, 2022

Choose a reason for hiding this comment

serathius commented Sep 27, 2022 • edited

serathius commented Sep 27, 2022 • edited

k8s-triage-robot commented Dec 27, 2022

k8s-triage-robot commented Jan 26, 2023

pohly commented Feb 13, 2023

pohly commented Feb 13, 2023

thockin commented Feb 13, 2023 • edited

killianmuldoon commented Mar 10, 2023

pohly commented Mar 10, 2023

k8s-triage-robot commented Jun 8, 2023

pohly commented Jun 12, 2023

killianmuldoon commented Oct 4, 2023

killianmuldoon commented Oct 4, 2023

k8s-ci-robot commented Oct 4, 2023

sbueringer commented Sep 15, 2022 •

edited

pohly Sep 27, 2022 •

edited

serathius commented Sep 27, 2022 •

edited

serathius commented Sep 27, 2022 •

edited

thockin commented Feb 13, 2023 •

edited