Skip to content

Conversation

@robscott
Copy link
Member

@robscott robscott commented Apr 17, 2025

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
We missed this when reviewing the ListenerSet GEP. Although an optional port that would require implementations to assign a port could be legitimately useful, I think we haven't fully processed this concept. Moving this back to required allows us to make it optional in the future, where starting with optional would make it much more painful to transition to required.

Some potential concerns of an optional port on ListenerSet:

  • Automatic port allocation could become a bit complicated for Gateway implementations to maintain
  • Spec.Listeners.Port would no longer be a reliable source for understanding which Port a Listener was listening on
  • Each Listener with an optional Port would require a status entry (not currently the case)
  • If ListenerSet ever supported multiple parents, the same Listener might have different Ports assigned

With all that said, I think this is potentially a very useful idea, I just don't think we spent enough time on this while reviewing ListenerSet and would prefer to take the safest possible path forward towards RC2. This is not "never", just "not yet".

Does this PR introduce a user-facing change?:

XListenerSet Port is required. (It was already effectively required due to 0 being an invalid value, but had been marked optional).

/cc @dprotaso @youngnick

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 17, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: robscott

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 17, 2025
@youngnick
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 17, 2025
@k8s-ci-robot k8s-ci-robot merged commit 75aed66 into kubernetes-sigs:main Apr 17, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants