[gnoi] SetPackage: auto-resolve version from image when not provided by ryanzhu706 · Pull Request #679 · sonic-net/sonic-gnmi

ryanzhu706 · 2026-05-21T20:35:17Z

Description:

Problem

The System.SetPackage gNOI RPC requires the caller to provide the version field. If hwproxy or other callers don't provideit, the request fails with "version is missing in package request". The caller often doesn't know the version string ahead of time — it's embedded in the image binary itself.

Solution

Instead of failing when version is empty, the API now automatically resolves it by running sonic-installer binary_version after installation. This extracts the version from the image and uses it for sonic-installer set-default.

Behavior:

If version is provided → used directly (no change)
If version is empty and activate=true → auto-resolved via sonic-installer binary_version
If version is empty and activate=false → install-only succeeds without version

Tests

TestHandleSetPackage_ActivateWithAutoResolvedVersion — happy path
TestHandleSetPackage_AutoResolveVersionFails — error propagation
TestGetBinaryVersion_Success / _EmptyOutput / _CommandError — unit tests for helper

Why I did it

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

When the caller does not provide the version (to_version) field in the SetPackage request, instead of failing with an error, the API now automatically resolves the version by running 'sonic-installer binary_version <filename>' on the installed image. This allows hwproxy and other callers to omit the version field while still getting proper activation (set-default) behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: ryanzhu <ryanzhu@microsoft.com>

mssonicbld · 2026-05-21T20:35:26Z

/azp run

azure-pipelines · 2026-05-21T20:35:37Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull request overview

This PR updates the gNOI System.SetPackage implementation to allow callers to omit Package.Version, and (when activation is requested) auto-resolves the version from the installed image using sonic-installer binary_version so it can still run sonic-installer set-default.

Changes:

Removes strict validation that rejects requests with an empty Package.Version.
When activate=true and version is empty, resolves version via sonic-installer binary_version <filename> and uses it for activation.
Adds unit tests covering auto-resolution success/failure and the new helper.

Reviewed changes

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

File	Description
pkg/gnoi/system/system.go	Adds `getBinaryVersion()` helper and activates with an auto-resolved version when `activate=true` and `version` is empty.
pkg/gnoi/system/system_test.go	Updates validation expectations so empty `version` is no longer rejected up-front.
pkg/gnoi/system/system_monkey_test.go	Adds monkey-patched unit tests for auto-resolution paths and helper behavior.

hdwhdw

Can you include a manual test, possible with logs to make sure this work well? We can put it in PR description

Restructure HandleSetPackage to validate and resolve the version from the image binary BEFORE performing install. This ensures we fail early without side effects if version resolution fails. Order is now: validate inputs -> resolve version -> install -> activate Also addresses PR review comments: - Fix args bounds check (len >= 2) before accessing args[1] - Add test for Version='' + Activate=false (install-only) - Update test names to accurately reflect scenarios Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: ryanzhu <ryanzhu@microsoft.com>

mssonicbld · 2026-05-21T21:16:28Z

/azp run

azure-pipelines · 2026-05-21T21:16:40Z

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 5 comments.

+	// Resolve version: use provided version, or auto-resolve from image binary
+	version := pkg.Package.Version
+	if version == "" {
+		resolved, err := getBinaryVersion(ctx, pkg.Package.Filename)
+		if err != nil {
+			log.Errorf("Failed to resolve version from image %s: %v", pkg.Package.Filename, err)
+			return nil, status.Errorf(codes.Internal, "failed to resolve version from image: %v", err)
+		}
+		version = resolved
+		log.V(1).Infof("Auto-resolved version from image: %s", version)
+	}


+		return "", fmt.Errorf("sonic-installer binary_version failed with exit code %d: %s",
+			result.ExitCode, result.Stderr)


@@ -50,8 +50,7 @@ func TestHandleSetPackageValidation(t *testing.T) {
 					},
 				},
 			},
-			wantErr: true,
-			errMsg:  "version is missing",
+			wantErr: false, // Version is auto-resolved from image; no longer rejected
 		},


+func TestHandleSetPackage_EmptyVersionNoActivate(t *testing.T) {
+	patches := gomonkey.NewPatches()
+	defer patches.Reset()
+
+	// Mock: binary_version and install succeed, set-default should NOT be called
+	patches.ApplyFunc(exec.RunHostCommand, func(ctx context.Context, cmd string, args []string, opts *exec.RunHostCommandOptions) (*exec.CommandResult, error) {
+		if cmd == "sonic-installer" {
+			if len(args) >= 1 && args[0] == "binary_version" {
+				return &exec.CommandResult{
+					Stdout:   "SONiC-OS-4.2.0-Enterprise\n",
+					ExitCode: 0,
+				}, nil
+			}
+			if len(args) >= 1 && args[0] == "install" {
+				return &exec.CommandResult{
+					Stdout:   "Installation completed successfully",
+					ExitCode: 0,
+				}, nil
+			}
+			if len(args) >= 1 && args[0] == "set-default" {
+				return nil, fmt.Errorf("set-default should not be called when activate=false")
+			}
+		}
+		return nil, fmt.Errorf("unexpected command: %s %v", cmd, args)
+	})


+func TestHandleSetPackage_AutoResolveVersionFails(t *testing.T) {
+	patches := gomonkey.NewPatches()
+	defer patches.Reset()
+
+	// Mock: binary_version fails — should reject before install
+	patches.ApplyFunc(exec.RunHostCommand, func(ctx context.Context, cmd string, args []string, opts *exec.RunHostCommandOptions) (*exec.CommandResult, error) {
+		if cmd == "sonic-installer" && len(args) >= 1 && args[0] == "binary_version" {
+			return nil, fmt.Errorf("binary_version command failed")
+		}
+		return nil, fmt.Errorf("unexpected command (install should not be called): %s %v", cmd, args)
+	})
+
+	ctx := context.Background()
+	req := &syspb.SetPackageRequest{
+		Request: &syspb.SetPackageRequest_Package{
+			Package: &syspb.Package{
+				Filename: "/tmp/test-image.bin",
+				Version:  "",
+				Activate: true,
+			},
+		},
+	}
+
+	_, err := HandleSetPackage(ctx, req)
+	if err == nil {
+		t.Fatal("HandleSetPackage() should return error when binary_version fails")
+	}
+	if !containsSubstring(err.Error(), "failed to resolve version from image") {
+		t.Errorf("HandleSetPackage() error = %v, should contain 'failed to resolve version from image'", err)
+	}
+}


Copilot AI review requested due to automatic review settings May 21, 2026 20:35

Copilot started reviewing on behalf of ryanzhu706 May 21, 2026 20:35 View session

ryanzhu706 requested a review from hdwhdw May 21, 2026 20:36

Copilot AI reviewed May 21, 2026

View reviewed changes

Comment thread pkg/gnoi/system/system_test.go Outdated

Comment thread pkg/gnoi/system/system.go

Comment thread pkg/gnoi/system/system_monkey_test.go Outdated

hdwhdw reviewed May 21, 2026

View reviewed changes

Comment thread pkg/gnoi/system/system.go

ryanzhu706 requested review from Copilot and hdwhdw May 21, 2026 21:33

Copilot started reviewing on behalf of ryanzhu706 May 21, 2026 21:33 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

hdwhdw approved these changes May 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[gnoi] SetPackage: auto-resolve version from image when not provided#679

[gnoi] SetPackage: auto-resolve version from image when not provided#679
ryanzhu706 wants to merge 2 commits into
sonic-net:masterfrom
ryanzhu706:fix/setpackage-auto-resolve-version

ryanzhu706 commented May 21, 2026

Uh oh!

mssonicbld commented May 21, 2026

Uh oh!

azure-pipelines Bot commented May 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hdwhdw left a comment •

edited

Loading

Uh oh!

Uh oh!

mssonicbld commented May 21, 2026

Uh oh!

azure-pipelines Bot commented May 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		return "", fmt.Errorf("sonic-installer binary_version failed with exit code %d: %s",
		result.ExitCode, result.Stderr)

Conversation

ryanzhu706 commented May 21, 2026

Why I did it

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Uh oh!

mssonicbld commented May 21, 2026

Uh oh!

azure-pipelines Bot commented May 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hdwhdw left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mssonicbld commented May 21, 2026

Uh oh!

azure-pipelines Bot commented May 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hdwhdw left a comment •

edited

Loading