Standardize True/False/Unknown status throughout eventing

[capri-xiyue]

Problem
Standardize True/False/Unknown status throughout eventing.
Background: We have some open questions of the difference between status "Unknown" and status "False".Currently, in some cases, we follow this doc https://github.com/knative/docs/blob/master/docs/serving/spec/knative-api-specification-1.0.md to make a decision. (

1. False MUST indicate a failure condition.
2. Unknown SHOULD indicate that reconciliation is not yet complete and success or failure is not yet determined.
3. True SHOULD indicate that the application is fully reconciled and operating correctly.)

But in some cases, we don't follow it. We have not kept it consistent in the eventing repo. Even following the docs mentioned above, the boundary of "Unknown" and "False" is not so clear in some scenarios. We should follow up with this open question and modify the code to standardize True/False/Unknown status throughout eventing

Exit Criteria
The standard of True/False/Unknown status throughout eventing is consistent.

Time Estimate (optional):
How many developer-days do you think this may take to resolve?

Additional context (optional)
Add any other context about the feature request here.

[sixolet]

Using the knative standard will make the command line support much easier to build!

[capri-xiyue]

@sixolet
Hi Naomi, does the knative standard look as follows?

• True - the reconcile was successful. Clients should stop polling, and show a success result

• False - the reconcile was unsuccessful. Clients should stop polling, and show a failure result. This should be accompanied with a message, and ideally reason code, to help detail the failure and if it is system or user caused. Note that the reconcile could still complete and go True after this point if the situation changes, i.e. a down backend service comes back online

• Unknown - the reconcile is still in progress. Clients should continue polling, and show a busy spinner/incremental status. This should be accompanied with a message and ideally reason code, to show incremental status

[grantr]

The Trigger's readiness is complex because it depends on other objects outside its control. As currently defined, the Trigger requires readiness of both its subscriber and its dependency, but the Trigger controller doesn't know when those objects will exist (or more accurately, when they will appear in its cache).

If the Trigger gets a "not found" error when trying to read its dependency, that should be considered a terminal condition by the rules above, and the Trigger should be Ready=False. Yet in practice, if the client is creating both the source and trigger at the same time, the Trigger would flip to Ready=True once the source appears in cache a second or two later.

Is this acceptable to the command line client?

[sixolet]

The trigger's readiness should allow for a short unknown period before going false if the dependency doesn't exist, with a "waiting for dependency" message. After the grace period, it can go False with "timed out waiting for dependency". You can use the creation time to determine this. If you must have a condition that is false while the dependency doesn't exist, it should be info or warning.

…

On Nov 15, 2019, at 10:52 AM, Grant Rodgers ***@***.***> wrote:  The Trigger's readiness is complex because it depends on other objects outside its control. As currently defined, the Trigger requires readiness of both its subscriber and its dependency, but the Trigger controller doesn't know when those objects will exist (or more accurately, when they will appear in its cache). If the Trigger gets a "not found" error when trying to read its dependency, that should be considered a terminal condition by the rules above, and the Trigger should be Ready=False. Yet in practice, if the client is creating both the source and trigger at the same time, the Trigger would flip to Ready=True once the source appears in cache a second or two later. Is this acceptable to the command line client? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[capri-xiyue]

Currently, the status of each condition type of resources is set to unknown at initial stage(before reconcile process starts). And the reconcile process follows a "fail-fast" philosophy which means when we fail to reconcile one condition type, it will return error and the reconcile process will stop, for all the other condition types, they will still be unknown since they have not been reconciled due to the terminal error of the reconcile process of one condition type.

@sixolet in your proposal, you mentioned that "The trigger's readiness should allow for a short unknown period before going false if the dependency doesn't exist". I feel it's a little bit confusing that "Unknown" is used when we know the reconcile process is already complete and we encounter a terminal error.

In my understanding of knative standard,

• False MUST indicate a failure condition. (conditions should always go to False when they cannot complete reconciling, regardless of if it is a client-caused or system-caused failure. The reconcile could still complete and go True after this point if the situation changes, i.e. a down backend service comes back online)
• Unknown SHOULD indicate that reconciliation is not yet complete. It will be only used when condition type has not started reconcile process and is still at initial stage.
• True SHOULD indicate that the application is fully reconciled and operating correctly.

For example, for the trigger readiness,
If the Trigger gets a "not found" error when trying to read its dependency, that should be considered a terminal condition by the rules above, and the Trigger should be Ready=False. And in practice, if the client is creating both the source and trigger at the same time, the Trigger will change to Ready=True once the source appears in cache a second or two later. "Unknown" should not appear unless the condition type is still in the initial stage.

[sixolet]

The rules should be taken to include normative client behaviour in terms of interpreting what is an "error". Clients are supposed to be able to create a trigger and then immediately create a related source. That means that doing so it's not an error and should not be interpreted as such. That, in turn, means that a "not found" for the dependant source in the first ten seconds after the create time of a trigger should result in an unknown state and a reschedule of the controller to check again soon. Not a fatal error.

