Skip to content

Comments

add reusable package for managing windows shims#2601

Open
rawahars wants to merge 1 commit intomicrosoft:mainfrom
rawahars:feat/shim-pkg
Open

add reusable package for managing windows shims#2601
rawahars wants to merge 1 commit intomicrosoft:mainfrom
rawahars:feat/shim-pkg

Conversation

@rawahars
Copy link
Contributor

This pull request introduces a new reusable Windows containerd shim bootstrap package. The main goal is to provide a generic, shareable shim entrypoint and eventing logic to avoid code duplication across Windows shim binaries.

Key changes:

Shim Bootstrap Implementation:

  • Introduced the pkg/shim package, which provides a reusable shim entrypoint for Windows containerd shims, handling CLI commands (start, serve, delete), ttrpc server setup, event publishing, and logging. Includes a detailed README.md with usage instructions and responsibilities.
    Core Logic:
  • Implemented the serve command to set up ttrpc server, handle log redirection, event publishing, and clean shutdowns.
  • Implemented the delete command to clean up container resources, handle panic logs, and ensure protocol-compliant responses.
  • Centralized CLI setup and argument parsing in shim.go, including ETW logging and OpenCensus tracing.

Event Publishing Infrastructure:

  • Added a Publisher interface and eventPublisher implementation for sending task events back to containerd, including OpenCensus tracing integration.

Testing:

  • Added unit tests for the new logic: limited file reading, option parsing, error trapping, and event publishing no-op behavior.

Documentation:

  • Added README.md for the new shim package, describing its purpose, usage, and integration points for consumers.

@helsaawy
Copy link
Contributor

theres already containerd/pkg/shim that implements a lot of the management and parsing needed, can we leverage that?

@rawahars
Copy link
Contributor Author

@helsaawy Using the upstream shim pkg would have been the absolute right way to go. However, Windows implementation is not yet robust upstream. Reference- https://github.com/containerd/containerd/blob/main/pkg/shim/shim_windows.go

Also, we have custom logic related to setting up ETW provider among other things.

In my opinion, as a follow-up to the new shim, we can work to ensure that containerd's shim pkg has robust Windows implementation and then move our new shim to the upstream package once it is released.

What are your thoughts?

Existing `containerd-shim-runhcs-v1` was tightly coupled with the code used for managing the shim lifecycle and other boilerplate code. Since we are adding new shims for Windows, the newly added `shim` package will be used for the boilerplate management code.

This commit adds the same.

Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
@helsaawy
Copy link
Contributor

Even without the Run logic to start and manage a shim server, integrating with it now would allow moving to the BootstrapParams containerd is now expecting (instead of the "address" file)
especially for utilities like ReadRuntimeOptions and the event publisher plugin

@rawahars
Copy link
Contributor Author

@helsaawy While I totally agree, the upstream shim implementation has stubbed out even newServer method which is used for creating the TTRPC server. Without that we cannot integrate at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants