Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docker: Return undetected before first detection #5362

Merged
merged 1 commit into from
Feb 25, 2019

Conversation

endocrimes
Copy link
Contributor

@endocrimes endocrimes commented Feb 25, 2019

Preview: https://asciinema.org/a/229631

fixes #5320

This causes the docker driver to return undetected before the daemon is available, for a better experience when hosts do not have docker installed.
After the first detection it then returns unhealthy if it subsequently goes away.

sidenote:
It appears that we do keep fingerprinting drivers if they return undetected, which I did not think was the case.

This commit causes the docker driver to return undetected before it
first establishes a connection to the docker daemon.

This fixes a bug where hosts without docker installed would return as
unhealthy, rather than undetected.
Copy link
Contributor

@notnoop notnoop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

I may suggest reusing fingerprintLock and having setFingerprintSuccess set detected to true; as fingerprintSuccess and detected (which is more like fingerprintEverWasSuccessful) are related and should be updated together.

Also, anyway we add a test for this?

@endocrimes
Copy link
Contributor Author

There's no particularly useful way to write a test for this without a lot of faking things for go-dockerclient afaik :/ (we currently don't really have anything in the way of fingerprint tests for docker :()

I actually want to explicitly keep this as is, because it would be fairly easy to introduce a bug if we added further validation steps if it was rolled in to setFingerprintSuccessful. (e.g detected but unhealthy if we were to validate more network things, or after #5356 is merged)

@endocrimes endocrimes merged commit d3c7e4d into master Feb 25, 2019
@endocrimes endocrimes deleted the dani/f-fingerprint-undetected branch February 25, 2019 14:17
@shantanugadgil
Copy link
Contributor

waiting for beta3 ... I too noticed this when I started some experiments with beta2 for nvidia 👍

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Detected docker driver and unhealthy
3 participants