Skip to content

Smartlyio/fetch latest runner prerelease #141

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

Conversation

sjagoe
Copy link
Contributor

@sjagoe sjagoe commented Aug 17, 2020

Identify the Bug

#74: Runners don't pick up the first job that triggered scale-up due to updating to a pre-release of github actions runner.

Description of the Change

This change causes the runner sync lambda to pre-cache the latest pre-release of the github actions runner to prevent auto-update on start.

It achieves this by listing all releases, which includes pre-releases, and selecting the first release, which should be the most recent.

Alternate Designs

N/A

Possible Drawbacks

It is not explicitly documented that releases are provided in the order expected by this change. If the order provided by github changes, this will fail unless all pages of releases are retrieved and sorted by the client.

Verification Process

  • The unit tests are updated to match the expected API.
  • The code included here has been running live for about a week without any issues.

Release Notes

  • syncer will now sync the latest github actions runner pre-release, preventing any auto-updating on startup

@npalm
Copy link
Member

npalm commented Aug 18, 2020

Really great the work you did, but I having a second thought. I assume you had the same problem as we have. At GitHub they have crearted a pre-release. and did not make the release final. Which is in my opnion a mistake. Or otherwise the update script of the runner is trying to update with a pre release. Which I would call a bug. (see actions/runner#581)

As you mentioned the order of releases is not documented, but it seems the config.sh scripts uses also the listReleases api. I still hope to get a answer from GitHub what they think should be the approach. @gertjanmaas any opinion?

@gertjanmaas
Copy link
Contributor

I agree with @npalm

@sjagoe
Copy link
Contributor Author

sjagoe commented Aug 20, 2020

I agree; I'd really like to see this fixed by github; it shouldn't be up to this module to work around the pre-releases.

In any case, I'll continue to maintain the branch in my fork of your repo and use it internally, because we saw the pre-release update issues occur again only two weeks ago (see https://github.com/philips-labs/terraform-aws-github-runner/issues/74#issuecomment-669074264), and that pre-release was left in that state for at least a week; during that time we had jobs not running in our AWS pool.

I see that now we have runner v2.273.0 in pre-release. I would expect jobs to fail to run again due to the update on start.

@gertjanmaas
Copy link
Contributor

Based on this PR and the trouble with the latest prerelease, I've created #165 to make this an option. Does this suit your needs @sjagoe ? If so I propose to close this PR in favour of #165

@sjagoe
Copy link
Contributor Author

sjagoe commented Aug 25, 2020

@gertjanmaas Thanks, #165 looks great. I'll go ahead and close this.

@sjagoe sjagoe closed this Aug 25, 2020
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.

3 participants