Coder Social home page Coder Social logo

rust-vmm-ci's Introduction

rust-vmm-ci

The rust-vmm-ci repository contains integration tests and Buildkite pipeline definitions that are used for running the CI for all rust-vmm crates.

CI tests are executed on the container image maintained at rust-vmm/rust-vmm-container repo with builds available on Docker Hub.

Having a centralized place for the tests is one of the enablers for keeping the same quality standard for all crates in rust-vmm.

Getting Started with rust-vmm-ci

To run the integration tests defined in the pipeline as part of the CI:

  1. Add rust-vmm-ci as a git submodule to your repository
# Add rust-vmm-ci as a submodule. This will point to the latest rust-vmm-ci
# commit from the main branch. The following command will also add a
# `.gitmodules` file and the `rust-vmm-ci` to the index.
git submodule add https://github.com/rust-vmm/rust-vmm-ci.git
# Commit the changes to your repository so that the CI can run using the
# rust-vmm-ci pipeline and tests.
git commit -s -m "Added rust-vmm-ci as submodule"
  1. Create the coverage test configuration file named coverage_config_ARCH.json in the root of the repository, where ARCH is the architecture of the machine. There are two coverage test configuration files, one per each platform. The example of the configuration file for the x86_64 architecture can be found in coverage_config_x86_64.json.sample, and the example of the configuration file for the aarch64 architecture can be found in coverage_config_aarch64.json.sample.

The json must have the following fields:

  • coverage_score: The coverage of the repository.
  • exclude_path: This field is used for excluding files from the report. File paths that match the given regular expression are skipped. (for example, if multiple files are to be skipped, they must be separated with |). It should be used to exclude autogenerated files. If the repository does not have any autogenerated files, exclude_path should be an empty string.
  • crate_features: cargo kcov does not build crate features by default. To get the coverage report including optional features, these need to be specified in crate_features separated by comma. If the crate does not have any features, this field should be empty.

This file is required for the coverage integration so it needs to be added to the repository as well.

  1. Copy one of the two provided dependabot configurations to .github/dependabot.yml, e.g. run cp rust-vmm-ci/dependabot-{weekly,monthly}.yml .github/dependabot.yml. Note that just symlinking the file does not work, as dependabot will not follow symlinks into submodules. This means that updates to these files made in rust-vmm-ci will need to be manually consumed for now. We recommend setting up weekly dependabot updates only if the crate receives multiple contributions a week, and if you expect to have the bandwidth to address weekly dependency PRs.

  2. Create a new pipeline definition in Buildkite. For this step ask one of the rust-vmm Buildkite admins to create one for you. The process is explained here.

  3. There is a script that autogenerates a dynamic Buildkite pipeline. Each step in the pipeline has a default timeout of 5 minutes. To run the CI using this dynamic pipeline, you need to add a step that is uploading the rust-vmm-ci pipeline:

./rust-vmm-ci/.buildkite/autogenerate_pipeline.py | buildkite-agent pipeline upload

This allows overriding some values and extending others through environment variables.

  • X86_LINUX_AGENT_TAGS: overrides the tags by which the x86_64 linux agent is selected; the default values are

    {"os": "linux", "platform": "x86.metal"}

  • AARCH64_LINUX_AGENT_TAGS: overrides the tags by which the aarch64 linux agent is selected. The default values are

    {"os": "linux", "platform": "arm.metal"}

  • DOCKER_PLUGIN_CONFIG: specifies additional configuration for the docker plugin. For available configuration, please check the https://github.com/buildkite-plugins/docker-buildkite-plugin.

  • TESTS_TO_SKIP: specifies a list of tests to be skipped.

  • TIMEOUTS_MIN: overrides the timeout value for specific tests.

  • DEFAULT_AGENT_TAG_HYPERVISOR: sets the hypervisor on which all the tests in the pipeline run. By default, the selected hypervisor is KVM because the hosts running KVM at the time of this change showed better performance and experienced timeouts less often. NOTE: This will not override the hypervisor defined at the test step level. If a test already defines a hypervisor tag that will remain intact.

The variable TESTS_TO_SKIP is specified as a JSON list with the names of the tests to be skipped. The variable TIMEOUTS_MIN is a dictionary where each key is the name of a test and each value is the number of minutes for the timeout. The other variables are specified as dictionaries, where the first key is tests and its value is a list of test names where the configuration should be applied; the second key is cfg and its value is a dictionary with the actual configuration.

For example, we can skip the test commit-format, have a timeout of 30 minutes for the test style and extend the docker plugin specification as follows:

