Skip to content
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

Adding annotation to help discovery #9

Merged
merged 5 commits into from
Apr 24, 2020
Merged

Adding annotation to help discovery #9

merged 5 commits into from
Apr 24, 2020

Conversation

arthurdm
Copy link
Member

This PR adds an annotation into bindable resources, allowing tools to quickly discover them and making it easy to create ServiceBinding CRs.

@arthurdm
Copy link
Member Author

CC @sbose78

@sbose78
Copy link
Contributor

sbose78 commented Mar 27, 2020

Thanks, reading.

README.md Outdated Show resolved Hide resolved
@arthurdm arthurdm self-assigned this Apr 1, 2020
@sbose78
Copy link
Contributor

sbose78 commented Apr 1, 2020

I was thinking we need to have this information at a higher level, example, at the bundle level? Of course, if a bundle doesn't exist for a CRD, then it could be at the CRD level.

@arthurdm
Copy link
Member Author

arthurdm commented Apr 1, 2020

interesting @sbose78 - do you have some examples of the bundles and how we could annotate them?

@sbose78
Copy link
Contributor

sbose78 commented Apr 1, 2020

Haven't got around thinking this through yet, thinking out aloud:

  • A category in the CSV : we could propose a specific new category called "Developer service" in the pre-defined list of categories ?
  • And/Or, have something in the CRD too ( for non-OLM operators )?
  • Semi-related: have something in the helm chart's chart.yaml if it provides us a specific kind of service.

And, then we encourage every "developer service" irrespective of where they come from to have binding metadata in it in the form of CSV descriptions and/or CRD annotations.

Usage

odo wil then be able to list only these in odo services list , and so will other tools which have a flow where you

  1. deploy a Node app, ( and ..Now your are looking to connect something to it ? )
  2. This is where you could query "everything helm or installed operators" which has the metadata "developer service"

@arthurdm
Copy link
Member Author

arthurdm commented Apr 1, 2020

my concern with placing this in a higher level is that these services may have not been provisioned yet. In other words, you need a KafkaCluster CR in other to bind to it, not just to discover its Kafka CRD. That was the rationale for adding servicebinding/bindable: "true" in the bindable-and-provisioned CR - so it's a list of "what can I bind to right now".

Unless we're saying that we want to list all possible (present and future) bindable services, and odo would do the provisioning as part of odo link?

@sbose78
Copy link
Contributor

sbose78 commented Apr 1, 2020

we want to list all possible (present and future) bindable services, and odo would do the provisioning as part of odo link?

Yeah, that's what odo does today with service classes.

We should support this at CSV, CRD and the CR level as well - however, the initial step in a lot of tools would be "Show me the subset of developer services available for provisioning" . svcat get classes in service catalog used to do a really good job there.

@arthurdm
Copy link
Member Author

arthurdm commented Apr 1, 2020

Yeah, that's what odo does today with service classes.

cool, that's what I was missing. I do wonder though, how that will be actually done in terms of implementation.

From what I can tell service catalog was based on open broker, so it has a prescribed way for provisioning. Our service binding spec consciously stayed away from defining how to provision services, so do we have a gap in terms of what odo needs?

For example, we have nothing in the spec equivalent to:

$ svcat provision ups-instance --class user-provided-service --plan default

@sbose78
Copy link
Contributor

sbose78 commented Apr 1, 2020

Correct, it was based on open broker.

No, we are good, we don't have a gap, we should happily stay away from provisioning :)

Here's how the flow should look like:

  1. Filter out relevant services that a developer might want provisioned using the metadata we agree on.

  2. Let the tool figure out how to provision it ( not our problem :) )

  3. After (2) is done, invoke service binding.

Our effort in this PR is to guide users in discovering the right services which after provisioning, developers would want their apps to bind to.

@arthurdm
Copy link
Member Author

arthurdm commented Apr 1, 2020

sounds good - thanks for the clarifications @sbose78. I will happily let the tools figure out provisioning. :D

So to summarize the changes we can do in this PR for the different ways to become discoverable:

  • add Bindable to the CSV's metadata.annotations.categories (we'll need to open a PR with them) I like your suggestion for this, because it gives a nice human-readable way to filter this in OperatorHub
  • add bindable to your Chart.yaml's keyword list (sample for mongodb and postgres)
  • add the servicebinding/bindable: "true" annotation to your CRD or any CR (Secret, Service, etc)

looks good?

@sbose78
Copy link
Contributor

sbose78 commented Apr 9, 2020

Let me get to this, again.

@arthurdm
Copy link
Member Author

@sbose78 - I have simplified the PR based on the suggestions above. ready for review.

@arthurdm arthurdm removed the RC1 label Apr 21, 2020
@arthurdm arthurdm requested a review from sbose78 April 24, 2020 16:28
@sbose78
Copy link
Contributor

sbose78 commented Apr 24, 2020

Looks good!

Could you mark this as 'experimental' please? I'm in the process of defining it out side of the context of binding too and things might change and I will ensure this stays in sync.

If there are things being tracked for RC2, let's have this tracked please.

@arthurdm
Copy link
Member Author

thanks @sbose78

Added experimental and opened issue #35 for continuing this in RC2.

@arthurdm arthurdm merged commit cb4062a into master Apr 24, 2020
@arthurdm arthurdm deleted the discovery branch April 24, 2020 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants