-
Notifications
You must be signed in to change notification settings - Fork 611
[Experimental] Object reference resolver to support non-Addressable objects #5593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Questions, since I missed the presentation:
|
All, thanks for the chat on today's WG call. A couple of additional thoughts:
|
Thanks @evankanderson for looking at this. Answers:
|
apiVersion: my.batch.job/v1alpha1
kind: JobDefinition
metadata:
name: pi
spec:
template:
spec:
containers:
- name: pi
image: perl
command: ["perl", "-Mbignum=bpi", "-wle", "print bpi(2000)"]
restartPolicy: Never
backoffLimit: 4
---
apiVersion: sources.knative.dev/v1
kind: PingSource
metadata:
name: trigger-job
spec:
schedule: "@daily"
sink:
adapter:
apiVersion: my.batch.job/v1alpha1
kind: JobDefinition
name: pi
again, as I said I'm all for leveraging the experimental feature process to give this a shot, I'm just trying to surface some of our design goals we're trying to stick to and entertain other alternatives that provide needed UX. |
I agree with @devguyio's sentiment. It feels like what you want is a resource in between, that can then implement whichever semantics you like (in your case calling some external service to fetch the URL the source should sink into) while also implementing the |
I haven't yet read fully through the issue, but my first question about the use case given:
This is where we should try to influence the respective community to implement the
|
Side note to Still trying to find out the real use cases. Because when the real use case is to use PingSource as a full-featured cron-scheduler, then I would recommend that the PingSource should be checked if it is suitable for production (like checking those edge cases like those mentioned in https://metacpan.org/release/ROLAND/Schedule-Cron-1.01/source/t/execution_time.t) If using |
Thanks guys for looking at this. @devguyio @markusthoemmes: there is an intermediate data-plane object, but no corresponding CRD. See my comment above about double reffing. @devguyio Agree with all you points expect:
|
@lionelvillard ack. that's my understanding as well.
I also understood the same, and it's what I meant by
My understanding is that you are trying to avoid making the user use an adapter CRD, right? and the example I gave above will do exactly that. The user doesn't have to know anything about a SinkAdapter CRD. The ´platform´ is the one that'll create that CRD for the user under the hood. This is basically the resolver service you're asking for but using k8s API (the CR is the request to resolve, and the CR status is the response with the URI) . There's no extra adapter data plane in this model. |
@devguyio this is an interesting solution. I don't really like the idea of changing the Destination API. I really like @dug idea about the configmap that allows people to define a mapping between "Kind" and "URL-Template". Easy to implement, no external dependencies, no extra network hop (one of Scott's concerns). It also addresses @evankanderson question/concern about re-triggering the address resolution, which now becomes technically feasible. |
I just realized this has an impact on DomainMapping |
If we aren't as worried about an extra network hop, it also seems feasible to create a service which uses URLs containing k8s Note that the key behavior we're depending on for Destination (the thing that references a K8s Service or Addressable) is that the |
@evankanderson The current approach (kind->URL mapping in ConfigMap) preserves this key behavior:
|
Just to speak it out loud (re @devguyio's suggestion): You could install a webhook that leaves the API intact and does the translation for you ref:
apiVersion: my.batch.job/v1alpha1
kind: JobDefinition
name: pi becomes ref:
apiVersion: adapters.com/v1alpha1
kind: MySinkAdapter
name: pi automatically and all interfaces can stay intact. That'd leave the interface to be purely K8s API based, which has some level of beauty to it, imo anyway. |
@markusthoemmes that's an interesting solution. However I don't like the idea of having the webhook changing the spec, too confusing. It's fine for the webhook to populate the defaults but nothing more than that IMO. It also makes exporting applied resources to yaml non-trivial.
that's nice to have but not a requirement.
k8s or knative? ref is k8s, Addressable is Knative. FWIW, the resolved sink appears in the status section so it's clear where events are sent |
I'm not quite parsing in which way that's more confusing vs. having the resolution been done by some service that the user can't even inspect. In the outlined approach, the user would see exactly what's happening and can check the respective adapter to see what the actually resolved URL was (aka: Where their stuff is sent). This is similar behavior to what istio sidecar-injection is doing as well too, so we're not out of bounds wrt. changing spec on apply. After all, the response the user gets from the apply would contain the mutations. |
we moved away for this solution. Sorry I didn't update the issue description (will do).
This is still the case with the mapping solution (see above): the user sees exactly where events are sent by looking at the resolved URL in the status section. The adapter may or may not send events. In the example above, the adapter creates jobs.
Is this just adding stuff or also changing the original spec? Anyhow, one of the goals here is to hide (for the lack of a better word) the inner working. The user does not care about the adapter, that's just plumbing. |
Just to make sure I understand, in the JobDefinition example, is it the case that if we wrote status.address.url to the JobDefinition in a controller (i.e. made it implement Addressable) things would "just work" without needing this? Putting aside whether doing that is a good idea or not, are there examples where something like that (or indeed Evan's suggestion of an EventingJobDefinition with the same semantics as JobDefinition but that implements Addressable) wouldnt fulfill the use case? |
yes
Yes for the use case and no for the desired UX. We would like to hide (in the spec, not in the status) EventingJobDefinition. |
Just to push on this for a moment, in what way is the UX different in this approach? If we added a controller - or indeed a webhook - which placed status.address on JobDefinition (which I think we could do, and which could even be driven by a configmap), wouldn't the UX be identical, other than - to @markusthoemmes's point - the user could predict which URL would be used by looking at the JobDefinition, without having to wait for the sink's status to be resolved? |
That's an interesting idea. It's not always possible though, since it's possible to define closed schema. |
This issue is stale because it has been open for 90 days with no |
/triage-accepted |
This issue is stale because it has been open for 90 days with no |
This issue is stale because it has been open for 90 days with no |
/remove-lifecycle stale |
/triage accepted |
Description
This proposal is about extending the KReference resolver to support non-addressable objects (no
status.address
field). For those objects, we propose todelegate the reference resolution to an external service which upon receiving a KReference object replies with a resolved URI or an "not supported" errorallow people to define a mapping between "Kind" and "URL-Template", where the URL-Template can use fields from the Object being referenced.Use Case
The driving use case is this one: for instance, as a user I want to be able to trigger a batch job every day by having a PingSource CR targeting a batch job definition:
IMPORTANT NOTE: this is just an example! The source can be anything (Github, Kafka, you name it) and JobDefinitition can also be anything that is not Addressable.
Exit Criteria
Users are now able to reference non-addressable object
Experimental flag name:
kreference-custom-resolvers
Experimental feature stages plan
Below the proposed plan for the feature stages (this list implicitly includes the requirements defined in the process)
URIFromObjectReference
to support custom resolvers in knative/pkgAffected WG
The text was updated successfully, but these errors were encountered: