Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
97 changes: 97 additions & 0 deletions pkg/exec/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,103 @@ func buildNsenterArgs(opts *RunHostCommandOptions, command string, args []string
return nsenterArgs
}

// AsyncCommandHandle represents a handle to a running asynchronous command.
// The underlying process is decoupled from any caller context so it will
// continue running even if the originating gRPC stream is cancelled.
type AsyncCommandHandle struct {
done chan struct{}
result *CommandResult
}

// Wait blocks until the command completes and returns the result.
func (h *AsyncCommandHandle) Wait() *CommandResult {
<-h.done
return h.result
}

// Done returns a channel that is closed when the command completes.
func (h *AsyncCommandHandle) Done() <-chan struct{} {
return h.done
}

// RunHostCommandAsync starts a command on the host using nsenter, but unlike
// RunHostCommand the process is NOT tied to any caller-supplied context.
// This is critical for long-running operations (e.g. sonic-installer) that
// must survive gRPC client disconnects.
//
// The Timeout field in opts is still honoured via an internal background
// context. If Timeout is zero the defaultTimeout is used.
func RunHostCommandAsync(command string, args []string, opts *RunHostCommandOptions) (*AsyncCommandHandle, error) {
if command == "" {
return nil, fmt.Errorf("command cannot be empty")
}

if opts == nil {
opts = &RunHostCommandOptions{}
}

timeout := opts.Timeout
if timeout == 0 {
timeout = defaultTimeout
}

// Use a background context so the process is never killed by the
// caller's context cancellation (e.g. gRPC stream teardown).
ctx, cancel := context.WithTimeout(context.Background(), timeout)

nsenterArgs := buildNsenterArgs(opts, command, args)
cmd := exec.CommandContext(ctx, "nsenter", nsenterArgs...)

if opts.WorkingDir != "" {
cmd.Dir = opts.WorkingDir
}
if len(opts.Environment) > 0 {
cmd.Env = append(cmd.Env, opts.Environment...)
}

var stdout, stderr bytes.Buffer
cmd.Stdout = &stdout
cmd.Stderr = &stderr

if err := cmd.Start(); err != nil {
cancel()
return nil, fmt.Errorf("failed to start command: %v", err)
}

handle := &AsyncCommandHandle{
done: make(chan struct{}),
}

go func() {
defer cancel()
defer close(handle.done)

err := cmd.Wait()
result := &CommandResult{
Stdout: stdout.String(),
Stderr: stderr.String(),
Error: err,
}
if exitError, ok := err.(*exec.ExitError); ok {
result.ExitCode = exitError.ExitCode()
}
handle.result = result
}()

return handle, nil
}

// NewCompletedAsyncHandle creates an AsyncCommandHandle that is already
// completed with the given result. This is useful for testing.
func NewCompletedAsyncHandle(result *CommandResult) *AsyncCommandHandle {
h := &AsyncCommandHandle{
done: make(chan struct{}),
result: result,
}
close(h.done)
return h
}

// IsNsenterAvailable checks if nsenter is available in the system
func IsNsenterAvailable() bool {
cmd := exec.Command("which", "nsenter")
Expand Down
94 changes: 94 additions & 0 deletions pkg/exec/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,100 @@ func TestParseCommand(t *testing.T) {
}
}

func TestRunHostCommandAsync(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip("nsenter tests can only run on Linux")
}
if !IsNsenterAvailable() {
t.Skip("nsenter is not available on this system")
}

testResult, _ := RunHostCommand(context.Background(), "true", nil, nil)
if testResult != nil && testResult.Error != nil && strings.Contains(testResult.Stderr, "Permission denied") {
t.Skip("Insufficient permissions to run nsenter tests")
}

t.Run("empty command", func(t *testing.T) {
_, err := RunHostCommandAsync("", nil, nil)
if err == nil {
t.Error("RunHostCommandAsync() with empty command should return error")
}
})

t.Run("simple echo", func(t *testing.T) {
handle, err := RunHostCommandAsync("echo", []string{"async-test"}, &RunHostCommandOptions{
Timeout: 5 * time.Second,
})
if err != nil {
t.Fatalf("RunHostCommandAsync() failed to start: %v", err)
}

result := handle.Wait()
if result.Error != nil {
t.Errorf("command failed: %v", result.Error)
}
if !strings.Contains(result.Stdout, "async-test") {
t.Errorf("expected stdout to contain 'async-test', got %q", result.Stdout)
}
})

