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

emit-translation-events CLI flag to suppress event creation #5252

Closed
1 of 2 tasks
mflendrich opened this issue Nov 29, 2023 · 7 comments · Fixed by #5296
Closed
1 of 2 tasks

emit-translation-events CLI flag to suppress event creation #5252

mflendrich opened this issue Nov 29, 2023 · 7 comments · Fixed by #5296
Assignees
Labels
release/highlight This part of the release is worth bragging about.
Milestone

Comments

@mflendrich
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Problem Statement

Today it is not possible to suppress creation of k8s events for translation successes/failures.
Those events can be a source of load on k8s API that is higher than necessary (#4127) and can be unwanted (e.g. in an emergency where the load on the API server is impacting the cluster).

Proposed Solution

Add a config knob to disable KIC's module that creates events.

There is prior art: update-status

https://github.com/Kong/kubernetes-ingress-controller/blob/cff127d03acf818a3865ef6d8ee016ffd1f15087/internal/manager/config.go#L229C42-L230

The idea is that we add a new flag alongside update-status (proposed name: emit-translation-events) defaulting to true.

Additional information

Related to #4127. #4127 is the long-term solution that, if #4127 is enabled by default, the likelihood of translation events causing trouble emit-translation-events flag should be way less.

Acceptance Criteria

  • emit-translation-events flag implemented, set as default=true, if disabled: KIC creates no events in the Kubernetes API.
@czeslavo
Copy link
Contributor

I suggest we rename the flag to --emit-kubernetes-events and use it to determine whether we should emit any type of Events (not only KongConfigurationTranslationFailed, but also KongConfigurationSucceeded and KongConfigurationApplyFailed).

@rainest
Copy link
Contributor

rainest commented Nov 30, 2023

tl;dr from #4127 (comment) is that we should already be limited to 1 Event update per failure per 300s after an initial ramp up period. The exact rate is tunable, though documentation is sparse and the the limiter behavior varies depending on your Even emission patterns.

There is a corner case at startup where, if you've let a large quantity N of broken resources accumulate, you would see N Event updates every 3s until you hit the 25 Event duplicate count before throttling starts.

@randmonkey
Copy link
Contributor

Reopen since the backport to 2.12.x and 3.0.x is not done.

@czeslavo
Copy link
Contributor

czeslavo commented Dec 6, 2023

Posted a proposal PR (#5299) to rename the flag to --emit-kubernetes-events (because of comment reasons).

@tao12345666333
Copy link
Member

Let me backport them to other branches.

@tao12345666333
Copy link
Member

tao12345666333 commented Dec 7, 2023

@tao12345666333
Copy link
Member

Already backported.
Let's close this one

@mflendrich mflendrich added the release/highlight This part of the release is worth bragging about. label Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/highlight This part of the release is worth bragging about.
Projects
None yet
5 participants