Skip to content

Refactor notification mechanism#360

Merged
k8s-ci-robot merged 9 commits intokubernetes-sigs:mainfrom
johananl:johananl/refactor-notifications
Mar 5, 2026
Merged

Refactor notification mechanism#360
k8s-ci-robot merged 9 commits intokubernetes-sigs:mainfrom
johananl:johananl/refactor-notifications

Conversation

@johananl
Copy link
Copy Markdown
Member

@johananl johananl commented Feb 23, 2026

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR refactors the notification mechanism. Here is what it does specifically:

  • Replace the global notifications map with a new Report type.
  • One local Report instance is created in ToGatewayAPIResources().
  • A *Report pointer is passed to each provider/emitter constructor.
  • Each provider/emitter calls Report.Notifier() to create its own provider/emitter-scoped NotifyFunc and stores this function as a property.
  • Conversion code can now emit notifications by calling a package-scoped notify() function.

There are no user-facing changes -- this is an implementation change only. The size of the PR is largely due to repetitive replacements of the "instrumentation" everywhere.

Which issue(s) this PR fixes:

Fixes #340

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Feb 23, 2026
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 23, 2026
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 23, 2026
@johananl johananl force-pushed the johananl/refactor-notifications branch from 723f2c3 to a1612a1 Compare February 24, 2026 11:55
@johananl johananl force-pushed the johananl/refactor-notifications branch from a1612a1 to 32beae2 Compare February 24, 2026 11:59
@johananl johananl force-pushed the johananl/refactor-notifications branch from 32beae2 to f5bbf4a Compare February 24, 2026 14:53
@johananl johananl force-pushed the johananl/refactor-notifications branch from f5bbf4a to f4a8e55 Compare February 24, 2026 15:00
@johananl johananl force-pushed the johananl/refactor-notifications branch from f4a8e55 to ecd57d4 Compare February 24, 2026 15:07
@johananl johananl force-pushed the johananl/refactor-notifications branch from ecd57d4 to 105737b Compare February 24, 2026 15:13
@johananl johananl force-pushed the johananl/refactor-notifications branch from 105737b to f9ecf5d Compare February 24, 2026 15:15
@johananl johananl force-pushed the johananl/refactor-notifications branch from f9ecf5d to 8bcb4e4 Compare February 24, 2026 15:38
@johananl johananl force-pushed the johananl/refactor-notifications branch from 8bcb4e4 to a9206bf Compare February 24, 2026 15:44
@johananl johananl force-pushed the johananl/refactor-notifications branch from a9206bf to ea0be27 Compare February 24, 2026 15:48
@johananl
Copy link
Copy Markdown
Member Author

/test pull-ingress2gateway-e2e

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 3, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johananl, LiorLieberman

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:

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 the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 3, 2026
@johananl
Copy link
Copy Markdown
Member Author

johananl commented Mar 3, 2026

We have no character limit on the notification message which can mess up the tables when the terminal isn't big enough.

I'm fine with either format. The colored box design was done using an slog handler. I can look into "porting" the visuals without reintroducing the logging library (since we've agreed to keep notifications as distinct from logs).

@Stevenjin8
Copy link
Copy Markdown
Contributor

@johananl makes sense.
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 3, 2026
@johananl
Copy link
Copy Markdown
Member Author

johananl commented Mar 4, 2026

I'm porting the design from #341.

@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Mar 5, 2026
@johananl johananl force-pushed the johananl/refactor-notifications branch from 1112e49 to 0f82553 Compare March 5, 2026 09:59
johananl added 5 commits March 5, 2026 12:00
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
@johananl johananl force-pushed the johananl/refactor-notifications branch from 0f82553 to 5221299 Compare March 5, 2026 10:08
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 5, 2026
johananl added 4 commits March 5, 2026 12:43
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
We use the standard log package instead of klog in a few places.

Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>
@johananl johananl force-pushed the johananl/refactor-notifications branch from 5221299 to 88955af Compare March 5, 2026 10:43
@Stevenjin8
Copy link
Copy Markdown
Contributor

/unhold
/lgtm

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Mar 5, 2026
@k8s-ci-robot k8s-ci-robot merged commit b338dc0 into kubernetes-sigs:main Mar 5, 2026
6 checks passed
@johananl johananl deleted the johananl/refactor-notifications branch March 5, 2026 16:13
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/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notifications should be logs

4 participants