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

Remove Engine from InstanceConfig #774

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub trait Instance {
type Engine: Send + Sync + Clone;

/// Create a new instance
fn new(id: String, cfg: Option<&InstanceConfig<Self::E>>) -> Self;
fn new(id: String, cfg: &InstanceConfig) -> Self;
/// Start the instance
/// The returned value should be a unique ID (such as a PID) for the instance.
/// Nothing internally should be using this ID, but it is returned to containerd where a user may want to use it.
Expand Down
1 change: 0 additions & 1 deletion benches/containerd-shim-benchmarks/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ edition.workspace = true
containerd-shim-wasm = { path = "../../crates/containerd-shim-wasm", features = ["testing"] }
containerd-shim-wasmedge = { path = "../../crates/containerd-shim-wasmedge" }
containerd-shim-wasmtime = { path = "../../crates/containerd-shim-wasmtime" }
wasmtime = { workspace = true }

[dev-dependencies]
criterion = { version = "0.5", features = ["html_reports"] }
Expand Down
17 changes: 2 additions & 15 deletions benches/containerd-shim-benchmarks/benches/wasmtime-benchmarks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::time::Duration;
use containerd_shim_wasm::container::Instance;
use containerd_shim_wasm::sandbox::Error;
use containerd_shim_wasm::testing::WasiTest;
use containerd_shim_wasmtime::instance::{WasiConfig, WasmtimeEngine};
use containerd_shim_wasmtime::instance::WasmtimeEngine;
use criterion::measurement::WallTime;
use criterion::{criterion_group, criterion_main, BenchmarkGroup, Criterion};

Expand Down Expand Up @@ -53,21 +53,8 @@ use criterion::{criterion_group, criterion_main, BenchmarkGroup, Criterion};
of a longer benchmarking time). Running the whole suite on a desktop
computer takes now a bit over 10 minutes.
*/
#[derive(Clone)]
struct WasiTestConfig {}

impl WasiConfig for WasiTestConfig {
fn new_config() -> wasmtime::Config {
let mut config = wasmtime::Config::new();
// Disable Wasmtime parallel compilation for the tests
// see https://github.com/containerd/runwasi/pull/405#issuecomment-1928468714 for details
config.parallel_compilation(false);
config.wasm_component_model(true); // enable component linking
config
}
}

type WasmtimeTestInstance = Instance<WasmtimeEngine<WasiTestConfig>>;
type WasmtimeTestInstance = Instance<WasmtimeEngine>;

fn run_wasmtime_test_with_spec(wasmbytes: &[u8]) -> Result<u32, Error> {
let (exit_code, _, _) = WasiTest::<WasmtimeTestInstance>::builder()?
Expand Down
3 changes: 3 additions & 0 deletions crates/containerd-shim-wasm/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),

### Changed
- Reuse and synchronise access to `Container` object instead of reloading form disk ([#763](https://github.com/containerd/runwasi/pull/763))
- Require `Engine` generic in `Instance` to implement `Default` ([#774](https://github.com/containerd/runwasi/pull/774))
- The method `Instance::new` now takes an `&InstanceConfig` instead of `Option<&InstanceConfig>` ([#774](https://github.com/containerd/runwasi/pull/774))

### Removed
- Removed `containerd_shim_wasm::sandbox::instance_utils::get_instance_root` and `containerd_shim_wasm::sandbox::instance_utils::instance_exists` functions ([#763](https://github.com/containerd/runwasi/pull/763))
- Removed `Engine` generic from `InstanceConfig` ([#774](https://github.com/containerd/runwasi/pull/774))

## [v0.8.0] — 2024-12-04

Expand Down
2 changes: 1 addition & 1 deletion crates/containerd-shim-wasm/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ struct MyInstance {
impl Instance for MyInstance {
type Engine = ();

fn new(_id: String, _cfg: Option<&InstanceConfig<Self::Engine>>) -> Result<Self, Error> {
fn new(_id: String, _cfg: &InstanceConfig) -> Result<Self, Error> {
todo!();
}
fn start(&self) -> Result<u32, Error> {
Expand Down
4 changes: 2 additions & 2 deletions crates/containerd-shim-wasm/src/container/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub trait Engine: Clone + Send + Sync + 'static {
/// The cached, precompiled layers will be reloaded on subsequent runs.
/// The runtime is expected to return the same number of layers passed in, if the layer cannot be precompiled it should return `None` for that layer.
/// In some edge cases it is possible that the layers may already be precompiled and None should be returned in this case.
fn precompile(&self, _layers: &[WasmLayer]) -> Result<Vec<Option<Vec<u8>>>> {
fn precompile(_layers: &[WasmLayer]) -> Result<Vec<Option<Vec<u8>>>> {
bail!("precompile not supported");
}

Expand All @@ -78,7 +78,7 @@ pub trait Engine: Clone + Send + Sync + 'static {
/// "runwasi.io/precompiled/<Engine.name()>/<unique_string>"
///
/// When it returns None the runtime will not be asked to precompile the module. This is the default value.
fn can_precompile(&self) -> Option<String> {
fn can_precompile() -> Option<String> {
None
}
}
Loading
Loading