Kubernetes names of ServiceClass and Plan versus OSB Guids

[duglin]

See: https://docs.cloudfoundry.org/services/managing-service-brokers.html

Upon refreshing a catalog from a broker a Service and/or Plan name can change. This is problematic for us because we map those names to Kube resource names, which are immutable.

I think we need to use the "id" from the Broker as the Kube "name" since in both cases they'll be immutable. We can then create a property called OSBName (or something like that) to hold the "name" returned from the broker. In UIs we need to remind people to show the OSBName not the Kube name.

[pmorie]

I agree with what you've said here if we can pin down for certain whether the names of these entities can change. If you just look at the OSB spec, it's not super clear: https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#catalog-management

The link says not to change the ID of services and plans, which I would agree implies that the names might be changed. I think we should clarify this in the spec before we make any code changes.

If we do this, it's not an amazing experience if you're just writing yaml, but porcelain commands and good user interface can make this easier.

[pmorie]

@vaikas-google your thoughts appreciated here

[duglin]

yep - I shutter at the idea of asking people to type GUIDs instead of nicer names in their kube yaml but we may not have a choice.

[pmorie]

Opened openservicebrokerapi/servicebroker#187 to determine the answer to this

[arschles]

We discussed in the SIG call that, if brokers are allowed to change names on catalog refreshes, we need to use the GUIDs, and reference them in Instances. This is of course a poor user experience, so I propose that we build a layer of abstraction (the solution to everything in CS, of course!) for naming. I've detailed it in #805

[MHBauer]

One of the original conversations was to use the osb-guid as the k8s-meta-name for exactly this reason. We ended up using the osb-name for the usability reason.

[duglin]

On today's call we agreed to using ID as the metadata.name on ServiceClass and ServicePlan but we should keep ExternalID too just in case we decide to use something else for the name.

I'll open a new issue to discuss possible designs for how to add on something for a better UX - and that discuss (IMO) should happen before beta.

[jwforres]

On Thu, May 11, 2017 at 5:02 PM, Doug Davis ***@***.***> wrote: On today's call we agreed to using ID as the metadata.name on ServiceClass and ServicePlan but we should keep ExternalID too just in case we decide to use something else for the name. I'll open a new issue to discuss possible designs for how to add on something for a better UX - and that discuss (IMO) should happen before beta.

Agreed about that happening before beta, please include me in that

…

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#802 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABZk7TpOFp99fEQaceIx6LaF3j3MYxFZks5r43dYgaJpZM4NTPUn> .

[pmorie]

It looks like this isn't going to be in 0.0.7, moving to 0.1.0

[arschles]

If we switch to using GUIDs as the primary key for serviceclasses and plans, I'd like to include a way to assign human-readable names to the services and plans in the same change. I agree that a porcelain CLI may solve the naming problem, but it won't be a complete solution because Instances will be created from helm charts as well.

[duglin]

while the most common pattern in kube for referencing other objects is by name, what other mechanisms are there? I know labelSelectors is one but are there others?

[pmorie]

The only way to reference individual resources from another resource is by name.

I would be very curious to know how CF deals with name changes for services / plans. Does someone want to take an action item to find out about that?

[duglin]

they expose the GUIDs for things so even if the user says "foo", they'll use the guid under the covers.

[duglin]

this is why using "names" as keys is a bad idea. Typically names are human readable and may want to be changed from time to time - but keys/guids/ids/.... being static for all time is usually ok.

[duglin]

Just wondering... are there any cases in kube of people being allowed to modify the name property on a resource? While not common if its technically allowed it might be something to consider.

[pmorie]

Just wondering... are there any cases in kube of people being allowed to modify the name property on a resource? While not common if its technically allowed it might be something to consider.

No

[pmorie]

Here's another requirement that I think exists:

• It should be possible to write a template, helm chart, resource YAML that reliably addresses a particular service class and plan that will be usable in multiple clusters

[pmorie]

@smarterclayton curious to know your thoughts here

[pmorie]

One thought I had: it would be possible conceivably to support different modes of addressing a serviceclass and plan with a union type.

Also, I would like to point out that this is related to #1010

[duglin]

I think there are multiple ways in which to view how a service would be used across clusters.

1 - from within each cluster there's a cross-cluster reference to the service - @pmorie I think this is the mode you're referring to in your comment above. And in that case I would imagine the "normal" Kube mechanism would be used. Meaning, the reference would need to include some kind of cluster ID/name. In this case though I would imagine that the service would need to be owned by just one cluster and the one would be responsible for managing it.

2 - if people want to share services across clusters then the service is registered with each cluster independently. This has the advantage of the admin being able to managed which clusters sees which service/plan. This is the model we use in Bluemix today except its not just across Kube clusters but even CF and Docker clusters.

I'm not sure I see the advantage of the first model since there really isn't any logic associated with the registration process - its more a question of visibility and access control - which is better done via the 2nd model. The Broker is really the thing that's shared.

[pmorie]

I did not have cross-cluster in mind as a usage model at all, actually, might need to go and clarify some of my above comment

[pmorie]

Let me clarify my above comments. So far in our discussion of this issue we have implicitly been hunting for a single 'just-right' way to address service classes and service plans from the instance spec. Given the differences of opinion on that subject, I started thinking about supporting both of the following options:

(note, assumes we used the OSB ID of service classes/plans as the kubernetes name of these resources):

1. Reference the kubernetes name (which in this case would be likely not to be human readable) of these resources
2. Reference the human-readable name of the service class/plan at any given time (I think this is the mode @duglin seems to favor in this comment
3. Create 'alias' resources for service class and plan that have user-supplied, stable names that resolve to a specific OSB ID/kubernetes name of service class and plan

I'm not sure whether we would necessarily want to support all of these, but there are other variations of 'where does this service instance come from' (like user provided services, see #1010), that could also be modeled as siblings to these behaviors.

As I said in this comment, my big takeaway is that we probably ought to use a union type to hold information about where a service instance comes from in order to add new options in a backward compatible way and clearly delineating exactly what option a user is requesting.

[MHBauer]

https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/kubernetes-sig-architecture/rJGgMjrFIQc/rNm6uwlnBQAJ looks like the upstream discussion to follow.

[pmorie]

Proposal

Assumptions

• name changes of services and plans are deprecated (as agreed to in OSB face to face) do not have to be accomodated in a first-class way
• users should never have to look at non-human-readable names of services or plans

Proposed Changes

• Move existing fields under ServiceClass and ServicePlan under new Spec structs and add Status to these objects
• Retain existing ExternalID fields and ensure these values are immutable for a given name of service and plan
• Kubernetes names:
  • ServiceClass: $brokerName-$serviceClassName
  • ServicePlan: $brokerName-$serviceClassName-$servicePlanName
• Add a new Status.Inactive (boolean) field to ServiceClass and ServicePlan
• If a broker changes the name of a service:
  • set the Status.Inactive field on that ServiceClass
  • set a status message saying that the service has been renamed to $new-name
  • do not allow new instances to be created of this service
• If a broker changes the name of a plan:
  • set the Status.Inactive field on that ServicePlan
  • set a status message saying that the plan has been renamed to $new-name
  • do not allow new instances to be created on this plan
• ServiceInstance keeps the same fields and uses the full kubernetes names to reference ServiceClass and ServicePlan

API Changes

type ServiceClass struct {
	TypeMeta
	ObjectMeta

	Spec ServiceClassSpec
	Status ServiceClassStatus
}

type ServiceClassSpec struct {
	// all existing fields of ServiceClass here

}

type ServiceClassStatus struct {
	// Inactive indicates whether a service is inactive due to being deleted or renamed by the broker
	Inactive        boolean
	// InactiveMessage has information about why the ServiceClass was made inactive
	InactiveMessage *string
}

type ServicePlan struct {
	TypeMeta
	ObjectMeta

	Spec ServicePlanSpec
	Status ServicePlanStatus
}

type ServicePlanSpec struct {
	// all existing fields of ServicePlan here

}

type ServicePlanStatus struct {
	// Inactive indicates whether a plan is inactive due to being deleted or renamed by the broker
	Inactive        boolean
	// InactiveMessage has information about why the ServicePlan was made inactive
	InactiveMessage *string
}

Examples

Given a broker test-broker whose catalog contains a service test-service with plan example-plan-1, the following resources would be created:

apiVersion: servicecatalog.k8s.io/v1alpha1
kind: ServiceClass
metadata:
  name: test-broker-test-service
spec:
  brokerName: test-broker
  externalID: d35b55b2-b1fd-4123-8045-5b9c619cb629
  description: "service description"
  bindable: true
---
apiVersion: servicecatalog.k8s.io/v1alpha1
kind: ServicePlan
metadata:
  name: test-broker-test-service-example-plan-1
spec:
  brokerName: test-broker
  serviceClassName: test-broker-test-service
  externalID: 10e03cb7-b2cf-40dd-a954-16a382b92446
  description: "plan 1 description"
  free: true

A ServiceInstance referencing these would look like:

apiVersion: servicecatalog.k8s.io/v1alpha1
kind: ServiceInstance
metadata:
  name: test-instance
  namespace: test-ns
spec:
  serviceClassName: test-broker-test-service
  planName: test-broker-test-service-example-plan-1

If the broker is poorly behaved and intentionally chooses to use deprecated behaviors and renames example-plan-1 to example-plan-2, we would have the following ServicePlans:

apiVersion: servicecatalog.k8s.io/v1alpha1
kind: ServicePlan
metadata:
  name: test-broker-test-service-example-plan-1
spec:
  brokerName: test-broker
  serviceClassName: test-broker-test-service
  externalID: 10e03cb7-b2cf-40dd-a954-16a382b92446
  description: "plan 1 description"
  free: true
status:
  inactive: true
  inactiveMessage: "renamed to test-broker-test-service-example-plan-2"
---
apiVersion: servicecatalog.k8s.io/v1alpha1
kind: ServicePlan
metadata:
  name: test-broker-test-service-example-plan-2
spec:
  brokerName: test-broker
  serviceClassName: test-broker-test-service
  externalID: 10e03cb7-b2cf-40dd-a954-16a382b92446
  description: "plan 1 description"
  free: true

[duglin]

Proposal discussed on today's call: https://docs.google.com/presentation/d/1ZdRqLgAbE8uTQpJrXmdTucitPknURMoQITAJSxr3LhE/edit#slide=id.g25fb676d6e_0_91

[pmorie]

I might have another idea!

[pmorie]

I am starting to think that we should bail on solving all the complexities of name changes in the initial beta release and solve them later. It's most important to get things into the hands of users and this seems to be a real bear of a problem to solve. So, I'm not 100% sure, but I think we should consider doing it.

I do still have another idea I will write up ASAP, but I have exhausted my time budget for this problem for this week and need to move onto other things.

[pmorie]

Proposal:

• Use OSB ID for k8s names for now
• Add an ExternalName field to ServiceClassSpec and ServicePlanSpec that holds the value of the human-readable OSB name
• ExternalName should be a selectable field
• Users use the human-readable names to specify which service and plan they want via the ExternalServiceName and ExternalPlanName fields
• Add ServiceClassRef and ServicePlanRef fields into ServiceInstanceSpec
• Controller writes the ServiceClassRef and ServicePlanRef fields if they are not set
• Controller updates the ServicePlanRef field if the ExternalPlanName is updated by the user (the existing admission controller will block updates when the service is does not support plan updates) - this also triggers updating the instance at the broker

type ServiceInstanceSpec struct {
	// ExternalServiceName is the human-readable name of the service
	// as reported by the broker.
	ExternalServiceName string
	// ExternalPlanName is the human-readable name of the plan
	// as reported by the broker.
	ExternalPlanName string

	// ServiceClassRef is a reference to the ServiceClass
	// that the user selected.
	ServiceClassRef *ObjectReference
	// ServicePlanRef is a reference to the ServicePlan
	// that the user selected.
	ServicePlanRef  *ObjectReference
}

type ServiceClassSpec struct {
	ExternalID   string
	ExternalName string
}

type ServicePlanSpec struct {
	ExternalID   string
	ExternalName string
}

[vaikas]

@pmorie @duglin
Do we still need ExternalID fields in Service[Class|Plan]Spec? Seems like that's simple .Name in both cases if we use the ID as the name and it's immutable?

[pmorie]

@vaikas-google I do want both external ID and Name fields so we can support different naming strategies in the future

[vaikas]

ack