servicebinding / spec

Specification for binding services to k8s workloads
https://servicebinding.io
Apache License 2.0
92 stars 35 forks source link

Configuration hints to enable or disable performance optimizations (with trade-offs) #167

Closed baijum closed 3 years ago

baijum commented 3 years ago

"Premature optimization is the root of all evil." -- Sir Tony Hoare (popularized by Donald Knuth)

In a production environment, some optimizations are desirable to reduce the workload in the Kubernetes cluster. These are some of the choices that an implementation can make to improve the performance:

  1. Make Service Binding resource immutable
  2. Avoid watching Provisioned Service resource for changes
  3. Avoid watching Application resource for changes

Each of these optimizations improves the performance but comes with its trade-offs -- the optimizations work against the inherent behavior of a custom resource and implementation patterns in Kubernetes. So, it's better to make it configurable so that those who need it can enable it explicitly, and those who don't need it can disable it explicitly.

Making ServiceBinding resource immutable can be performed after a successful reconciliation. A successful reconciliation of the ServiceBinding resource can be decided based on conditions of type Ready with status True as given below:

status:
  conditions:
  - type:   Ready
    status: 'True'
    reason: 'Projected'
    message: ''
    lastTransitionTime: '2021-01-20T17:00:00Z'

The spec can provide these configuration hints through annotations in the ServiceBinding resource:

   performance.service.binding/immutable: "true"
   performance.service.binding/watch-service: "false"
   performance.service.binding/watch-application: "false"

In the absence of these configuration hints, implementation can decide the default value. All these flags are configuration hints. If an implementation doesn't support these optimizations, those implementations can ignore these configuration hints.

FAQ

Question 1: If these all are performance optimizations, why not enable it always without any configuration hints? Answer: These optimizations come with trade-offs, and the requirements for Kubernetes clusters will be different. For example, these optimizations may not be suitable for a development Kubernetes cluster.

Question 2: How a new optimization is going to work based on these configuration hints? Answer: Any optimization with a trade-off needs to be explicitly enabled based on some configuration hints. If such an optimization is being introduced in the future, the spec needs to change accordingly. Note: There is no issue with introducing optimizations without any trade-offs.

Question 3: If these optimizations come with trade-offs, why make them part of the implementation? Answer: These optimizations will be enabled by those who need them to improve the performance, and it's safe to assume they understand the trade-offs.

Question 4: What are the potential trade-offs of enabling these performance optimizations? Answer: The Kubernetes custom resource, particularly the ServiceBiniding, is inherently not immutable. So, a user might attempt to change one of the field values, say .spec.provider or .spec.env. If the user relies on a GitOps tool, reconciling the desired state in Git is a norm. Deleting a resource to achieve the same result is against the expected state reconciliation behavior. Watching resources to reconcile is a typical implementation pattern in Kubernetes. So working against that would be unexpected, even if it provides performance gain. But these optimizations controlled through explicit configuration hints help to avoid any confusion.

Question 5: Why use annotation for configuration hints, and why not spec fields? Answer:

With all these considerations, I felt annotation would be a better choice. Remember, these are "hints" and not mandates.


Normative text:

Configuration Hints to Enable or Disable Performance Optimizations

Configuration hints are optional annotation-based flags to enable or disable performance optimizations with trade-offs in the implementation. Implementations with support for these optimizations MUST respect these values to enable or disable them. Implementations without support for these optimizations MAY ignore them. In the absence of these configuration hints, the implementation MAY decides the default value.

To make the ServiceBinding resource immutable after a successful reconciliation, the user MAY set this annotation:

performance.service.binding/immutable: "true"

To remove watching the Provisioned Service resource for any changes, the user MAY set this annotation:

performance.service.binding/watch-service: "false"

To remove watching the Application resource for any changes, the user MAY set this annotation:

performance.service.binding/watch-application: "false"
baijum commented 3 years ago

We can consider this proposal post 1.0 release. Probably after some real-world testing and feedback.

pedjak commented 3 years ago

We should try to minimize leakage of implementation strategies/details into the spec and answer the following questions in the spec itself:

We can consider this proposal post 1.0 release. Probably after some real-world testing and feedback.

Agree. We could easily add the proposed hints first as a part of an operator configuration - not sure if we need such fain-grained control per service binding from the start.

baijum commented 3 years ago

Suppose an implementation makes the ServiceBinding resource immutable and stops all watches. That implementation can design as a CLI tool instead of an operator with CRD and controller.

The CLI tool can still use the ServiceBinding resource as a regular YAML configuration file.

For example, let's take a simple YAML configuration (conig.yaml):

apiVersion: service.binding/v1alpha2
kind: ServiceBinding
metadata:
  name: account-service

spec:
  application:
    apiVersion: apps/v1
    kind:       Deployment
    name:       online-banking

  service:
    apiVersion: v1
    kind:       Secret
    name:       prod-account-service-secret

To bind:

$ sb bind -f config.yaml

To unbind:

$ sb unbind -f config.yaml

The installation scripts can run these commands to bind and unbind. Few parts of the spec will be irrelevant for a CLI tool (e.g, whole status block).

scothis commented 3 years ago

I have a few concerns with this proposal:

  1. this spec should be focused on the semantics of a service binding. Performance is typically in the domain of an implementation. If the spec is mandating something that is difficult to implement efficiently, then let's have that discussion in context.
  2. this discussion is occurring without numbers or a means to measure impact.
  3. asking every user to opt-in to a flag for every resource is never going to happen (the commons remain tragic). The flags either need to be defined system wide (in which case there's no need for it to be part of the spec) or framed in a way that users will actually care about the setting.
baijum commented 3 years ago

Based on the discussion here, I am closing this issue.