build: separate Go caches from project tools by hdwhdw · Pull Request #767 · sonic-net/sonic-gnmi

hdwhdw · 2026-08-25T04:08:17Z

Why

GOPATH=/tmp/go makes privileged and unprivileged Go commands share module and checksum cache state. Root-owned cache files can then block later unprivileged commands after tests pass.

Go caches are user state, not repository artifacts. Repository tools also need stable locations that do not depend on ambient PATH state.

What changes

The Makefile does not assign GOPATH.
Direct module source lookups use go list instead of GOPATH/pkg/mod.
Versioned build and test tools use build/tools/bin and exact invocation paths.
Cross-builds compile host tools for the host architecture.
Existing Go commands that use sudo run in an explicit root environment with root default caches.

Long-term direction

Users own their Go caches.
Product binaries stay in build/bin.
Repository tools stay in build/tools/bin.
Tests eventually run without sudo and use project-local fixtures.
Cleanup eventually removes repository artifacts only.
Debian packaging eventually consumes an explicit product manifest.

Scope

This PR changes cache and tool ownership only. It does not change product binaries, packaging, fixtures, sudo usage, or cleanup behavior.

The sonic-mgmt-common patch helper remains unchanged. It still assumes its default $GOPATH/pkg/mod module-cache path.

Validation

Azure Pipelines run 1205748 passed all sonic-gnmi checks for commit afb6db7.
make -f pure.mk junit-xml passed 853 tests and generated Cobertura XML.
Cross-target tool installation produced an amd64 host executable.
Environment checks confirmed that privileged Go commands use the explicit root environment and root cache paths.

Tracking: Azure DevOps Task 39393252.

mssonicbld · 2026-08-25T04:08:25Z

/azp run

azure-pipelines · 2026-08-25T04:08:26Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

azure-pipelines · 2026-08-25T04:08:38Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Pull request overview

This PR refactors the build and test workflow to keep Go tool/cache state in the invoking user’s normal Go locations while ensuring repository artifacts (products, pinned tools, and test fixtures) live under build/. It also updates tests and runtime code to make filesystem paths (checkpoints, host roots, DB configs, shared-memory keys) configurable so tests can run without sudo and without writing to system-owned locations.

Changes:

Move product outputs to build/bin and pin build/test tools under build/tools/bin (Makefile + pure.mk).
Rework test fixture/runtime setup to use repo-owned paths and env vars (DB config path, runtime root, checkpoint dir, shared-memory key), removing sudo from test runs.
Update documentation and CI to align with the new build/test layout and pinned-tool workflow.

Reviewed changes

Copilot reviewed 25 out of 26 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tools/test/env.sh	Wrapper now forwards CLI args to the upstream test env script; retains a default mode for local usage.
test/utils.py	Removes `sudo` from `gnmi_dump` invocation to support non-root test execution.
test/test_gnmi_configdb.py	Makes checkpoint location configurable via `SONIC_GNMI_CHECKPOINT_DIR`.
test/test_gnmi_configdb_patch.py	Makes checkpoint location configurable via `SONIC_GNMI_CHECKPOINT_DIR`.
test_utils/test_utils.go	Builds DB fixture files under a configurable runtime root and writes global DB config dynamically.
swsscommon/Makefile	Cleans only the SWIG-generated artifacts and copied interface file.
sonic_db_config/db_config.go	Uses `DB_CONFIG_PATH` and derived global config path to initialize DB config via swsscommon.
sonic_db_config/db_config_test.go	Updates fatal messages for clearer errors on fixture setup failures.
sonic_data_client/mixed_db_client.go	Makes checkpoint base dir configurable for tests/runtime.
sonic_data_client/client_test.go	Adds `TestMain` setup for configurable checkpoint directory.
README.md	Updates build instructions and clarifies artifact/tool output locations.
pure.mk	Pins test/coverage/security tools under `build/tools/bin` and uses `-mod=readonly` for purity targets.
proto/gnoi/README.md	Updates proto regeneration guidance to use Make targets + pinned tools.
Makefile	Major build/test refactor: pinned tools, repo-owned fixtures, no-sudo Go tests, explicit product binary manifest for packaging.
go.sum	Adds checksum entry for `golang.org/x/time v0.11.0`.
go.mod	Bumps `github.com/golang/glog` and adds explicit `golang.org/x/time v0.11.0`.
gnmi_server/server.go	Makes host log root overridable for tests via an unexported variable.
gnmi_server/server_test.go	Uses temp directories for journal/healthz/pathz/checkpoint test artifacts.
gnmi_server/gnsi_pathz_test.go	Switches Pathz test file paths to variables (set by TestMain).
gnmi_server/gnoi_healthz.go	Makes host root configurable for tests by replacing hard-coded `/mnt/host` with a variable.
gnmi_server/gnoi_healthz_test.go	Updates healthz tests to build container paths using the configurable host root.
gnmi_server/db_journal.go	Uses overridable host log root for journal file IO.
gnmi_server/db_journal_test.go	Updates journal tests to use overridable host log root.
doc/telemetry-dev-env/Dockerfile	Removes GOPATH customization from the dev image; keeps Go on PATH via GOROOT.
common_utils/shareMem.go	Allows overriding the SysV shared-memory key via `SONIC_GNMI_SHM_KEY` for tests.
azure-pipelines.yml	Removes GOPATH/bin from PATH setup, aligning with pinned-tool approach.

