Skip to content

V cshekar/setpackage 202511 #689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
v-cshekar wants to merge 3 commits into
base: 202511
Choose a base branch
from
+245 −19
Open

V cshekar/setpackage 202511 #689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e9c3e3c
[gnoi] SetPackage: auto-resolve version from image when not provided
ryanzhu706 May 21, 2026
d2e9d06
[gnoi] SetPackage: resolve version before install, fix test coverage
ryanzhu706 May 21, 2026
0842312
[gnoi] SetPackage: use absolute path for sonic-installer
v-cshekar May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 47 additions & 13 deletions pkg/gnoi/system/system.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,27 +86,35 @@ func HandleSetPackage(ctx context.Context, req *syspb.SetPackageRequest) (*syspb
log.Errorf("Filename is missing in package request")
return nil, status.Errorf(codes.InvalidArgument, "filename is missing in package request")
}
if pkg.Package.Version == "" {
log.Errorf("Version is missing in package request")
return nil, status.Errorf(codes.InvalidArgument, "version is missing in package request")
}

// Reject RemoteDownload - require local image
if pkg.Package.RemoteDownload != nil {
log.Errorf("RemoteDownload is not supported - image must be local")
return nil, status.Errorf(codes.InvalidArgument, "remote download is not supported, image must be local")
}

// Log the package details
log.V(1).Infof("Installing package: filename=%s, version=%s, activate=%v",
pkg.Package.Filename, pkg.Package.Version, pkg.Package.Activate)

// Validate filename is absolute path
if !filepath.IsAbs(pkg.Package.Filename) {
log.Errorf("Filename must be an absolute path: %s", pkg.Package.Filename)
return nil, status.Errorf(codes.InvalidArgument, "filename must be an absolute path")
}

// 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)
}

// Log the package details
log.V(1).Infof("Installing package: filename=%s, version=%s, activate=%v",
pkg.Package.Filename, version, pkg.Package.Activate)