t.Run("done channel closes on completion", func(t *testing.T) {
handle, err := RunHostCommandAsync("true", nil, &RunHostCommandOptions{
Timeout: 5 * time.Second,
})
if err != nil {
t.Fatalf("RunHostCommandAsync() failed to start: %v", err)
}

select {
case <-handle.Done():
// expected
case <-time.After(5 * time.Second):
t.Fatal("timed out waiting for Done() channel")
}

result := handle.Wait()
if result.ExitCode != 0 {
t.Errorf("expected exit code 0, got %d", result.ExitCode)
}
})

t.Run("captures non-zero exit code", func(t *testing.T) {
handle, err := RunHostCommandAsync("false", nil, &RunHostCommandOptions{
Timeout: 5 * time.Second,
})
if err != nil {
t.Fatalf("RunHostCommandAsync() failed to start: %v", err)
}

result := handle.Wait()
if result.ExitCode == 0 {
t.Error("expected non-zero exit code for 'false' command")
}
})
}

func TestNewCompletedAsyncHandle(t *testing.T) {
expected := &CommandResult{
Stdout: "test output",
ExitCode: 0,
}
handle := NewCompletedAsyncHandle(expected)

// Done channel should already be closed
select {
case <-handle.Done():
// expected
default:
t.Fatal("Done() channel should be closed for completed handle")
}

result := handle.Wait()
if result != expected {
t.Error("Wait() should return the provided result")
}
}

func TestIsNsenterAvailable(t *testing.T) {
// This test just verifies the function runs without error
// The actual result depends on the system
Expand Down
23 changes: 14 additions & 9 deletions pkg/gnoi/system/system.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ func HandleSetPackage(ctx context.Context, req *syspb.SetPackageRequest) (*syspb
return nil, status.Errorf(codes.InvalidArgument, "filename must be an absolute path")
}

// Install the package using sonic-installer
if err := installPackage(ctx, pkg.Package.Filename); err != nil {
// Install the package using sonic-installer (async, survives gRPC disconnect)
if err := installPackage(pkg.Package.Filename); err != nil {
log.Errorf("Failed to install package %s: %v", pkg.Package.Filename, err)
return nil, status.Errorf(codes.Internal, "failed to install package: %v", err)
}
Expand All @@ -117,7 +117,7 @@ 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 {
if err := activatePackage(pkg.Package.Version); err != nil {
log.Errorf("Failed to activate package %s: %v", pkg.Package.Version, err)
return nil, status.Errorf(codes.Internal, "failed to activate package: %v", err)
}
Expand All @@ -128,19 +128,22 @@ func HandleSetPackage(ctx context.Context, req *syspb.SetPackageRequest) (*syspb
}

// installPackage installs a SONiC image using sonic-installer install command.
func installPackage(ctx context.Context, filename string) error {
// The command is started asynchronously and decoupled from any caller context
// so that it survives gRPC client disconnects.
func installPackage(filename string) error {
log.V(1).Infof("Installing package: %s", filename)

// Execute sonic-installer install command with -y flag for non-interactive installation
// Use a longer timeout as sonic-installer can take several minutes
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)
handle, err := exec.RunHostCommandAsync("sonic-installer", []string{"install", "-y", filename}, opts)
if err != nil {
return fmt.Errorf("failed to run sonic-installer install: %v", err)
return fmt.Errorf("failed to start sonic-installer install: %v", err)
}

result := handle.Wait()
if result.Error != nil {
return fmt.Errorf("sonic-installer install failed with exit code %d: %s",
result.ExitCode, result.Stderr)
Expand All @@ -151,19 +154,21 @@ func installPackage(ctx context.Context, filename string) error {
}

// activatePackage sets a SONiC image as the next boot image using sonic-installer set-default.
func activatePackage(ctx context.Context, version string) error {
// Like installPackage, the command is decoupled from any caller context.
func activatePackage(version string) error {
log.V(1).Infof("Activating package version: %s", version)

// Execute sonic-installer set-default command
// Use a longer timeout for consistency
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)
handle, err := exec.RunHostCommandAsync("sonic-installer", []string{"set-default", version}, opts)
if err != nil {
return fmt.Errorf("failed to run sonic-installer set-default: %v", err)
return fmt.Errorf("failed to start sonic-installer set-default: %v", err)
}

result := handle.Wait()
if result.Error != nil {
return fmt.Errorf("sonic-installer set-default failed with exit code %d: %s",
result.ExitCode, result.Stderr)
Expand Down
Loading
Loading