Suppressed comments (4)

gnmi_server/gnoi_healthz_test.go:658

filepath.Join(healthzHostRoot, req.GetId()) ignores healthzHostRoot because the request ID is absolute (starts with "/"). Trim the leading slash so the test creates the artifact under the configured host root.

			realPath := filepath.Join(healthzHostRoot, req.GetId())

gnmi_server/gnoi_healthz_test.go:706

filepath.Join(healthzHostRoot, req.GetId()) ignores healthzHostRoot because the request ID is absolute (starts with "/"). Trim the leading slash so the test creates the artifact under the configured host root.

			realPath := filepath.Join(healthzHostRoot, req.GetId())

gnmi_server/gnoi_healthz_test.go:744

filepath.Join(healthzHostRoot, req.GetId()) ignores healthzHostRoot because the request ID is absolute (starts with "/"). Trim the leading slash so the test creates the artifact under the configured host root.

			realPath := filepath.Join(healthzHostRoot, req.GetId())

gnmi_server/gnoi_healthz_test.go:783

filepath.Join(healthzHostRoot, req.GetId()) ignores healthzHostRoot because the request ID is absolute (starts with "/"). Trim the leading slash so the test creates the artifact under the configured host root.

			realPath := filepath.Join(healthzHostRoot, req.GetId())

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

 		return nil, status.Errorf(codes.InvalidArgument, "Invalid artifact path")
 	}
-	file_path := filepath.Join("/mnt/host", cleanPath)
+	file_path := filepath.Join(healthzHostRoot, cleanPath)


 		return status.Errorf(codes.InvalidArgument, "Invalid artifact path")
 	}
-	file_path := filepath.Join("/mnt/host", cleanPath)
+	file_path := filepath.Join(healthzHostRoot, cleanPath)


 			req := &healthz.ArtifactRequest{Id: "/tmp/dump/seek_fail.txt"}

-			realPath := "/mnt/host/tmp/dump/seek_fail.txt"
+			realPath := filepath.Join(healthzHostRoot, req.GetId())


 func TestMain(m *testing.M) {
+	checkpointDir, err := os.MkdirTemp("", "sonic-checkpoint-")
+	if err != nil {
+		panic(err)
+	}
+	testFile = filepath.Join(checkpointDir, "ut.cp.json")
+	os.Setenv("SONIC_GNMI_CHECKPOINT_DIR", checkpointDir)
+	defer os.RemoveAll(checkpointDir)
 	defer test_utils.MemLeakCheck()
 	m.Run()
 }


+    . ${TOPDIR}/../sonic-mgmt-common/tools/test/env.sh \
+        --dest=${TOPDIR}/build/test \
+        --dbconfig-in=${TOPDIR}/testdata/database_config.json


 			// Create both host and container path versions
 			dummy_hostfile := "/tmp/dump/fake-collect-success"
-			dummy_containerfile := "/mnt/host/tmp/dump/fake-collect-success"
+			dummy_containerfile := filepath.Join(healthzHostRoot, dummy_hostfile)


mssonicbld · 2026-08-25T09:32:46Z

/azp run

azure-pipelines · 2026-08-25T09:33:00Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-08-25T14:02:02Z

/azp run

azure-pipelines · 2026-08-25T14:02:16Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-08-26T18:23:10Z

/azp run

azure-pipelines · 2026-08-26T18:23:25Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-08-27T18:46:51Z

/azp run

azure-pipelines · 2026-08-27T18:47:04Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Suppressed comments (2)

gnmi_server/server_test.go:6941

After introducing an exit-code variable in TestMain, capture the return value of m.Run() into it so the deferred os.Exit uses the real test status.

	defer test_utils.MemLeakCheck()
	m.Run()
}

gnmi_server/server_test.go:6914

TestMain must call os.Exit with the result of m.Run(); otherwise go test exits with status 0 even when tests fail. Adding a deferred os.Exit wrapper at the top preserves the existing defers (temp dir cleanup + MemLeakCheck) while still returning the correct exit code.