…

On Nov 20, 2019, at 10:23 AM, capri-xiyue ***@***.***> wrote:  Currently, the status of each condition type of resources is set to unknown at initial stage(before reconcile process starts). And the reconcile process follows a "fail-fast" philosophy which means when we fail to reconcile one condition type, it will return error and the reconcile process will stop, for all the other condition types, they will still be unknown since they have not been reconciled due to the terminal error of the reconcile process of one condition type. @sixolet in your proposal, you mentioned that "The trigger's readiness should allow for a short unknown period before going false if the dependency doesn't exist". I feel it's a little bit confusing that "Unknown" is used when we know the reconcile process is already complete and we encounter a terminal error. In my understanding of knative standard, False MUST indicate a failure condition. (conditions should always go to False when they cannot complete reconciling, regardless of if it is a client-caused or system-caused failure. The reconcile could still complete and go True after this point if the situation changes, i.e. a down backend service comes back online) Unknown SHOULD indicate that reconciliation is not yet complete. It will be only used when condition type has not started reconcile process and is still at initial stage. True SHOULD indicate that the application is fully reconciled and operating correctly. For example, for the trigger readiness, If the Trigger gets a "not found" error when trying to read its dependency, that should be considered a terminal condition by the rules above, and the Trigger should be Ready=False. And in practice, if the client is creating both the source and trigger at the same time, the Trigger will change to Ready=True once the source appears in cache a second or two later. "Unknown" should not appear unless the condition type is still in the initial stage. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[capri-xiyue]

I summarized the eventing core resources condition summary in this doc https://docs.google.com/document/d/1iwWowvCEh_5wiBp8TvgNRkrVoxPLiezoF-s7018EV-M/edit#heading=h.567syk6hive0
In this doc, we can see that the standard of True/False/Unknown status is not consistent through eventing.
For example,
For apiserversource, when the deployment(receiver adapter) is not available, the status of condition type "Deployed" is false. However, for container source, the status in the same scenario is "Unknown".

[capri-xiyue]

@sixolet
I feel we don't have the same understanding for the "fatal error" and how the reconcile process happen.
For example, when dependency(source) of trigger is not ready in the first ten seconds. For those reconcile process happened in the first ten seconds, it will terminate. However, it won't stop them from reconciling again.
In my understanding, everytime when we encounter error during reconcile process, it will immediately return error. But we will re enqueue the error and the controller framework we use will check again soon. So it will heal again and the status will jump to "True" once things become ready.

In addition, based on your rule, "The trigger's readiness should allow for a short unknown period before going false if the dependency doesn't exist, with a "waiting for dependency" message. After the grace period, it can go False with "timed out waiting for dependency". So the user will see spinner status for the short period(maybe about 10 secs), then the status will go to False and it will stop spinning. But when the source becomes ready maybe after 1 minutes, the status will jump to "True", right ? What do you expect user to do here when the status goes to "False" with "timed out waiting for dependency" message here? Should user keep polling or xxx ?

I don't know whether it's a good approach to bind user's expected behavior with the intermediate status of condition type since K8s system is built on eventual consistency. The system should keep reconciling until the status of resources reaches to the expected status that user wants(In this case, the expected status should be True)

[akashrv]

@dgerd @vaikas-personal @vaikas @cooperneil
This is an interesting discussion. Comments, suggestions?
I wonder what "short unknown period" means in a system like K8s where reconcilers watch and reconcile forever to make the observed stated eventually consistent with the desired state.

[hrasadi]

@capri-xiyue In your proposal, what will happen if the trigger is in False state and a second round of reconciliation is ongoing (before it hopefully goes to True). There are two possibilities that I can think of:
1- The trigger goes to 'Unknown' state everytime we start a reconcile phase. IMO, this will provide a bad customer experience (for an ever failing resource, it will show flip state between False and Unknown every XX seconds).
2- The whole Unknown concept is only used for the first time the trigger is being initialized. Well, in that case, why not calling it that state 'Initializing' instead of 'Unknown'?

[capri-xiyue]

@hrasadi
The whole Unknown concept is only used for the first time the trigger is being initialized or the trigger or the reconcile process already gets terminated before it reconcile a specific resource.
For example, the trigger has condition type as follows: "Broker", "SubscriberResolved" and "Dependency". When we fail to get broker, the condition type "Broker" will set to "False". And the reconcile process will get terminated. The condition type "SubscriberResolved" and "Dependency" will still be "Unknown" since they have not been reconciled(The reconcile process already got terminated before reconciling "SubscriberResolved" and "Dependency").

Regarding the naming, I don't have a preference between "Unknown" and "Initializing" as long as we have a consistent standard and a clear documentation.

[yolocs]

