Change where the keychain dockercreds are read #1436
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1436 +/- ##
==========================================
+ Coverage 67.19% 67.33% +0.14%
==========================================
Files 140 140
Lines 8827 8887 +60
==========================================
+ Hits 5931 5984 +53
- Misses 2389 2395 +6
- Partials 507 508 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I don't think this is a good idea, opening and reading the file every time a secret is requested (aka every single reconcile) is going to be a lot of wasted resources.
What do you think of calling os.Stat() instead? We could cache the last modified time of the file and re-parse the file if the new modified time is newer. stat()should be a lot more performant than open() since it's only doing a lookup and not allocating anything that needs to be cleaned up (i.e. every open() must have a close())
|
Is this an opportunity a single reconcile loop could continually re-read them? |
|
@chenbh I can add a condition in the NewVolumeSecretKeychain method to os.stat the secret file but where are you thinking would we cache the time to compare against? |
|
@xtreme-shane-lattanzio I would have the @matthewmcnew I'm not too convinced about the benefits of continuously reading the file especially since we don't expect the underlying Secret to be rotated often. |
xtreme-shane-lattanzio
force-pushed
the
secret_watcher
branch
4 times, most recently
from
0bac2fb to
722e33a
Compare
xtreme-shane-lattanzio
force-pushed
the
secret_watcher
branch
from
722e33a to
f0a5528
Compare
Resolves #1353