[dev-gwapi-0.6] replace Gateway/Listener Ready conditions with Programmed by skriss · Pull Request #4849 · projectcontour/contour

skriss · 2022-11-15T18:40:04Z

Replaces the "Ready" condition used for Gateways
and Listeners with "Programmed".

Updates #4848.

Signed-off-by: Steve Kriss krisss@vmware.com

Replaces the "Ready" condition used for Gateways and Listeners with "Programmed". Updates projectcontour#4848. Signed-off-by: Steve Kriss <krisss@vmware.com>

skriss

This was mostly straightforward, but there are a couple questionable combos of Condition type + reason that need to be resolved.

skriss · 2022-11-15T18:44:11Z

internal/dag/gatewayapi_processor.go

 	if !isAddressAssigned(p.source.gateway.Spec.Addresses, p.source.gateway.Status.Addresses) {
-		gatewayNotReadyCondition = &metav1.Condition{
-			Type:    string(gatewayapi_v1beta1.GatewayConditionReady),
+		// TODO(sk) resolve condition type-reason mismatch


The "AddressNotAssigned" reason did not move over to the "Programmed" condition, it's still documented as being for the "Ready" condition (ref. https://github.com/kubernetes-sigs/gateway-api/blob/eb822e981bd8f7b4f52e89cafbe2d0397b5f01fb/apis/v1beta1/gateway_types.go#L643-L648), so need to figure out what to do here. It would be a little odd to set "Ready: false" for this scenario, but never set "Ready: true" (since we're not currently able to meet the requirements for the "Ready: true" condition).

I'm feeling like this is probably fine as-is for now since it's not breaking any conformance tests, but possibly something to follow up on down the road.

hm yeah this one is a little weird, I could go either way on whether it belongs with the Programmed or Ready condition, but seems find to leave as you have it here until an upstream issue/conformance test changes things

👍 yeah agree this one could potentially go either way. I'll raise an issue upstream for it.

skriss · 2022-11-15T18:45:32Z

internal/dag/gatewayapi_processor.go

-			gwAccessor.AddCondition(gatewayapi_v1beta1.GatewayConditionReady, metav1.ConditionFalse, gatewayapi_v1beta1.GatewayReasonListenersNotValid, "Listeners are not valid")
+		if !allListenersProgrammed {
+			// If we have invalid listeners, set Programmed=false.
+			// TODO(sk) resolve condition type-reason mismatch


Similarly here, the "ListenersNotValid" reason did not move over to the "Programmed" condition (ref. https://github.com/kubernetes-sigs/gateway-api/blob/eb822e981bd8f7b4f52e89cafbe2d0397b5f01fb/apis/v1beta1/gateway_types.go#L634-L637), so need to decide what to do here.

I'm feeling like this is probably fine as-is for now since it's not breaking any conformance tests, but possibly something to follow up on down the road.

yeah the wording on that Reason makes it sounds like it should be used with the Programmed Condition

one or more Listeners have an invalid or unsupported configuration and cannot be configured on the Gateway.

good for us to keep now and open an upstream issue

Agreed. Will raise an upstream issue.

codecov · 2022-11-15T18:54:22Z

Codecov Report

Merging #4849 (d830bcc) into dev-gwapi-0.6 (ec6bb08) will increase coverage by 0.04%.
The diff coverage is 97.67%.

Additional details and impacted files

@@                Coverage Diff                @@
##           dev-gwapi-0.6    #4849      +/-   ##
=================================================
+ Coverage          76.30%   76.34%   +0.04%     
=================================================
  Files                139      139              
  Lines              16688    16680       -8     
=================================================
+ Hits               12734    12735       +1     
+ Misses              3703     3694       -9     
  Partials             251      251

Impacted Files	Coverage Δ
internal/status/gatewaystatus.go	`72.93% <ø> (+5.10%)`	⬆️
internal/dag/gatewayapi_processor.go	`96.61% <97.61%> (+<0.01%)`	⬆️
internal/gatewayapi/listeners.go	`100.00% <100.00%> (ø)`
internal/sorter/sorter.go	`98.46% <0.00%> (-0.52%)`	⬇️

…tour#4849) Replaces the "Ready" condition used for Gateways and Listeners with "Programmed". Updates projectcontour#4848. Signed-off-by: Steve Kriss <krisss@vmware.com>

* drop support for ReferencePolicy (#4830) Drops support for ReferencePolicy which has been replaced by ReferenceGrant in Gateway API. * update to latest Gateway API commit (#4839) - Update to v1beta1 ReferenceGrant - HTTPRequestHeaderFilter -> HTTPHeaderFilter - GatewayConditionScheduled -> GatewayConditionAccepted - ListenerConditionDetached -> ListenerConditionAccepted * updates for SupportedFeatures changes (#4844) * replace Gateway/Listener Ready conditions with Programmed (#4849) Replaces the "Ready" condition used for Gateways and Listeners with "Programmed". * update to Gateway API v0.6.0-rc1 (#4891) * Use generics for Gateway API pointer helpers (#4905) Can get rid of our existing gatewayapi package helpers for getting pointers to gateway api types based on primitive types Also found some more instances of v1alpha2 package being used where we should switch to v1beta1 Fixes #4555 Fixes #4699 Fixes #4848 Signed-off-by: Steve Kriss <krisss@vmware.com> Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: Steve Kriss <krisss@vmware.com>

* drop support for ReferencePolicy (projectcontour#4830) Drops support for ReferencePolicy which has been replaced by ReferenceGrant in Gateway API. * update to latest Gateway API commit (projectcontour#4839) - Update to v1beta1 ReferenceGrant - HTTPRequestHeaderFilter -> HTTPHeaderFilter - GatewayConditionScheduled -> GatewayConditionAccepted - ListenerConditionDetached -> ListenerConditionAccepted * updates for SupportedFeatures changes (projectcontour#4844) * replace Gateway/Listener Ready conditions with Programmed (projectcontour#4849) Replaces the "Ready" condition used for Gateways and Listeners with "Programmed". * update to Gateway API v0.6.0-rc1 (projectcontour#4891) * Use generics for Gateway API pointer helpers (projectcontour#4905) Can get rid of our existing gatewayapi package helpers for getting pointers to gateway api types based on primitive types Also found some more instances of v1alpha2 package being used where we should switch to v1beta1 Fixes projectcontour#4555 Fixes projectcontour#4699 Fixes projectcontour#4848 Signed-off-by: Steve Kriss <krisss@vmware.com> Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: Steve Kriss <krisss@vmware.com> Signed-off-by: yy <yang.yang@daocloud.io>

* drop support for ReferencePolicy (projectcontour#4830) Drops support for ReferencePolicy which has been replaced by ReferenceGrant in Gateway API. * update to latest Gateway API commit (projectcontour#4839) - Update to v1beta1 ReferenceGrant - HTTPRequestHeaderFilter -> HTTPHeaderFilter - GatewayConditionScheduled -> GatewayConditionAccepted - ListenerConditionDetached -> ListenerConditionAccepted * updates for SupportedFeatures changes (projectcontour#4844) * replace Gateway/Listener Ready conditions with Programmed (projectcontour#4849) Replaces the "Ready" condition used for Gateways and Listeners with "Programmed". * update to Gateway API v0.6.0-rc1 (projectcontour#4891) * Use generics for Gateway API pointer helpers (projectcontour#4905) Can get rid of our existing gatewayapi package helpers for getting pointers to gateway api types based on primitive types Also found some more instances of v1alpha2 package being used where we should switch to v1beta1 Fixes projectcontour#4555 Fixes projectcontour#4699 Fixes projectcontour#4848 Signed-off-by: Steve Kriss <krisss@vmware.com> Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: Steve Kriss <krisss@vmware.com>

replace Gateway/Listener Ready conditions with Programmed

d830bcc

Replaces the "Ready" condition used for Gateways and Listeners with "Programmed". Updates projectcontour#4848. Signed-off-by: Steve Kriss <krisss@vmware.com>

skriss commented Nov 15, 2022

View reviewed changes

skriss marked this pull request as ready for review November 17, 2022 15:22

skriss requested a review from a team as a code owner November 17, 2022 15:22

skriss requested review from stevesloka and sunjayBhatia and removed request for a team November 17, 2022 15:22

sunjayBhatia approved these changes Nov 17, 2022

View reviewed changes

skriss merged commit 3097f7f into projectcontour:dev-gwapi-0.6 Nov 18, 2022

arkodg mentioned this pull request Dec 6, 2022

feat: upgrade GWAPI to v0.6.0-rc1 envoyproxy/gateway#778

Merged

sunjayBhatia mentioned this pull request Dec 14, 2022

Bump Gateway API to 0.6.0-rc.1 #4929

Merged

skriss deleted the pr-gwapi-programmed branch January 31, 2023 19:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dev-gwapi-0.6] replace Gateway/Listener Ready conditions with Programmed#4849

[dev-gwapi-0.6] replace Gateway/Listener Ready conditions with Programmed#4849
skriss merged 1 commit intoprojectcontour:dev-gwapi-0.6from
skriss:pr-gwapi-programmed

skriss commented Nov 15, 2022

Uh oh!

skriss left a comment

Uh oh!

skriss Nov 15, 2022

Uh oh!

skriss Nov 17, 2022

Uh oh!

sunjayBhatia Nov 17, 2022

Uh oh!

skriss Nov 17, 2022

Uh oh!

skriss Nov 15, 2022

Uh oh!

skriss Nov 17, 2022

Uh oh!

sunjayBhatia Nov 17, 2022

Uh oh!

skriss Nov 17, 2022

Uh oh!

codecov bot commented Nov 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

skriss commented Nov 15, 2022

Uh oh!

skriss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 15, 2022

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants