Clean up notifications by Stevenjin8 · Pull Request #384 · kubernetes-sigs/ingress2gateway

Stevenjin8 · 2026-03-05T21:14:42Z

I think we have a lot of notifications that are really just debug logs that we should not present to the user.

Also, most of our errors are recoverable (say, by falling back to a default value), so we should just make an Error notification instead and continue on.

This also adds some other notifications

GRPC isn't fully supported
We wont serve traffic using a self-signed cert
regex matches are case insenstive and prefix
path normalization isn't super well defined.

~

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

add regex warnings

# Conflicts: # pkg/i2gw/providers/ingressnginx/bodysize.go # pkg/i2gw/providers/ingressnginx/iprange.go

Stevenjin8 · 2026-03-05T21:16:38Z

 					if config.isWeight || !config.isHeader {
 						httpRouteContext.HTTPRoute.Spec.Rules[ruleIdx].BackendRefs[canaryBackendIdx].Weight = &canaryWeight
 						httpRouteContext.HTTPRoute.Spec.Rules[ruleIdx].BackendRefs[nonCanaryBackendIdx].Weight = &nonCanaryWeight
-						notify(notifications.InfoNotification, fmt.Sprintf("parsed canary annotations of ingress %s/%s and set weights (canary: %d, non-canary: %d, total: %d)",


IMO notifications liek these ones don't really add anything. The user already expects ingress2gateway to do this.

i agree, we should make sure the README is update to date though in that case. otherwise the user would have to look through the translated output and confirm the annotation was parsed. i guess they would know by the fact that there wasn't an unsupported annotation notification but that's a bit less intuitive.

Stevenjin8 · 2026-03-05T21:17:22Z


 			ingress := getNonCanaryIngress(sources)
 			if ingress == nil {
-				notify(notifications.InfoNotification, "Found canary ingress rule without non-canary ingress rule", &httpRouteContext.HTTPRoute)


maybe this is an error? In theory we should never get to this code path.

kkk777-7 · 2026-03-16T07:55:44Z

thanks @Stevenjin8 ! I left some comments.

k8s-ci-robot · 2026-03-17T05:34:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kkk777-7, Stevenjin8

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Stevenjin8,kkk777-7]

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

kkk777-7 · 2026-03-17T05:34:47Z

LGTM, thanks!
/lgtm

* Get rid of Info notifications * Make most errors notifications add regex warnings * use methods * lint * fmt * cleanup some warning messages * add more notifications * verify * fix test * grpcroute warnings * review

* feat: Implement PR1 Infrastructure and Standard CORS support Signed-off-by: aashish kumar <aashtrek@google.com> * Restore PR 303 feature files * feat: apply gke session cookie parser stacked on main * fix: suppress warnings for affinity annotations * refactor: use generic session affinity in EmitterIR * refactor: use provider-neutral SessionAffinity enum * fix(gce): correctly strip HTTPRoute rule names without breaking generic tests * fix(common): remove unsupported HTTPRouteRule Name generation and fix all tests * fix(gce): remove implicit GatewayClass upselling to gke-l7-global-external-managed * style: update copyright header to remove specific year * updated formatting * Implement session affinity tracking and restore rule naming * Update standard emitter to warn on unconsumed SessionAffinity * remove copyright years (#389) * remove copyright years after 2025 * change some other ocpyrights --------- Co-authored-by: Lior Lieberman <liorlib7+riskified@gmail.com> * fix: use binary suffixes (Ki/Mi/Gi) for body size conversion (#375) * fix: use binary suffixes (Ki/Mi/Gi) for body size conversion * Fix tests and comment * Update main and ingress-nginx README (#390) * Some README updates for the ingress-nginx provider.for in * Fix readme * remove version stuff * remove comments in CONTRIBUTING.md * Better install instructions * Reorganize e2e tests (#366) * Move e2e logic to e2e/framework package Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Move providers and implementations to sub packages Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Consolidate Kong Kong is special in that it serves both as an ingress provider and a Gateway API implementation. However, having a separate helper for each deployment type is unnecessary and creates problems in some tests. Merge the two helpers into one while allowing importing the helper via both implementation.DeployKong and provider.DeployKong (via a thin wrapper) for consistency with other providers. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Use correct consts to identify providers and implementations deployProviders() matches ingress provider names defined in pkg/i2gw/providers — these must stay in sync with the library because they form the CLI contract. deployGatewayImplementation() decides which gateway backend to deploy — an e2e concern that pkg/i2gw has no idea about. Even when a provider and an implementation share the same string value (e.g. "kong"), the consts carry different meanings and should come from different packages so the two concerns don't get conflated. Use consts from pkg/i2gw/providers for the deployProviders switch and consts from e2e/implementation for the deployGatewayImplementation switch. Kong is a special case: it serves as both an ingress provider and a gateway implementation, backed by a single Helm deployment. Both code paths use the same Acquire key (implementation.KongName) so the resource manager deduplicates the deployment and avoids races between test cases. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Reorganize tests by category Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Update e2e/README.md Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Remove old comment This info lives under e2e/README.md now. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Fix slow-deleting namespaces Adding the Envoy Gateway implementation seems to have triggered a racy situation upon test teardown where some app namespaces get stuck in "Terminating" for multiple minutes despite not having any resources, likely due to transient errors which trigger exponential backoff. Fix the issue by watching for stuck namespaces and "nudging" the namespace controller by touching an annotation on the namespace object. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Generate kind.yaml dynamically No need to commit this file to the repo - we can generate it on the fly and remove at test cleanup. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> --------- Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> * Remove LiorLieberman from reviewers list (#392) * Clean up notifications (#384) * Get rid of Info notifications * Make most errors notifications add regex warnings * use methods * lint * fmt * cleanup some warning messages * add more notifications * verify * fix test * grpcroute warnings * review * fix tests and unparsed stuff * clean up service errors * simplify notification cahnge * undo nginx changes --------- Signed-off-by: aashish kumar <aashtrek@google.com> Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com> Co-authored-by: aashish kumar <aashtrek@google.com> Co-authored-by: Lior Lieberman <liorlib7+riskified@gmail.com> Co-authored-by: Johanan Liebermann <jliebermann@microsoft.com> Co-authored-by: Lior Lieberman <liorlieberman@google.com>

Stevenjin8 added 5 commits March 5, 2026 13:44

Get rid of Info notifications

7e27837

Make most errors notifications

a034e1c

add regex warnings

use methods

0220699

lint

8d3e269

Merge commit '8d3e269' into general-notifications

f099636

# Conflicts: # pkg/i2gw/providers/ingressnginx/bodysize.go # pkg/i2gw/providers/ingressnginx/iprange.go

Stevenjin8 requested review from LiorLieberman, robscott and youngnick as code owners March 5, 2026 21:14

Stevenjin8 temporarily deployed to release March 5, 2026 21:14 — with GitHub Actions Inactive

k8s-ci-robot requested a review from rikatz March 5, 2026 21:14

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 5, 2026

Stevenjin8 commented Mar 5, 2026

View reviewed changes

fmt

a3f7ba7

Stevenjin8 temporarily deployed to release March 5, 2026 21:44 — with GitHub Actions Inactive

Stevenjin8 commented Mar 6, 2026

View reviewed changes

Comment thread pkg/i2gw/emitters/standard/standard.go Outdated

Stevenjin8 added 4 commits March 6, 2026 16:06

cleanup some warning messages

bc379ed

add more notifications

18413ce

verify

985537d

fix test

228a6a8

Stevenjin8 temporarily deployed to release March 9, 2026 19:54 — with GitHub Actions Inactive

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 9, 2026

grpcroute warnings

e26d9c1

Stevenjin8 temporarily deployed to release March 9, 2026 20:04 — with GitHub Actions Inactive

Stevenjin8 self-assigned this Mar 10, 2026

kkk777-7 reviewed Mar 16, 2026

View reviewed changes

Comment thread pkg/i2gw/providers/ingressnginx/iprange.go Outdated

Comment thread pkg/i2gw/providers/ingressnginx/redirect.go

review

826a134

Stevenjin8 temporarily deployed to release March 16, 2026 18:05 — with GitHub Actions Inactive

Stevenjin8 requested a review from kkk777-7 March 16, 2026 18:05

kkk777-7 approved these changes Mar 17, 2026

View reviewed changes

k8s-ci-robot assigned kkk777-7 Mar 17, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 17, 2026

k8s-ci-robot merged commit 4a7a91e into kubernetes-sigs:main Mar 17, 2026
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up notifications#384

Clean up notifications#384
k8s-ci-robot merged 12 commits intokubernetes-sigs:mainfrom
Stevenjin8:general-notifications

Stevenjin8 commented Mar 5, 2026 •

edited

Loading

Uh oh!

Stevenjin8 Mar 5, 2026

Uh oh!

jgreeer Mar 5, 2026

Uh oh!

Stevenjin8 Mar 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kkk777-7 commented Mar 16, 2026

Uh oh!

k8s-ci-robot commented Mar 17, 2026

Uh oh!

kkk777-7 commented Mar 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Stevenjin8 commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Stevenjin8 Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

jgreeer Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Stevenjin8 Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kkk777-7 commented Mar 16, 2026

Uh oh!

k8s-ci-robot commented Mar 17, 2026

Uh oh!

kkk777-7 commented Mar 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Stevenjin8 commented Mar 5, 2026 •

edited

Loading