TESTS_TO_SKIP='["commit-format"]' TIMEOUTS_MIN='{"style": 30}' DOCKER_PLUGIN_CONFIG='{
    "tests": ["coverage"],
    "cfg": {
        "devices": [ "/dev/vhost-vdpa-0" ],
        "privileged": true
    }
}' ./rust-vmm-ci/.buildkite/autogenerate_pipeline.py | buildkite-agent pipeline upload

For most use cases, overriding or extending the configuration is not necessary. We may want to do so if, for example, the platform needs a custom device that is not available on the existing test instances or if we need a specialized hypervisor.

  1. The code owners of the repository will have to setup a WebHook for triggering the CI on pull request and push events.

Buildkite Pipeline

The Buildkite pipeline is the definition of tests to be run as part of the CI. It includes steps for running unit tests and linters (including coding style checks), and computing the coverage.

Currently the tests can run on Linux x86_64 and aarch64 hosts.

Example of step that checks the build:

steps:
- label: build-gnu-x86_64
  command: cargo build --release
  retry:
    automatic: false
  agents:
    os: linux
    platform: x86_64.metal
  plugins:
  - docker#v3.8.0:
      image: rustvmm/dev:v16
      always-pull: true
  timeout_in_minutes: 5

To see all steps in the pipeline check the output of the .buildkite/autogenerate_pipeline.py script.

Custom Pipeline

Some crates might need to test functionality that is specific to that particular component and thus cannot be added to the common pipeline.

In this situation, the repositories need to create a JSON file with a custom test configuration. The preferred path is .buildkite/custom-tests.json.

For example to test the build with one non-default feature enabled, the following configuration can be added:

{
  "tests": [
    {
      "test_name": "build-bzimage",
      "command": "cargo build --release --features bzimage",
      "platform": [
        "x86_64"
      ]
    }
  ]
}

To run this custom pipeline, you need to add a step that is uploading it in Buildkite. The same script that autogenerates the main pipeline can be used with the option -t PATH_TO_CUSTOM_CONFIGURATION:

./rust-vmm-ci/.buildkite/autogenerate_pipeline.py -t .buildkite/custom-tests.json | buildkite-agent pipeline upload

Integration Tests

In addition to the one-liner tests defined in the Buildkite Pipeline, the rust-vmm-ci also has more complex tests defined in integration_tests.

Test Profiles

The integration tests support two test profiles:

  • devel: this is the recommended profile for running the integration tests on a local development machine.
  • ci (default option): this is the profile used when running the integration tests as part of the Continuous Integration (CI).

The test profiles are applicable to pytest, the integration test framework used with rust-vmm-ci. Currently only the coverage test follows this model as all the other integration tests are run using the Buildkite pipeline.

The difference between is declaring tests as passed or failed:

  • with the devel profile the coverage test passes if the current coverage is equal or higher than the upstream coverage value. In case the current coverage is higher, the coverage file is updated to the new coverage value.
  • with the ci profile the coverage test passes only if the current coverage is equal to the upstream coverage value.

Further details about the coverage test can be found in the Adaptive Coverage section.

Adaptive Coverage

The line coverage is saved in tests/coverage. To update the coverage before submitting a PR, run the coverage test:

CRATE="kvm-ioctls"
# NOTE: This might not be the latest container version, you can check which one we're using
# by looking into the .buildkite/autogenerate_pipeline.py file.
LATEST=16
docker run --device=/dev/kvm \
           -it \
           --security-opt seccomp=unconfined \
           --volume $(pwd)/${CRATE}:/${CRATE} \
           rustvmm/dev:v${LATEST}
cd ${crate}
pytest --profile=devel rust-vmm-ci/integration_tests/test_coverage.py

If the PR coverage is higher than the upstream coverage, the coverage file needs to be manually added to the commit before submitting the PR:

git add tests/coverage

Failing to do so will generate a fail on the CI pipeline when publishing the PR.

NOTE: The coverage file is only updated in the devel test profile. In the ci profile the coverage test will fail if the current coverage is higher than the coverage reported in tests/coverage.

Performance tests

rust-vmm-ci includes an integration test that can run a battery of benchmarks at every pull request, comparing the results with the tip of the upstream main branch. The test is not included in the default Buildkite pipeline. Each crate that requires the test to be run as part of the CI must add a custom pipeline.

An example of a pipeline that runs the test for ARM platforms and prints the results:

steps:
- label: bench-aarch64
  command: pytest rust-vmm-ci/integration_tests/test_benchmark.py -s
  retry:
    automatic: false
  agents:
    os: linux
    platform: arm.metal
  plugins:
  - docker#v3.8.0:
      image: rustvmm/dev:v16
      always-pull: true

The test requires criterion benchmarks to be exported by the crate. The test expects the entry point into the performance benchmarks to be named main. In other words, the following configuration is expected in Cargo.toml:

[[bench]]
name = "main"

All benchmarks need to be collected in a main.rs file placed in benches/.

criterion collects performance results by running a function for a user-configured number of iterations, timing the runs, and applying statistics. The individual benchmark tests must be added in the crate. They can be run outside the CI with:

cargo bench [--all-features] OR [--features <features>]

rust-vmm-ci uses critcmp to compare the results yielded by cargo bench --all-features on the PR being tested with those from the tip of the upstream main branch. The test runs cargo bench twice, once on the current HEAD, then again after git checkout origin/main. critcmp takes care of the comparison, making use of criterion's stable format for output files. The results are printed to stdout and can be visually inspected in the pipeline output. In its present form, the test cannot fail.

To run the test locally:

docker run --device=/dev/kvm \
           -it \
           --security-opt seccomp=unconfined \
           --volume $(pwd)/${CRATE}:/${CRATE} \
           rustvmm/dev:v${LATEST}
cd ${CRATE}
pytest rust-vmm-ci/integration_tests/test_benchmark.py -s

Note that performance is highly dependent on the underlying platform that the tests are running on. The raw numbers obtained are likely to differ from their counterparts on a CI instance.

Running the tests locally

To run the integration tests locally, you can run the following from the crate you need to test. You can find the latest container version in the script that autogenerates the pipeline. For example:

cd ~/vm-superio
CRATE="vm-superio"
# NOTE: This might not be the latest container version, you can check which one we're using
# by looking into the .buildkite/autogenerate_pipeline.py file.
LATEST=16
docker run -it \
           --security-opt seccomp=unconfined \
           --volume $(pwd):/${CRATE} \
           --volume ~/.ssh:/root/.ssh \
           rustvmm/dev:v${LATEST}
cd vm-superio
./rust-vmm-ci/test_run.py

Known issues:

  • When running the cargo-audit test, the following error may occur:
test_cargo-audit (__main__.TestsContainer) ... error: couldn’t fetch advisory database: git operation failed: reference ‘refs/heads/main’ not found; class=Reference (4); code=NotFound (-3)

A fix for this is to remove ~/.cargo/advisory-db in the container, and then rerun test_run.py:

rm -rf ~/.cargo/advisory-db
./rust-vmm-ci/test_run.py

rust-vmm-ci's People

Contributors

ablu avatar aghecenco avatar ahmedaabouzied avatar alexandruag avatar alindima avatar andreeaflorescu avatar anth1y avatar catalindumitru avatar catdum avatar endeneer avatar epilys avatar federicoponzi avatar jonathanwoollett-light avatar keiichiw avatar lauralt avatar mathieupoirier avatar mrxinwang avatar roypat avatar russell-islam avatar shadowcurse avatar stefanharh avatar stefano-garzarella avatar vireshk avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

rust-vmm-ci's Issues

Find a better way to separate the mshv and kvm hosts

We need to be able to select the hypervisor for certain pipeline (for example, for kvm-ioctls, the hypervisor needs to be kvm). Right now the EC2 hosts don't specify a hypervisor, while the mshv hosts do. This needs to be configured on the host, so it's not directly related to this repository.

Nevertheless, we do need to update the code in this crate as well because we need some way of selecting the hypervisor at the crate level and or at the test level. When using the tests defined in rust-vmm-ci, we need to be able to override the agent tags. There is already some functionality to override tags using environment variables, so we should experiment with that as well.

Add test to check signed-off-by

A recurrent problem we have in rust-vmm PRs is that we have unsigned commits. Let's make this part of the CI as it is easy to miss during reviews.

Add support for workpace tests

For example the coverage test will not work on workspaces because we are using the --features flag which is invalid in workspaces and only allowed for packages.

Add possibility to ignore specific advisories in audit test

We've recently hit an issue where an advisory was issued for an unmaintained crate (atty - advisory here) that several rust-vmm crates depend on. Since there's no apparent way to fix the issue (unresponsive maintainer), we need to allow for certain advisories such as this one to be skipped in the cargo audit test. Otherwise, CI will fail and block.

Figure out the infrastructure for long running tests

Some, if not all, crates will require long running tests for more relevant performance measurements, as well as stability and more. This might resolve into dedicated Buildkite pipelines for each crate and may not be centralized in rust-vmm-ci, but until that's ironed out, we'll be tracking the matter in one issue. We can follow up with per-crate issues if it resolves to that.

