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

Add E2E tests on arm64 #15230

Merged
merged 1 commit into from
Feb 2, 2023
Merged

Add E2E tests on arm64 #15230

merged 1 commit into from
Feb 2, 2023

Conversation

kevinzs2048
Copy link
Contributor

@kevinzs2048
Copy link
Contributor Author

This PR is regarded with #14748

Just a PR to add e2e test for Arm64 to check if it is green.
If the job is OK, will change it to a periodical job.

@kevinzs2048
Copy link
Contributor Author

@dims @geetasg @chaochn47 first time commit here, so needs maintainers to approve first. Thanks!

@kevinzs2048
Copy link
Contributor Author

Hi @ahrtr, thanks!
This one is to add e2e test for Arm64 to verify the function.
After it work, I will add the integration test in the tests.yaml and remove the functional test since it is going to be deprecated.

The reason why it is located to a separate file is that it needs to run on a special Arm64 node and also this will be changed to periodical job.

@@ -0,0 +1,31 @@
name: E2E-arm64
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding a separate workflow, can you add arm64 as a job to existing "E2E" workflow?

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't work because the ARM machines have different labels. FYI. #15172 (comment)

Probably we can add a template to be shared by both the existing E2E workflow and ARM64. But I don't think it's a big deal, It's OK to leave it as it's for now.

Copy link
Member

@serathius serathius Feb 2, 2023

Choose a reason for hiding this comment

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

Labels are per job not per workflow.

jobs:
  test:
    runs-on: [Linux, ARM64]

You can change it to:

jobs:
  amd64:
    runs-on: ubuntu-latest
    ...
  arm64:
    runs-on: [Linux, ARM64]

@ahrtr
Copy link
Member

ahrtr commented Feb 1, 2023

this will be changed to periodical job.

Why?

@dims
Copy link
Contributor

dims commented Feb 1, 2023

Why?

@ahrtr the aim is not to be a bottleneck to the current set of CI jobs that are run before we land a PR. The problem is we have only 2 github runners for arm64 and if we have a lot of PRs in flight the time that folks have to wait to land something will be a lot.

@ahrtr
Copy link
Member

ahrtr commented Feb 1, 2023

The problem is we have only 2 github runners for arm64 and if we have a lot of PRs in flight the time that folks have to wait to land something will be a lot.

Makes sense. Thanks @dims for the clarification.

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you @kevinzs2048

You might want to change it to periodical job before merging.

@kevinzs2048
Copy link
Contributor Author

@ahrtr Thanks! Modify to periodical job now.

The perviously submit for the e2e-arm64 job is green.

@ahrtr
Copy link
Member

ahrtr commented Feb 2, 2023

Please squash the commits.

Now it is daily nightly build at 1 am.

Signed-off-by: Kevin Zhao <kevin.zhao@linaro.org>
@kevinzs2048
Copy link
Contributor Author

Please squash the commits.

Thanks! Done

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @kevinzs2048

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

Successfully merging this pull request may close these issues.

4 participants