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

Windows self hosted runners #3379

Merged
merged 3 commits into from
Feb 13, 2025
Merged

Windows self hosted runners #3379

merged 3 commits into from
Feb 13, 2025

Conversation

DaMandal0rian
Copy link
Member

@DaMandal0rian DaMandal0rian commented Feb 10, 2025

This PR builds upon #3373 by introducing Windows self-hosted runners utilizing spot instances.

TODO

  • Replace the current custom image with the official Windows runner image once all required packages are available:
    Windows2022-Readme.md.

Current Status

  • The current image is a base Windows installation and is missing many essential packages listed in the official runner image readme.
  • The goal is to use AWS-compatible images that provide 1:1 compatibility with the official GitHub-hosted Windows image.

Temporary Workaround

  • Until the full Windows image is published by the maintainer, we are using a custom-built image with the necessary dependencies pre-installed.
  • This custom image is derived from our dedicated runners image and is required for this configuration.
  • More details on custom images can be found here:
    Custom Image Configuration.

Next Steps

Code contributor checklist:

Copy link
Member

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

This is just temporary until the Windows full image is published by the maintainer/provider, since the base image lacks packages required for our CI workflow

Which packages, what maintainer/provider? Do you have any links, maybe add TODO that points to upstream issue?

BTW is ubuntu22-full-x64 an official image, can we also switch to upstream there?

@DaMandal0rian DaMandal0rian force-pushed the windows-self-hosted-runners branch from 278147d to b406747 Compare February 10, 2025 11:06
@DaMandal0rian
Copy link
Member Author

DaMandal0rian commented Feb 10, 2025

This is just temporary until the Windows full image is published by the maintainer/provider, since the base image lacks packages required for our CI workflow

Which packages, what maintainer/provider? Do you have any links, maybe add TODO that points to upstream issue?

Powershell 7, bash and cargo, it's a simple base image and the provider runs-on is working on getting a 1:1 full image compatible with AWS. I have added the todo, and i had opened a service request with the developer directly for the issues.

BTW is ubuntu22-full-x64 an official image, can we also switch to upstream there?

No we can't the official images are built for Azure, not AWS, but they maintain 1:1 sync/compatibility with the upstream.

@nazar-pc
Copy link
Member

TODO: Usually explains what needs to be done and any preconditions, right now it just points to a readme with a list of software, I have no idea what needs to be done/why it is not used from the beginning.

Powershell 7, bash and cargo, it's a simple base image and the provider runs-on is working on getting a 1:1 full image compatible with AWS. I have added the todo, and i had opened a service request with the developer directly for the issues.

All 3 of those are in the list of available software at the link you have added as TODO, can you elaborate what you mean exactly?

No we can't the official images are built for Azure, not AWS, but they maintain 1:1 sync/compatibility with the upstream.

By official I meant from runs-on, not GitHub itself.

@DaMandal0rian
Copy link
Member Author

TODO: Usually explains what needs to be done and any preconditions, right now it just points to a readme with a list of software, I have no idea what needs to be done/why it is not used from the beginning.

Doesn't the PR description describe this? I can add all that in again for the "todo".

Powershell 7, bash and cargo, it's a simple base image and the provider runs-on is working on getting a 1:1 full image compatible with AWS. I have added the todo, and i had opened a service request with the developer directly for the issues.

All 3 of those are in the list of available software at the link you have added as TODO, can you elaborate what you mean exactly?

The developer only has a working minimal image, not the full image with all packages on a 1:1 compatibility with Official GH windows image. SImilar comment here

No we can't the official images are built for Azure, not AWS, but they maintain 1:1 sync/compatibility with the upstream.

By official I meant from runs-on, not GitHub itself.

It is the runs-on official image, i'm not using a custom image for Linux.

@nazar-pc
Copy link
Member

Doesn't the PR description describe this? I can add all that in again for the "todo".

Imagine you are a new person, looking at the code. You see some strange image used and explanation is:

todo: replace with official image from maintainer https://github.com/runs-on/runner-images-for-aws/blob/main/releases/windows22/x64/images/windows/Windows2022-Readme.md

You, obviously, follow the link and what do you see? A list of software available on official image.

Questions:

  • why is that image not used? all the official software seems to be there
  • if image was not fine at the time of the introduction what was wrong with it exactly? this is not clear to me still
  • if there was something wrong with an image, what is the tracking issue I can check to make sure the image is fixed today? again, I still don't understand what needs to happen and how can we confirm that it did happen

Just check some of the countless TODOs we have in the code. We sometimes use downstream forks temporarily and basically every time we have TODOs with links to upstream issues that explain the problem and with clear guidance when TODO can be resolved.

Nothing like this is present in the code right now and explanation that "Powershell 7, bash and cargo" are missing is misleading at best because the very link you have in TODO claims they are not in fact missing.

I'm sure you keep necessary information in your head and follow upstream issues, but you have to communicate these things clearly for reviewers and whoever might have to resolve the TODO in the future, which is sometimes a completely different person that will be scratching their head wondering what is going on there.

I see you've updated TODO just not saying:

The custom image will be replaced when this new full image becomes available.

How do I know if it is available? Is there an upstream issue about it to follow? If not can you create one and put a link to it so we have continuity of information?

The developer only has a working minimal image, not the full image with all packages on a 1:1 compatibility with Official GH windows image.

Did you create an issue with packages we're missing? Or maybe issues already exist, then can you provide an exhaustive list of them?

It is the runs-on official image, i'm not using a custom image for Linux.

👍

@DaMandal0rian DaMandal0rian force-pushed the windows-self-hosted-runners branch from 113f9f0 to dae2b18 Compare February 11, 2025 09:23
@DaMandal0rian DaMandal0rian merged commit d554caa into main Feb 13, 2025
5 of 8 checks passed
@DaMandal0rian DaMandal0rian deleted the windows-self-hosted-runners branch February 13, 2025 08:35
DaMandal0rian added a commit that referenced this pull request Feb 13, 2025
…unners"

This reverts commit d554caa, reversing
changes made to deff3e2.
DaMandal0rian added a commit that referenced this pull request Feb 13, 2025
…unners"

This reverts commit d554caa, reversing
changes made to deff3e2.
DaMandal0rian added a commit that referenced this pull request Feb 13, 2025
…unners"

This reverts commit d554caa, reversing
changes made to deff3e2.
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