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

Ensures GitHub is available prior to downloading the runner #56

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

simonbs
Copy link
Contributor

@simonbs simonbs commented Nov 15, 2023

The changes in this PR ensures that we do not attempt downloading the runner or setting up the runner until we have verified that the virtual machine is able to contact GitHub.

Failure to communicate with github.com, e.g. because the virtual machine did not have an Internet connection, would previously result in an error in the script, at which point the virtual machine would reboot. This could result in a boot loop if we were unable to communicate with github.com for a long period.

@simonbs simonbs self-assigned this Nov 15, 2023
@simonbs simonbs merged commit 3458b7c into main Nov 15, 2023
@simonbs simonbs deleted the bugfix/ensure-github-available branch November 15, 2023 12:40
@xavierLowmiller
Copy link
Contributor

Would you consider changing the test from a ping to something like curl -s https://github.com > /dev/null?
Today, I ran into an issue where curl works fine on Github, but ping doesn't, so this might be a bad proxy for detecting availability.

It's also what reddit suggests for similar issues (in more profane words)

@simonbs
Copy link
Contributor Author

simonbs commented Jan 24, 2024

@xavierLowmiller Sure! I didn't know about this issue. I won't have time to address it the next few days but I'm happy to review and merge a PR that fixes it 🙏

@xavierLowmiller
Copy link
Contributor

Great! I'm happy to put one up ✌️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants