Consistent status reasons

[howardjohn]

GatewayConditionReady is extended. It has a set of reasons:
// * "ListenersNotValid"
// * "ListenersNotReady"
// * "AddressNotAssigned"

ListenerConditionProgrammed is core, but it only has a few reasons:
// * "Invalid"
// * "Pending"

Given these represent almost the same thing, it seems like these should be the same set, or GatewayConditionReady should be a superset

similar concern on Listener status

[robscott]

Good catch, agree that these should all be present for both conditions.

/help

[howardjohn]

Also I think the spec is pretty clear but while I am here to double check... if I cannot say "traffic is ready to flow immediately" I should just not set Ready at all, right? And only use Programmed?

[arkodg]

Also I think the spec is pretty clear but while I am here to double check... if I cannot say "traffic is ready to flow immediately" I should just not set Ready at all, right? And only use Programmed?

+1 to this, helps users understand the difference between CP programmed and DP ready
cc @dprotaso

[howardjohn]

So my interpretation means:

• Accepted: I got far enough to start sending some data to the data plane
• Programmed: I got far enough to start send all the data to the data plane
• Ready (optional): I sent all the data to the data plane and it fully processed it and is ready to go

[robscott]

Also I think the spec is pretty clear but while I am here to double check... if I cannot say "traffic is ready to flow immediately" I should just not set Ready at all, right? And only use Programmed?

Yep, the requirements for setting ready have become pretty strict:

gateway-api/apis/v1beta1/gateway_types.go

Lines 609 to 611 in 1c4ae61

	// Ready is an optional Condition that has Extended support. When it's set,
	// the condition indicates whether the Gateway has been completely configured
	// and traffic is ready to flow through the data plane immediately.

If there were some kind of delay after which you could guarantee readiness that may also be a good reason to set "Ready", but not sure if that's a good idea.

[robscott]

@gauravkghildiyal has volunteered to work on this one

[robscott]

/remove-help

[arkodg]

would be good to add conformance tests to make sure if Route reports status as Ready, all requests through it must succeed

[robscott]

@arkodg completely agree, mind creating an issue to track that?

[howardjohn]

One other issue I noticed is I would assume Ready > Programmed > Accepted. However, consider this status:"

  - lastTransitionTime: fake
    message: 'failed to assign
      to all requested addresses: hostname "istio-ingressgateway.not-default.svc.domain.suffix"
      not found'
    reason: NoResources
    status: "False"
    type: Accepted
  - lastTransitionTime: fake
    message: Resource programmed
    reason: Programmed
    status: "True"
    type: Programmed

I want to make Programmed=False because its not Accepted, but the only valid reasons for Programmed=False is Invalid or Pending, and Pending says "controller has not reconciled yet"; neither of these seem quite true

[gauravkghildiyal]

@howardjohn The issue says that "ListenerConditionProgrammed" should be subset of "GatewayConditionReady".

Is that perhaps a typo and you meant to say GatewayConditionProgrammed" should be subset of "GatewayConditionReady"?

(I'm new to this and hence cannot confidently differentiate a typo from an actual ask)

[gauravkghildiyal]

/assign

[youngnick]

@howardjohn I agree that Programmed should have a NotAccepted Reason, which says "Can't be Programmed because it wasn't Accepted".

[howardjohn]

The issue says that "ListenerConditionProgrammed" should be subset of "GatewayConditionReady".

That was a typo, good catch

[dprotaso]

Would we consider GatewayConditionReady a top-level terminal condition?

Meaning

• it requires all the child conditions (Accepted, Programmed) to be True before it can be True
• If any of the child conditions are False then it should be False?

Also it would be good for implementations to hint that they don't support GatewayConditionReady by setting the Status to Unknown and some hard coded Reason - ie. NotSupported

[howardjohn]

+1 to Unknown and NotSupported

[youngnick]

+1 to Unknown and NotSupported being available.

But I don't think they should be required. The absence of a Condition means that it should be read as Unknown anyway, so it shouldn't be necessary for implementations to add it if they don't want to support it.

[gauravkghildiyal]

Alright, collating all the information here, does the following look agreeable? (Adding all related condition/reasons here for a holistic view)

(Also, lets take this step by step and tackle the issue #1877 regarding "removing GatewayConditionReady completely" as a separate item)

Resource	Condition name	Value	Reason	Comment
Gateway condition	Accepted	TRUE	Accepted
		FALSE	NotReconciled
		FALSE	NoResources
		FALSE	ListenersNotValid	Moved from Ready. This should be set when one of the Listnerers has Accepted=False
		UNKNOWN	Pending
	Programmed	TRUE	Programmed
		FALSE	Invalid
		FALSE	NotAccepted	Newly Added. Question: Is there another reason to set (Programmed, FALSE, Invalid) besides Accepted=False? If not, then this might be redundant
		FALSE	Pending
		UNKNOWN	Pending
	Ready (Extended)	TRUE	Ready
		FALSE	ListenersNotValid	Deleted/Deprecated Moving this to Accepted since it represents that some listener is "syntactically or semantically" invaid.
		FALSE	ListenersNotReady
		FALSE	AddressNotAssigned
		UNKNOWN	ConditionNotSupported	Newly added. Ready condition already has Extended support, meaning that it is not necessary for implementations to explicitly set Ready to Unknown with this reason
Listener Condition	Accepted	TRUE	Accepted
		FALSE	PortUnavailable
		FALSE	UnsupportedProtocol
		FALSE	UnsupportedAddress
		UNKNOWN	Pending
	Programmed	TRUE	Programmed
		FALSE	Invalid
		FALSE	NotAccepted	Newly Added. Question: Is there another reason to set (Programmed, FALSE, Invalid) besides Accepted=False? If not, then this might be redundant
		FALSE	Pending
		UNKNOWN	Pending
	Ready (Extended)	TRUE	Ready
		FALSE	Invalid
		FALSE	Pending
		UNKNOWN	Pending

[youngnick]

Wow @gauravkghildiyal, thanks for that table, that's excellent.

Here's my answers to your questions, but I'm interested to hear if I'm the outlier here:

• For Gateway ListenersNotValid, if we're having one Listener being not accepted mean the entire Gateway is not accepted, that means that we can't do partial Gateway validity. I'd prefer ListenersNotValid to be a reason that's allowable with either Accepted as true or false - in either case, that Reason means that one or more listeners are invalid, but if Accepted is true, that means at least one Listener is valid. Accepted being false means that there are no valid Listeners. This is analogous to how Route partial validity works as well.
• I agree that NotAccepted seems like a better description for the same state as Invalid at first glance, but I think we should leave Invalid in there until we're sure there are no other use cases for it. We should enforce NotAccepted for both Listener and Gateway Programmed Conditions though, I think that it should probably always be added.

[howardjohn]

Here is how I view things. Changes are bolded.

GatewayAccepted: "The Gateway is accepted by the controller"
Reasons:

• Accepted: All Good
• ListenersNotValid: one or more listeners are invalid. Accepted can still be True if at least some configuration was programmed
• Pending: Still waiting to be reconciled.
• UnsupportedAddress (moved from listener): Address is not supported)

Moved: AddressNotAssigned (to programmed)

GatewayProgrammed: "The Gateway is ready to go, (but maybe not immediately; eventual consistency allowed). This only applies to the entire GW; a new listener would not make this flip to False for example"
Reasons:

• Programmed: All good
• Invalid: Gateway was no accepted, so we cannot be programmed of course
• AddressNotAssigned (moved from Accepted): Could not allocate addresses
• Pending: Still waiting to be reconciled.
• NoResources (moved from Ready): Could not allocate non-address resources

GatewayReady: reserved for future use

---

ListenerAccepted: "The listener is accepted and valid"
Reasons:

• Accepted: all good
• PortUnavailable
• UnsupportedProtocol
  Moved: UnsupportedAddress (listener does not have an Address... this is handled by AddressNotAssigned in Gateway)

ListenerResolvedRefs: "all references resolved
No changes

ListenerProgrammed: "The Listener is ready to go, (but maybe not immediately; eventual consistency allowed)"
Reasons:

• Programmed: All good
• Invalid: set when Accepted=False
• Pending: waiting

ListenerReady: reserved

---