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

runner: add labels to distinguish OS types #15

Merged
merged 1 commit into from
Jan 16, 2024
Merged

runner: add labels to distinguish OS types #15

merged 1 commit into from
Jan 16, 2024

Conversation

rootfs
Copy link
Contributor

@rootfs rootfs commented Jan 12, 2024

No description provided.

@rootfs rootfs requested a review from a team as a code owner January 12, 2024 13:51
Copy link
Contributor

@SamYuan1990 SamYuan1990 left a comment

Choose a reason for hiding this comment

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

https://github.com/sustainable-computing-io/aws_ec2_self_hosted_runner/blob/main/.github/workflows/ci_unit.yml#L61
ref to here, as the label is a sub set of the runner we wanted, may I know why this change ensure the test running at specific runner?

@SamYuan1990
Copy link
Contributor

ref to log at https://github.com/sustainable-computing-io/aws_ec2_self_hosted_runner/actions/runs/7490317153/job/20388985460#step:1:2 and https://github.com/sustainable-computing-io/aws_ec2_self_hosted_runner/actions/runs/7490317153/job/20388981956#step:1:2 as currently the label is default or a sub set of instance labels, the github runner random pick up an instance to run the test. Hence I am curious on the reason.

entrypoint.sh Outdated
yum install -y curl jq libicu
else
# add ubuntu to the label
RUNNER_LABEL="ubuntu,${RUNNER_LABEL}"
Copy link
Contributor

@SamYuan1990 SamYuan1990 Jan 12, 2024

Choose a reason for hiding this comment

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

ref https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#choosing-self-hosted-runners

If you specify an array of strings or variables, your workflow will execute on any runner that matches all of the specified runs-on values. For example, here the job will only run on a self-hosted runner that has the labels linux, x64, and gpu:

IMO, when the test cases running in parallel, we will have three instances.

  1. rhel,self-hosted,linux,x86
  2. ubuntu, self-hosted,linux,x86 # 22.04
  3. ubuntu, self-hosted,linux,x86 # 20.04

as our condition as belowing

runs-on: [self-hosted, linux, x64]

how does the label work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SamYuan1990 yes, this is going to be interesting. Would you like to make a matrix that way? We don't have ubuntu version tag (22.04 and 20.04 are the same in this sense), but rhel and ubuntu are now in the runner labels.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, I would like to use instance id to specific an instance for test.
@rootfs , could you please help test instance id can be a validate label or instance id as a hash value which too long for label?
if instance id is too long, we can use a label group as Operator system.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is not possible to know the instance id before hand so we cannot use it to set up tests. The instance type is known (e.g. t2.micro). See the attached image.

@rootfs
Copy link
Contributor Author

rootfs commented Jan 12, 2024

here is the label
image

@rootfs
Copy link
Contributor Author

rootfs commented Jan 15, 2024

@SamYuan1990 any feedback? once we have the additional labels, can you refactor the CI? thanks.

@rootfs rootfs merged commit dfa5bbb into main Jan 16, 2024
22 checks passed
@rootfs rootfs mentioned this pull request Jan 16, 2024
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