feat(sharding): Additional legacy shard algorithm that provides consistent shard on duplicate server URLs (#18118) #18713
Conversation
…icate server URLs Signed-off-by: Ezzahhh <38420555+Ezzahhh@users.noreply.github.com>
✅ Preview Environment deployed on Bunnyshell
See: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } else { | ||
| h := fnv.New32a() | ||
| _, _ = h.Write([]byte(server)) | ||
| shard := int32(h.Sum32() % uint32(replicas)) |
Check failure
Code scanning / CodeQL
Incorrect conversion between integer types High
Signed-off-by: Ezzahhh <38420555+Ezzahhh@users.noreply.github.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #18713 +/- ##
==========================================
+ Coverage 50.64% 50.66% +0.01%
==========================================
Files 315 315
Lines 43359 43381 +22
==========================================
+ Hits 21960 21978 +18
- Misses 18893 18895 +2
- Partials 2506 2508 +2 ☔ View full report in Codecov by Sentry. |
|
Sorry for pinging @ishitasequeira, not sure who to ask about this PR so am happy to take critique or guidance. Does this look like an acceptable addition to address this edge case? If not, is there any suggested method other than the workaround described (setting |
Checklist:
Closes #18118
There is an edge case where cluster secrets may be duplicated (by server URL, or in other words multiple cluster secrets that point to the same physical cluster) to be able to easily target tenants for an ApplicationSet (multiple deployments of the same helm chart into different namespaces) within the same physical cluster.
Unfortunately, this causes an issue when there are more than 1 controller replicas, because the sharding algorithms assume that the cluster ID uniquely identifies each physical cluster. This results in potentially the same physical cluster being sharded to more than one controller and causing sync/race condition issues (e.g. helm hooks breaking as two controllers race against each other).
This PR adds an additional sharding algorithm that is a modification of the existing legacy distribution algorithm. It shards by the cluster's server URL instead so that any cluster with the same server URL will return the same consistent shard to avoid the above described issue. The downside is that sharding on the server URL may potentially be less evenly distributed compared to using the ID.
Please let me know if there is a better method or preferred solution to this issue; this seemed like the least harmful to cause issue to existing users as it is opt-in. Our current workaround in production is to specify the
shardfor all clusters that are shared but it could become difficult over time to maintain this and ensure consistency.