refactor: introduce 'byClusterName' secret index to speedup cluster server URL lookup #8133
Conversation
alexmt
force-pushed
the
cluster-lookup-performance
branch
from
6f761c3
to
8ff0aef
Compare
util/db/cluster.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| if len(localClusterSecrets) == 0 && name == string(localClusterSecrets[0].(*apiv1.Secret).Data["name"]) { |
There was a problem hiding this comment.
Should this be len(localClusterSecrets) > 0?
There was a problem hiding this comment.
ops, good catch! Fixed.
There was a problem hiding this comment.
Unit test also catch it so I had to modify implementation only
…erver URL lookup Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
alexmt
force-pushed
the
cluster-lookup-performance
branch
from
8ff0aef
to
0d5ace4
Compare
|
Just double checking that it will be cheery picked to 2.2 right? |
Codecov Report
@@ Coverage Diff @@
## master #8133 +/- ##
==========================================
+ Coverage 41.51% 41.53% +0.01%
==========================================
Files 174 174
Lines 22712 22736 +24
==========================================
+ Hits 9430 9443 +13
- Misses 11941 11949 +8
- Partials 1341 1344 +3
Continue to review full report at Codecov.
|
|
@yeya24 , that is correct. |
|
Hello @alexmt, thanks for the quick fix and good work! Can I ask when will it get into v2.2? Anything else is blocking now? |
| } | ||
|
|
||
| // if local cluster name is not overridden and specified name is local cluster name, return local cluster server | ||
| localClusterSecrets, err := informer.GetIndexer().ByIndex(settings.ByClusterURLIndexer, appv1.KubernetesInternalAPIServerAddr) |
There was a problem hiding this comment.
Can we check db.getLocalCluster().Name == name first and fetch the local cluster index later? I think it is not necessary to do this if the specified name is not local
There was a problem hiding this comment.
This check is required because the local cluster can be renamed. So we cannot assume it is named "in-cluster" and have to check cluster secrets.
…erver URL lookup (#8133) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
|
@yeya24 v2.2.3 is being created: https://github.com/argoproj/argo-cd/runs/4857175142?check_suite_focus=true |
…erver URL lookup (argoproj#8133) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Signed-off-by: Alexander Matyushentsev AMatyushentsev@gmail.com
Closes #8132
PR introduces 'byClusterName' secret index to speedup cluster server URL lookup