Move docs check #86
Merged
Move docs check #86
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The method name has changed in commit 1c3c411 Author: Egor Lazarchuk <yegorlz@amazon.co.uk> Date: Fri Nov 3 11:35:49 2023 +0000 test(integration): updated vhost-user-block test but was not updated in a parallel PR during a rebase. Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
Missing slash in socket path construction. Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
This is to visually separate virtio and vhost-user block test groups. Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
Rename block/block_metrics.rs to block/metrics.rs since it makes more sense to deduce metrics from block (or other device) namespaces. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Since the per METRICS pool is static to the file and it is understood which module it belongs to, we don't need to prefix module name to it so, remove module prefix from per device METRICS pool. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Moving vsock metrics out of logger helps decouple vsock metrics from logger and be consistent with block and net which too have moved their metrics out of logger. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Since vsock metrics are moved to vsock::metrics module and are now accessible via vsock::metrics::METRICS, use that instead of METRICS.vsock. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Use existing vsock tests to validate to make sure that there is no breaking change in vsock metrics. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Qemu vhost-user-blk backend uses O_DIRECT when accessing the block file. Retrieving randwrite metric for this test does not represent significant value, because it is unlikely to be used in production setups due to low performance. Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
Introduces a function contract for GCD. The contract is checked against the implementation to ensure that it is satisfied and can also be used as a stub in other Kani harnesses for increased efficiency. The effect is a setup very similar to what was done before manually with stubs and a check for the implementation, but now with only a single source of truth (the contract). Signed-off-by: Justus Adam <justbldr@amazon.com>
Without this, cargo ignores the .cargo/config file in the temporary checkout, and instead uses the one from the branch on which the A/B-test script is being run. This means it would be impossible to A/B-test the impact of compiler options. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
We used to pass the -m nonci option to pytest invocations in A/B-testing because the performance tests are marked as nonci. However, we want to be able to also run some PR CI tests are A/B tests now. For this, pass -m '' to pytest instead, which instructs it to collect all tests, regardless of how they are marked. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
Make the test A/B compatible by outputing multiple metrics. We do not need a large sample size here, as the test is very stable. Also add it as an A/B-test to the post-PR pipeline. We choose a low nosie-threshold of 0.01 because the test produces very stable values. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
It used to be 2024, which is seemingly a typo. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
Define metrics for vhost-user devices. These are metrics that are independent of the device type. They refer to events during initialization, configuration and activation of a vhost-user device. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Use 3 metrics for vhost user block device: 1. to report activate failures. 2. to report failure to read config space. 3. to report time taken to init the device in micro seconds. 4. to report time taken to activate the device in micro seconds. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Report addition of vhost user device metrics with an example. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Flush vhost user device metrics as part of METRICS flush using a proxy serializer. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Make FirecrackerMetrics schema local to validate function so that vhost-user schema added by different tests don't conflict with each other. Move timestamp validation up so that the check doesn't generate any false results. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Add schema to validate breaking change in vhost-user metrics. Re-organize metrics validation function to accommodate vhost-user which has per device metrics but does not have aggregate metrics. Use existing vhost-user tests to check that there is no breaking change in vhost-user-block device metrics. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Add basic test to check shared inc and store metrics. Signed-off-by: Sudan Landge <sudanl@amazon.com>
c5n.metal instances have an Skylake CPU like:
Intel(R) Xeon(R) Platinum 8124M CPU
Identify it as such.
Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
This makes the tests work in other instances that use the same CPU. Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
Move EntropyDeviceMetrics out of logger to entropy device module. Add a proxy in logger to flush serialized entropy metrics. Add a basic unit test for entropy metrics. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Move `report_balloon_event_fail` to ballon since it is used only within that module. This will also help in limiting visibility of BallonDeviceMetrics when we move it to balloon as part of next refactor commit. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Move balloon device metrics out of logger to devices. Add a proxy in logger to flush serialized metrics. Add a basic unit test for balloon metrics. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Move i8042 device metrics out of logger to be consistent with the approach to have device metrics in device modules. Instead of having flush_metrics() in i8042 module, define it in legacy so that same function can later be used to flush other legacy metrics. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Move rtc device metrics out of logger to devices. Flush rtc device metrics through legacy flush_metrics(). Signed-off-by: Sudan Landge <sudanl@amazon.com>
Move serial device metrics out of logger to devices. Flush serial device metrics through legacy flush_metrics(). Signed-off-by: Sudan Landge <sudanl@amazon.com>
Rename METRICS.vsock to METRICS.vsock_ser to be consistent with naming of other proxy serializers. Signed-off-by: Sudan Landge <sudanl@amazon.com>
Because now `vmm` thread can update vhost-user-block config it needs `sendmsg` and `recvmsg` to talk to the backend. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
This is to allow PATCH /drives for vhost-usr-blk. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk> Co-authored-by: Nikita Kalyazin <kalyazin@amazon.com>
The test issues a PATCH /drives request to a vhost-user-blk drive and expects for the guest to observe the new device size. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk> Co-authored-by: Nikita Kalyazin <kalyazin@amazon.com>
Added `config_change_time_us` to the metrics for vhost-user devices. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Added `config_change_time_us` metric to the vhost-user-block device. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Currently, our runs get triggered on `/ref/heads/main`. For consistency with other pipelines, normalize that to just `main`. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
This caused the triggered buildkite run to be on the literal string `"$GITHUB_REF_NAME"` instead of the value of the environment variable. Fixes: cfc8040 Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
Use the double fork method to daemonize jailer as suggested in https://0xjet.github.io/3OHA/2022/04/11/post.html 1st fork to make sure setsid doesn't fail and 2nd fork is to make sure the daemon code cannot reacquire a controlling terminal. Signed-off-by: Sudan Landge <sudanl@amazon.com>
firecracker-microvm#4047 updated parsing the logger level filter. It removed all case variants outside fully uppercase (e.g. `INFO`) and the 1st character being upper case (e.g. `Info`) this removed support for other previously supported variants e.g. `info`. This commit re-introduces this support such that all variants should again be supported. Fixes commit 332f218 Signed-off-by: Jonathan Woollett-Light <jcawl@amazon.co.uk> Signed-off-by: Sudan Landge <sudanl@amazon.com>
Add unit test to cover all cases of LevelFilter. Remove redundant logic from levelfilter_from_str because levelfilter_from_str_all_variants is a better place to put the logic. Signed-off-by: Jonathan Woollett-Light <jcawl@amazon.co.uk> Signed-off-by: Sudan Landge <sudanl@amazon.com>
This error variant was used to encode that no error actually happened, which does not make much sense conceptually. What made this worse is that is contained a FcExitCode, which is itself just a fake Result<(), non-zero-exit code>. This means it was possible to get Firecracker to exit with status "error, but not actually error, but actually there is an error after all", or: "Firecracker exited with an error: Microvm stopped without an error: GenericError". The underlying problem here is the fact that we are using `FcExitCode` as an error variant for `Result`. Since `FcExitCode::Ok` exists, `FcExitCode` is a kind of `Result` itself, meaning we are dealing with `Result<_, Result<_, _>>` as a type, which has no well-defined interpretation. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
Using FcExitCode as an error type is undesirable, as it allows us to construct Err(FcExitCode::Ok), e.g. an object that says "error: everything's okay!". This is confusing and has caused problems in different contexts before, so replace FcExitCode with a proper error type here. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
Previously, when a VM exited, we looked for the first vcpu that reported an exit status, and then indiscriminately reported that back. However, it is possible to one vcpu to exit successfully while another exits with an error, and this could lead us to report "Firecracker Exited Successfully" even though it did not. Now, we explicitly look for the status code of all vcpus. If any of them report an error, this takes precedence over non-error status codes. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
Its helpful to know percentage Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
It did not have a help message before. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
Updated `BlockDeviceConfig` to have `file_engine_type` as an optional filed, instead of a filed with default value. This is done, because for vhost-user-block this field is not used, but it would still be have some value when user would request the block config. With this change config for vhost-user-block will return `None` for `file_engine_type`. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
It is technically a bug fix Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
It was the only entry where the PR link was missing. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
It was forgotten about in that PR. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
vhost-user-blk is in developer preview, which means that the API may change without deprecation process. Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
vhost-user-blk is in developer preview, which means that the API may change without deprecation process. Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
A developer preview warning log message is printed every time a new vhost-user-blk device is created. Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
This would be more visible to users. See also: https://github.com/orgs/community/discussions/16925 Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
Log exit code when exiting firecracker and read it during test Signed-off-by: Jing Yang <k.jingyang@gmail.com>
Remove unnecessary let Signed-off-by: Jing Yang <k.jingyang@gmail.com>
In the 7vcpu case, tests were getting very close to taking 120s, an din some cases even hitting this timeout during cleanup, causing spurious failures. Instead us pytests default timeout of 300s, as all other tests do (I forgot to relax the timeout of this test when I increased the number of iterations during A/B-ification). Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
Fixes: fc6a36e Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
Bumps [aiohttp](https://github.com/aio-libs/aiohttp) from 3.8.6 to 3.9.0. - [Release notes](https://github.com/aio-libs/aiohttp/releases) - [Changelog](https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst) - [Commits](aio-libs/aiohttp@v3.8.6...v3.9.0) --- updated-dependencies: - dependency-name: aiohttp dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Our internal check to alarm us if a post-PR A/B-test fails does not fire if a "bailed out" run (e.g. one where we decide not to run A/B-tests because we only modified, say, .md files) happens while a previous A/B-test is still running. This is because it only looks at the most recent buildkite run. Therefore, move the check whether to run an A/B-test at all from buildkite to the GitHub action that orchestrates the runs, so that "bail outs" happen outside of buildkite. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
...
Reason
...
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following
Developer Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.PR Checklist
CHANGELOG.md.TODOs link to an issue.rust-vmm.