func TestMain(m *testing.M) {
	journalDir, err := os.MkdirTemp("", "gnmi-journal-")
	if err != nil {
		panic(err)
	}

mssonicbld · 2026-08-28T01:27:37Z

/azp run

azure-pipelines · 2026-08-28T01:27:49Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-08-28T17:39:00Z

/azp run

azure-pipelines · 2026-08-28T17:39:13Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>

mssonicbld · 2026-08-28T18:19:34Z

/azp run

azure-pipelines · 2026-08-28T18:19:46Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

Makefile:41

Pinned tool versions diverge from pure.mk (gotestsum v1.13.0 here vs v1.12.3 in pure.mk; gocov-xml v1.2.0 here vs v1.1.0). Keeping multiple pinned versions for the same tooling can lead to different output/behavior across targets; consider aligning or centralizing versions.

GOTESTSUM := $(abspath $(TOOLS_BIN_DIR)/gotestsum-v1.13.0)
GOCOV := $(abspath $(TOOLS_BIN_DIR)/gocov-v1.1.0)
GOCOV_XML := $(abspath $(TOOLS_BIN_DIR)/gocov-xml-v1.2.0)

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>

mssonicbld · 2026-08-28T21:51:59Z

/azp run

azure-pipelines · 2026-08-28T21:52:12Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Copilot AI lite review requested due to automatic review settings August 25, 2026 04:08

Copilot started reviewing on behalf of hdwhdw August 25, 2026 04:08 View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

hdwhdw mentioned this pull request Aug 25, 2026

[DO NOT MERGE] test: validate sonic-gnmi PR 775 packaging manifest sonic-net/sonic-buildimage#29171

Draft

hdwhdw mentioned this pull request Aug 25, 2026

ci: enforce provider boundaries in pure build mode #765

Draft

hdwhdw force-pushed the fix/go-cache-artifact-boundary branch from 75fafc8 to 445f985 Compare August 27, 2026 18:46

hdwhdw requested a lite review from Copilot August 28, 2026 01:05

Copilot started reviewing on behalf of hdwhdw August 28, 2026 01:06 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Comment thread sonic_data_client/client_test.go

Comment thread test_utils/test_utils.go Outdated

Comment thread test/test_gnmi_configdb.py Outdated

Comment thread test/test_gnmi_configdb_patch.py Outdated

hdwhdw force-pushed the fix/go-cache-artifact-boundary branch from 006aaa3 to 8107ab5 Compare August 28, 2026 17:38

hdwhdw changed the title ~~build: separate Go caches from project artifacts~~ build: separate Go caches from project tools Aug 28, 2026

hdwhdw requested a lite review from Copilot August 28, 2026 18:03

Copilot started reviewing on behalf of hdwhdw August 28, 2026 18:04 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Comment thread Makefile Outdated

Comment thread pure.mk

Comment thread Makefile

Comment thread Makefile

build: separate Go caches from project tools

38cbcad

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>

hdwhdw force-pushed the fix/go-cache-artifact-boundary branch from 8107ab5 to 38cbcad Compare August 28, 2026 18:19

hdwhdw requested a lite review from Copilot August 28, 2026 21:02

Copilot started reviewing on behalf of hdwhdw August 28, 2026 21:03 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Comment thread pure.mk

Comment thread pure.mk

build: isolate privileged Go environment

afb6db7

Signed-off-by: Dawei Huang <daweihuang@microsoft.com>

hdwhdw requested a lite review from Copilot August 29, 2026 02:13

Copilot started reviewing on behalf of hdwhdw August 29, 2026 02:14 View session

Copilot AI reviewed Aug 29, 2026

View reviewed changes

hdwhdw requested a review from donghaolicd August 31, 2026 16:38

donghaolicd approved these changes Aug 31, 2026

View reviewed changes

hdwhdw merged commit 537a95b into sonic-net:master Aug 31, 2026
13 checks passed

Conversation

hdwhdw commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What changes

Long-term direction

Scope

Validation

Uh oh!

mssonicbld commented Aug 25, 2026

Uh oh!

azure-pipelines Bot commented Aug 25, 2026

Uh oh!

azure-pipelines Bot commented Aug 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

mssonicbld commented Aug 25, 2026

Uh oh!

azure-pipelines Bot commented Aug 25, 2026

Uh oh!

mssonicbld commented Aug 25, 2026

Uh oh!

azure-pipelines Bot commented Aug 25, 2026

Uh oh!

mssonicbld commented Aug 26, 2026

Uh oh!

azure-pipelines Bot commented Aug 26, 2026

Uh oh!

mssonicbld commented Aug 27, 2026

Uh oh!

azure-pipelines Bot commented Aug 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Aug 28, 2026

Uh oh!

azure-pipelines Bot commented Aug 28, 2026

Uh oh!

mssonicbld commented Aug 28, 2026

Uh oh!

azure-pipelines Bot commented Aug 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Aug 28, 2026

Uh oh!

azure-pipelines Bot commented Aug 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Aug 28, 2026

Uh oh!

azure-pipelines Bot commented Aug 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

hdwhdw commented Aug 25, 2026 •

edited

Loading