Normative text missing for ServiceBinding resource's .status.observedGeneration field

[baijum]

ServiceBinding resource type schema has a .status.observedGeneration field:

...
status:
  binding:              # LocalObjectReference, optional
    name:               # string
  conditions:           # []metav1.Condition containing at least one entry for `Ready`
  observedGeneration:   # int64

But there is no normative text about the .status.observedGeneration field explaining its significance. Probably the spec should give some info about that field's usage pattern.

There was some comment about that field in the past here: https://github.com/k8s-service-bindings/spec/issues/126#issuecomment-710105155

[baijum]

The .status.ObservedGeneration field might be useful for some implementations. I think we can make it an optional field and add a normative text something like this:

If the .status.observedGeneration field is defined, the value of .status.observedGeneration SHOULD be the generation of the ServiceBinding resource last processed by the controller. Note: The .metadata.generation field always points to the current generation of the ServiceBinding resource, which is incremented by the API server when writes are made to the ServiceBinding resource spec field.

BTW, KubePresent doesn't use .spec.observedGeneration field to track the spec generation. Instead, it uses the GenerationChangedPredicate predicate provided by the controller-runtime package. So, I am also fine with removing this field.

Let me know your thoughts.

[scothis]

The role of the ObservedGeneration field isn't for the controller, but for the consumer. When the Generation and ObservedGeneration values are the same, then the consumer knows that the resource's spec has been reconciled and the status is accurate. This is important because a ServiceBinding can be Ready=True and then have the spec updated. If someone observes the ServiceBinding resource after an update and before the reconciler runs, they may see the now stale status and think the spec's desired state is achieved.

it uses the GenerationChangedPredicate predicate

I would caution against using the GenerationChangePredicate in this way. You'll be missing changes to the resource that don't increment the generation, and all resync events.

[pedjak]

Does it make sense to allow updates for already successful binding, i.e. when Ready=True? What would be a usecase?

[baijum]

@scothis Thanks for the clarification.

Based on your input I changed the text like this:

The value of .status.observedGeneration MUST be the generation of the ServiceBinding resource last processed by the controller. Note: The .metadata.generation field always points to the current generation of the ServiceBinding resource, which is incremented by the API server when writes are made to the ServiceBinding resource spec field.

If this looks fine, I can send a PR. We can refine the text further from the PR.

[scothis]

It's worth clarifying whose responsibility it is to set the value, when it should be updated, and what users can do with the value.

How about something like:

When updating the status of the ServiceBinding resource, the controller MUST set value of .status.observedGeneration to the value of .metadata.generation. The .metadata.generation field is always the current generation of the ServiceBinding resource, which is incremented by the API server when writes are made to the ServiceBinding resource spec field. Consumers SHOULD compare the value of the observed and current generations to know if the status reflects the current resource definition.

[baijum]

Perfect :-)

Though, the last sentence looks a bit disconnected. Let's add a transition phrase to improve the flow.

Therefore, consumers SHOULD compare...

I will send a PR.

[baijum]

I will send a PR.

Done. #164