Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions apis/v1/gateway_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1398,8 +1398,10 @@ type ListenerStatus struct {
// attachment semantics can be found in the documentation on the various
// Route kinds ParentRefs fields). Listener or Route status does not impact
// successful attachment, i.e. the AttachedRoutes field count MUST be set
// for Listeners with condition Accepted: false and MUST count successfully
// attached Routes that may themselves have Accepted: false conditions.
// for Listeners with condition Accepted: false but MUST count successfully
// only attached Routes that may themselves have Accepted: true conditions.
// Attached Routes whose Accepted condition is False or Unknown (or that
// do not have an Accepted condition set) MUST NOT be included in this count.
Comment on lines +1401 to +1404
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't been following this as closely as I should, but maybe I can reword this to make sure I'm understanding it correctly:

  1. AttachedRoutes should always be set on each Listener, even if the Listener itself has not been accepted
  2. AttachedRoutes should only represent the number of Routes with the Accepted condition set to True that have been attached to this Listener

If so, I'm completely on board with point 2, but confused by point 1 (although I recognize that it's not new in this PR).

/cc @rikatz @youngnick

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robscott Exactly, I changed only point 2 since it is what the issue is requesting, instead for point 1 I can confirm that based on the doc it seems to work in this way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey Rob, so long story short, during the update of ListenerSet GEP (https://github.com/kubernetes-sigs/gateway-api/pull/4286/files) Nick questioned why we were counting Listeners not accepted as well, and well, me and @davidjumani copied the text from here.

In fact, the implementations today count just the accepted ones, but the text on the API description was wrong, so we are now "fixing it" and making it clear that AttachedRoutes MUST count just accepted routes.

Maybe @youngnick has some better wording here tho :)

//
// Uses for this field include troubleshooting Route attachment and
// measuring blast radius/impact of changes to a Listener.
Expand Down
8 changes: 5 additions & 3 deletions apisx/v1alpha1/xlistenerset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,12 @@ type ListenerEntryStatus struct {
// AND the Route has a valid ParentRef selecting the whole Gateway
// resource or a specific Listener as a parent resource (more detail on
// attachment semantics can be found in the documentation on the various
// Route kinds ParentRefs fields). Listener or Route status does not impact
// Route kinds ParentRefs fields). Listener status does not impact
// successful attachment, i.e. the AttachedRoutes field count MUST be set
// for Listeners with condition Accepted: false and MUST count successfully
// attached Routes that may themselves have Accepted: false conditions.
// for Listeners with condition Accepted: false but MUST count successfully
// only attached Routes that may themselves have Accepted: true conditions.
// Attached Routes whose Accepted condition is False or Unknown (or that
// do not have an Accepted condition set) MUST NOT be included in this count.
//
// Uses for this field include troubleshooting Route attachment and
// measuring blast radius/impact of changes to a Listener.
Expand Down
12 changes: 8 additions & 4 deletions config/crd/experimental/gateway.networking.k8s.io_gateways.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 8 additions & 4 deletions config/crd/standard/gateway.networking.k8s.io_gateways.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions geps/gep-1713/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,11 @@ type ListenerEntryStatus struct {
// resource or a specific Listener as a parent resource (more detail on
// attachment semantics can be found in the documentation on the various
// Route kinds ParentRefs fields). Listener or Route status does not impact
// successful attachment, i.e. the AttachedRoutes field count MUST be set
// for Listeners with condition Accepted: false and MUST count successfully
// attached Routes that may themselves have Accepted: false conditions.
// successful attachment, i.e. the AttachedRoutes field count MUST be set
// for Listeners with condition Accepted: false but MUST count successfully
// only attached Routes that may themselves have Accepted: true conditions.
// Attached Routes whose Accepted condition is False or Unknown (or that
// do not have an Accepted condition set) MUST NOT be included in this count.
//
// Uses for this field include troubleshooting Route attachment and
// measuring blast radius/impact of changes to a Listener.
Expand Down
4 changes: 2 additions & 2 deletions pkg/generated/openapi/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.