Fix out-of-date path

Some paths are out of date in section "Adaptive Coverage".
Some error messages, like the one below, have out-of-date path too.
AssertionError: Coverage is increased from 67.9 to 69.6. Please update the coverage in tests/coverage.

Provide a way to configure the buildkite pipeline

The rust-vmm-ci provides the common tests to be run by all rust-vmm crates. The problem is that we have a few things that should be configurable.

One example is the possibility to configure the devices passed when running the unit tests. For example, vhost tests need access to the vdpa device, but adding it explicitly in the rust-vmm-ci excessively constrains the CI of all other components when this is not in fact needed (i.e. all other components need to run on a host that has access to the vdpa device, and the vdpa device needs to be passed to the docker image when running the tests). More details about this here: #64

Add support for selecting agent based on the hypervisor tag

Right now we only support selecting a buildkite agent to run the job based on the os (either windows or linux) and the platform tag (x86_64.metal or arm.metal). We need to add support for another agent tag as well so that the mshv CI can use the autogeneration tool to generate the tests.

Here is an example of the tags: https://github.com/rust-vmm/mshv/blob/1a9ca01801e78f8ddc2630660cbab3bc3300e39e/.buildkite/pipeline.yml#L43

The code that needs to be updated to support this is: https://github.com/rust-vmm/rust-vmm-ci/blob/main/.buildkite/autogenerate_pipeline.py#L211. We need to do a get for hypevisor, and set it in the agents tags. The code should be similar to the way we set the platform.

consider running cargo audit with --deny-warnings enabled

According to this comment, breaking changes related to cargo audit (which may turn into errors in time, see this thread and this PR) can be caught earlier if we run cargo audit with --deny-warnings enabled.

If we choose to enable this flag, cargo audit will also fail if we expose in Cargo.toml a version that was yanked. An example would be vm-superio: 0.1.0 was yanked, but master still points to 0.1.0 in Cargo.toml (with other commits on top). Basically, after we yank a version, cargo audit --deny-warning will force us to release a new version on the branch on which we are running the test if the toml file is pointing to the yanked version (which sounds pretty reasonable).

Deprecate static Buildkite pipeline

To be able to eventually delete the static pipeline from rust-vmm-ci we need to update the CI for the existing components to use the autogenerate script instead (as specified in the readme).

This issue tracks the progress per component:

  • vm-fdt
  • vhost-device
  • kvm-bindings
  • crate-template (N/A)
  • io-rate-limiter (N/A)
  • mshv
  • seccompiler
  • vhost-user-backend
  • vfio-ioctls
  • vm-memory
  • vm-virtio
  • vm-device
  • linux-loader
  • vhost
  • event-manager
  • vm-superio
  • vmm-sys-util
  • virtio-bindings
  • kvm-ioctls
  • vmm-reference
  • vfio-bindings
  • vm-allocator (N/A)

Expand the scope of clippy checks

The current way of invoking clippy as part of the CI pipeline (cargo clippy --all -- -D warnings) does not check some parts of the code. It looks like the exhaustive list of relevant arguments is something along the lines of cargo clippy --workspace --bins --examples --tests --benches --all-targets --all-features -- -D warnings, with the caveat that --all-features might not make sense for all crates.

cargo kcov on aarch64 seems a bit unreliable

Running kcov on aarch64 doesn't seem to work as expected.

For the same code on x86 vs aarch64 we get differences from 1-4% which seems a lot for medium sized code bases (>10k lines of code).

We should either fix the kcov problem or remove the test altogether.

Add test that checks the formatting of code examples

Formatting for code examples is only available in nightly for now. So we first need to resolve: rust-vmm/rust-vmm-container#36

I would propose we add a rustfmt.toml file as part of the test such that not all repositories need to be changed (also, cargo fmt will fail if you add some option in rustfmt.toml which is not supported in stable). The content of the file needs to be:

format_code_in_doc_comments = true

After running the test using cargo +nightly fmt, we need to delete the created file.

Running test_bench on the commit that introduces the benchmarks fails

The way test_bench works is that it checks weather there are any performance regressions introduced by running cargo bench on both the PR commits and on master.
This introduces an interesting problem because typically you add both the tests and the pipeline update to run those tests in the same PR. The PR will thus always fail because running cargo bench on master (when there are no tests) will fail with:

error: Unrecognized option: 'noplot'
error: bench failed

