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

GHA: Enable static check for s390x, aarch64 and ppc64le #8485

Merged
merged 15 commits into from Jan 19, 2024

Conversation

BbolroC
Copy link
Member

@BbolroC BbolroC commented Nov 21, 2023

This is to enable the static check (including a unit test) for s390x by incorporating it with the existing one for x86_64.
Once this PR is merged and the relevant workflows settle down, it will replace a Jenkins CI http://jenkins.katacontainers.io/job/kata-containers-2.0-ubuntu-s390x-unit-PR/.

Fixes: #8482

Test result: https://github.com/BbolroC/kata-containers/actions/runs/6945353895/job/18894516175 (all failing tests are for x86_64 and they have nothing to do with the change for this PR)

Signed-off-by: Hyounggyu Choi Hyounggyu.Choi@ibm.com
Signed-off-by: Jianyong Wu jianyong.wu@arm.com
Signed-off-by: Amulyam24 amulmek1@in.ibm.com

Copy link
Member

@fidencio fidencio 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 @BbolroC!

@BbolroC
Copy link
Member Author

BbolroC commented Nov 21, 2023

/test

Copy link
Member

@gkurz gkurz left a comment

Choose a reason for hiding this comment

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

lint: fix cargo clippy errors on s390x
This is to fix linting errors reported by cargo clippy for s390x.

This calls for two comments:

  1. this change doesn't have anything to do with this PR
  2. we should enforce linting to happen before code is merged into the repo

I'd prefer this fix to go to another PR unless it is required for this PR (in which case it would be appreciated to provide details in the commit log).

@BbolroC
Copy link
Member Author

BbolroC commented Nov 21, 2023

lint: fix cargo clippy errors on s390x
This is to fix linting errors reported by cargo clippy for s390x.

This calls for two comments:

  1. this change doesn't have anything to do with this PR

The change is necessary to go all the tests for this PR green on s390x.

  1. we should enforce linting to happen before code is merged into the repo

Yes, what you suggested is already configured for x86_64, which this PR will follow naturally. (e.g. you can see the linting result for s390x below https://github.com/kata-containers/kata-containers/actions/runs/6946479768?pr=8485)

I'd prefer this fix to go to another PR unless it is required for this PR (in which case it would be appreciated to provide details in the commit log).

As I mentioned above shortly, the commit is required for this PR because it was identified (e.g. https://github.com/BbolroC/kata-containers/actions/runs/6942202736/job/18884635456) while enabling make check for the {agent|kata-ctl} on s390x which had not been triggered by the Jenkins unit test (i.e. only make test was used). This error has not been found on x86_64 against the same code base (e.g. https://github.com/kata-containers/kata-containers/actions/runs/6944105755/job/18890547068?pr=8483) as follows:

let matcher = VirtioBlkCCWMatcher::new(&root_bus, &device);

uev_other_device.devpath = format!(
"{}/card{}/{}",
AP_ROOT_BUS_PATH,
card,
format!("{}.0002", card)

So, it would more make sense for me to handle the commit in this PR and supplement the commit messages if necessary. Thanks. 😉

@BbolroC BbolroC changed the title GHA: Add unit test for s390x GHA: Enable static check for s390x Nov 21, 2023
@BbolroC
Copy link
Member Author

BbolroC commented Nov 22, 2023

/test

@BbolroC
Copy link
Member Author

BbolroC commented Nov 22, 2023

I've noticed one thing which looks weird to me:

The following workflows were triggered by this PR:

Static checks / build-checks (log-parser-rs, make vendor, ubuntu-20.04) (pull_request)
Static checks / build-checks (log-parser-rs, make vendor, s390x) (pull_request) 

which have been successfully finished. But I can see another pending workflow like:

build-checks (log-parser-rs, make vendor)

which is waiting for status to be reported and never finished.

@fidencio and @gkurz any thoughts on this? Thanks!

Copy link
Member

@gkurz gkurz 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 @BbolroC !

@katacontainersbot katacontainersbot added the size/small Small and simple task label Nov 24, 2023
@BbolroC
Copy link
Member Author

BbolroC commented Nov 24, 2023

/test

@BbolroC
Copy link
Member Author

BbolroC commented Nov 29, 2023

The merge will be suspended due to an issue at #8527 until a similar PR for aarch64 and ppc64le is ready. Thanks.

@BbolroC BbolroC force-pushed the add-unit-test-s390x branch 2 times, most recently from 802e5be to 8373efb Compare December 4, 2023 12:27
@BbolroC
Copy link
Member Author

BbolroC commented Dec 4, 2023

For the reviewers: #8545 (aarch64) has been incorporated into this PR.

@BbolroC BbolroC changed the title GHA: Enable static check for s390x GHA: Enable static check for s390x, aarch64 and ppc64le Dec 4, 2023
@katacontainersbot katacontainersbot added size/medium Average sized task and removed size/small Small and simple task labels Jan 9, 2024
jongwu and others added 15 commits January 18, 2024 16:31
This is to add a runner for arm64 to the workflow.

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
PathBuf here is only used for x86.

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
As part of the CI migration from Jenkins to GitHub Action, a CI job named
`kata-containers-2.0-ubuntu-s390x-unit-PR` is covered by the static check.
This commit is to enable the check for s390x by incorporating a runner
`s390x` with the corresponding workflow.

Fixes: kata-containers#8482

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
If `yq_path` is set to `/usr/local/bin/yq`, there could be a situation
where the `yq` cannot be installed without `sudo`.
This commit handles the situation by putting `sudo` in front of `curl`
and `chmod`, respectively.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
Some linting errors were identified during the enablement of `make check`.
These have not been found by the Jenkins CI job because `make test` was
only triggered.

The errors for the `agent` occurs under the s390x specific tests while
the other ones for the `kata-ctl` are the architecture-specific code.

This commit is to fix those errors.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
At the moment, a project `dragonball` and `runtime-rs` does not support
for s390x. During the enablement, some errors due to the misconfiguration
of Makefile for `make check` and `make vendor` were identified.

This is to skip the build for the affected target of the projects.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
It was observed that a tmporary file `/tmp/kata_hybrid_vsock02.hvsock`
for test_setup_hvsock_failed() is not removed from time to time.
This leads to a test failure for the same test next time due to the
file permission on a self-hosted runner.
This commit is to explicitely delete the file before the check starts.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
This PR adds ppc64le runner to the static-checks workflow.

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
Since dragonball is not currently supported on ppc64le, skip running the targets for static-checks.

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
A few CPU related test cases were failing as the version was being verified against Power8 while the CI machine is Power9.

Fixes: kata-containers#5531

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
This minor PR removes the extra space in the makefiles.

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
…64le

This is to skip the test_init_container_create_launcher if not root on ppc64le.

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
kata-ctl currently fails to build on ppc64le. Skip it for running static checks and the issues will be fixed and tracked in a seperate issue.

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
This is to fix a compile error raised for aarch64.

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
 - test_volume_capacity_stats: verify the file block size against the fetched size via statfs()
 - test_reseed_rng: Correct the request codes for RNDADDTOENTCNT and RNDRESEEDCRNG when platform is ppc64le
 - test list_routes: Add the route only if destination is not empty
 - test_new_fs_manager: skip the test if cgroups v2 is used by default
 - skip test cases rpc::tests::test_do_write_stream, sandbox::tests::test_find_process, sandbox::t
ests::test_find_container_process and sandbox::tests::add_and_get_container on ppc64le as they are fl
aky

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
@katacontainersbot katacontainersbot added size/large Task of significant size and removed size/medium Average sized task labels Jan 18, 2024
@BbolroC
Copy link
Member Author

BbolroC commented Jan 18, 2024

/test

@gkurz gkurz merged commit b7d6b18 into kata-containers:main Jan 19, 2024
288 of 359 checks passed
@BbolroC BbolroC deleted the add-unit-test-s390x branch January 19, 2024 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/large Task of significant size
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GHA: no static check for s390x
6 participants