Service label selector

[arthurdm]

Fixes #169

This PR adds parity between spec.application and spec.service by allowing both to be selected via labels.

Given the nature that labels can match more than 1 resource I also edited the language around $SERVICE_BINDING_ROOT/<binding-name> and how to achieve uniqueness. I believe this was needed even before this PR, because you could currently have two different ServiceBinding CRs with the exact same spec.name field (eg. "database").

[baijum]

If there are different versions/targets of the same service, what will be the recommended strategy to pick the right version/target from an application?

Similar to the provider field, can the spec recommend another field to determine the right version/target? The field name could be environment or cluster or kube-cluster. (I expect values like dev, qa, stage, prod etc.)

[arthurdm]

hi @baijum - you still need to define the kind and apiVersion of the resource, the "oneOf" applies to {name, selector}. Is there a different scenario you're thinking about?

[baijum]

hi @baijum - you still need to define the kind and apiVersion of the resource, the "oneOf" applies to {name, selector}. Is there a different scenario you're thinking about?

I was thinking about the usage patterns that can be adopted by the language-specific application/framework.

For example, consider this Java library:

https://github.com/spring-cloud/spring-cloud-bindings/blob/12b88ac5fe4122459e560c5a98411f58c4694349/src/main/java/org/springframework/cloud/bindings/Bindings.java#L152

or this Python library:

https://pypi.org/project/pyservicebinding/

There are methods to filter bindings based on type and provider values. Since this PR is providing a mechanism to project different versions/targets of the same service, how will the application differentiate between the projected bindings? Or can we assume at a given time, only one bindings directory will be present in the $SERVICE_BINDING_ROOT?

[arthurdm]

how will the application differentiate between the projected bindings?

The key is the problem you describe is not introduced with this PR, it's already there. You can create 2 ServiceBinding CRs that point to different instances of the same service, so both type and provider would be the same.

[baijum]

how will the application differentiate between the projected bindings?

The key is the problem you describe is not introduced with this PR, it's already there. You can create 2 ServiceBinding CRs that point to different instances of the same service, so both type and provider would be the same.

Ok, let's handle this situation in the user guide. I have started working on that here: #175

[arthurdm]

hey everyone - I've updated the PR with the agreement from yesterday's community call to add the restriction that only a single label match (for either app or service) is taken into consideration.

[baijum]

What about the zero matches scenario? I think the spec should add a mandate to yield at least one result for label selectors. So, the reconciler can raise error when there is no matching service found.

[scothis]

I'm not in favor of this proposal. It increases complexity for both users and implementations while providing meager benefits.

Kubernetes resources should be level-triggered. That is, a given set of input resources produces a deterministic set of output resources over time. This stability over time is particularly crucial to us as to avoid thrashing the application workload resource with unnecessary mutations. The predictability of the output is even more important as the input state changes over time. It shouldn't matter how the input state evolved to get to the current state.

Here are several cases that need to have their semantics defined:

• existing binding with a selector and a single matching resource, a new matching resource is created
• two existing resources, a new binding is created with a selector matching both
• existing binding with a selector and a single matching resource, the selector is update to match more resources
• existing binding with a selector and a multiple matching resource, the selector is update to match fewer resources
• (depending on the actual proposed semantics, there will be more edge cases)

Most users don't expect that creating a new resource will break their existing system.

Binding a single service to multiple application workloads don't have these semantic issues. This is because the ServiceBinding resource is inherently asymmetric; it has extra knowledge about the service (name, type, provider) that is applied to the application workload. The binding name is particularly problematic since it has uniqueness constraint forced onto it by the file system. Trying to force symmetry on top of an asymmetric model is asking for trouble, or a best a bunch of edge case semantics that need to be defined. There's a time and place to break every rule-of-thumb, but the benefit here is too marginal to justify the high cost.

While I don't think this belongs in the spec, I'm in favor of experimenting with a resource that can define these semantics that in turn creates child ServiceBinding resources as necessary.

[scothis]

What does "first" mean? Label selectors are a filter that's applied to a set. There's no semantic order until you actively sort the set based on some criteria.

[arthurdm]