The rules should be taken to include normative client behaviour in terms of interpreting what is an "error". Clients are supposed to be able to create a trigger and then immediately create a related source. That means that doing so it's not an error and should not be interpreted as such. That, in turn, means that a "not found" for the dependant source in the first ten seconds after the create time of a trigger should result in an unknown state and a reschedule of the controller to check again soon. Not a fatal error.
…
On Nov 20, 2019, at 10:23 AM, capri-xiyue @.***> wrote:  Currently, the status of each condition type of resources is set to unknown at initial stage(before reconcile process starts). And the reconcile process follows a "fail-fast" philosophy which means when we fail to reconcile one condition type, it will return error and the reconcile process will stop, for all the other condition types, they will still be unknown since they have not been reconciled due to the terminal error of the reconcile process of one condition type. @sixolet in your proposal, you mentioned that "The trigger's readiness should allow for a short unknown period before going false if the dependency doesn't exist". I feel it's a little bit confusing that "Unknown" is used when we know the reconcile process is already complete and we encounter a terminal error. In my understanding of knative standard, False MUST indicate a failure condition. (conditions should always go to False when they cannot complete reconciling, regardless of if it is a client-caused or system-caused failure. The reconcile could still complete and go True after this point if the situation changes, i.e. a down backend service comes back online) Unknown SHOULD indicate that reconciliation is not yet complete. It will be only used when condition type has not started reconcile process and is still at initial stage. True SHOULD indicate that the application is fully reconciled and operating correctly. For example, for the trigger readiness, If the Trigger gets a "not found" error when trying to read its dependency, that should be considered a terminal condition by the rules above, and the Trigger should be Ready=False. And in practice, if the client is creating both the source and trigger at the same time, the Trigger will change to Ready=True once the source appears in cache a second or two later. "Unknown" should not appear unless the condition type is still in the initial stage. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

An error state (ready=false) doesn't mean it's not recoverable. A user (or client) has to look at the reason to understand if any intervention is required. It's less desirable (or even impossible) for the control plane to differentiate a "first-time" error vs. an error happened later.

I don't know what's the best client behavior - maybe a client-side timeout.

[sixolet]

To differentiate first time from later: look at the create time. The entire purpose of having rules for conditions is so clients can use them. Let's make them actually useful to clients!

…

On Nov 20, 2019, at 1:46 PM, cshou ***@***.***> wrote:  The rules should be taken to include normative client behaviour in terms of interpreting what is an "error". Clients are supposed to be able to create a trigger and then immediately create a related source. That means that doing so it's not an error and should not be interpreted as such. That, in turn, means that a "not found" for the dependant source in the first ten seconds after the create time of a trigger should result in an unknown state and a reschedule of the controller to check again soon. Not a fatal error. … On Nov 20, 2019, at 10:23 AM, capri-xiyue @.***> wrote:  Currently, the status of each condition type of resources is set to unknown at initial stage(before reconcile process starts). And the reconcile process follows a "fail-fast" philosophy which means when we fail to reconcile one condition type, it will return error and the reconcile process will stop, for all the other condition types, they will still be unknown since they have not been reconciled due to the terminal error of the reconcile process of one condition type. @sixolet in your proposal, you mentioned that "The trigger's readiness should allow for a short unknown period before going false if the dependency doesn't exist". I feel it's a little bit confusing that "Unknown" is used when we know the reconcile process is already complete and we encounter a terminal error. In my understanding of knative standard, False MUST indicate a failure condition. (conditions should always go to False when they cannot complete reconciling, regardless of if it is a client-caused or system-caused failure. The reconcile could still complete and go True after this point if the situation changes, i.e. a down backend service comes back online) Unknown SHOULD indicate that reconciliation is not yet complete. It will be only used when condition type has not started reconcile process and is still at initial stage. True SHOULD indicate that the application is fully reconciled and operating correctly. For example, for the trigger readiness, If the Trigger gets a "not found" error when trying to read its dependency, that should be considered a terminal condition by the rules above, and the Trigger should be Ready=False. And in practice, if the client is creating both the source and trigger at the same time, the Trigger will change to Ready=True once the source appears in cache a second or two later. "Unknown" should not appear unless the condition type is still in the initial stage. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. An error state (ready=false) doesn't mean it's not recoverable. A user (or client) has to look at the reason to understand if any intervention is required. It's less desirable (or even impossible) for the control plane to differentiate a "first-time" error vs. an error happened later. I don't know what's the best client behavior - maybe a client-side timeout. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[yolocs]

To differentiate first time from later: look at the create time. The entire purpose of having rules for conditions is so clients can use them. Let's make them actually useful to clients!

I doubt that you can tell if it's a first time error by looking at just the create time. From your first comment, it seems you want to use the create time for timeout, which itself could be debatable (see questions from @capri-xiyue).
In my understanding, conditions should reflect the actual states of resources. The timeout strategy seems like an optimization for human interaction in the cost of state accuracy.