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

Add a to_any helper in the containerd client crate #363

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

estesp
Copy link
Member

@estesp estesp commented Jan 14, 2025

Fixes: #362

This provides a solution to Any->type matching that happens on the containerd Go-based server side where the Any types in prost use typeurl as defined in the protobuf spec and the server side is matching on the fullname property of the type.

@github-actions github-actions bot added the C-client Containerd client label Jan 14, 2025
@estesp
Copy link
Member Author

estesp commented Jan 15, 2025

ping @mxpv - not sure what is going on with CI; looks like an error in the shim package tests, but not sure how it could be related to my change?

@mxpv
Copy link
Member

mxpv commented Jan 15, 2025

Looks like the formatter is not happy with imports:
image

@estesp
Copy link
Member Author

estesp commented Jan 15, 2025

Thanks! Fixed that, but looks like many CI jobs are having the same output about a type definition in the shim, with a recommended fix (but I have no idea if that's a change somewhere in the CI underpinning platform that caused that or a weird by-product of my change?):

   Compiling containerd-shim v0.7.4 (/Users/runner/work/rust-extensions/rust-extensions/crates/shim)
error[E0308]: mismatched types
   --> crates/shim/src/asynchronous/mod.rs:181:53
    |
181 |             let task_service = create_task(Arc::new(task));
    |                                            -------- ^^^^ expected `Box<dyn Task + Send + Sync>`, found associated type
    |                                            |
    |                                            arguments to this function are incorrect
    |
    = note:       expected struct `std::boxed::Box<dyn containerd_shim_protos::shim_async::Task + std::marker::Send + Sync>`
            found associated type `<T as asynchronous::Shim>::T`
    = help: consider constraining the associated type `<T as asynchronous::Shim>::T` to `std::boxed::Box<dyn containerd_shim_protos::shim_async::Task + std::marker::Send + Sync>`
    = note: for more information, visit https://doc.rust-lang.org/book/ch19-03-advanced-traits.html
    = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html
note: associated function defined here
   --> /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/alloc/src/sync.rs:386:12
help: store this in the heap by calling `Box::new`
    |
181 |             let task_service = create_task(Arc::new(Box::new(task)));
    |                                                     +++++++++    +

For more information about this error, try `rustc --explain E0308`.
error: could not compile `containerd-shim` (lib) due to 1 previous error

@estesp
Copy link
Member Author

estesp commented Jan 16, 2025

Not sure how to debug CI here; Taking a simple job like “Timings” and running it locally (even with my PR change), I can’t reproduce the error. How are Rust versions managed in GH Actions; any chance something changed?

Here is my output:

cargo build --all-features --timings
   Compiling containerd-shim-protos v0.7.2 (/home/ubuntu/go/src/github.com/containerd/rust-extensions/crates/shim-protos)
   Compiling containerd-snapshots v0.3.0 (/home/ubuntu/go/src/github.com/containerd/rust-extensions/crates/snapshots)
   Compiling containerd-client v0.7.0 (/home/ubuntu/go/src/github.com/containerd/rust-extensions/crates/client)
   Compiling runc v0.2.0 (/home/ubuntu/go/src/github.com/containerd/rust-extensions/crates/runc)
   Compiling containerd-shim-logging v0.1.1 (/home/ubuntu/go/src/github.com/containerd/rust-extensions/crates/logging)
   Compiling containerd-shim v0.7.4 (/home/ubuntu/go/src/github.com/containerd/rust-extensions/crates/shim)
   Compiling containerd-runc-shim v0.1.1 (/home/ubuntu/go/src/github.com/containerd/rust-extensions/crates/runc-shim)
      Timing report saved to /home/ubuntu/go/src/github.com/containerd/rust-extensions/target/cargo-timings/cargo-timing-20250116T182114Z.html
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 24.34s

@mxpv
Copy link
Member

mxpv commented Jan 16, 2025

Ya, there is something weird going on with ttrpc, the whole CI is broken. I'll TAL in a bit.

@mxpv mxpv mentioned this pull request Jan 16, 2025
This provides a solution to Any->type matching that happens on
the containerd Go-based server side where the Any types in prost
use typeurl as defined in the protobuf spec and the server side
is matching on the fullname property of the type.

Signed-off-by: Phil Estes <estesp@amazon.com>
@estesp
Copy link
Member Author

estesp commented Jan 16, 2025

Looks like we are good now! Thanks

@mxpv mxpv added this pull request to the merge queue Jan 16, 2025
Merged via the queue into containerd:main with commit 2d47d88 Jan 16, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-client Containerd client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

containerd client needs a to_any() helper
2 participants