Merge pull request #728 from andreiltd/doc-tests

Fix documentation

.github/workflows/action-fmt.yml → .github/workflows/action-check.yml

@@ -13,7 +13,7 @@ on:
         type: string
 
 jobs:
-  fmt:
+  check:
     name: lint on ${{ inputs.os }}
     runs-on: ${{ inputs.os }}
     steps:
@@ -32,5 +32,3 @@ jobs:
           rustup toolchain install nightly --component rustfmt
       - name: Run checks
         run: make check-${{ inputs.runtime }}
-      - name: Check unused dependencies
-        uses: bnjbvr/[email protected]

.github/workflows/ci.yml

@@ -19,13 +19,13 @@ jobs:
       - uses: actions/checkout@v4
       - uses: actions/dependency-review-action@v4
 
-  fmt:
+  check:
     name: ${{ matrix.runtime }}
     strategy:
       matrix:
         os: ["ubuntu-latest", "windows-latest"]
         runtime: ["common", "wasmedge", "wasmtime", "wasmer"]
-    uses: ./.github/workflows/action-fmt.yml
+    uses: ./.github/workflows/action-check.yml
     with:
       os: ${{ matrix.os }}
       runtime: ${{ matrix.runtime }}
@@ -132,3 +132,36 @@ jobs:
     with:
       os: ${{ matrix.os }}
       runtime: ${{ matrix.runtime }}
+
+  docs:
+    name: docs
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - name: Setup build env
+      run: ./scripts/setup-linux.sh
+      shell: bash
+    - uses: actions-rust-lang/setup-rust-toolchain@v1
+    - uses: Swatinem/rust-cache@v2
+    - name: Check documentation
+      run: make test-doc
+    - name: Generate documentation
+      run: make generate-doc
+
+  deps:
+    name: unused dependencies
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - uses: actions-rust-lang/setup-rust-toolchain@v1
+    - uses: Swatinem/rust-cache@v2
+    - name: Check unused dependencies
+      uses: bnjbvr/[email protected]
+
+  spelling:
+    name: spell check with typos
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - name: Spell Check Repo
+      uses: crate-ci/typos@master

CONTRIBUTING.md

@@ -129,7 +129,7 @@ make fix
 
 ## Adding new features
 
-Most features will likely have most of the code in the `containerd-shim-wasm` project and a few runtime specific addtions to each runtime shim.  The general expectation is that the feature should be added to all runtimes. We will evaluate on a case by case basis exceptions, where runtimes may not be able to support a given feature or requires changes that make it hard to review.  In those cases we it may make sense to implement in follow up PR's for other runtimes.
+Most features will likely have most of the code in the `containerd-shim-wasm` project and a few runtime specific additions to each runtime shim.  The general expectation is that the feature should be added to all runtimes. We will evaluate on a case by case basis exceptions, where runtimes may not be able to support a given feature or requires changes that make it hard to review.  In those cases we it may make sense to implement in follow up PR's for other runtimes.
 
 A tip for developing a new feature is to implement it and test it with one runtime you are familiar with then add it to all the runtimes.  This makes it easier to test and iterate before making changes across all the runtimes.
 
@@ -150,7 +150,7 @@ This is a fast moving space, with lots of innovation happening and some shims ma
 A Shim implementation maybe subject to removal if:
  - If a shim runtime has not been maintained for 6 months it will be subject to removal.  
  - If required changes to the runtime can't be merged or not supported by runtime maintainers.
- - If it falls behind in new features added to the `containerd-shim-wasm` due to lack of maintainance
+ - If it falls behind in new features added to the `containerd-shim-wasm` due to lack of maintenance
 
 Before removal:
 - We will create an issue in the repository, pinned to the top. 

Makefile

@@ -130,6 +130,12 @@ test-%:
 	# run tests in one thread to prevent parallelism
 	RUST_LOG=trace $(CARGO) test $(TARGET_FLAG) --package containerd-shim-$* $(FEATURES_$*) --lib --verbose $(TEST_ARGS_SEP) --nocapture --test-threads=1
 
+test-doc:
+	RUST_LOG=trace $(CARGO) test --doc -- --test-threads=1
+
+generate-doc:
+	RUST_LOG=trace $(CARGO) doc --workspace --all-features --no-deps --document-private-items --exclude wasi-demo-app
+
 test-oci-tar-builder:
 	RUST_LOG=trace $(CARGO) test $(TARGET_FLAG) --package oci-tar-builder $(FEATURES_$*) --verbose $(TEST_ARGS_SEP) --nocapture --test-threads=1
 

README.md

@@ -18,7 +18,7 @@ on the CNCF slack.
 
 ## Usage
 
-runwasi is intended to be consumed as a library to be linked to from your own wasm host implementation. It creates one shim process per container or k8s pod. 
+runwasi is intended to be consumed as a library to be linked to from your own wasm host implementation. It creates one shim process per container or k8s pod.
 
 You need to implement a trait to teach runwasi how to use your wasm host.
 
@@ -145,7 +145,7 @@ make build
 sudo make install
 ```
 
-> Note: `make build` will only build one binary. The `make install` command copies the binary to $PATH and uses symlinks to create all the component described above. 
+> Note: `make build` will only build one binary. The `make install` command copies the binary to $PATH and uses symlinks to create all the component described above.
 
 Build the test image and load it into containerd:
 
@@ -174,15 +174,15 @@ So they'll continue singing it forever just because...
 (...)
 ```
 
-To kill the process, you can run in other session: `sudo ctr task kill -s SIGKILL testwasm`. 
+To kill the process, you can run in other session: `sudo ctr task kill -s SIGKILL testwasm`.
 
 The test binary supports commands for different type of functionality, check [crates/wasi-demo-app/src/main.rs](crates/wasi-demo-app/src/main.rs) to try it out.
 
 ### Demo 2 using OCI Images with custom WASM layers
 
 The previous demos run with an OCI Container image containing the wasm module in the file system.  Another option is to provide a cross-platform OCI Image that that will not have the wasm module or components in the file system of the container that wraps the wasmtime/wasmedge process.  This OCI Image with custom WASM layers can be run across any platform and provides for de-duplication in the Containerd content store among other benefits. To build OCI images using your own images you can use the [oci-tar-builder](./crates/oci-tar-builder/README.md)
 