There is an obvious workaround:

  • merge the performance tests first (including the Cargo.toml configuration that makes that error disappear)
  • merge the pipeline (aka the update so that the tests run) in a following PR

I find this workaround to be pretty ugly because it requires tribal knowledge and can introduce confusion. Granted, it happens only the first time performance tests are added, but I still think it's pretty ugly.

Alternatively, what we can do is that we can only run cargo bench when the benchmarks exists.

I would propose the following change. Instead of running cargo bench --all-features -- --noplot --save-baseline {} and use the [lib] bench = false in Cargo.toml to avoid the problem specified here, we can use cargo bench --all-features --bench main -- --noplot --save-baseline {}. This implies that all benchmarks for all crates would need to be named main (or at least their entry point), which seems to be a lesser evil than having to merge first the tests and the Cargo.toml update and then the pipeline update. With this change then we can check if main benchmark is defined or not, and if it is not defined, we simply do not compare after with before.

The advantages of these proposal:

  • the performance tests run the first time they're introduced; this way we can see if the performance tests actually work (there will be no comparison, which sounds reasonable)
  • there is no need to have 2 PRs when you need to introduce the performance tests for the first time

WDYT?

[test_bench] Allow for different names for benchmarks

In test_bench, we now expect all benchmarks to be named main. This is because we are running the benchmarks as follows:
cargo bench --bench main --all-features -- --noplot --save-baseline {}.

Instead of harcoding the name to be main, we can parse the Cargo.toml file to check what is the name of the benchmarks.
We need to look for the bench target in Cargo.toml:

[[bench]]
name = "main"

Add test to check the commit message format

