Fixing test flake #1534
Fixing test flake #1534
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kfswain The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
k8s-ci-robot
added
the
size/S
| p.wg.Add(1) | ||
| go func() { | ||
| p.indexer.Add(state.PrefixHashes, ServerID(targetPod.NamespacedName)) | ||
| p.wg.Done() |
There was a problem hiding this comment.
mmmmmm.
this fix introduces a wg only for test purpose. the thing is we don't have access to this wg outside of prefix plugin package, so llm-d tests will fail (we test PD, where prefix is used and profile handler uses it, reads state from CycleState, but with this go routine the update is not ready on time.
it solves the issue only in prefix unit tests
|
as temp fix to unblock flaky tests this is good enough as we don't want to fail incoming PRs. /lgtm |
k8s-ci-robot
added
the
lgtm
|
/hold @kfswain you know what - maybe we can make the WG public? as long as we have this solution in place, I'll need to add wg.wait also in llm-d unit tests |
k8s-ci-robot
added
the
do-not-merge/hold
discussed with kellen, this will be handled in a follow up. /unhold |
k8s-ci-robot
removed
the
do-not-merge/hold
Fixes: #1529
Adding a waitGroup should not impact prod code as we do not wait on this WG anywhere in the system, just allows unit tests to be synchronous