-To learn more about this approach checkout the [design document](https://docs.google.com/document/d/11shgC3l6gplBjWF1VJCWvN_9do51otscAm0hBDGSSAc/edit).  
+To learn more about this approach checkout the [design document](https://docs.google.com/document/d/11shgC3l6gplBjWF1VJCWvN_9do51otscAm0hBDGSSAc/edit).
 
 > **Note**: This requires containerd 1.7.7+ and 1.6.25+.  If you do not have these patches for both `containerd` and `ctr` you will end up with an error message such as `mismatched image rootfs and manifest layers` at the import and run steps. Latest versions of k3s and kind have the necessary containerd versions.
 
@@ -198,7 +198,7 @@ Run the image with `sudo ctr run --rm --runtime=io.containerd.[ wasmedge | wasmt
 ```
 sudo ctr run --rm --runtime=io.containerd.wasmtime.v1 ghcr.io/containerd/runwasi/wasi-demo-oci:latest testwasmoci wasi-demo-oci.wasm echo 'hello'
 hello
-exiting 
+exiting
 ```
 
 ### Demo 3 using Wasm OCI Artifact
@@ -212,3 +212,6 @@ make test/k8s-oci-wasmtime
 ```
 
 > note: We are using a kubernetes cluster to run here since containerd's ctr has a bug that results in ctr: `unknown image config media type application/vnd.wasm.config.v0+json`
+
+### WASI/HTTP Demo for `wasmtime-shim`
+See [wasmtime-shim documentation](./crates/containerd-shim-wasmtime/README.md#WASI/HTTP).

crates/containerd-shim-wasm/README.md

@@ -6,31 +6,52 @@ A library to help build containerd shims for wasm workloads.
 
 ## Usage
 
-```rust
+```rust,no_run
 use containerd_shim as shim;
-use containerd_shim_wasm::sandbox::{ShimCli, Instance, Nop}
+use containerd_shim_wasm::sandbox::instance::Nop;
+use containerd_shim_wasm::sandbox::{ShimCli, Instance};
 
-shim::run::<ShimCli<Nop>>("io.containerd.nop.v1", opts);
+shim::run::<ShimCli<Nop>>("io.containerd.nop.v1", None);
 ```
 
 The above example uses the built-in `Nop` instance which does nothing.
 You can build your own instance by implementing the `Instance` trait.
 
-```rust
+```rust,no_run
+use std::time::Duration;
+use chrono::{DateTime, Utc};
+
 use containerd_shim as shim;
-use containerd_shim_wasm::sandbox::{ShimCli, Instance}
+use containerd_shim_wasm::sandbox::{Error, Instance, InstanceConfig, ShimCli};
 
 struct MyInstance {
- // ...
+    // ...
 }
 
 impl Instance for MyInstance {
-    // ...
+    type Engine = ();
+
+   fn new(_id: String, _cfg: Option<&InstanceConfig<Self::Engine>>) -> Result<Self, Error> {
+       todo!();
+    }
+    fn start(&self) -> Result<u32, Error> {
+       todo!();
+    }
+    fn kill(&self, signal: u32) -> Result<(), Error> {
+       todo!();
+    }
+    fn delete(&self) -> Result<(), Error> {
+       todo!();
+    }
+    fn wait_timeout(&self, t: impl Into<Option<Duration>>) -> Option<(u32, DateTime<Utc>)> {
+       todo!();
+    }
 }
 
-shim::run::<ShimCli<MyInstance>>("io.containerd.myshim.v1", opts);
+shim::run::<ShimCli<MyInstance>>("io.containerd.myshim.v1", None);
 ```
 
 containerd expects the shim binary to be installed into `$PATH` (as seen by the containerd process) with a binary name like `containerd-shim-myshim-v1` which maps to the `io.containerd.myshim.v1` runtime which would need to be configured in containerd. It (containerd) also supports specifying a path to the shim binary but needs to be configured to do so.
 
-This crate is not tied to any specific wasm engine.
+This crate is not tied to any specific wasm engine.
+

crates/containerd-shim-wasm/src/container/path.rs

@@ -4,9 +4,9 @@ pub trait PathResolve {
     // Resolve the path of a file give a set of directories as the `which` unix
     // command would do with components of the `PATH` environment variable, and
     // return an iterator over all candidates.
-    // Resulting candidates are files that exist, but no other constraing is
+    // Resulting candidates are files that exist, but no other constraint is
     // imposed, in particular this function does not check for the executable bits.
-    // Further contraints can be added by calling filtering the returned iterator.
+    // Further constraints can be added by calling filtering the returned iterator.
     fn resolve_in_dirs(
         &self,
         dirs: impl IntoIterator<Item = impl AsRef<Path>>,

crates/containerd-shim-wasm/src/sandbox/cli.rs

@@ -3,7 +3,7 @@ use std::path::PathBuf;
 use containerd_shim::{parse, run, Config};
 
 #[cfg(feature = "opentelemetry")]
-use crate::sandbox::shim::{otel_traces_enabled, OTLPConfig};
+use crate::sandbox::shim::{otel_traces_enabled, OtlpConfig};
 use crate::sandbox::{Instance, ShimCli};
 
 pub mod r#impl {
@@ -53,7 +53,7 @@ pub fn shim_main<'a, I>(
         use tokio::runtime::Runtime;
         let rt = Runtime::new().unwrap();
         rt.block_on(async {
-            let _guard = OTLPConfig::build_from_env()
+            let _guard = OtlpConfig::build_from_env()
                 .expect("Failed to build OtelConfig.")
                 .init()
                 .expect("Failed to initialize OpenTelemetry.");
@@ -84,9 +84,9 @@ fn shim_main_inner<'a, I>(
     {
         // read TRACECONTEXT env var that's set by the parent process
         if let Ok(ctx) = std::env::var("TRACECONTEXT") {
-            OTLPConfig::set_trace_context(&ctx).unwrap();
+            OtlpConfig::set_trace_context(&ctx).unwrap();
         } else {
-            let ctx = OTLPConfig::get_trace_context().unwrap();
+            let ctx = OtlpConfig::get_trace_context().unwrap();
             std::env::set_var("TRACECONTEXT", ctx);
         }
     }

crates/containerd-shim-wasm/src/sandbox/instance.rs

@@ -141,7 +141,7 @@ pub trait Instance: 'static {
         self.wait_timeout(None).unwrap()
     }
 
-    /// Waits for the instance to finish and retunrs its exit code
+    /// Waits for the instance to finish and returns its exit code
     /// Returns None if the timeout is reached before the instance has finished.
     /// This is a blocking call.
     fn wait_timeout(&self, t: impl Into<Option<Duration>>) -> Option<(u32, DateTime<Utc>)>;

crates/containerd-shim-wasm/src/sandbox/shim/local/tests.rs

@@ -15,11 +15,11 @@ use crate::sandbox::instance::Nop;
 use crate::sandbox::shim::events::EventSender;
 use crate::sandbox::shim::instance_option::InstanceOption;
 
-struct LocalWithDescrutor<T: Instance + Send + Sync, E: EventSender> {
+struct LocalWithDestructor<T: Instance + Send + Sync, E: EventSender> {
     local: Arc<Local<T, E>>,
 }
 
-impl<T: Instance + Send + Sync, E: EventSender> LocalWithDescrutor<T, E> {
+impl<T: Instance + Send + Sync, E: EventSender> LocalWithDestructor<T, E> {
     fn new(local: Arc<Local<T, E>>) -> Self {
         Self { local }
     }
@@ -31,7 +31,7 @@ impl EventSender for Sender<(String, Box<dyn MessageDyn>)> {
     }
 }
 
-impl<T: Instance + Send + Sync, E: EventSender> Drop for LocalWithDescrutor<T, E> {
+impl<T: Instance + Send + Sync, E: EventSender> Drop for LocalWithDestructor<T, E> {
     fn drop(&mut self) {
         self.local
             .instances
@@ -83,7 +83,7 @@ fn test_delete_after_create() {
         "test_namespace",
         "/test/address",
     ));
-    let mut _wrapped = LocalWithDescrutor::new(local.clone());
+    let mut _wrapped = LocalWithDestructor::new(local.clone());
 
     local
         .task_create(CreateTaskRequest {
@@ -115,7 +115,7 @@ fn test_cri_task() -> Result<()> {
         "/test/address",
     ));
 
-    let mut _wrapped = LocalWithDescrutor::new(local.clone());
+    let mut _wrapped = LocalWithDestructor::new(local.clone());
 
     let temp = tempdir().unwrap();
     let dir = temp.path();
@@ -288,7 +288,7 @@ fn test_task_lifecycle() -> Result<()> {
         "/test/address",
     ));
 
-    let mut _wrapped = LocalWithDescrutor::new(local.clone());
+    let mut _wrapped = LocalWithDestructor::new(local.clone());
 
     let temp = tempdir().unwrap();
     let dir = temp.path();

crates/containerd-shim-wasm/src/sandbox/shim/mod.rs

@@ -13,4 +13,4 @@ mod task_state;
 
 pub use cli::Cli;
 #[cfg(feature = "opentelemetry")]
-pub use otel::{traces_enabled as otel_traces_enabled, Config as OTLPConfig};
+pub use otel::{traces_enabled as otel_traces_enabled, Config as OtlpConfig};

crates/containerd-shim-wasm/src/sandbox/shim/otel.rs

@@ -7,11 +7,12 @@
 //! # Usage
 //!
 //! ```rust
-//! use containerd_shim_wasm::sandbox::shim::otel::Config;
+//! use containerd_shim_wasm::sandbox::shim::OtlpConfig;
+//! use containerd_shim_wasm::sandbox::shim::otel_traces_enabled;
 //!
 //! fn main() -> anyhow::Result<()> {
-//!     if traces_enabled() {
-//!         let otel_config = Config::build_from_env()?;
+//!     if otel_traces_enabled() {
+//!         let otel_config = OtlpConfig::build_from_env()?;
 //!    
 //!         let _guard = otel_config.init()?;
 //!    
@@ -69,7 +70,7 @@ pub fn traces_enabled() -> bool {
 ///
 /// Returns a `Result` containing the initialized tracer or a `TraceError` if initialization fails.
 ///
-/// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#configuration-options
+/// <https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#configuration-options>
 impl Config {
     pub fn build_from_env() -> anyhow::Result<Self> {
         let traces_endpoint = traces_endpoint_from_env()?;
@@ -98,7 +99,7 @@ impl Config {
 
     /// Returns the current trace context as a JSON string.
     pub fn get_trace_context() -> anyhow::Result<String> {
-        // propogate the context
+        // propagate the context
         let mut injector: HashMap<String, String> = HashMap::new();
         global::get_text_map_propagator(|propagator| {
             // retrieve the context from `tracing`

crates/containerd-shim-wasm/src/sandbox/stdio.rs

@@ -119,7 +119,7 @@ mod test {
 
     use super::*;
 
-    /// containerd can send an empty path or a non-existant path
+    /// containerd can send an empty path or a non-existent path
     /// In both these cases we should just assume that the stdio stream was not setup (intentionally)
     /// Any other error is a real error.
     #[test]