// Install the package using sonic-installer
if err := installPackage(ctx, pkg.Package.Filename); err != nil {
log.Errorf("Failed to install package %s: %v", pkg.Package.Filename, err)
Expand All @@ -117,11 +125,11 @@ func HandleSetPackage(ctx context.Context, req *syspb.SetPackageRequest) (*syspb

// If activate is requested, set as next boot image
if pkg.Package.Activate {
if err := activatePackage(ctx, pkg.Package.Version); err != nil {
log.Errorf("Failed to activate package %s: %v", pkg.Package.Version, err)
if err := activatePackage(ctx, version); err != nil {
log.Errorf("Failed to activate package %s: %v", version, err)
return nil, status.Errorf(codes.Internal, "failed to activate package: %v", err)
}
log.V(1).Infof("Successfully activated package %s", pkg.Package.Version)
log.V(1).Infof("Successfully activated package %s", version)
}

return &syspb.SetPackageResponse{}, nil
Expand All @@ -136,7 +144,7 @@ func installPackage(ctx context.Context, filename string) error {
opts := &exec.RunHostCommandOptions{
Timeout: 10 * time.Minute, // Allow up to 10 minutes for installation
}
result, err := exec.RunHostCommand(ctx, "sonic-installer", []string{"install", "-y", filename}, opts)
result, err := exec.RunHostCommand(ctx, "/usr/local/bin/sonic-installer", []string{"install", "-y", filename}, opts)
if err != nil {
return fmt.Errorf("failed to run sonic-installer install: %v", err)
}
Expand All @@ -150,6 +158,32 @@ func installPackage(ctx context.Context, filename string) error {
return nil
}

// getBinaryVersion extracts the version string from a SONiC image using sonic-installer binary-version.
func getBinaryVersion(ctx context.Context, filename string) (string, error) {
log.V(1).Infof("Resolving binary version for: %s", filename)

opts := &exec.RunHostCommandOptions{
Timeout: 1 * time.Minute,
}
result, err := exec.RunHostCommand(ctx, "/usr/local/bin/sonic-installer", []string{"binary-version", filename}, opts)
if err != nil {
return "", fmt.Errorf("failed to run sonic-installer binary-version: %v", err)
}

if result.Error != nil {
return "", fmt.Errorf("sonic-installer binary-version failed with exit code %d: %s",
result.ExitCode, result.Stderr)
}

version := strings.TrimSpace(result.Stdout)
if version == "" {
return "", fmt.Errorf("sonic-installer binary-version returned empty output for %s", filename)
}

log.V(1).Infof("Resolved binary version: %s", version)
return version, nil
}

// activatePackage sets a SONiC image as the next boot image using sonic-installer set-default.
func activatePackage(ctx context.Context, version string) error {
log.V(1).Infof("Activating package version: %s", version)
Expand All @@ -159,7 +193,7 @@ func activatePackage(ctx context.Context, version string) error {
opts := &exec.RunHostCommandOptions{
Timeout: 2 * time.Minute, // Allow up to 2 minutes for setting default
}
result, err := exec.RunHostCommand(ctx, "sonic-installer", []string{"set-default", version}, opts)
result, err := exec.RunHostCommand(ctx, "/usr/local/bin/sonic-installer", []string{"set-default", version}, opts)
if err != nil {
return fmt.Errorf("failed to run sonic-installer set-default: %v", err)
}
Expand Down
199 changes: 196 additions & 3 deletions pkg/gnoi/system/system_monkey_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func TestHandleSetPackage_SuccessWithoutActivation(t *testing.T) {

// Mock exec.RunHostCommand to return successful installation
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] == "install" {
if cmd == "/usr/local/bin/sonic-installer" && len(args) >= 1 && args[0] == "install" {
return &exec.CommandResult{
Stdout: "Installation completed successfully",
Stderr: "",
Expand Down Expand Up @@ -48,13 +48,206 @@ func TestHandleSetPackage_SuccessWithoutActivation(t *testing.T) {
}
}

func TestHandleSetPackage_ActivateWithAutoResolvedVersion(t *testing.T) {
patches := gomonkey.NewPatches()
defer patches.Reset()

// Mock exec.RunHostCommand to handle binary-version, install, and set-default
patches.ApplyFunc(exec.RunHostCommand, func(ctx context.Context, cmd string, args []string, opts *exec.RunHostCommandOptions) (*exec.CommandResult, error) {
if cmd == "/usr/local/bin/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) >= 2 && args[0] == "set-default" {
if args[1] != "SONiC-OS-4.2.0-Enterprise" {
return nil, fmt.Errorf("unexpected version: %s", args[1])
}
return &exec.CommandResult{
Stdout: "Default image set successfully",
ExitCode: 0,
}, nil
}
}
return nil, fmt.Errorf("unexpected command: %s %v", cmd, args)
})

ctx := context.Background()
req := &syspb.SetPackageRequest{
Request: &syspb.SetPackageRequest_Package{
Package: &syspb.Package{
Filename: "/tmp/test-image.bin",
Version: "", // Empty! Should auto-resolve before install
Activate: true,
},
},
}

resp, err := HandleSetPackage(ctx, req)
if err != nil {
t.Fatalf("HandleSetPackage() returned error: %v", err)
}
if resp == nil {
t.Fatal("HandleSetPackage() returned nil response")
}
}

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 == "/usr/local/bin/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)
}
}

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 == "/usr/local/bin/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)
})

ctx := context.Background()
req := &syspb.SetPackageRequest{
Request: &syspb.SetPackageRequest_Package{
Package: &syspb.Package{
Filename: "/tmp/test-image.bin",
Version: "",
Activate: false,
},
},
}

resp, err := HandleSetPackage(ctx, req)
if err != nil {
t.Fatalf("HandleSetPackage() returned error: %v", err)
}
if resp == nil {
t.Fatal("HandleSetPackage() returned nil response")
}
}

func TestGetBinaryVersion_Success(t *testing.T) {
patches := gomonkey.NewPatches()
defer patches.Reset()

patches.ApplyFunc(exec.RunHostCommand, func(ctx context.Context, cmd string, args []string, opts *exec.RunHostCommandOptions) (*exec.CommandResult, error) {
if cmd == "/usr/local/bin/sonic-installer" && len(args) >= 1 && args[0] == "binary-version" {
return &exec.CommandResult{
Stdout: "SONiC-OS-4.2.0-Enterprise\n",
ExitCode: 0,
}, nil
}
return nil, fmt.Errorf("unexpected command: %s %v", cmd, args)
})

ctx := context.Background()
version, err := getBinaryVersion(ctx, "/tmp/test-image.bin")
if err != nil {
t.Fatalf("getBinaryVersion() returned error: %v", err)
}
if version != "SONiC-OS-4.2.0-Enterprise" {
t.Errorf("getBinaryVersion() = %q, want %q", version, "SONiC-OS-4.2.0-Enterprise")
}
}

func TestGetBinaryVersion_EmptyOutput(t *testing.T) {
patches := gomonkey.NewPatches()
defer patches.Reset()

patches.ApplyFunc(exec.RunHostCommand, func(ctx context.Context, cmd string, args []string, opts *exec.RunHostCommandOptions) (*exec.CommandResult, error) {
return &exec.CommandResult{
Stdout: "",
ExitCode: 0,
}, nil
})

ctx := context.Background()
_, err := getBinaryVersion(ctx, "/tmp/test-image.bin")
if err == nil {
t.Fatal("getBinaryVersion() should return error for empty output")
}
if !containsSubstring(err.Error(), "returned empty output") {
t.Errorf("getBinaryVersion() error = %v, should contain 'returned empty output'", err)
}
}

func TestGetBinaryVersion_CommandError(t *testing.T) {
patches := gomonkey.NewPatches()
defer patches.Reset()

patches.ApplyFunc(exec.RunHostCommand, func(ctx context.Context, cmd string, args []string, opts *exec.RunHostCommandOptions) (*exec.CommandResult, error) {
return nil, fmt.Errorf("command not found")
})

ctx := context.Background()
_, err := getBinaryVersion(ctx, "/tmp/test-image.bin")
if err == nil {
t.Fatal("getBinaryVersion() should return error when command fails")
}
if !containsSubstring(err.Error(), "failed to run sonic-installer binary-version") {
t.Errorf("getBinaryVersion() error = %v, should contain 'failed to run sonic-installer binary-version'", err)
}
}

func TestHandleSetPackage_SuccessWithActivation(t *testing.T) {
patches := gomonkey.NewPatches()
defer patches.Reset()

// Mock exec.RunHostCommand to handle both install and set-default commands
patches.ApplyFunc(exec.RunHostCommand, func(ctx context.Context, cmd string, args []string, opts *exec.RunHostCommandOptions) (*exec.CommandResult, error) {
if cmd == "sonic-installer" {
if cmd == "/usr/local/bin/sonic-installer" {
if len(args) >= 1 && args[0] == "install" {
return &exec.CommandResult{
Stdout: "Installation completed successfully",
Expand Down Expand Up @@ -132,7 +325,7 @@ func TestHandleSetPackage_ActivateCommandError(t *testing.T) {

// Mock exec.RunHostCommand: install succeeds, set-default fails
patches.ApplyFunc(exec.RunHostCommand, func(ctx context.Context, cmd string, args []string, opts *exec.RunHostCommandOptions) (*exec.CommandResult, error) {
if cmd == "sonic-installer" {
if cmd == "/usr/local/bin/sonic-installer" {
if len(args) >= 1 && args[0] == "install" {
return &exec.CommandResult{
Stdout: "Installation completed successfully",
Expand Down
5 changes: 2 additions & 3 deletions pkg/gnoi/system/system_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestHandleSetPackageValidation(t *testing.T) {
errMsg: "filename is missing",
},
{
name: "missing version",
name: "empty version - resolved from image before install",
req: &syspb.SetPackageRequest{
Request: &syspb.SetPackageRequest_Package{
Package: &syspb.Package{
Expand All @@ -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
},
{
name: "remote download not supported",
Expand Down
Loading