Eventing: 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 [e-mail] 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 [e-mail] 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 [e-mail] 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.

[sixolet]

"unknown" in the ready condition is meant to reflect that a normal process is still going on that may result in success or failure.

That is true during FaaS-style trigger creation by a client.

That is both state accuracy and better for human interaction.

After all, humans are the ultimate arbiters of meaning.

On Nov 20, 2019, at 3:08 PM, cshou [e-mail] wrote:


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.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.

[yolocs]

"unknown" in the ready condition is meant to reflect that a normal process is still going on that may result in success or failure. That is true during FaaS-style trigger creation by a client. That is both state accuracy and better for human interaction. After all, humans are the ultimate arbiters of meaning.

But who knows there is an on going process? E.g. a trigger is waiting for a subscriber to appear. Controller doesn’t know and shouldn’t care if the subscriber will eventually appear. But the client (the user) who created the trigger should have the context - they know whether the subscriber will appear soon.

I’m advocating that when the controller finds out the subscriber doesn’t exist, it should honestly mark Ready=False with reason “subscriber not found” rather than pretending Ready=Unknown and hoping the subscriber will appear soon.

[xorver]

I'm integrating Trigger statuses with UI, We have some prior work on handling statuses for other Knative resources, and basically if the status is Unknown we keep pooling, when the status becomes True/False, we stop pooling. I'm using the same logic for triggers.

I create the trigger first, it has dependency on yet nonexistent PubSub, then I create the PubSub owned by the the trigger. At this point both entities were created from my point of view. Then I fetch the trigger to check the status. In such case the trigger Ready condition is set to False with error

Dependency is not ready: PullSubscription has no Ready type status.

This is a PubSub dependency that I'm not aware of as user, so the objects that I created are clearly reconciling yet. IMO Missing PullSubscription status, should indicate unknown status of PubSub, and Unknown status of Trigger.

