appsody / appsody-operator

An Operator for deploying Appsody based applications to Kubernetes. This repo will be archived soon.
Apache License 2.0
18 stars 15 forks source link

Support basic service binding #152

Closed arthurdm closed 4 years ago

arthurdm commented 5 years ago

There's an opportunity for the Appsody Operator to help with basic service binding.

The design has been moved to the following document, to allow for collaborative comments, etc.

https://docs.google.com/document/d/1riOX0iTnBBJpTKAHcQShYVMlgkaTNKb4m8fY7W1GqMA/edit?usp=sharing

seabaylea commented 5 years ago

This looks good - some initial thoughts:

  1. Because of how some libraries assemble URLs, we might need to have the secret provide host, port and contextRoot separately as well as a combined url.
  2. We'd need there to be a reference to the secret or the config from the Service CR that the Appsody Operator generates (so its possible for other things to find/access the secret as well).
  3. Is the provides: metadata.name needed, or can be infer that from the project name (do we have name collisions, or scope to provide more than one endpoint we need to cater for?)
  4. Do we need to deal with lazy resolution/availability? ie, what happens if you deploy a microservice with a dependency on a microservice that is not yet available?
arthurdm commented 5 years ago

thanks @seabaylea

  1. I thought about this when driving home today. :) I think at a minimum we need a path field, for the context root. I have added that now. We can go through a few examples and see if we need to split it up.

  2. and 3. Originally I had provides: true and just always use metadata.name. That would mean that you always know the name of the secret, since it's the same as the service name. But then you lose the flexibility of naming that secret arbitrarily. I am leaning to go back to provides: true, specially if we need a way to always link to the secret.

4.I think that's a complex scenario that I am not sure we want to support right now, as it would require the runtime code (app) itself to be able to handle lazy resolution of a needed service. In this current design the pod initialization would eventually fail because it could not find a required k8s secret to bind.

seabaylea commented 5 years ago

for #4, we avoid any need to provide retry etc in the app, if we can deplay the deployment of the microservice until its dependent app is available. This does however introduce a possible deadlock scenario should there, for whatever reason, be a cyclic dependency.

arthurdm commented 5 years ago

that's a good point @seabaylea - the operator could indeed hold things up until the secret is available. something for us to consider. Perhaps if a deployment is waiting for the secret to be available we can have its reconcile status to be waiting for dependency.

arthurdm commented 5 years ago

updated the proposal to make the provider's secret always match metadata.name (which is already unique to the service), and added a section to talk about delayed activation.

arthurdm commented 5 years ago

added a section on an alternate binding method, via mounting a properties file instead of env vars.

arthurdm commented 5 years ago

updated the design to include namespaces - allowing consumers to specify services from other namespaces.

arthurdm commented 5 years ago

added section (at the end) about OpenShift 4.2's topology view

arturdzm commented 5 years ago

Couple things:

  1. the service provides DNS hostname record not a URL, it can be any protocol, HTTP/HTTPS/TCP or even proprietary URL for DB connections like mongodb://

  2. You can't use/mount secrets/config maps across namespaces AFAIK

  3. Most modern authentication methods like OAuth have per client credentials, like client_id, client_secret etc.

Also the yaml snippets are not syntactically correct, as you need another object key to hold indented values

arthurdm commented 5 years ago

