Skip to content
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.

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

Support NoMatchingListenerHostname reason in HTTPRoute status #290

Merged

Conversation

pleshakov
Copy link
Contributor

Proposed changes

If an HTTPRoute references a listener with the hostname that does not match any hostnames of the HTTPRoute, NGINX Kubernetes Gateway will set the condition "Accepted" with the status "False" and the reason "NoMatchingListenerHostname" in the status of the HTTPRoute.

The commit introduces a new type RouteCondition. It is used to capture status-related conditions when building the graph. It will be used to capture other conditions too, including when building configuration.

Note: the implementation required implementing a few additional conditions. In one case, to wait until the corresponding reasons (v1beta1.RouteConditionReason) are added to the Gateway API and in the other case, to not increase the scope, those conditions are left to be implemented (FIXMEs).

@pleshakov pleshakov requested a review from a team as a code owner November 2, 2022 21:31
@github-actions github-actions bot added the enhancement New feature or request label Nov 2, 2022
@pleshakov pleshakov force-pushed the feature/httproute-status-NoMatchingListenerHostname-reason branch from 4281a36 to a26e3ca Compare November 2, 2022 21:37
@kate-osborn
Copy link
Contributor

@pleshakov does the compatibility doc need to be updated to reflect these changes?

internal/state/conditions/httproute.go Outdated Show resolved Hide resolved
internal/state/conditions/httproute.go Show resolved Hide resolved
internal/state/statuses.go Outdated Show resolved Hide resolved
internal/status/httproute.go Outdated Show resolved Hide resolved
internal/status/httproute.go Outdated Show resolved Hide resolved
internal/status/httproute_test.go Outdated Show resolved Hide resolved
@pleshakov pleshakov requested a review from kate-osborn November 4, 2022 16:45
@pleshakov
Copy link
Contributor Author

not resolving the conflicts yet -- those conflicts are because of the recent line-length changes. I will resolve them before merging so that the review-related changes to the PR are only about the PR comments.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Nov 4, 2022
@pleshakov pleshakov force-pushed the feature/httproute-status-NoMatchingListenerHostname-reason branch 2 times, most recently from b31a452 to 00dccce Compare November 8, 2022 00:14
If an HTTPRoute references a listener with the hostname that does not
match any hostnames of the HTTPRoute, NGINX Kubernetes Gateway will
set the condition "Accepted" with the status "False" and the reason
"NoMatchingListenerHostname" in the status of the HTTPRoute.

The commit introduces a new type RouteCondition. It is used to capture
status-related conditions when building the graph. It will be used to
capture other conditions too, including when building configuration.

Note: the implementation required implementing a few additional
conditions. In one case, to wait until the corresponding reasons
(v1beta1.RouteConditionReason) are added to the Gateway API and in the
other case, to not increase the scope, those conditions are left to be
implemented (FIXMEs).
@pleshakov pleshakov force-pushed the feature/httproute-status-NoMatchingListenerHostname-reason branch from 00dccce to 4c82cc0 Compare November 8, 2022 00:18
@pleshakov pleshakov merged commit 67b326d into main Dec 5, 2022
@pleshakov pleshakov deleted the feature/httproute-status-NoMatchingListenerHostname-reason branch December 5, 2022 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants