Repository: knative/pkg
Pull request #1284 (2020-05-04 20:34:46)

Create a common method for processing reconciliation on generic KRShaped
Issue #1226

- Creates a post-process function for reasoning generically about KRShaped resources
    - Automatically bumps ObservedGeneration
    - Sets Ready=Unknown if an error is encountered and the reconciler did not update conditions.

This is not yet invoked, the next PR will integrate this with genreconciler.

knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1284#" title="Author self-approved">whaught</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **grantr**
You can assign the PR to them by writing `/assign @grantr` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[reconciler/OWNERS](https://github.com/knative/pkg/blob/master/reconciler/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["grantr"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257408439001288705/artifacts/line-cov.html#file0) | Do not exist | 100.0% | 

whaught: /assign @grantr @n3wscott 
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257441029670309888/artifacts/line-cov.html#file0) | Do not exist | 100.0% | 

knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257452353406111746/artifacts/line-cov.html#file0) | Do not exist | 100.0% | 

knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257469088842649601/artifacts/line-cov.html#file0) | Do not exist | 100.0% | 

knative-prow-robot: @whaught: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | a379c4b2f4abc83751c8b111663427aeb3f74a39 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-unit-tests/1257469088842649600) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1284). [Your PR dashboard](https://gubernator.knative.dev/pr/whaught).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
whaught: /retest
knative-prow-robot: @whaught: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | a379c4b2f4abc83751c8b111663427aeb3f74a39 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-unit-tests/1257470975625138176) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1284). [Your PR dashboard](https://gubernator.knative.dev/pr/whaught).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
whaught: /retest
antoineco: A general question about this PR: why would we want to force the Ready condition to Unknown instead of letting the user decide what to do with it in case of error?
knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1284#" title="Author self-approved">whaught</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **grantr**, **n3wscott**
You can assign the PR to them by writing `/assign @grantr @n3wscott` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[apis/duck/OWNERS](https://github.com/knative/pkg/blob/master/apis/duck/OWNERS)**
- **[reconciler/OWNERS](https://github.com/knative/pkg/blob/master/reconciler/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["grantr","n3wscott"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[apis/duck/v1/kresource_type.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257702501784752130/artifacts/line-cov.html#file0) | Do not exist | 60.0% | 

knative-prow-robot: @whaught: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | ac9806fff0242949df2afd03667249e55f975791 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-unit-tests/1257702501784752128) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1284). [Your PR dashboard](https://gubernator.knative.dev/pr/whaught).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: @whaught: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 9e36f8065253fd9beda68a1deb838bd7af240260 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-unit-tests/1257707786964307969) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1284). [Your PR dashboard](https://gubernator.knative.dev/pr/whaught).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[apis/duck/v1/kresource_type.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257707786964307968/artifacts/line-cov.html#file0) | Do not exist | 60.0% | 

knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[apis/duck/v1/kresource_type.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257708792888430593/artifacts/line-cov.html#file0) | Do not exist | 60.0% | 
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257708792888430593/artifacts/line-cov.html#file1) | Do not exist | 100.0% | 

knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[apis/duck/v1/kresource_type.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257712568319873026/artifacts/line-cov.html#file0) | Do not exist | 60.0% | 
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257712568319873026/artifacts/line-cov.html#file1) | Do not exist | 100.0% | 

whaught: > A general question about this PR: why would we want to force the Ready condition to Unknown instead of letting the user decide what to do with it in case of error?

We always want the implementer of the reconciler to be setting a condition when there is a failure. However now we will be incrementing observed generation automatically.

The client is typically waiting for obsGen == gen && ready to be terminal. If we were ready=true and then bumped generation on a failure without resetting the condition, clients would erroneously stop listening. The solution here is to catch failures and if the ready condition is unchanged, move it to unknown.

In a correctly implemented reconciler, the hope is that this status never gets hit. One alternative to this might be to set this status on generation bump _before_ the reconciler code runs and not bother with the old/new comparison?
antoineco: What about a transient failure? E.g. some external service the reconciler depends on is unreachable, but the reconciled objects can be assumed to be _just fine_, unless we observe that failure for a certain amount of time, or a certain amount of retries? In that case I want some level of control, Knative shouldn't do some magic and unconditionally update the `Ready` condition on my behalf, other components may depend on it.
whaught: > I agree that this might be OK for some/most implementations, but what about a transient failure? E.g. some external service the reconciler depends on is unreachable, but the reconciled objects can be assumed to be _just fine_, unless we observe that failure for a certain amount of time, or a certain amount of retries? In that case I want some level of control, Knative shouldn't do some magic and unconditionally update the `Ready` condition on my behalf, other components may depend on it.

I think in that case it would be reasonable for the reconciler to set conditions to _something_ (perhaps ready=true) as it processes the new generation rather than returning a reconciler.Event
antoineco: That would be fine, but your code forces the Ready condition to `Unknown` as long as `Semantic.DeepEqual` returns `true`, so if the Ready condition is already `True`, it will flip to `Unknown`.
whaught: > That would be fine, but your code forces the Ready condition to `Unknown` as long as `Semantic.DeepEqual` returns `true`, so if the Ready condition is already `True`, it will flip to `Unknown`.

Sorry I've changed the strategy with the above gist. I think preprocessing will be much simpler than trying to compare to the old resource.
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[apis/duck/v1/kresource_type.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257722382974455808/artifacts/line-cov.html#file0) | Do not exist | 60.0% | 
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257722382974455808/artifacts/line-cov.html#file1) | Do not exist | 90.0% | 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1284#" title="Author self-approved">whaught</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **grantr**
You can assign the PR to them by writing `/assign @grantr` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[reconciler/OWNERS](https://github.com/knative/pkg/blob/master/reconciler/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["grantr"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257729806619906049/artifacts/line-cov.html#file0) | Do not exist | 90.0% | 

knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257735846853873665/artifacts/line-cov.html#file0) | Do not exist | 90.0% | 

knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257799264549998592/artifacts/line-cov.html#file0) | Do not exist | 82.4% | 

knative-prow-robot: @whaught: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-build-tests | 4d9f0e35f5fef8bdae6cc2284658f1b5e3fcf7e7 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-build-tests/1257799264554192896) | `/test pull-knative-pkg-build-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1284). [Your PR dashboard](https://gubernator.knative.dev/pr/whaught).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257801781010763777/artifacts/line-cov.html#file0) | Do not exist | 82.4% | 

knative-prow-robot: @whaught: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-build-tests | 9ddd78f93d8929f43d21e2d169f9ec7f5796f4d9 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-build-tests/1257801781010763776) | `/test pull-knative-pkg-build-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1284). [Your PR dashboard](https://gubernator.knative.dev/pr/whaught).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
whaught: /retest
knative-prow-robot: @whaught: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-build-tests | 9ddd78f93d8929f43d21e2d169f9ec7f5796f4d9 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-build-tests/1257804045960089600) | `/test pull-knative-pkg-build-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1284). [Your PR dashboard](https://gubernator.knative.dev/pr/whaught).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257806185080295425/artifacts/line-cov.html#file0) | Do not exist | 82.4% | 

knative-prow-robot: @whaught: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-build-tests | 9ddd78f93d8929f43d21e2d169f9ec7f5796f4d9 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-build-tests/1257804045960089600) | `/test pull-knative-pkg-build-tests`
pull-knative-pkg-unit-tests | e5741b153a9ba0e9fb8e8a08d7190572f4250cad | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-unit-tests/1257806185084489728) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1284). [Your PR dashboard](https://gubernator.knative.dev/pr/whaught).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
whaught: /retest
knative-prow-robot: @whaught: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | e5741b153a9ba0e9fb8e8a08d7190572f4250cad | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-unit-tests/1257806185084489728) | `/test pull-knative-pkg-unit-tests`
pull-knative-pkg-build-tests | e5741b153a9ba0e9fb8e8a08d7190572f4250cad | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-build-tests/1257806185076101120) | `/test pull-knative-pkg-build-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1284). [Your PR dashboard](https://gubernator.knative.dev/pr/whaught).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
whaught: /retest
knative-prow-robot: @whaught: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-build-tests | e5741b153a9ba0e9fb8e8a08d7190572f4250cad | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-build-tests/1257816754558603264) | `/test pull-knative-pkg-build-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1284). [Your PR dashboard](https://gubernator.knative.dev/pr/whaught).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1284#" title="Author self-approved">whaught</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **mattmoor**
You can assign the PR to them by writing `/assign @mattmoor` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/knative/pkg/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["mattmoor"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1257818895062929408/artifacts/line-cov.html#file0) | Do not exist | 82.4% | 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1284#" title="Author self-approved">whaught</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **grantr**
You can assign the PR to them by writing `/assign @grantr` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[reconciler/OWNERS](https://github.com/knative/pkg/blob/master/reconciler/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["grantr"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1258021730773897216/artifacts/line-cov.html#file0) | Do not exist | 82.4% | 

antoineco: /lgtm
knative-prow-robot: New changes are detected. LGTM label has been removed.
knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1284#" title="Author self-approved">whaught</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **grantr**, **n3wscott**
You can assign the PR to them by writing `/assign @grantr @n3wscott` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[apis/OWNERS](https://github.com/knative/pkg/blob/master/apis/OWNERS)**
- **[reconciler/OWNERS](https://github.com/knative/pkg/blob/master/reconciler/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["grantr","n3wscott"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[apis/condition_set.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1258155989249560576/artifacts/line-cov.html#file0) | 97.5% | 96.7% | -0.8
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1258155989249560576/artifacts/line-cov.html#file1) | Do not exist | 91.7% | 

n3wscott: This is great, nice work. I think this will blend well with our controllers and let us drop some easily forgotten copy/paste logic from the current reconcilers. 

👏 

/lgtm
/approve

knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1284#issuecomment-624919153" title="Approved">n3wscott</a>*, *<a href="https://github.com/knative/pkg/pull/1284#" title="Author self-approved">whaught</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **grantr**
You can assign the PR to them by writing `/assign @grantr` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- ~~[apis/OWNERS](https://github.com/knative/pkg/blob/master/apis/OWNERS)~~ [n3wscott]
- **[reconciler/OWNERS](https://github.com/knative/pkg/blob/master/reconciler/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["grantr"]} -->
knative-prow-robot: New changes are detected. LGTM label has been removed.
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[apis/condition_set.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1258185560124559360/artifacts/line-cov.html#file0) | 97.5% | 96.7% | -0.8
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1284/pull-knative-pkg-go-coverage/1258185560124559360/artifacts/line-cov.html#file1) | Do not exist | 91.7% | 

vagababov: /lgtm
/approve
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1284#issuecomment-624919153" title="Approved">n3wscott</a>*, *<a href="https://github.com/knative/pkg/pull/1284#issuecomment-624954937" title="Approved">vagababov</a>*, *<a href="https://github.com/knative/pkg/pull/1284#" title="Author self-approved">whaught</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[apis/OWNERS](https://github.com/knative/pkg/blob/master/apis/OWNERS)~~ [n3wscott]
- ~~[reconciler/OWNERS](https://github.com/knative/pkg/blob/master/reconciler/OWNERS)~~ [vagababov]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1284#pullrequestreview-407068349" title="LGTM">grantr</a>*, *<a href="https://github.com/knative/pkg/pull/1284#issuecomment-624919153" title="Approved">n3wscott</a>*, *<a href="https://github.com/knative/pkg/pull/1284#issuecomment-624954937" title="Approved">vagababov</a>*, *<a href="https://github.com/knative/pkg/pull/1284#" title="Author self-approved">whaught</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[apis/OWNERS](https://github.com/knative/pkg/blob/master/apis/OWNERS)~~ [n3wscott]
- ~~[reconciler/OWNERS](https://github.com/knative/pkg/blob/master/reconciler/OWNERS)~~ [grantr,vagababov]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/apis/condition_set.go b/apis/condition_set.go
index 49f9d2e9b..8d02fec48 100644
--- a/apis/condition_set.go
+++ b/apis/condition_set.go
@@ -142,6 +142,11 @@ type conditionsImpl struct {
 	accessor ConditionsAccessor
 }
 
+// GetTopLevelConditionType is an accessor for the top-level happy condition.
+func (r ConditionSet) GetTopLevelConditionType() ConditionType {
+	return r.happy
+}
+
 // Manage creates a ConditionManager from an accessor object using the original
 // ConditionSet as a reference. Status must be a pointer to a struct.
 func (r ConditionSet) Manage(status ConditionsAccessor) ConditionManager {
diff --git a/reconciler/reconcile_common.go b/reconciler/reconcile_common.go
new file mode 100644
index 000000000..38e3bbe38
--- /dev/null
+++ b/reconciler/reconcile_common.go
@@ -0,0 +1,55 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package reconciler
+
+import (
+	"context"
+
+	duckv1 "knative.dev/pkg/apis/duck/v1"
+	"knative.dev/pkg/logging"
+)
+
+const failedGenerationBump = "NewObservedGenFailure"
+
+// PreProcessReconcile contains logic to apply before reconciliation of a resource.
+func PreProcessReconcile(ctx context.Context, resource duckv1.KRShaped) {
+	newStatus := resource.GetStatus()
+
+	if newStatus.ObservedGeneration != resource.GetObjectMeta().GetGeneration() {
+		condSet := resource.GetConditionSet()
+		manager := condSet.Manage(newStatus)
+
+		// Reset Ready/Successful to unknown. The reconciler is expected to overwrite this.
+		manager.MarkUnknown(condSet.GetTopLevelConditionType(), failedGenerationBump, "unsuccessfully observed a new generation")
+	}
+}
+
+// PostProcessReconcile contains logic to apply after reconciliation of a resource.
+func PostProcessReconcile(ctx context.Context, resource duckv1.KRShaped) {
+	logger := logging.FromContext(ctx)
+	newStatus := resource.GetStatus()
+	mgr := resource.GetConditionSet().Manage(newStatus)
+
+	// Bump observed generation to denote that we have processed this
+	// generation regardless of success or failure.
+	newStatus.ObservedGeneration = resource.GetObjectMeta().GetGeneration()
+
+	rc := mgr.GetTopLevelCondition()
+	if rc.Reason == failedGenerationBump {
+		logger.Warn("A reconciler observed a new generation without updating the resource status")
+	}
+}


Test patch:
diff --git a/reconciler/reconcile_common_test.go b/reconciler/reconcile_common_test.go
new file mode 100644
index 000000000..b61cc0399
--- /dev/null
+++ b/reconciler/reconcile_common_test.go
@@ -0,0 +1,96 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package reconciler
+
+import (
+	"context"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"knative.dev/pkg/apis"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
+)
+
+func makeResource(topLevelCond string) *duckv1.KResource {
+	fooCond := apis.Condition{
+		Type:    "Foo",
+		Status:  corev1.ConditionTrue,
+		Message: "Something something foo",
+	}
+	readyCond := apis.Condition{
+		Type:    apis.ConditionType(topLevelCond),
+		Status:  corev1.ConditionTrue,
+		Message: "Something something bar",
+	}
+
+	return &duckv1.KResource{
+		ObjectMeta: metav1.ObjectMeta{
+			Generation: 42,
+		},
+
+		Status: duckv1.Status{
+			ObservedGeneration: 0,
+			Conditions:         duckv1.Conditions{fooCond, readyCond},
+		},
+	}
+}
+
+func TestPreProcessResetsReady(t *testing.T) {
+	testCases := []struct {
+		name                      string
+		initTopLevelCond          string
+		expectedTopLevelCondition apis.ConditionType
+	}{{
+		name:                      "top level Ready",
+		initTopLevelCond:          "Ready",
+		expectedTopLevelCondition: apis.ConditionReady,
+	}, {
+		name:                      "top level Succeeded",
+		initTopLevelCond:          "Succeeded",
+		expectedTopLevelCondition: apis.ConditionSucceeded,
+	}}
+
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			resource := makeResource(test.initTopLevelCond)
+			krShape := duckv1.KRShaped(resource)
+
+			PreProcessReconcile(context.Background(), krShape)
+
+			if rc := resource.Status.GetCondition(test.expectedTopLevelCondition); rc.Status != "Unknown" {
+				t.Errorf("Expected unchanged ready status got=%s want=Unknown", rc.Status)
+			}
+		})
+	}
+}
+
+func TestPostProcessReconcileBumpsGeneration(t *testing.T) {
+	resource := makeResource("Ready")
+
+	krShape := duckv1.KRShaped(resource)
+	PostProcessReconcile(context.Background(), krShape)
+
+	if resource.Status.ObservedGeneration != resource.Generation {
+		t.Errorf("Expected observed generation bump got=%d want=%d", resource.Status.ObservedGeneration, resource.Generation)
+	}
+
+	if krShape.GetStatus().ObservedGeneration != krShape.GetObjectMeta().GetGeneration() {
+		t.Errorf("Expected observed generation bump got=%d want=%d", resource.Status.ObservedGeneration, resource.Generation)
+	}
+}