Conversation
|
✅ All required contributors have signed the F5 CLA for this PR. Thank you! |
|
I have hereby read the F5 CLA and agree to its terms |
|
Hi @matei, thank you for the PR. Can you please rebase and commit again using the @matei user or accept the CLA using the @MateiStefanescu-MRM user to pass our F5 CLA. |
e202a78 to
17d0ca3
Compare
17d0ca3 to
b648175
Compare
|
Hey @matei thanks for the PR, can you please create an issue in the repo with the same description as the PR and attach it to the PR please, |
Hey @vepatel, this is related to this issue |
There was a problem hiding this comment.
Pull request overview
This PR addresses a startup-time performance issue where findResourcesForResourceReference can return the same resource multiple times (once per host key), causing downstream processing to repeat work for multi-host resources.
Changes:
- Add de-duplication logic to
findResourcesForResourceReferenceusingResource.GetKeyWithKind()to ensure each matching resource is returned at most once. - Apply the same de-duplication across both
hostsandlistenerHostsscans.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var result []Resource | ||
| seen := make(map[string]struct{}) | ||
|
|
There was a problem hiding this comment.
This change adds de-duplication behavior in findResourcesForResourceReference, but there’s no unit test covering the regression scenario described in the PR (a single resource associated with many host keys returning duplicates). Please add a test case with an Ingress (or VirtualServer) that has multiple hosts and references the same Service/Secret, and assert the returned slice contains that resource only once.
566524c to
c497e1e
Compare
Proposed changes
Problem:
when the ingress controller pods start, we have:
findResourcesForResourceReference
Point 4 is the issue in
findResourcesForResourceReference: if an ingress is serving 50 hosts, this method will duplicate the same IngressConfiguration / VirtualServerConfiguration / etc. object in result 50 times.This is causing a big delay from container start to container ready (in our tests 2min+ for an ingress with 57 hosts).
Solution:
simple de-duplication in
findResourcesForResourceReference. With the fix to de-duplicate, this time is reduced to 2-3sChecklist
Before creating a PR, run through this checklist and mark each as complete.