Always to a live lookup of version info instead of caching. #860
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
Can one of the admins verify this patch? |
|
ok to test |
| @@ -658,7 +656,9 @@ func (m *manager) GetMachineInfo() (*info.MachineInfo, error) { | |||
| } | |||
|
|
|||
| func (m *manager) GetVersionInfo() (*info.VersionInfo, error) { | |||
| return &m.versionInfo, nil | |||
| // TODO: Consider caching this and periodically updating. The VersionInfo may change if | |||
| // the docker daemon is started after the cAdvisor client is created | |||
There was a problem hiding this comment.
nit: Add to TODO - Might also be helpful to cache, so we can return last known docker version if docker is down at the time of query.
|
LGTM. Thanks for fixing it, @pwittrock ! |
|
LGTM |
|
CLAs look good, thanks! |
|
Thanks everyone. PTAL. Feedback addressed. |
|
retest this please |
|
@cadvisorJenkinsBot: retest this please |
pwittrock
force-pushed
the
no_cache_versioninfo
branch
from
9970983 to
b7bbefd
Compare
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
Recent push was to squash the commit for the PR review comments |
|
LGTM |
Always to a live lookup of version info instead of caching.
This PR changes how VersionInfo is served
old: lookup/create and cache the version info at manager creation time
new: lookup/create the version info each time it is requested
This is necessary to correctly report the VersionInfo in the event that the docker daemon is not running at the time the manager is created. This case may be encountered when cAdvisor is started in the same program that initializes the docker daemon / container bridge.