thanks for the feedback @arturdzm

  1. That's a good point - we'll need to add a field for the protocol (although a DB wouldn't necessarily be a valid scenario for an AppsodyApplication producer)
  2. The operator will create the secret in the consumer's namespace, so we're ok there.
  3. In that case it would behave like the scenario where no credentials were provided by the producer (i.e. the token / clientID is passed in by the client)
  4. Yup, good catch - I will remove the boolean value as it is implied by the existence of the key
arthurdm commented 5 years ago

updated design with latest feedback

cmoulliard commented 5 years ago

I think that the spec of the service provided is missing a level of information or aggregation.

As a service exposed could be :

Example for an endpoint

service:
    provides:
        endpoint:
            path: "/portfolio"
            type: ClusterIP
            port: 9080
           protocol: https

Example for a DB

service:
    provides:
        database:
           DB_NAME:
           DB_USER:
           DB_PWD: 

... with such info, appsody (or hal - https://github.com/halkyonio/hal) client tool OR UI could filter the information when a user will select a service to bind, ...

WDYT ? @arthurdm

arthurdm commented 5 years ago

thanks for the feedback @cmoulliard

I think the category of the service is a great idea, but I am thinking that fits much better in the service.consumes side of the AppsodyApplication CR, rather than the service.provider side - i.e. we wouldn't model a DB in the AppsodyApplication CR, as that is meant to deploy runtime apps.

So focusing on the service.consumes obj, that's an area where a type / category could be placed:

  service:
     - consumes: "service1"
        type: endpoint
        namespace: "lob1"
        mountPath: "/opt/endpoint"
     - consumes: "service2"
        type: database
        namespace: "couchdb"
        mountPath: "/opt/db"

In this scenario the Appsody Operator (& other tools) can understand that they should look for a k8s / knative service that matches service1 in namespace lob1 - creating the mounted bindings for the consuming service from that.

In the case of a DB, it could either call a service broker to provision that (advanced case), or simply look for a k8s secret that matches that name (simple case) in that namespace - creating the mounted bindings for the consuming service from that.

As a side note: can you give an example of the integration you were thinking for prometheus and jaeger? We have recently added support to connect to a prometheus instance in our app CR, to have it scrape metrics from the runtime pod. Both prometheus and jaeger have operators, so that's what we would use for providing these services.

cmoulliard commented 5 years ago

can you give an example of the integration you were thinking for prometheus and jaeger? We have recently added support to connect to a prometheus instance

We are on the same page as we will also use the ServiceMonitor CRD resource to request to the prometheus operator to monitor a microservice. BTW, such YAML resource could be created OOTB using our Dekorate Tool -> see: http://dekorate.io/dekorate/#prometheus-annotations

For jaeger: here is the info about what we did : http://dekorate.io/dekorate/#jaeger-annotations

Remark: Our current operator don't yet support to handle a capability of type prometheus or jaeger but this is on our todo list, roadmap

cmoulliard commented 5 years ago

In the case of a DB, it could either call a service broker to provision that (advanced case), or simply look for a k8s secret that matches that name (simple case) in that namespace - creating the mounted bindings for the consuming service from that.

That will be the most tricky part as (Template/Ansible) ServiceBroker are deprecated on ocp4 and should be replaced by operators.

Adopting operators is certainly a great idea but the problem is that currently no spec really exist to allow to expose the needed parameters of an operator in order to delegate to a Capability Operator the process to create a DB capability.

The Openshift DevTools team is proposing to define such informations using the ClusterServiceVersion - StatusDescriptors -> see: https://github.com/redhat-developer/service-binding-operator#introduction but of course that will only work if all the Capability OLM writers follow the spec ;-)

arthurdm commented 5 years ago

Attaching a small set of slides that illustrates the proposed design. I suggest that in this first iteration of the design we focus on category : openapi.

service binding.pptx

cmoulliard commented 5 years ago

As this issue is already a long thread discussion, we should perhaps continue the discussion using a Doc to capture the comments/questions and to have the possibility to review/discuss it like also to add diagrams, ... and finally to create a Tech Spec doc. WDYT ? @arthurdm

arthurdm commented 5 years ago

that's a good idea @cmoulliard - do you have a suggestion for a collaboration tool? Google docs?

cmoulliard commented 5 years ago

Google Doc is perhaps a good idea if all the participants have a gmail address

arthurdm commented 5 years ago

I moved the design into a google doc and updated the summary to point to it. Comments are welcome either in here or right at the doc. =)

arthurdm commented 4 years ago

Great job on the implementation @navidsh! I think we can close this issue now?

navidsh commented 4 years ago

Yea, it's done. Closing