workflow icon indicating copy to clipboard operation
workflow copied to clipboard

doc(applications): deploying add-ons

Open bacongobbler opened this issue 7 years ago • 15 comments

This is the design document for deis addons and is currently a work-in-progress. This document is expected to change over time as work is being done.

closes https://github.com/deis/workflow/issues/741 once the work has been laid out and implemented.

bacongobbler avatar Apr 10 '17 20:04 bacongobbler

@slack, @mboersma and @krancour are potential reviewers of this pull request based on my analysis of git blame information. Thanks @bacongobbler!

deis-bot avatar Apr 10 '17 20:04 deis-bot

added deis addons:services to the docs to list the service catalog.

bacongobbler avatar Apr 10 '17 20:04 bacongobbler

removed the --as support in the doc. Heroku's implementation is shoddy at best and I couldn't think of a perfect solution out the door, so for an initial implementation we'll just accept whatever envvars the service broker gives us.

bacongobbler avatar Apr 10 '17 22:04 bacongobbler

@bacongobbler, I think one needs to carefully read docs or interactive help to understand that addons:list is provisioned and bound services while addons:services are available services. Would the intent of each of those list operations be less ambiguous if we used addons:provisioned (or addons:installed) and addons:available, respectively?

krancour avatar Apr 11 '17 02:04 krancour

I was trying to make it a 1:1 mapping with heroku CLI commands, but I agree it is a little confusing. I'd be amenable to deis addons:list being the catalog list and deis addons:installed being a list of addons provisioned under the user's account. How about that?

bacongobbler avatar Apr 11 '17 02:04 bacongobbler

Is parity with Heroku still a concrete objective? In either event, I'm happy with what you have just proposed.

krancour avatar Apr 11 '17 02:04 krancour

Is parity with Heroku still a concrete objective?

Yes, because it's still a familiar toolset to many people, and that's still the general inspiration for Deis Workflow functionality. But it's not set in stone, if there's a good argument for doing things a different way--and it sounds like there might be here.

mboersma avatar Apr 11 '17 15:04 mboersma

I think this is a very good opportunity to change the heroku CLI names for specific commands. I agree that addons:list and addons:services is too confusing. I'll update the docs with the commands I proposed.

bacongobbler avatar Apr 11 '17 15:04 bacongobbler

So as it turns out, instances are namespaced. Makes sense, but we haven't had to provision user-namespaced resources before... Everything was either an annotation on the application's service or on the router.

What I think that means going forward is that we can introduce a myuser namespace creation at deis register (along with a data migration for existing users), then each instance that is provisioned from a broker is created in that user's namespace. Instance bindings just reference instances by name which is globally unique, so having the instance in one namespace and the binding in another shouldn't matter, right? This would also have the added benefit that all instances are removed when the user is deleted. This would also be a useful transition into deis teams, in that instances are deployed to a team's namespace and can be managed by the cluster operator accordingly.

Any opinions/thoughts on the matter, @krancour or @ultimateboy?

bacongobbler avatar Apr 12 '17 21:04 bacongobbler

From what it looks like, my assumption was wrong. instances and instance bindings must exist in the same namespace, so instances must be bound to the application then. That kinda sucks, because I had expected instances to be able to bind to an account and the bindings would be what the application cares about. I'll file a ticket upstream and see if there's a way we can work around this.

As seen here, I have an instance in the bacongobbler namespace...

><> kubectl --context=service-catalog get instances --namespace bacongobbler
NAME           KIND
ups-instance   Instance.v1alpha1.servicecatalog.k8s.io

...And an instance binding in the test-ns namespace following the basic walkthrough.

><> kubectl --context=service-catalog get bindings -n test-ns ups-binding -o yaml
apiVersion: servicecatalog.k8s.io/v1alpha1
kind: Binding
metadata:
  creationTimestamp: 2017-04-12T22:44:01Z
  finalizers:
  - kubernetes
  name: ups-binding
  namespace: test-ns
  resourceVersion: "76"
  selfLink: /apis/servicecatalog.k8s.io/v1alpha1/namespaces/test-ns/bindings/ups-binding
  uid: 85e8d1d6-1fd1-11e7-b4a0-0242ac110006
spec:
  instanceRef:
    name: ups-instance
  osbGuid: 17633cb4-dff1-4221-909d-e3dadc2f3707
  secretName: my-secret
status:
  conditions:
  - message: The binding references an Instance that does not exist. Binding "test-ns/ups-binding"
      references a non-existent Instance "test-ns/ups-instance"
    reason: ReferencesNonexistentInstance
    status: "False"
    type: Ready

bacongobbler avatar Apr 12 '17 22:04 bacongobbler

You know what, the idea of the instance and the instance binding living in the same namespace makes more sense anyways because in a team-centric world, all apps will be deployed in the same namespace as the team name so the instance should live in there.

I'll be white-boarding this more overnight and try to come up with a better solution tomorrow.

bacongobbler avatar Apr 12 '17 22:04 bacongobbler

@bacongobbler whether it should be possible to bind to instances in other namespaces was the subject of lots and lots of discussion very early on, but not so much as of late. I am of the opinion that in large enterprises and in microservice architectures, the notion of n applications all binding to a common instance that belongs to none of them (but is managed as its own thing) is probably not an uncommon use case. But that's not something that service catalog currently does because it would simply defy k8s existing permissions model and I don't think we were about to go re-inventing that.

The practical implication of that is exactly what you seem to have concluded already-- that service instances and bindings for those instances should all be provisioned within an app's own namespace (or within a team namespace that contains multiple apps? is that a thing now?)

Also, since Workflow is very app centric, would provision + bind be combined into a single step for Workflow users?

krancour avatar Apr 13 '17 00:04 krancour

or within a team namespace that contains multiple apps? is that a thing now?

Nope! Just a twinkle in my eye.

Also, since Workflow is very app centric, would provision + bind be combined into a single step for Workflow users?

Yes. Honestly I've just revamped the documentation again to only have 5 commands:

  • addons:list (list instances provisioned for this app)
  • addons:create (create an instance + binding, using format serviceclassname:plan as argument)
  • addons:destroy (destroy an instance + binding using service class name as the argument)
  • addons:catalog (list available service classes)
  • addons:plans (list available plans for the given service class name)

addons:plans is still yet to be added to this PR, but will be up in a sec.

At the current time let's keep the instance + instance binding together with the app since all apps are in their own namespace. Had we deployed apps in a user's personal namespace (or implemented deis teams) then separating instance bindings from the app might've made more sense, but not with the current model.

bacongobbler avatar Apr 13 '17 19:04 bacongobbler

@bacongobbler I like where this is headed. It's clean and simple. There's a lot more complexity that could be exposed, but given Workflow's Heroku-esque focus on 12factor apps, this looks to be just right. 💯

krancour avatar Apr 13 '17 19:04 krancour

k, deis addons:plans has now been added as well.

bacongobbler avatar Apr 13 '17 20:04 bacongobbler