Have you considered treating missing dependency status as non-terminal failure and returning "Unknown" with the same message (I've seen you do already similar thing when the sink is missing). The alternative for me is checking create/update time and switching the returned status from False to Unknown in case it's been created recently, which is not ideal.

[dgerd]

I have been spending some time looking at this for Serving. See https://github.com/knative/serving/issues/5076#issuecomment-560711908.

We had an issue that seems fairly similar to the Trigger problem in our Route (https://github.com/knative/serving/issues/4173). The issue was that if you create a new revision and reference that newly created Revision at the same time you likely go through this Unknown -> False -> Unknown -> True flow. This is because Routes don't own Revisions so it doesn't know if the thing you referenced is going to ever be created or is just not quite ready yet.

The only difference with our problem is that our client was also a controller (the KService controller). To fix the issue we kept the same NotCreated == False and NotYetReady == Unknown logic in our Route, but added serialization in our controller to wait until the Revision was ready before updating the Route to reconcile.

I understand that Trigger is not owned by a controller, but it seems like any client could adopt a similar pattern to validate both Broker and Subscription existence before starting Trigger creation.

[xorver]

I think the main problems that I'm hitting right now is that PubSub depends on PullSubscription (which is created internally) and it's following (PullSubscription) Unknown -> (Pubsub) False logic, whereas more reasonable would be (PullSubscription) Unknown -> (Pubsub) Unknown.

This can be seen when creating PubSub and Trigger for nonexistent topic, the resources for around a minute are in state (PullSubscription) Unknown -> (PubSub) False: PullSubscriptionNotReady -> (Trigger) False: DependencyNotReady (http://gpaste/5326467166633984), then PubSub fails with missing topic message and failures are propagated (in case of successful reconciliation it's similar in flow but happens quicker).

[capri-xiyue]

@dgerd Just wanna clarify your points, you mentioned that NotCreated == False and NotYetReady == Unknown logic.
For example, in the Trigger and Source scenario:

CRD | Scenario | ConditionType | ConditionStatus | Trigger Status
-- | -- | -- | -- | --
trigger | Source does not exist | "Dependency" | False | False
trigger | Failed to get Source | "Dependency" | False | False
trigger | Source is not ready, the status of Source is Unknown | "Dependency" | Unknown | False
trigger | Source is not ready, the status of Source is False | "Dependency" | False | False

Is the above sheet the same as what serving does?

I agree with the point that client could adopt a similar pattern to validate both Broker and Subscription existence before starting Trigger creation.
@xorver Why do you think (PullSubscription) Unknown -> (Pubsub) Unknown is more reasonable? Does it involve some user experience or xxx? Could you help explain it? Thank you !

[dgerd]

@capri-xiyue It looks more like:

CRD | Scenario | ConditionType | ConditionStatus | Trigger Status
-- | -- | -- | -- | --
trigger | Source does not exist | "Dependency" | False | False
trigger | Source is not ready, the observedGeneration (status) != generation (spec) | "Dependency" | Unknown | Unknown
trigger | Source is not ready, the status of Source is False | "Dependency" | False | False
trigger | Source is not ready, the status of Source is Unknown | "Dependency" | Unknown | Unknown
trigger | Source is ready, the status of Source is true | "Dependency" | True | True (if other conditions true)

We currently don't touch status for failing to get, but this is something that I would like for us to change in serving to what you outlined:

CRD | Scenario | ConditionType | ConditionStatus | Trigger Status
-- | -- | -- | -- | --
trigger | Failed to get Source | "Dependency" | False | False

[vaikas]

These are all what I'd expect as well to be like so, with the only question being why on failure to get source it's marked as False instead of Unknown. But, I do think it makes sense since if we're unable to get the Source (Not Exist, or auth failure, or ...) we're unable to make forward progress since we need to get it, so I'm totes fine with calling it False.

[xorver]

@xorver Why do you think (PullSubscription) Unknown -> (Pubsub) Unknown is more reasonable?

@capri-xiyue I think it'll more closely follow the rules described in https://github.com/knative/docs/blob/master/docs/serving/spec/knative-api-specification-1.0.md#error-signalling

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

If we have PubSub with the only dependency in PullSubscription, and PullSubscription status tells reconciliation is not yet complete and success or failure is not yet determined, then telling the same about status of PubSub makes most sense. PubSub's status is directly dependent on PullSubscription so we don't yet know if it's gonna be True or False. Returning Unknown encourages users to keep pooling for status, assuming that we'll have better answer soon. Returning False tells them that it's failed.

Note that returning Unknown "should" indicate reconciliation, so it may be permanently broken, but returning False "must" indicate failure condition, and in this case we just need more time and status is not yet determined.

[lionelvillard]

In general, I would expect the dependent to report the status of the dependency, as is. If the dependency do not exist, then I would expect the condition to be false. Going back to the trigger example, @dgerd's table is what I would expect.

[dgerd]

Following up on @vaikas comment, I do think that the "failed to get" is probably the most interesting example and I think I am becoming more convinced on Unknown/Unknown instead of False/False for this case. It is a failure to be able to make progress on the dependency, but the Source actually could be Ready. We don't know as there is the problem with our informer or api server.

[capri-xiyue]

@dgerd Regarding the "failed to get" case, if there is something wrong with the informer or api server and the source actually could be Ready, and we set the status to "Unknown", what do we expect users or upper layer to do in this case? Keep pooling and wait it to become True or False which will probably not happen if there is no human intervention regarding informer or api server? Should we avoid using "Unknown" in cases that no progress will be made without human intervention? IMO, we can only use "Unknown" when progress will happen without human intervention or for initial status. Does it make sense?

[n3wscott]

@capri-xiyue I think @dgerd is saying that there is a difference between "auth errors to the api server" and "the resource does not exist".

I think auth errors end up as false, not exist is unknown.

As a general comment, the "keep polling" thing is a bit funny because you should always be polling. The state can change for any number of reasons at any time.

[grantr]

Rereading this thread, I think @sixolet's suggestion of a grace period in https://github.com/knative/eventing/issues/2187#issuecomment-554494137 is a good compromise. The grace period doesn't violate the rules above, because the trigger does eventually progress to False if the dependency fails to be detected within the grace period.

I'm not sure how to implement this correctly. I think it would require adding a temporary condition to the trigger signaling to the future reconciler that we have started the grace period, then requeueing the trigger with a delay regardless of success or failure of the reconcile. I don't know if pkg/controller supports requeue with delay.

I haven't thought about this too carefully, but what about this logic in the reconciler code path that's hit when the dependency doesn't exist:

if dependency does not exist:
  if condition WaitForDependency is not present:
    add it
    set DependencyReady to Unknown
  else if condition WaitForDependency is present:
    check transition time of WaitForDependency condition
    if transition time is older than the grace period:
      remove WaitForDependency condition
      set DependencyReady to False

Then, at the end of the reconcile, when the reconcile would otherwise be successful:

if condition WaitForDependency is present:
  set RequeueAfter to a polling interval

[grantr]

Oops, hit send too soon. Meant to ask this to clarify where people's current thoughts are:

Do we agree that the only remaining issue is how to avoid this sequence of Ready states: Unknown -> False (<10 seconds) -> Unknown/True?

Do we agree that the case "expected resource does not exist and I have permission to check" is the only one that will cause that progression?

[dgerd]

if there is something wrong with the informer or api server and the source actually could be Ready, and we set the status to "Unknown", what do we expect users or upper layer to do in this case?

I would expect them to wait and then eventually timeout. The API server or informer problem may actually be transient (i.e. 500 type errors, throttling, etc)

Should we avoid using "Unknown" in cases that no progress will be made without human intervention?

The problem with this definition is that "no progress without human intervention" is extremely difficult to define. Take out of capacity errors for example. If my cluster is full I will never progress until something disappears or my cluster gets bigger. That may happen automatically (i.e. scale to zero or cluster scaling event), or that may take human action to fix.

that there is a difference between "auth errors to the api server" and "the resource does not exist".

Yes I am saying that, but with the opposite outcome. If get get back a "dependent resource does not exist" from the API server we trust it and we say "DependentReady=False". Where as if we try to get the dependent resources and we get back a throttle/auth/server/random error we don't know anything about the dependent resource. It could be Ready=True/Unknown/False or not even exist. In this case our knowledge about the resource is unknown so "DependentReady=Unknown".

As a general comment, the "keep polling" thing is a bit funny because you should always be polling. The state can change for any number of reasons at any time.

While true I get the desire to have a command-line or SDK wait for a settled state before continuing. We do it in our integration tests for example. We create a Service and then wait until it is ready to serve traffic and then run through our workflow. It could go unready during the test, but we don't care to keep polling. We care that it was Ready before we started throwing traffic at it.

[vaikas]

Yes I am saying that, but with the opposite outcome. If get get back a "dependent resource does not exist" from the API server we trust it and we say "DependentReady=False". Where as if we try to get the dependent resources and we get back a throttle/auth/server/random error we don't know anything about the dependent resource. It could be Ready=True/Unknown/False or not even exist. In this case our knowledge about the resource is unknown so "DependentReady=Unknown".

Yeah, I wasn't quite sure if we wanted to tack on the additional complexity at that layer, I mentioned different failure cases, but wanted to point out that there's more knowledge in some cases than others. If we know something doesn't exist, that does seem like False is more appropriate and on errors where we fail to actually get the resource for other reasons, Unknown seems more appropriate. Just curious if adding that complexity is necessary or helpful.

[dgerd]

@grantr We do have something like this grace period in Serving for Pod/Deployment status. While I agree that it is possible and an option I did want to understand more why in this case a client (or controller) couldn't/wouldn't just serialize these operations if flipping to "False" on failure to find dependency isn't what is desired:

CreateBroker()
CreateSource()
WaitForBrokerAndSource()
CreateTrigger(broker, source)

[grantr]

@dgerd I'm not sure what Subscription you're referring to. Is this the source that the trigger's creator (the client) has declared a dependency on?

I imagine the desire from the client perspective is to avoid encoding the relationship graph and the set of possible states into the client.

[dgerd]

I'm not sure what Subscription you're referring to. Is this the source that the trigger's creator (the client) has declared a dependency on?

Whoops. I mean't Source. Edited the comment.

[xorver]

There are two things here

1. Returning Unknown when dependency exists and returns Unknown (as shown in the table). Does anyone disagree and prefer to keep returning False?
2. Returning Unknown when dependency does not exist. I don't have strong opinion on it. From my point of view as user the best would be (the higher the better):
3. Return Unknown if we're within some grace period. Then False.
4. Return Unknown all the time until the dependency is there.
5. Return False.

@dgerd As for WaitForBrokerAndSource(). I create PubSub which is my source, but some reconciler creates also PullSubscribtion for it, which is the source for PubSub. So When I create PubSub and fetch its status, it's False. In order to wait for it I need to assume that for some time I'm treating all False statuses as Unknown. I need to know what time is the right one, and I need to accept that I'll treat some terminal errors as Unknown, and present a "spinner" for them to the user (it's ok if the time is reasonably small. However if we don't change (1), some errors take 1min to reconcile).

[lionelvillard]

For me:

• when the dependency exists, then the dependant status should be the dependency Ready status. Simple.
• when the dependency readiness cannot be established (the dependant couldn't get the dependency for various reason), the status should be Unknown
• when the dependency does not exist, the status should be False.

I don't like the grace period because that's yet something else to explain to the end user. We need to come up with simple rules since this is user-facing.

The CLI can decide to implement a grace period because CLI implementors know what can change quickly.

[capri-xiyue]

@xorver
I think the dependency listed in https://github.com/knative/eventing/issues/2187#issuecomment-561325883 is different from the dependency you mentioned in PubSub and PullSubscription.
I think we have two kinds of dependencies

For the dependency listed in https://github.com/knative/eventing/issues/2187#issuecomment-561325883, the Source(dependency) is not created or owned by Trigger. But in your case PubSub and PullSubscription, PullSubscription is owned and created by PubSub. During the reconciling process of PubSub, if PubSub find the PullSubscription it needs doesn't exist, it will try to create a new one. So for PubSub, I don't think we will need to handle the case that whether the status of PubSub should be set to False or Unknown when PullSubscription does not exist. PubSub will always try to create PullSubscription when it finds PullSubscription does not exist.

Regarding the case the Source(dependency) is not created or owned by Trigger and Source doesn't exist, I think the status of Trigger should be set to False. As what @dgerd mentioned, CLI could adopt a similar pattern to validate Source existence before starting Trigger creation which will create a better user experience.

[capri-xiyue]

For me, I agree with what @dgerd mentioned. Just update the table for the case "Failed to get source"

CRD | Scenario | ConditionType | ConditionStatus | Trigger Status
-- | -- | -- | -- | --
trigger | Source does not exist | "Dependency" | False | False
trigger | Source is not ready, the observedGeneration (status) != generation (spec) | "Dependency" | Unknown | Unknown
trigger | Source is not ready, the status of Source is False | "Dependency" | False | False
trigger | Source is not ready, the status of Source is Unknown | "Dependency" | Unknown | Unknown
trigger | Failed to get Source | "Dependency" | Unknown | Unknown
trigger | Source is ready, the status of Source is true | "Dependency" | True | True (if other conditions true)

The CLI can decide to implement a grace period because CLI implementors know what can change quickly or adopt a pattern to validate Source existence before starting Trigger creation. Both of them will create a better user experience.

[aaron-lerner]

What does the process look like for a trigger transitioning from False for non-existent source to True for source is True? Does it go to Unknown after the source is created but before the source is True? Is it guaranteed to go to Unknown in between or if a source becomes True very quickly after creation, can the trigger jump from False to True?

[aaron-lerner]

Also, one thing I don't see mentioned is that in practice we've been adding an ownerReference on that the source pointing to the trigger. Correct me if I'm wrong, but the trigger being referenced needs to exist before adding the ownerRef can be added. This means we can't realistically create the source before the trigger.

[capri-xiyue]

What does the process look like for a trigger transitioning from False for non-existent source to True for source is True? Does it go to Unknown after the source is created but before the source is True? Is it guaranteed to go to Unknown in between or if a source becomes True very quickly after creation, can the trigger jump from False to True?

I think it can be False -> True if a source becomes True very quickly after creation.
It can also be False -> Unknown -> True if a source becomes Unknown after creation and is still in reconciliation process.
I don't think we can guarantee it goes to Unknown in between.

[capri-xiyue]

If it is not possible to create the source before trigger, does it work for you guys that CLI decides to implement a grace period because CLI implementors know what can change quickly? @aaron-lerner @xorver ?

[aaron-lerner]

Yes, I think a grace period on the client side should be ok.

[xorver]

Yes, if the PubSub -> PullSubscription does not produce this problem, then for PubSub -> Trigger I can solve it either by waiting for PubSub or applying grace period.