service: add unit tests for LCOW v2 shim service layer by shreyanshjain7174 · Pull Request #2649 · microsoft/hcsshim

shreyanshjain7174 · 2026-03-26T14:09:50Z

Adds unit tests for the LCOW v2 shim service layer. The service orchestrates sandbox lifecycle (create / start / stop / wait / shutdown / status / metrics) by delegating to vm.Controller; without these tests every code change had to be validated against a live HCS environment.

Design

The Service struct holds a single vmController interface field (defined in types.go). Production passes *vm.Controller; tests pass a generated mock. The interface is a superset that includes the methods pod.New needs (Guest(), SCSIController(), Plan9Controller(), NetworkController() etc.) so the same field satisfies both Service's direct calls and pod.New's narrower interface via Go structural typing - no concrete-pointer leakage. This mirrors the merged internal/controller/pod pattern.

Coverage

49 tests across three files matching the production layout:

service_sandbox_internal_test.go (23): duplicate sandbox rejection, missing config.json, VM create / start failure, sandbox-id mismatch guards, stop success / failure / idempotency, wait clean / error / wait-failure / exit-status-failure exits, status mapping for every state, ping not-implemented, shutdown idempotency / running-VM termination / terminate-error swallowed, metrics success and stats failure.
service_shimdiag_internal_test.go (10): pid passthrough, exec-in-host success / failure, tasks / share / stacks state guards, share missing host-path validation, stacks dump-failure and success.
service_task_internal_test.go (16): consolidated state-guard and unknown-container guard tables, not-implemented surfaces, shutdown no-op, update validation (nil resources rejected, dispatch by resource type, per-resource failure surfaces), enrichNotFoundError pass-through and ErrNotFound preservation.

Out of scope

The full CreateSandbox -> BuildSandboxConfig -> CreateVM path is exercised by internal/builder/vm/lcow/ tests and the test/parity/vm/ parity suite; the service-level surface is the orchestration logic only.
The confidential StartSandbox path reads boot files from disk; belongs in functional tagged tests.

Mocks

mocks/mock_service.go is checked in. Standardising mock generation (directive in test file + CI drift validation) is tracked in #2707.

Defines a narrow vmController interface in the service package and threads it through Service in place of the concrete *vm.Controller field. Production passes vm.New(); tests pass a generated gomock. The interface includes the methods pod.New requires (Guest, SCSIController, VPCIController, Plan9Controller, NetworkController) so the same field satisfies both Service's direct calls and pod.New's narrower interface via Go structural typing. Tests cover the production failure paths through the service RPC surface: Sandbox API (23 tests): - duplicate Create rejection, missing config.json, VM create / start failure - sandbox-id mismatch guards - stop success / failure / idempotency - wait clean / error / wait-failure / exit-status-failure exits - status mapping for every state, ping not-implemented - shutdown idempotency, running-VM termination, terminate-error swallowed - metrics success and stats failure Shimdiag API (10 tests): - pid passthrough, exec-in-host success / failure - tasks / share / stacks state guards - share missing host-path validation - stacks dump-failure and success Task API (16 tests): - consolidated state-guard and unknown-container guard tables - not-implemented surfaces, shutdown no-op - update validation: nil resources rejected, dispatch by resource type, per-resource failure surfaces - enrichNotFoundError pass-through and ErrNotFound preservation Mocks committed at mocks/mock_service.go with the standard build tag (windows && lcow). Standardising mock generation across controllers is tracked in microsoft#2707. Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>

…rt grouping Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>

…empDir() for missing-path test Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>

rawahars

service_task_internal_test.go does not include tests for-

createInternal

startInternal

stateInternal

deleteInternal

pidsInternal

killInternal

execInternal

resizePtyInternal

closeIOInternal

waitInternal

statsInternal

rawahars · 2026-06-30T08:18:27Z

+	}
+}
+
+// ─── diagShareInternal tests ──────────────────────────────────────────────


Add a success case as well. In the mock request, ensure that the params received by the controller are same as what were set in the test.

rawahars · 2026-06-30T08:18:48Z

+	}
+}
+
+// ─── diagTasksInternal tests ──────────────────────────────────────────────


Add a success case as well. In the mock request, ensure that the params received by the controller are same as what were set in the test.

rawahars · 2026-06-30T08:19:26Z

+	svc, mockCtrl := newTestService(t)
+
+	req := &shimdiag.ExecProcessRequest{Args: []string{"/bin/ls"}, Workdir: "/"}
+	mockCtrl.EXPECT().ExecIntoHost(gomock.Any(), req).Return(42, nil)


Can you verify in this mock request that the params received by the controller are same as the ones set in test?

rawahars · 2026-06-30T08:23:24Z

+	}
+}
+
+// ─── createSandboxInternal tests ──────────────────────────────────────────


Add a success case as well. In the mock request, ensure that the params received by the controller are same as what were set in the test.

rawahars · 2026-06-30T10:41:59Z

+	// over the concrete VM controller, so that the vmController interface only
+	// needs to declare the methods the service itself calls; the VM guest and
+	// device accessors that pod.New consumes are not surfaced on that interface.
+	newPod func(podID, networkNamespaceID string) *pod.Controller


Please don't introduce new abstractions in service struct. Please use pod.New itself.

rawahars · 2026-06-30T10:42:40Z


 // NewService creates a new instance of the Service with the shared state.
 func NewService(ctx context.Context, eventsPublisher shim.Publisher, sd shutdown.Service) *Service {
+	vmc := vm.New()


Why move this outside the struct?

Addresses review feedback (rawahars): createSandboxInternal had only error-path coverage (duplicate, missing config.json, CreateVM failure). Adds a happy-path test that asserts the CreateOptions forwarded to vmController.CreateVM are derived from the request (ID, Owner, BundlePath, parsed SandboxSpec) instead of matching gomock.Any(), and that sandboxID is recorded only after CreateVM succeeds. Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>

Introduce a consumer-side containerController interface that *linuxcontainer.Controller satisfies as-is: the child getters keep returning the concrete *process.Controller, so the production controller needs no adapter or wrapper. Make getContainerController an injectable package-level function variable so unit tests can substitute a mock container controller without building a real pod/container chain. Add success-path tests for the one-hop delegation methods (Pids, Stats, DeleteProcess exec, Kill single-container, Update container, Start init), asserting the forwarded arguments and response mapping. Methods that take a second hop through the process controller are intentionally left to functional coverage. Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>

shreyanshjain7174 · 2026-06-30T14:37:19Z

Added success-path unit tests for the one-hop container-delegation methods (Pids, Stats, DeleteProcess exec, Kill single-container, Update container, Start init) via a small consumer-side containerController interface that *linuxcontainer.Controller satisfies as-is — no adapter or production signature change — with getContainerController made an injectable package-level function variable for the mock.

On why the rest don't get success-path unit tests: state, exec, resizePty, closeIO, wait, start (exec path), create, and diagTasks make a second hop through the process controller via ctrCtrl.GetProcess() → *process.Controller. Mocking that hop would force GetProcess to return an interface, i.e. an adapter on the production controller, so they stay on functional coverage.

shreyanshjain7174 requested a review from a team as a code owner March 26, 2026 14:09

shreyanshjain7174 force-pushed the service/unit-tests branch 4 times, most recently from 72ac4ca to c703ec9 Compare March 30, 2026 09:27

shreyanshjain7174 force-pushed the service/unit-tests branch from c703ec9 to d0fa692 Compare April 6, 2026 09:31

shreyanshjain7174 mentioned this pull request Apr 8, 2026

Adds unit tests and parity tests for the V2 shim #2668

Open

shreyanshjain7174 force-pushed the service/unit-tests branch 2 times, most recently from 6612341 to 5315f01 Compare April 27, 2026 12:38

Kishon-Microsoft assigned marma-dev Apr 28, 2026

jterry75 reviewed Apr 28, 2026

View reviewed changes

Comment thread cmd/containerd-shim-lcow-v2/service/service.go Outdated

jterry75 requested changes Apr 28, 2026

View reviewed changes

shreyanshjain7174 force-pushed the service/unit-tests branch from affc35e to 0f930d4 Compare April 29, 2026 04:53

rawahars reviewed Apr 30, 2026

View reviewed changes

Comment thread cmd/containerd-shim-lcow-v2/service/service.go Outdated

rawahars reviewed Apr 30, 2026

View reviewed changes

Comment thread cmd/containerd-shim-lcow-v2/service/types.go Outdated

rawahars reviewed Apr 30, 2026

View reviewed changes

Comment thread cmd/containerd-shim-lcow-v2/service/service.go

rawahars reviewed Apr 30, 2026

View reviewed changes

Comment thread cmd/containerd-shim-lcow-v2/service/service_sandbox_internal_test.go Outdated

shreyanshjain7174 requested review from jterry75 and rawahars May 4, 2026 05:07

marma-dev approved these changes May 4, 2026

View reviewed changes

Comment thread cmd/containerd-shim-lcow-v2/service/service_sandbox_internal_test.go Outdated

Comment thread cmd/containerd-shim-lcow-v2/service/service_shimdiag_internal_test.go Outdated

shreyanshjain7174 force-pushed the service/unit-tests branch from 2729c12 to 14234dd Compare May 9, 2026 14:05

shreyanshjain7174 requested review from jterry75 and removed request for jterry75 June 30, 2026 06:32

Shreyansh Sancheti added 3 commits June 30, 2026 12:21

service: address review nits — remove test-mention comments, fix impo…

b4a255f

…rt grouping Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>

service: check ErrNotImplemented specifically in PingSandbox, use t.T…

055c422

…empDir() for missing-path test Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>

shreyanshjain7174 force-pushed the service/unit-tests branch from 14234dd to 055c422 Compare June 30, 2026 06:52

rawahars reviewed Jun 30, 2026

View reviewed changes

AdityaMittal1306 approved these changes Jun 30, 2026

View reviewed changes

rawahars reviewed Jun 30, 2026

View reviewed changes

rawahars requested changes Jun 30, 2026

View reviewed changes

Shreyansh Sancheti added 2 commits June 30, 2026 20:04

shreyanshjain7174 force-pushed the service/unit-tests branch from f23d623 to d53820f Compare June 30, 2026 14:35

rawahars approved these changes Jun 30, 2026

View reviewed changes

jterry75 approved these changes Jun 30, 2026

View reviewed changes

shreyanshjain7174 merged commit c697858 into microsoft:main Jun 30, 2026
32 of 33 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

service: add unit tests for LCOW v2 shim service layer#2649

service: add unit tests for LCOW v2 shim service layer#2649
shreyanshjain7174 merged 5 commits into
microsoft:mainfrom
shreyanshjain7174:service/unit-tests

shreyanshjain7174 commented Mar 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rawahars left a comment

Uh oh!

rawahars Jun 30, 2026

Uh oh!

rawahars Jun 30, 2026

Uh oh!

rawahars Jun 30, 2026

Uh oh!

rawahars Jun 30, 2026

Uh oh!

rawahars Jun 30, 2026

Uh oh!

rawahars Jun 30, 2026

Uh oh!

shreyanshjain7174 commented Jun 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

shreyanshjain7174 commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Design

Coverage

Out of scope

Mocks

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rawahars left a comment

Choose a reason for hiding this comment

Uh oh!

rawahars Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

rawahars Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

rawahars Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

rawahars Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

rawahars Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

rawahars Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

shreyanshjain7174 commented Jun 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

shreyanshjain7174 commented Mar 26, 2026 •

edited

Loading