Add a test to check that the commit message has an appropriate format:

  • it has a summary no longer than 50 chars (we can also adjust this number if we think it's too small)
  • each line of the body is less than 72 chars

cargo kcov does not run tests for non-default features

Right now the coverage test is not running for all features, but just the default ones.

Ideally, we should find a way to run coverage on all features (similar to what we have for unit tests with --all-features flag). The --all-features flag does not work with cargo kcov.

Make test commit work with repositories specified with `git`

test_commit_format forever hangs when the repository is specified with git instead of https.
The reason it hangs is that doing git fetch in the docker container awaits user input to fetch from the unknown host (in this case github). A quick fix here is to run ssh-keyscan -t rsa github.com >> ~/.ssh/known_hosts before calling git fetch. This way at least the test will just fail, and not hang.

The second problem is that we need to forward the ssh keys on the host for git to work (and configure the ssh agent). No idea how to do that in a safe/sane way.

Cleanup after test_bench

In test_bench we are doing a git checkout on the upstream master branch to check the diff in performance between the PR and the current master. This is problematic because if test_bench is not the last test to be run, all other tests will run on master instead of the PR branch.

We should make sure to properly clean up after the test (at all times, even when the test fails).

Do not delete the kcov_output in profile devel

When running the integration tests with profile devel, we can leave the kcov_output directory so that it is easier to inspect the coverage report.
Alternatively, we can add a --no-cleanup so we don't leave lingering files on the system without the user being aware.

Expose vhost vdpa device to container

To unblock rust-vmm/vhost#33, several conditions must be met:

  1. CI kernel version should be >= 5.7 (currently 5.4)
  2. vhost vdpa kernel modules should be loaded (see rust-vmm/vhost#33 (comment))
  3. the /dev/vhost-vdpa-0 device should be exposed to the container (see rust-vmm/vhost#33 (comment))

1 and 2 can be fixed by updating the AMIs for the CI runners. This issue tracks step 3, for which we need to update the Buildkite pipeline so that the steps that involve running the unit tests can access the device. The device needs to be passed in the Buildkite yaml as devices: [ "/dev/vhost-vdpa-0" ] (see documentation).

Make it easy to run tests locally

Most of the tests in rust-vmm-ci are just Buildkite pipeline steps which makes it impossible to run them locally before submitting a PR.

One idea would be to have something similar to the coverage test we already have, but we need to think about a flexible design so that the same thing can be used in buildkite pipelines and still keep the nice individual step/rust command.

Bad owner or permissions on /root/.ssh/config

Running ./rust-vmm-ci/test_run.py locally fails for vhost-device repository.

Commands I ran:

$ docker run --device=/dev/kvm -it --security-opt seccomp=unconfined --volume /home/vireshk/vhost-device:/vhost-device --volume ~/.ssh:/root/.ssh rustvmm/dev:v13

cd vhost-device; ./rust-vmm-ci/test_run.py

Failure Message:

test_commit-format (main.TestsContainer) ... =============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.10.0, pluggy-1.0.0
rootdir: /vhost-device
plugins: timeout-1.4.2
collected 1 item

rust-vmm-ci/integration_tests/test_commit_format.py F [100%]

==================================================================================================== FAILURES =====================================================================================================
_______________________________________________________________________________________________ test_commit_format ________________________________________________________________________________________________

def test_commit_format():
    """
    Checks commit message format for the current PR's commits.

    Checks if commit messages follow the 50/72 git commit rule
    [https://www.midori-global.com/blog/2018/04/02/git-50-72-rule]
    and if commits are signed.
    """
    # Fetch the upstream repository.
    fetch_base_cmd = "git fetch {} {}".format(REMOTE, BASE_BRANCH)
    try:
        subprocess.run(fetch_base_cmd, shell=True, check=True)
    except subprocess.CalledProcessError:
      raise NameError(
            "The name of the base branch or remote is invalid. "
            "See test documentation for more details."
        ) from None

E NameError: The name of the base branch or remote is invalid. See test documentation for more details.

rust-vmm-ci/integration_tests/test_commit_format.py:42: NameError
---------------------------------------------------------------------------------------------- Captured stderr call -----------------------------------------------------------------------------------------------
Bad owner or permissions on /root/.ssh/config
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
============================================================================================= short test summary info =============================================================================================
FAILED rust-vmm-ci/integration_tests/test_commit_format.py::test_commit_format - NameError: The name of the base branch or remote is invalid. See test documentation for more details.
================================================================================================ 1 failed in 0.03s ================================================================================================
ERROR

Update test_commit_format.py to work locally

This test is taking a dependency on Buildkite specific infrastructure. Specifically, it expect the following environment variables to be set in order to run the test: BUILDKITE_PULL_REQUEST_BASE_BRANCH and BUILDKITE_REPO.

We need to find good defaults for those environment variables in case they're not set so that we can run the tests locally. The challenge here comes from identifying a good default for the BUILDKITE_PULL_REQUEST_BASE_BRANCH as this is the branch we use for figuring out which commits are part of the PR.

From a short round of thinking, I could not find a perfect solution. A meh solution would be to always assume that the base branch is the master of the configured origin remote. This is a meh solution because it implies that the developer updates their local master branch from the upstream master branch so that the diff in commits is truly what is on the local branch from which they wish to open a PR. This assumes good practices (for example PRs are never opened from the master branch).

Other solutions are welcomed.

Add a test pipeline for this repo

At a minimum it should check that commits are signed.

It would also be nice that each commit in this repository runs all tests in a repository. This would be useful to catch bugs such as the one fixed by #48

Build infrastructure for running performance tests on rust-vmm crates

Add a dedicated Buildkite pipeline for performance tests for each crate that has such tests (alongside the "standard" CI pipeline). In the future, this can help separate performance runs from build / CI runs, and split tests on different machines according to our needs.

It's worth looking into critcmp as it seems easy to run and get a verdict from.

Long term, long duration performance tests will demand separate runs (nightly / on some other schedule). To begin with, we can add performance tests in the CI because:

  • it's easier to add them there short term
  • it will give us early indicators of how long they take
  • it will help us assess the stability of results

Please update docker image to Rust 1.34

For rust-vmm/vm-memory#19 I would like to use optionally the integer atomic types from Rust 1.34. Right now I would have to keep them out of CI (and not use --all-features in the pipeline files), it would be great if you could update the Rust image that buildkite pulls.

Check for correct crates.io ownership for every crate in the repository

According to the publishing documentation1, CODEOWNERS and the gatekeper team must be owners of a crate published on crates.io in addition to the original crate authors. We can add a check that runs for every crate located in a repository that verifies the ownership status is correct with the crates.io API2.

Example:

Find all crates:

for manifest_file in $(find . -type f -name Cargo.toml | grep --invert-match "/target/" | grep --invert-match "/fuzz/"); do
  NAME=$(grep --max-count=1 name "${manifest_file}" | sed -e 's/name = "\(.\+\)"/\1/')
  if ! [ -z "${NAME}" ]; then
    echo ${NAME}
  fi
done

Check owner for a crate:

CRATE_NAME="$1"

echo "INFO: querying https://crates.io/api/v1/crates/${CRATE_NAME}/owners"

OWNERS_REPLY=$(curl --silent "https://crates.io/api/v1/crates/${CRATE_NAME}/owners")

echo "INFO: API reply was ${OWNERS_REPLY}"

case "${OWNERS_REPLY}" in
  *'{"errors":[{"detail":"Not Found"}]}'* ) echo "INFO: Crate ${CRATE_NAME} was not found on crates.io and is assumed unpublished." ; exit 0 ;; # Crate is not published.
esac

ERROR=""

case "${OWNERS_REPLY}" in
  *github:rust-vmm:gatekeepers* ) echo "OK: rust-vmm:gatekeepers is an owner." ;;
  * ) echo "ERROR: rust-vmm:gatekeepers team must be in ${CRATE_NAME}'s owners." ; ERROR="yes";;
esac

for owner in $(sed -e '/^#.\+$/d' -e 's/^[*]\s*//' -e 's/[@]//g' < CODEOWNERS); do
  case "${OWNERS_REPLY}" in
    *${owner}* )
      echo "OK: ${owner} is an owner."
      ;;
    * )
      echo "ERROR: ${owner} is not an owner." ; ERROR="yes"
  esac
done

if ! [ -z "${ERROR}" ]; then
  echo "Missing crate owners for ${CRATE_NAME}, please add them."
  exit 1
fi

Running it for virtio-bindings outputs:

INFO: querying https://crates.io/api/v1/crates/virtio-bindings/owners
INFO: API reply was {"users":[{"avatar":"https://avatars.githubusercontent.com/u/1043863?v=4","id":35756,"kind":"user","login":"sameo","name":"Samuel Ortiz","url":"https://github.com/sameo"},{"avatar":"https://avatars.githubusercontent.com/u/2815944?v=4","id":37110,"kind":"user","login":"andreeaflorescu","name":"Andreea Florescu","url":"https://github.com/andreeaflorescu"},{"avatar":"https://avatars.githubusercontent.com/u/8278356?v=4","id":53759,"kind":"user","login":"epilys","name":"Manos Pitsidianakis","url":"https://github.com/epilys"},{"avatar":"https://avatars.githubusercontent.com/u/115481277?v=4","id":186043,"kind":"user","login":"roypat","name":"Patrick Roy","url":"https://github.com/roypat"},{"avatar":"https://avatars.githubusercontent.com/u/46028664?v=4","id":1481,"kind":"team","login":"github:rust-vmm:gatekeepers","name":"gatekeepers","url":"https://github.com/rust-vmm"}]}
OK: rust-vmm:gatekeepers is an owner.
ERROR: alexandruag is not an owner.
OK: andreeaflorescu is an owner.
ERROR: jiangliu is not an owner.
ERROR: slp is not an owner.
ERROR: stsquad is not an owner.
OK: epilys is an owner.
Missing crate owners for virtio-bindings, please add them.

Footnotes

  1. https://github.com/rust-vmm/community/blob/main/MAINTAINERS.md#becoming-a-repository-maintainer

  2. https://doc.rust-lang.org/cargo/reference/registry-web-api.html#owners-list

Problems with building `vhost-device-sound` in musl

          Setting `export PKG_CONFIG_ALLOW_CROSS=1` was enough to get through that, but:
  1. I can't build pipewire with musl, it complains stdbool.h is missing. I tried installing musl-dev and setting the library path to musl's dir in .cargo/config.toml:

    [target.x86_64-unknown-linux-musl]
    linker = "ld"
    rustflags = ["-Ctarget-feature=-crt-static", "-Clink-self-contained=on", "-L/usr/lib/x86_64-linux-musl", "-Clink-args=--dynamic-linker /lib/ld-musl-x86_64.so.1"]

    but no dice. Any ideas?

  2. Disabling default features with --no-default-features gives me errors from rust-vmm crates:

Compilation errors..

   Compiling vhost-user-backend v0.10.1
error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:145:23
    |
145 |             .add_used(self.mem.memory().deref(), desc_index, len)
    |              -------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |              |
    |              required by a bound introduced by this call
    |
note: required by a bound in `add_used`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:193:20
    |
193 |     fn add_used<M: GuestMemory>(&mut self, mem: &M, head_index: u16, len: u32)
    |                    ^^^^^^^^^^^ required by this bound in `QueueT::add_used`
help: consider further restricting the associated type
    |
143 |     pub fn add_used(&mut self, desc_index: u16, len: u32) -> Result<(), VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                                       +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:159:40
    |
159 |         self.queue.enable_notification(self.mem.memory().deref())
    |                    ------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |                    |
    |                    required by a bound introduced by this call
    |
note: required by a bound in `enable_notification`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:201:31
    |
201 |     fn enable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error>;
    |                               ^^^^^^^^^^^ required by this bound in `QueueT::enable_notification`
help: consider further restricting the associated type
    |
158 |     pub fn enable_notification(&mut self) -> Result<bool, VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                         +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:164:41
    |
164 |         self.queue.disable_notification(self.mem.memory().deref())
    |                    -------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |                    |
    |                    required by a bound introduced by this call
    |
note: required by a bound in `disable_notification`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:204:32
    |
204 |     fn disable_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<(), Error>;
    |                                ^^^^^^^^^^^ required by this bound in `QueueT::disable_notification`
help: consider further restricting the associated type
    |
163 |     pub fn disable_notification(&mut self) -> Result<(), VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                        +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:169:39
    |
169 |         self.queue.needs_notification(self.mem.memory().deref())
    |                    ------------------ ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |                    |
    |                    required by a bound introduced by this call
    |
note: required by a bound in `needs_notification`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:212:30
    |
212 |     fn needs_notification<M: GuestMemory>(&mut self, mem: &M) -> Result<bool, Error>;
    |                              ^^^^^^^^^^^ required by this bound in `QueueT::needs_notification`
help: consider further restricting the associated type
    |
168 |     pub fn needs_notification(&mut self) -> Result<bool, VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                                        +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

error[E0308]: mismatched types
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:185:41
    |
185 |             .try_set_desc_table_address(GuestAddress(desc_table))?;
    |              -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^^ expected `GuestAddress`, found a different `GuestAddress`
    |              |
    |              arguments to this method are incorrect
    |
    = note: `GuestAddress` and `GuestAddress` have similar names, but are actually distinct types
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.12.2/src/guest_memory.rs:109:1
    |
109 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.13.1/src/guest_memory.rs:111:1
    |
111 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `vm_memory` are being used?
note: method defined here
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/queue.rs:132:12
    |
132 |     pub fn try_set_desc_table_address(&mut self, desc_table: GuestAddress) -> Result<(), Error> {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:187:41
    |
187 |             .try_set_avail_ring_address(GuestAddress(avail_ring))?;
    |              -------------------------- ^^^^^^^^^^^^^^^^^^^^^^^^ expected `GuestAddress`, found a different `GuestAddress`
    |              |
    |              arguments to this method are incorrect
    |
    = note: `GuestAddress` and `GuestAddress` have similar names, but are actually distinct types
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.12.2/src/guest_memory.rs:109:1
    |
109 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.13.1/src/guest_memory.rs:111:1
    |
111 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `vm_memory` are being used?
note: method defined here
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/queue.rs:147:12
    |
147 |     pub fn try_set_avail_ring_address(&mut self, avail_ring: GuestAddress) -> Result<(), Error> {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:189:40
    |
189 |             .try_set_used_ring_address(GuestAddress(used_ring))
    |              ------------------------- ^^^^^^^^^^^^^^^^^^^^^^^ expected `GuestAddress`, found a different `GuestAddress`
    |              |
    |              arguments to this method are incorrect
    |
    = note: `GuestAddress` and `GuestAddress` have similar names, but are actually distinct types
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.12.2/src/guest_memory.rs:109:1
    |
109 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
note: `GuestAddress` is defined in crate `vm_memory`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vm-memory-0.13.1/src/guest_memory.rs:111:1
    |
111 | pub struct GuestAddress(pub u64);
    | ^^^^^^^^^^^^^^^^^^^^^^^
    = note: perhaps two different versions of crate `vm_memory` are being used?
note: method defined here
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/queue.rs:161:12
    |
161 |     pub fn try_set_used_ring_address(&mut self, used_ring: GuestAddress) -> Result<(), Error> {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the trait bound `<M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory` is not satisfied
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/vhost-user-backend-0.10.1/src/vring.rs:210:23
    |
210 |             .used_idx(self.mem.memory().deref(), Ordering::Relaxed)
    |              -------- ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `vm_memory::guest_memory::GuestMemory` is not implemented for `<M as GuestAddressSpace>::M`
    |              |
    |              required by a bound introduced by this call
    |
note: required by a bound in `used_idx`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/virtio-queue-0.9.1/src/lib.rs:190:20
    |
190 |     fn used_idx<M: GuestMemory>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error>;
    |                    ^^^^^^^^^^^ required by this bound in `QueueT::used_idx`
help: consider further restricting the associated type
    |
208 |     fn queue_used_idx(&self) -> Result<u16, VirtQueError> where <M as GuestAddressSpace>::M: vm_memory::guest_memory::GuestMemory {
    |                                                           +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
error: could not compile `vhost-user-backend` (lib) due to 8 previous errors

Originally posted by @epilys in rust-vmm/vhost-device#493 (comment)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.