Add DNS check to bootstrap

weaveworks / launcher

Weave Cloud Launcher

Apache License 2.0

10 stars 13 forks source link

Add DNS check to bootstrap #164

Closed lilic closed 6 years ago

lilic commented 6 years ago

Sometimes DNS is not configured correctly, but during the launcher process, DNS is required to setup all the needed resources. This creates a check by creating a pod where nslookup is performed.

Closes https://github.com/weaveworks/launcher/issues/158.

Note: Opened as more of a preview PR, if we agree on this implementation will add a test or two to this PR. :)

leth commented 6 years ago

Looks good! Just some minor questions!

lilic commented 6 years ago

@dlespiau @leth PTALA, thanks!

Not sure if we want add a note to the STDOUT saying we are performing checks or not. The last one right now before the DNS check is Installing Weave Cloud agents on minikube..., which might be slightly misleading or confusing when we then error out with There was an error while performing DNS check. DNS check failed. Timed out during DNS check. Thoughts, ideas, suggestions?
As mentioned elsewhere, the timeout cannot be set for nslookup, so right now we wait for 1 minute and timeout during the check. IMO if we can find a better way, or when we add other pre flight checks in the future we can use a different image rather then busybox, or build our own tool to do that. For now I would leave it as is, but am open for suggestions.

lilic commented 6 years ago

BTW not sure what's up with tests not running? :/ Could it be because I have this branch on my fork, did not have a look at circle ci setup yet in this project.

dlespiau commented 6 years ago

Ah yes, we push branches on the same repo. The reason is that we don't want anyone being able to start a test run by just opening a pull request (it could be a pull request with a commit changing what the tests will be running). The "Build forked pull requests" option in CircleCI is turned off for this project.

This way we make sure that only people with the rights to push to this repository can run test code in CI.

dlespiau commented 6 years ago

Adding a line on stdout saying that we're doing a pre-flight check sounds really useful "Checking the Kubernetes installation" or similar. It'll be especially useful for people having to wait for that 1 minute timeout, making it not awful!

lilic commented 6 years ago

Closing in favor of https://github.com/weaveworks/launcher/pull/166 to let the tests run.