Fix for Target Allocator not saving targets when collector instances take time to come up #2351
Conversation
|
Can you add a unit test for this case? Also, does the same problem not apply to the other allocation strategy as well? |
…ichandrashekar/opentelemetry-operator into rashmi/collector-instances
Thanks @swiatekm-sumo. I have made the changed for the other hashing algo as well and added a couple unit tests. please lmk if anything else is needed. |
|
Can you also check the case where you add targets to a strategy with 0 collectors, and later add collectors to see if targets get allocated to them correctly? There's also a lint error you need to fix. |
…ichandrashekar/opentelemetry-operator into rashmi/collector-instances
Thanks @swiatekm-sumo . That's a good test case, I have added that. Please take a look. |
|
@swiatekm-sumo - I see an e2e test failure due to timeouts during CR creation. Could you please rerun the tests? Thanks! |
|
@swiatekm-sumo - All the checks seem to have passed, could you pls help get this merged? Thanks! |
There was a problem hiding this comment.
I feel like there should be a way to do this without all the special cases, but we can refactor it later now that we have tests for them. I don't think there's any point in keeping this PR held up on DRY issues, so I'm approving it.
Thank you for the contribution!
|
Thanks @swiatekm-sumo! I dont have permissions to merge, could you pls merge this? |
…take time to come up (open-telemetry#2351) * change to account for targets discoverd before instances * adding change log * adding instance changed to leat weighted as well * adding tests * adding unit tests * updaintg tests * fixing lint error and updating tests and fixing bug in least weighted algo * fix indent
Description:
Fixing a bug - In a kubernetes cluster, have scrape jobs configured using TargetAllocator. Due to timing issue(which is very common with kubernetes clusters, where no ordering can be guaranteed), the instances are not discovered until after the first set of targets are discovered. The configuration applied until the instances are discovered are lost.
This PR fixes that issue where it saves the targets until the collector instances come up
Link to tracking Issue:
#2350
Testing:
Tested that in such scenarios, the targets are not lost and assigned to collector instances when they come up