-
Notifications
You must be signed in to change notification settings - Fork 35
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
Service Discovery Extension #81
Comments
We should evaluate https://github.com/knative-sandbox/discovery |
Thanks @scothis. The concern I would have with Knative's proposal is the fact it would miss non-duck type provisioned services, for example, those using the annotations / x-descriptions extension of the spec. I think a good starting point, which is friendly to all types, is for provisioned services to add a "discoverable" annotation or label into their binding CR - which could be as simple as a Secret. |
How would these resources be discoverable? Something would need to read the resource to see that it has the appropriate annotations. At the point you've read the resource, you don't need a way to discover it.
I don't follow, can you give an example? |
It's a discovery pattern via a simple annotation search. |
There's no way to query for a resource by annotation in Kubernetes; only labels can be queried. Moreover, you'd have to know which resource kinds to list, as queries are only for a specific kind. Discovery by brute force enumeration isn't really discovery. |
Perhaps we just start with a |
Instead of a label or annotation based brute force method. How about the backing service create a particular custom resource inoder to get discovered? An example custom resource would look like this:
This allows the Service Binding implementation to get the exact list of services available for binding. Well, I am still not sure how this information is going to be used. Can anyone please enlighten me on the use case? |
I feel like this would be very useful for app developers that want to connect to a Provisioned Service in a minimalist way e.g in @baijum's example using only |
@baijum That's making an assumption that backing services would be ready to create and maintain another object of a specific type on the cluster :) Before we do that, we need to have enough backing services onboard with the idea. I would do anything to reduce the amount of effort that backing service authors would have to make to make their services discoverable. That's why if we the discovery ecosystem can only look for a label and work with it, that'll be amazing. |
@baijum in your model, who creates the |
In normal cases, it can be created by the controller which creates the Provisioned Service. If that is not available an administrator of cluster can create it. |
As mentioned before, label-based approach requires brute force to identify arbitrary resources of different kinds. serviceBindingDiscovery := &unstructured.Unstructured{
Object: map[string]interface{}{
"kind": "ServiceBindingDiscovery",
"apiVersion": "service.binding/v1alpha2",
"metadata": map[string]interface{}{
"name": "account-service",
},
"spec": map[string]interface{}{
"service": map[string]interface{}{
"apiVersion": "com.example/v1alpha1",
"kind", "AccountService",
"name": "prod-account-service",
},
},
},
}
k8sClient.Create(contex.Background(), serviceBindingDiscovery) |
The brute force is on the side of the tooling that would encapsulate the logic of discovering services for provisioning. So the onus is on the folks/products offering developer tooling.
However, adding the above code is about convincing 100s of backing service authors/projects to maintain more lines of code for something that isn't even a part of core kubernetes 🤷♂️ In both cases, enough adoption needs to happen before it is useful. I feel, making folks decorate existing resources is easier than making folks create new artifacts. And, if we ask backing services to create fresh objects, we might as well ask folks to create the binding secret the service catalog way ? :) So, my point really is, how far do we go to ask backing services make their stuff usable/discoverable. |
We're confusing two different concepts in this issue: the discovery of compatible types and the discovery of compatible resources. If we solve for the discovery of types, then we get most compatible instances for free. I doubt it's possible to discover resources that are compatible via the Binding Secret Generation Strategies spec extension without enumerating every resource in the cluster, after every mutation. Adding a new resource to advertise that a different resource is bindable undercuts the value of the Provisioned Service duck type. It would force the author of that resource to create a type it doesn't currently know about, and may not be installed on the cluster. If that's the desired path, we should remove any mention of duck typed services and force users to create this new resource instead (I'm not advocating for this approach, just taking it to its logical conclusion). We can solve the ability to discover compatible types, the Knative Discovery API already does this for us. By listing resources for each known type, we also get compatible resources. |
Yeah. I was focusing on the resource ;-) Based on Knative Discovery API , the only mandate the spec need to make is to add a label to the Provisioned Service CRD. The Discovery API doesn't require any specific label. But I think for this spec, a specific label can be recommended. So the extension normative text could be something like this:
|
We may not need an extension. What about adding something like this to the Provisioned Service section:
|
+1 This looks good to me! |
Discussed in the hangout today and agreed to close this given the recent PR to add CRD discoverability and the ability to have Secret types. A new issue should be opened if anybody feels that CR discoverability is needed. |
We should evaluate whether there should be an extension specification for service discovery. Something like an annotation or an additional CRD that can represent implementors of a duck type. If this is appropriate, we should should add it.
The text was updated successfully, but these errors were encountered: