Skip to content

Add descriptive message to Gateway Programmed condition when no valid… #13152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Ayushd785 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add descriptive message to Gateway Programmed condition when no valid… #13152

Ayushd785 wants to merge 1 commit into from

Conversation

@Ayushd785
Copy link
Contributor

@Ayushd785 Ayushd785 commented Dec 16, 2025 •
edited
Loading

Description

When a Gateway has no valid listeners, the Programmed=False status condition shows an empty message, leaving users confused about why their Gateway isn't working. This fix adds a clear, descriptive message to help users understand the issue.

Fixes #11982

What Changed

  • Added a new message constant: GatewayNoValidListenersMessage = "No valid listeners found for Gateway"
  • Updated the Programmed=False condition to use this message instead of leaving it empty
  • Updated test golden files to reflect the new message

Before

  • lastTransitionTime: null
    message: ""
    reason: Invalid
    status: "False"
    type: Programmed

After

  • lastTransitionTime: null
    message: No valid listeners found for Gateway
    reason: Invalid
    status: "False"
    type: Programmed

Change Type

/kind fix

Added clearer status messages for Gateway and Listener conditions to improve debugging of invalid or missing listeners.

@gateway-bot gateway-bot added do-not-merge/release-note-invalid Indicates that a PR should not merge because it's missing one of the release note labels. kind/fix Categorizes issue or PR as related to a bug. labels Dec 16, 2025
@Ayushd785 Ayushd785 force-pushed the gateway-status-message-fix branch from 3d2d562 to 6cd82d0 Compare December 16, 2025 08:58
@Ayushd785 Ayushd785 mentioned this pull request Dec 16, 2025
Open
@Ayushd785
Copy link
Contributor Author

Ayushd785 commented Dec 16, 2025

The unit test failures in this PR are unrelated to my changes. My PR only adds a
descriptive message to the Gateway Programmed condition - it doesn't touch:

  • OAuth2/namespace creation code
  • WebSocket upgrade configuration
  • KRT/JSON marshaling logic

These appear to be pre-existing flaky tests or infrastructure issues.

@Ayushd785
Copy link
Contributor Author

Ayushd785 commented Dec 16, 2025

@timflannagan, can you please review it whenever it is possible?

jenshu
jenshu reviewed Dec 16, 2025
View reviewed changes
pkg/kgateway/translator/listener/validation.go Outdated
"github.com/kgateway-dev/kgateway/v2/pkg/kgateway/wellknown"
"github.com/kgateway-dev/kgateway/v2/pkg/pluginsdk/ir"
reports "github.com/kgateway-dev/kgateway/v2/pkg/pluginsdk/reporter"
reporter2 "github.com/kgateway-dev/kgateway/v2/pkg/pluginsdk/reporter"
Copy link
Contributor

@jenshu jenshu Dec 16, 2025

Choose a reason for hiding this comment

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

Suggested change
reporter2 "github.com/kgateway-dev/kgateway/v2/pkg/pluginsdk/reporter"
"github.com/kgateway-dev/kgateway/v2/pkg/pluginsdk/reporter"

can we just use reporter here?

Copy link
Contributor Author

@Ayushd785 Ayushd785 Dec 16, 2025

Choose a reason for hiding this comment

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

reporter is already used in the file it can cause redundancy.

Copy link
Contributor Author

@Ayushd785 Ayushd785 Dec 17, 2025

Choose a reason for hiding this comment

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

I updated validation.go to use the standard reporter package name and renamed the function parameter to rpt to avoid shadowing. This keeps the usage consistent and clean across the codebase.

pkg/kgateway/translator/listener/validation.go Outdated
reporter.Gateway(gw.Obj).SetCondition(reporter2.GatewayCondition{
Type: GatewayConditionAttachedListenerSets,
Status: metav1.ConditionUnknown,
Reason: GatewayReasonListenerSetsNotAllowed,
Copy link
Contributor

@jenshu jenshu Dec 16, 2025

Choose a reason for hiding this comment

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

it would be good to also add helpful messages to the other conditions that are missing them (search for SetCondition in this file - i see several are missing a Message)

Copy link
Contributor Author

@Ayushd785 Ayushd785 Dec 16, 2025

Choose a reason for hiding this comment

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

Okay got it, I'll definitely do it

Copy link
Contributor Author

@Ayushd785 Ayushd785 Dec 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I have added the messages to the other conditions as well as you suggested

@Ayushd785 Ayushd785 force-pushed the gateway-status-message-fix branch 2 times, most recently from a090cc2 to 4276faa Compare December 17, 2025 13:54
@gateway-bot gateway-bot added release-note and removed do-not-merge/release-note-invalid Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 17, 2025
@Ayushd785 Ayushd785 force-pushed the gateway-status-message-fix branch 8 times, most recently from 6be8403 to 9ce8316 Compare December 17, 2025 18:42
davidjumani
davidjumani reviewed Dec 17, 2025
View reviewed changes
pkg/kgateway/translator/listener/validation.go
Copy link
Contributor

@davidjumani davidjumani Dec 17, 2025

Choose a reason for hiding this comment

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

There are upcoming changes in GWAPI regarding the replacement of GatewayConditionAttachedListenerSets with a count of attached listener sets. Let's hold off until they have been finalized

Copy link
Contributor Author

@Ayushd785 Ayushd785 Dec 18, 2025

Choose a reason for hiding this comment

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

Thanks for the context! That makes sense. I’ll hold off on this change until the GWAPI updates are finalized and revisit it once the new condition semantics are clear.

@npolshakova
Copy link
Contributor

npolshakova commented Dec 26, 2025

Thanks for the contribution! It looks like one of the unit tests is failing with this change: https://github.com/kgateway-dev/kgateway/actions/runs/20425389198/job/58684609854?pr=13152#step:5:74839

@Ayushd785
Add descriptive messages to all Gateway and Listener status conditions
157a7cb 
This ensures all Gateway and Listener status conditions provide clear,
user-friendly messages explaining their state, improving observability
and debuggability.

Fixes kgateway-dev#11982

Signed-off-by: Ayush Dwivedi <[email protected]>
@Ayushd785 Ayushd785 force-pushed the gateway-status-message-fix branch from 722af85 to 157a7cb Compare December 26, 2025 20:43
@Ayushd785
Copy link
Contributor Author

Ayushd785 commented Dec 26, 2025

Thanks for the contribution! It looks like one of the unit tests is failing with this change: https://github.com/kgateway-dev/kgateway/actions/runs/20425389198/job/58684609854?pr=13152#step:5:74839

Hey, Thanks for the review i am trying to resolve that issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidjumani davidjumani davidjumani left review comments

@jenshu jenshu jenshu left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

kind/fix Categorizes issue or PR as related to a bug. release-note

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add Status Condition Messages to Gateway for Invalid Listeners

5 participants

@Ayushd785 @npolshakova @davidjumani @jenshu @gateway-bot