Skip to content

Remove rlp watcher for gateway rlps #242

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

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

eguzki
Copy link
Contributor

@eguzki eguzki commented Aug 30, 2023

This even mapper for RLP's targeting a gateway is no longer necessary. When a RLP targeting a Gateway is updated, all rate limit required resources are updated correcly

@eguzki eguzki requested a review from a team as a code owner August 30, 2023 19:52
@codecov
Copy link

codecov bot commented Aug 30, 2023

Codecov Report

Merging #242 (60d8195) into main (aa05f01) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #242      +/-   ##
==========================================
+ Coverage   62.96%   62.99%   +0.02%     
==========================================
  Files          33       32       -1     
  Lines        3224     3186      -38     
==========================================
- Hits         2030     2007      -23     
+ Misses       1014     1002      -12     
+ Partials      180      177       -3     
Flag Coverage Δ
integration 68.15% <ø> (+0.18%) ⬆️
unit 58.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
api/v1beta1 (u) ∅ <ø> (∅)
pkg/common (u) 76.99% <ø> (ø)
pkg/istio (u) 29.69% <ø> (ø)
pkg/log (u) 31.81% <ø> (ø)
pkg/reconcilers (u) 33.68% <ø> (ø)
pkg/rlptools (u) 57.63% <ø> (ø)
controllers (i) 68.15% <ø> (+0.18%) ⬆️
Files Changed Coverage Δ
controllers/ratelimitpolicy_controller.go 66.42% <ø> (-1.36%) ⬇️

... and 5 files with indirect coverage changes

@guicassolato
Copy link
Contributor

What about when the Gateway is modified (e.g. new listener added)?

@eguzki
Copy link
Contributor Author

eguzki commented Aug 30, 2023

That change will be picked by this watcher, which is still there.

		Watches(
			&source.Kind{Type: &gatewayapiv1beta1.Gateway{}},
			handler.EnqueueRequestsFromMapFunc(gatewayEventMapper.MapToRateLimitPolicy),
		).

@eguzki eguzki merged commit 1e63d5a into main Aug 30, 2023
@eguzki eguzki deleted the remove-rlp-watcher-for-gateway-rlps branch August 30, 2023 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants