service: add unit tests for LCOW v2 shim service layer#2649
Open
shreyanshjain7174 wants to merge 1 commit intomicrosoft:mainfrom
Open
service: add unit tests for LCOW v2 shim service layer#2649shreyanshjain7174 wants to merge 1 commit intomicrosoft:mainfrom
shreyanshjain7174 wants to merge 1 commit intomicrosoft:mainfrom
Conversation
shreyanshjain7174
force-pushed
the
service/unit-tests
branch
4 times, most recently
from
72ac4ca to
c703ec9
Compare
shreyanshjain7174
force-pushed
the
service/unit-tests
branch
from
c703ec9 to
d0fa692
Compare
shreyanshjain7174
force-pushed
the
service/unit-tests
branch
2 times, most recently
from
6612341 to
5315f01
Compare
jterry75
reviewed
Apr 28, 2026
jterry75
requested changes
Apr 28, 2026
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>
shreyanshjain7174
force-pushed
the
service/unit-tests
branch
from
affc35e to
0f930d4
Compare
rawahars
reviewed
Apr 30, 2026
| // vmController is responsible for managing the lifecycle of the underlying utility VM and its associated resources. | ||
| vmController *vm.Controller | ||
| // vmController is responsible for managing the lifecycle of the underlying | ||
| // utility VM. Tests substitute a mock; production passes [*vm.Controller]. |
Contributor
There was a problem hiding this comment.
nit: Please remove this statement Tests substitute a mock; production passes [*vm.Controller].
rawahars
reviewed
Apr 30, 2026
| ) | ||
|
|
||
| // vmController is the subset of the VM controller that [Service] depends on. | ||
| // Implemented by [*vm.Controller]. Tests substitute a mock. |
rawahars
reviewed
Apr 30, 2026
| vmController vmController | ||
|
|
||
| // podControllers maps podID -> PodController for each active pod. | ||
| podControllers map[string]*pod.Controller |
Contributor
There was a problem hiding this comment.
Should we create similar interface for pod.Controller too?
rawahars
reviewed
Apr 30, 2026
| "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/stats" | ||
| "github.com/Microsoft/hcsshim/internal/builder/vm/lcow" | ||
| "github.com/Microsoft/hcsshim/internal/controller/vm" | ||
|
|
Contributor
There was a problem hiding this comment.
Please format the imports.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Servicestruct holds a singlevmControllerinterface field (defined intypes.go). Production passes*vm.Controller; tests pass a generated mock. The interface is a superset that includes the methodspod.Newneeds (Guest(),SCSIController(),Plan9Controller(),NetworkController()etc.) so the same field satisfies bothService's direct calls andpod.New's narrower interface via Go structural typing - no concrete-pointer leakage. This mirrors the mergedinternal/controller/podpattern.Coverage
49 tests across three files matching the production layout:
service_sandbox_internal_test.go(23): duplicate sandbox rejection, missingconfig.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),enrichNotFoundErrorpass-through andErrNotFoundpreservation.Out of scope
CreateSandbox->BuildSandboxConfig->CreateVMpath is exercised byinternal/builder/vm/lcow/tests and thetest/parity/vm/parity suite; the service-level surface is the orchestration logic only.StartSandboxpath reads boot files from disk; belongs infunctionaltagged tests.Mocks
mocks/mock_service.gois checked in. Standardising mock generation (directive in test file + CI drift validation) is tracked in #2707.