Skip to content

Standardize go:generate for controller mocks + CI drift validation #2707

@shreyanshjain7174

Description

@shreyanshjain7174

Background

Per @rawahars's review on #2647: "Each controller has mocks needs to have the generate directive. The go:generate directive should reside in the test file."

This issue tracks the rollout for all controllers + a CI gate.

Goals

  1. Standardise mockgen invocation behind a wrapper script so individual //go:generate lines stay short and consistent.
  2. Add //go:generate directive in a TEST file (not source file) for each controller, using the wrapper.
  3. Add CI validation that runs go generate ./... and fails if there is any diff against the committed mock files.

Prior art — AWS amazon-ecs-agent

AWS uses a wrapper script: https://github.com/aws/amazon-ecs-agent/blob/master/scripts/mockgen.sh

Key design choices worth borrowing:

  • Single short //go:generate scripts/mockgen.sh <pkg> <source> <outdir> directive in source files
  • Wrapper runs mockgen | goimports to normalise imports
  • Wrapper prepends a standard licence/header banner so all generated files are byte-identical regardless of mockgen version drift

Suggested directive shape (hcsshim)

For platform-divergent guest interfaces (e.g. network.guestNetwork), the wrapper would resolve -build_constraint automatically based on filename suffix (_lcow.go -> windows&&lcow), so contributors don't have to spell it out.

Single short directive in a test file:

//go:generate scripts/mockgen.ps1 mocks types.go vmLifetime,guestManager

Wrapper handles -build_flags, -package, -destination, gofmt, banner.

Affected packages (current scope)

  • internal/controller/device/scsi/
  • internal/controller/device/plan9/
  • internal/controller/device/vpci/
  • internal/controller/network/
  • internal/controller/vm/
  • internal/windows/ (mock at internal/windows/mock/)
  • internal/gcs/ (GuestDefinedCapabilities mock at internal/gcs/mock/)
  • cmd/containerd-shim-lcow-v2/service/

Suggested CI step

Add a job (or extend an existing lint job) in .github/workflows/ci.yml:

- name: Verify generated mocks are up to date
  run: |
    go generate ./...
    git diff --exit-code -- '*/mocks/*.go' '*/mock/*.go'

Run on windows-latest so build tags resolve correctly.

Acceptance criteria

  • scripts/mockgen.ps1 (or .sh) wrapper exists and is documented
  • All listed controllers have a //go:generate directive in a test file using the wrapper
  • go generate ./... produces no diff against committed mocks on a fresh clone
  • CI fails if a contributor changes an interface without regenerating
  • Documentation (CONTRIBUTING.md or similar) explains: "After modifying an interface, run go generate ./... and commit the regenerated mocks"

Notes

Discussed in #2647 (rawahars review on internal/controller/network/network_lcow.go).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions