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

[Patch] Set the default runner to ubuntu-22.04 and expose it as input everywhere #135

Merged
merged 8 commits into from
Oct 17, 2024

Conversation

mbruns91
Copy link
Contributor

I pick up this one

@mbruns91
Copy link
Contributor Author

push-pull.yml, pyproject-release.yml and test-and-converage.yml already exposed the runner.

@mbruns91 mbruns91 requested a review from liamhuber October 16, 2024 13:39
@mbruns91
Copy link
Contributor Author

mbruns91 commented Oct 16, 2024

shall we also set the default to ubuntu-22.04 in this PR? From my perspective it make sense to define defaults that are compatible with the subsequent commands (e.g. pip install ...).

Edit: nevermind, I just did it. if you object this one, we can just revert the resp. commit.

@mbruns91 mbruns91 changed the title Expose runner everywhere [Patch] Set the default runner to ubuntu-22.04 and expose it as input everywhere Oct 16, 2024
Copy link
Member

@liamhuber liamhuber left a comment

Choose a reason for hiding this comment

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

Yep, looks good to me.

It turns out I was likely wrong that this was an image bug and is rather a new "feature" of the latest ubuntu itself. I still like this as a short term patch while we get things rolling, but I opened a new issue (#136) to discuss a more long-term solution -- almost certainly one of the ideas you already proposed elsewhere.

@liamhuber
Copy link
Member

Ah, @mbruns91, wait! It looks like "ubuntu-latest" will be rolled back to 22:

Comment:

Hey everyone - thank you for your input. Based on community feedback we have decided to rollback the ubuntu-latest migration. All runs on ubuntu-latest will go back to Ubuntu-22 while we work out the issues with the image. We apologize for any service disruption you all may have had, and appreciate everyone’s feedback and patience. The rollback will be completed in the next hour.

To give some context for the changes we made - our images have become very large over the past 2 years and we were trying to free up some space to give us some breathing room going forward. There is currently no extra free space left for installing any more tooling, while still maintaining our free space SLA of 14gb. The size of the image also impacts performance of the VMs as well. Since a new OS is always a cutover, we took the opportunity to free up some space. Clearly this was not the right approach, and we apologize for the pain we have caused.

We will re-evaluate our communication and rollout strategy before beginning this migration again.

This PR should still go forward to expose the runner arg, but we can probably get away without updating the default image.

@liamhuber
Copy link
Member

liamhuber commented Oct 16, 2024

Yep, it's rolled back to 22.04 already. Let's ditch the default change, but keep the runner exposure at input

@mbruns91
Copy link
Contributor Author

Yep, it's rolled back to 22.04 already. Let's ditch the default change, but keep the runner exposure at input

I do not strongly object this, but personally I'd prefer to "properly" define the default runner. This makes it less ambiguous on what backend our CI tasks are running.
From a (OS) support perspective, ubuntu 22.04 is totally fine for the next 4 years or so. So no realistic risk of using outdated stuff. But more importantly, we will avoid similar issues in the future. When we want to upgrade the runner image being used as default, it will be much easier by testing on a selected repository - via the exposed runner input - before rolling it out as a new default.

@liamhuber
Copy link
Member

I can live with an explicitly pinned default, go for merge

@mbruns91 mbruns91 merged commit 23bb441 into main Oct 17, 2024
@mbruns91 mbruns91 deleted the expose_runner_everywhere branch October 17, 2024 14:41
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