The main point here is that the spec is allowing for selectors to be used in the cases where the name is not known a head of time, but it is strongly recommending that an appropriate set of labels be used so that 1 and only 1 resource matches. I think that's a reasonable comprise to allow for the dynamic name use case while still keeping a 1-to-1 cardinality between app - Service Binding - service.

[scothis]

but it is strongly recommending that an appropriate set of labels be used so that 1 and only 1 resource matches

The proposed change is not a strong recommendation, but a new requirement that changes the existing behavior.

Current text:

implementors MUST handle each matching resource as if it was specified by name in a distinct ServiceBinding resource.

Proposed text:

the reconciler implementation MUST select the first match and ignore any other matches.

[scothis]

From an implementors perspective, I'm not sure what "first match" means. Can you elaborate on the desired semantics?

[arthurdm]

The proposed change is not a strong recommendation, but a new requirement that changes the existing behavior.

There are two personas: the author of the ServiceBinding CR (let's call "the user") and the implementation of the spec. There's a strong recommendation for the user to use a combination of labels to yield a single match, because the implementation has a requirement to pick the first one it finds.

I think we want to leave the matching order as undefined, hence the strong recommendation to the user.

[scothis]

The proposed text is specifically calling out that "reconciler implementation MUST" do something, but that something is ambiguous. The word "first" is troubling me because k8s label selectors don't have an order. We're asking implementations to distinguish between two things without giving them any guidance on how to make that distinction.

[arthurdm]

I've now updated the PR to be a bit more descriptive, let me know if this helps:

As label selectors are inherently queries that return zero-to-many resources, the reconciler implementation MUST select whichever resource it first matched and ignore any other matches. Given that ordering of resource matching is undefined, it is RECOMMENDED that ServiceBinding authors use a combination of labels that yield a single resource.

[scothis]

I'm still rather down on adding a label selector to the service reference. As an attempt to create more symmetry between the application workload and service references, I created a draft proposal in #182 that will remove the label selector for application workloads.

One of the driving factors in adding the selector support in the first place was binding into CronJob resources where the target resource itself was not PodSpecable, but has children that are although they have a generated name. The ClusterApplicationResourceMapping support is a better way to manage this case since it can bind directly to CronJob resources.

[arthurdm]

One of the driving factors in adding the selector support in the first place was binding into CronJob resources where the target resource itself was not PodSpecable

I thought the main reason for the workload labels was for resources that had generated names (independent if they were PodSpecable or not). In my mind that's the biggest value-add for supporting labels.

[scothis]

I thought the main reason for the workload labels was for resources that had generated names

If the resource is mutable, you can create the target resource, get the actual name and then create the ServiceBinding resource with that name. It was immutable resource with generated names that were tricky (like a CronJob decomposing to a Job)

[arthurdm]

Maybe we can go through this a bit more in the hangout, as I am confused. :)

If I am using GitOps and I have my ServiceBinding CR in a Git repo I need to either know the name of the target resource or need a label that will be used.

[scothis]

If I am using GitOps and I have my ServiceBinding CR in a Git repo I need to either know the name of the target resource or need a label that will be used.

It's not that I don't think application label selectors have a unique use. I argued hard to get the capability added to the spec in the first place. ClusterApplciationMappings can pick up some of the original need that drove the proposal, but there are still use cases that would be lost.

However, I would rather remove the ability to define an application label selector than to nerf the existing capability by limiting it to a single "first" resource. The proposed language is not only going to surprise users (hopefully not in production), it's also significantly more complicated to implement. On the call community call where this proposal was discussed, there was a desire for symmetry between the service and application selectors. If symmetry is a goal, #182 is a better approach.

My desired resolution is to close both this PR and #182 unmerged. I still believe the behavior for a service label selector can be prototyped in a high level resource either decomposes to a ServiceBinding or acts as a mapping resource that can be referenced by a service binding. Once the semantics are stable and the need demonstrated, then we can expand the scope of the spec to add the behavior.

[arthurdm]

As discussed in today's hangout I will close this PR and we'll work towards merging #182 and then working on a more robust solution for both workloads and service labels.