metadata.observedGeneration behaves differently across CRDs

[dgerd]

In what area(s)?

/area API

/area autoscale
/area build
/area monitoring
/area networking
/area test-and-release

What version of Knative?

0.2.x
0.3.x
Output of git describe --dirty

0.7

Expected Behavior

From Kubernetes Guidance,

If the primary resource your controller is reconciling supports ObservedGeneration in its status, make sure you correctly set it to metadata.Generation whenever the values between the two fields mismatches.

This lets clients know that the controller has processed a resource. Make sure that your controller is the main controller that is responsible for that resource, otherwise if you need to communicate observation via your own controller, you will need to create a different kind of ObservedGeneration in the Status of the resource.

I expect observedGeneration to always be bumped when a controller picks up a new spec to reconcile regardless of what happens later in the reconcile loop.

Actual Behavior

Many of our controllers wait until the end before bumping observedGeneration or bump it only in certain situations. This can lead to:

1. Our status changes, but our observedGeneration does not increment
2. Our status doesn't change due to an error processing and our observedGeneration doesn't increment

In the first case the client will believe that we are still reflecting the status of the previous spec which is false. While in the second case the client will sit there waiting (potentially forever) depending on the type of error ( #4561 fixed one particular example of this case )

Given the coupling between client's (including the Service controller) use of observedGeneration and Conditions, this may likely require changes to either how we report reconcile errors (some of them are not reported in sub-conditions) or how we consume observedGeneration.

Steps to Reproduce the Problem

An example of this:

1. Put in a bad domainTemplate in the ConfigMap
   • updateRouteStatusURL should fail https://github.com/knative/serving/blob/master/pkg/reconciler/route/route.go#L194
   • This failure does not update Conditions so the status will not change
   • This failure occurs before observedGeneration is updated so even with a new spec observedGeneration will never change
2. Update the Route with a new spec
   • As observedGeneration is not updating it looks like the controller is not running, but really it is spinning attempting to retry

We have similar areas in Configuration reconciler where an err can occur before ObservedGeneration is updated resulting in spinning without update.

https://github.com/knative/serving/blob/master/pkg/reconciler/configuration/configuration.go#L129

[dgerd]

/assign

[dgerd]

@dgerd to scope this down further this release. Most likely work will and in 0.11.

[dgerd]

This has tumbled through the milestones and I have not been able to get to it so unassigning.

[vagababov]

/cc @whaught
Weston, is this something you're doing now with gen reconciler?

[whaught]

/assign

[whaught]

Yes this is an issue I am aiming to solve.

The precursor is in /pkg with knative/pkg#1226