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.

Sign up for GitHub

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

Jump to bottom

Introduce riscv64 CI #159

Merged
merged 1 commit into from
Sep 2, 2024
Merged

Introduce riscv64 CI #159

merged 1 commit into from
Sep 2, 2024

Conversation

TimePrinciple
Copy link
Contributor

Summary of the PR

Introduce logic necessary for generating YAML needed by BuildKite, which are designed to work with image introduced in
rust-vmm/rust-vmm-container#106.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

@roypat roypat mentioned this pull request Aug 2, 2024
Merged
4 tasks
stefano-garzarella
stefano-garzarella reviewed Aug 5, 2024
View reviewed changes
.buildkite/autogenerate_pipeline.py Outdated Show resolved Hide resolved
.buildkite/autogenerate_pipeline.py Outdated Show resolved Hide resolved
.buildkite/autogenerate_pipeline.py Show resolved Hide resolved
@TimePrinciple TimePrinciple force-pushed the introduce-riscv-ci branch 2 times, most recently from 10d9b98 to ffb6948 Compare August 6, 2024 11:50
roypat
roypat reviewed Aug 8, 2024
View reviewed changes
Copy link
Collaborator

@roypat roypat left a comment

Choose a reason for hiding this comment

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

Thanks :) Just some grammar nits.

.buildkite/autogenerate_pipeline.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@TimePrinciple TimePrinciple force-pushed the introduce-riscv-ci branch 2 times, most recently from 0bc69ce to 1421aac Compare August 8, 2024 09:36
stefano-garzarella
stefano-garzarella reviewed Aug 8, 2024
View reviewed changes
.buildkite/autogenerate_pipeline.py Outdated Show resolved Hide resolved
.buildkite/autogenerate_pipeline.py Outdated Show resolved Hide resolved
@stefano-garzarella
Copy link
Member

The PR LGTM, please rebase it.

@TimePrinciple
Copy link
Contributor Author

The PR LGTM, please rebase it.

Thanks for reviewing, rebased now :)

roypat
roypat reviewed Aug 27, 2024
View reviewed changes
Copy link
Collaborator

@roypat roypat left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Let's wait for the rust-vmm-container CI to build the new containers, and then let's bump CONTAINER_VERSION here to get a final sanity check on everything :) But then we're good to go I think!

@TimePrinciple
Copy link
Contributor Author

Let's wait for the rust-vmm-container CI to build the new containers, and then let's bump CONTAINER_VERSION here to get a final sanity check on everything :) But then we're good to do I think!

Shall I do it in this PR or raise another one 🧐

@roypat
Copy link
Collaborator

roypat commented Aug 27, 2024

Let's wait for the rust-vmm-container CI to build the new containers, and then let's bump CONTAINER_VERSION here to get a final sanity check on everything :) But then we're good to do I think!

Shall I do it in this PR or raise another one 🧐

In this PR please :)

@TimePrinciple
Copy link
Contributor Author

Let's wait for the rust-vmm-container CI to build the new containers, and then let's bump CONTAINER_VERSION here to get a final sanity check on everything :) But then we're good to do I think!

Shall I do it in this PR or raise another one 🧐

In this PR please :)

Container version updated to v39 :)

@roypat
Copy link
Collaborator

roypat commented Aug 27, 2024

Let's wait for the rust-vmm-container CI to build the new containers, and then let's bump CONTAINER_VERSION here to get a final sanity check on everything :) But then we're good to do I think!

Shall I do it in this PR or raise another one 🧐

In this PR please :)

Container version updated to v39 :)

I think it'll need to be v40 (... which is my fault, I forgot to do a PR to update it to v39 when my rust toolchain update container got merged. Sorry!)

@TimePrinciple
Copy link
Contributor Author

Let's wait for the rust-vmm-container CI to build the new containers, and then let's bump CONTAINER_VERSION here to get a final sanity check on everything :) But then we're good to do I think!

Shall I do it in this PR or raise another one 🧐

In this PR please :)

Container version updated to v39 :)

I think it'll need to be v40 (... which is my fault, I forgot to do a PR to update it to v39 when my rust toolchain update container got merged. Sorry!)

Never mind, I've force pushed to v40 :)

@roypat
Copy link
Collaborator

roypat commented Aug 28, 2024

Mh, I just realized that it didn't even try to spawn the riscv64 CI jobs for this PR. Can we add the .platform file to this repository, so that the CI sanity checks we have also always run on riscv64? I think for this we'll need to make the .platform file discovery code slightly more clever, see for instance how the coverage test does it: https://github.com/rust-vmm/rust-vmm-ci/blob/main/integration_tests/test_coverage.py#L16

@TimePrinciple
Copy link
Contributor Author

TimePrinciple commented Aug 28, 2024 •
edited
Loading

Mh, I just realized that it didn't even try to spawn the riscv64 CI jobs for this PR. Can we add the .platform file to this repository, so that the CI sanity checks we have also always run on riscv64? I think for this we'll need to make the .platform file discovery code slightly more clever, see for instance how the coverage test does it: https://github.com/rust-vmm/rust-vmm-ci/blob/main/integration_tests/test_coverage.py#L16

I gather that we need a mechanism to detect where this script is being executed, in a normal repo or submodule, and invoke CI accordingly. I will work it out.

@TimePrinciple
Introduce riscv64 CI
96e401e 
Introduce logic necessary for generating YAML needed by BuildKite, which
are designed to work with image introduced in
rust-vmm/rust-vmm-container#106.

The container version is updated to v44 to enable CI on RISC-V platform.

Signed-off-by: Ruoqing He <[email protected]>
roypat
roypat approved these changes Sep 2, 2024
View reviewed changes
Copy link
Collaborator

@roypat roypat left a comment

Choose a reason for hiding this comment

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

🎉

@stefano-garzarella stefano-garzarella merged commit 752ad13 into rust-vmm:main Sep 2, 2024
2 checks passed
@stefano-garzarella
Copy link
Member

I just opened #162 to fix formatting issue highlighted by black on main: https://github.com/rust-vmm/rust-vmm-ci/actions/runs/10662874591/job/29550878188

@TimePrinciple TimePrinciple deleted the introduce-riscv-ci branch September 2, 2024 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stefano-garzarella stefano-garzarella stefano-garzarella approved these changes

@roypat roypat roypat approved these changes

@andreeaflorescu andreeaflorescu Awaiting requested review from andreeaflorescu andreeaflorescu is a code owner

@lauralt lauralt Awaiting requested review from lauralt lauralt is a code owner

@sameo sameo Awaiting requested review from sameo sameo is a code owner

@ShadowCurse ShadowCurse Awaiting requested review from ShadowCurse ShadowCurse is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TimePrinciple @stefano-garzarella @roypat