feat: Add labels for root parent object #14578
Conversation
Signed-off-by: Scott Fleener <[email protected]>
sfleen
force-pushed
the
sfleen/trace-parent
branch
from
e57df5a to
1d643ee
Compare
sfleen
changed the title
controller/k8s/metadata_api.go
Outdated
|
|
||
| resource := schema.FromAPIVersionAndKind(parentRef.APIVersion, parentRef.Kind). | ||
| GroupVersion(). | ||
| WithResource(parentRef.Kind) |
There was a problem hiding this comment.
This part doesn't seem to work consistently, not sure if there's a better way to generate a GroupVersionResource from a parent ref?
There was a problem hiding this comment.
Under what circumstances it's not consistent?
There was a problem hiding this comment.
See 11ca6b1, turns out it was actually incorrect since "resource" is a subtly different format from "kind"
| if err != nil { | ||
| return Address{}, PodID{}, err | ||
| } | ||
| ownerKind, ownerName := pp.metadataAPI.GetOwnerKindAndName(context.Background(), pod, false) |
There was a problem hiding this comment.
Thoughts about replacing this with the new function?
There was a problem hiding this comment.
Seems reasonable. I can put out a follow-up for this, since the current implementation doesn't support controlling retry logic.
| ownerKind, ownerName := wp.metadataAPI.GetOwnerKindAndName(context.Background(), wp.addr.Pod, true) | ||
| wp.addr.OwnerKind = ownerKind | ||
| wp.addr.OwnerName = ownerName |
There was a problem hiding this comment.
Same as above, saving for follow-up
controller/k8s/api_test.go
Outdated
| api.Sync(nil) | ||
| metadataAPI.Sync(nil) | ||
| } | ||
| metadataAPI.Sync(nil) |
There was a problem hiding this comment.
Turns out this was actually incorrect, since the API expects resources in a different format than I was using.
The GroupVersionResource requires the resource to be both lowercase and plural. Signed-off-by: Scott Fleener <[email protected]>
Signed-off-by: Scott Fleener <[email protected]>
Currently, we add a label that identifies the parent resource that a proxy is attached to, usually a deployment, statefulset, daemonset, etc. These are populated via
linkerd.io/proxy-<resource>annotations, with a different label for each parent resource.There are a few deficiencies with the current implementation. Currently, the implementation is specialized to only built-in k8s resources, so things like argocd rollouts will not appear. Additionally it does not recurse beyond two levels, so any more levels of parenting will not be captured.
This adds a pair of new labels,
linkerd.io/proxy-root-parentandlinkerd.io/proxy-root-parent-kind, that identify the name and kind of the root parent of a proxy workload. It also correctly recurses to the true root resource, at least as far as cluster role permissions for the proxy injector permit.Note that the proxy already consumes all of the pod labels via the downward API, so there's no changes required to the proxy injector templates.