Align event Recorder with Controller

[nightkr]

Would you like to work on this feature?

yes

What problem are you trying to solve?

Currently, Controller.run() returns ObjectRef<K> objects, while Recorder takes ObjectReferences. These mostly fulfil the same purpose, but are currently incompatible. In particular, the type fields of ObjectRef<K> are hidden from the user, and ObjectRef<K> does not track UIDs. On the other hand, ObjectRef<K> provides nicer string formatting, is slightly more efficient, and offers optional type safety.

Describe the solution you'd like

1. Track object UIDs in ObjectRef
2. Make it easy to convert an ObjectRef<K> into ObjectReference
3. Make Recorder take ObjectRef<K> rather than ObjectReference

Describe alternatives you've considered

We could also standardize on ObjectReference, but I'm not sure the drawbacks there are worth it.

Documentation, Adoption, Migration Strategy

Should object UIDs participate in ObjectRef equality? I'm inclined to say no for backwards compatibility reasons, but that would be inconsistent with ObjectReference.

Perhaps a middle ground would be to say that they participate in equality IFF uid.is_some() on both sides, but that also feels like a bit of a footgun...

Target crate for feature

kube-runtime

[clux]

A reason that we use ObjectReference inside the Recorder is because the Event type embeds the ref inside the spec as the involved object, so we have to convert to it anyway. There is also an easy way to convert to an ObjectReference via Resource (e.g. usage).

If we change our inteface to use ObjectRef then are we not dropping info here? Do we backfill it from the Api impl on Resource? Does UUID not have a legitimate use? Thought kubectl or client-go would use them for comparison on duplicate names (statefulset pods / other determinisms).

[nightkr]

There is also an easy way to convert to an ObjectReference via Resource (e.g. usage).

ObjectRef already covers this via ObjectRef::from_obj.

If we change our inteface to use ObjectRef then are we not dropping info here? Do we backfill it from the Api impl on Resource? Does UUID not have a legitimate use? Thought kubectl or client-go would use them for comparison on duplicate names (statefulset pods / other determinisms).

I'm not saying we should lose features. Anything that ObjectRef is currently missing would have to be added before it's a viable replacement.

[clux]

Trying to summarise to see if there's anything here we should do.

We can create and convert (and conversions carry uids):

• generate an ObjectReference via an object via Resource::object_ref
• create ObjectRef<K> from an object via the ObjectRef::from_obj* methods
• convert an From ObjectRef<K> for ObjectReference using above referenced pr

however we cannot easily convert between an ObjectReference into ObjectRef<K> i think?

---

alignment; fully aligning these is not super practical, since they have different use cases:

• ObjectRef is much better for memory usage on Store (no need to store repeat type info in keys)
• ObjectRef is not serialize (as people note in Adding ObjectRef to custom resources #1310 (reply in thread)) and adding this would bring a bunch more complexity to it
• ObjectReference is serialize and its default structure is what the event recorder api expects
• ObjectReference is the upstream type that exists on many crds already

We could maybe make the recorder (et al) take an ObjectRef, but that feels equivalent to lifting the From impl into the recorder, and probably not worth it.

---

I suggest we do the following two things:

• create a converter from ObjectReference to ObjectRef<K> so users coming from a CRD ref can pass it into the controller
• point out that it's easy to convert between the two in kube.rs#watched-relations as per the last question. Particularly since the above From impl does not actually show up on docs.rs for some reason - would have expected it here: https://docs.rs/kube/latest/kube/runtime/reflector/struct.ObjectRef.html#trait-implementations - Point out how to ObjectRef in relations website#45

[nightkr]

ObjectReference is the upstream type that exists on many crds already

It's far from standardized though (#1310 (reply in thread) has a very incomplete list). I'm also not convinced that there is such a thing as a universal valid Serialize impl for it. :/

My current thought process would be something like this:

struct LocalObjectRef<K> {
  name: String,
  dt: &K::UnversionedDynType,
}

struct ClusterObjectRef<K> {
  name: String,
  namespace: K::Namespace,
  dt: &K::UnversionedDynType,  
}

struct ResolvedObjectRef<K> {
  name: String,
  namespace: K::Namespace,
  dt: &K::DynType,  
}

impl<K> LocalObjectRef<K> {
  fn in_namespace(self, ns: impl Into<String>) -> ClusterObjectRef<K>;
  fn in_namespace_of<K2>(self, obj: &K2) -> ClusterObjectRef<K> where K2: Resource<Scope = K::Scope>;
}

impl ClusterObjectRef<DynamicObject> {
  fn with_version(self, version: impl Into<String>) -> ResolvedObjectRef<DynamicObject>;
  fn with_recommended_version(self, discovery: &Discovery) -> Result<ResolvedObjectRef<DynamicObject>>;
  // return Some if K's API group and kind match self.dt
  // probably need to either take `self` by ref or return the COR in the error case...
  fn try_match_type<K>(self) -> Option<ResolvedObjectRef<K>>;
}

impl<K> ClusterObjectRef<K> where K: Resource<DynamicType = ()> {
  // not a fan of having to do this explicitly tbh..
  fn with_typed_version(self) -> ResolvedObjectRef<K>;
}