Add async command execution for long-running SetPackage operations#621
Open
sigabrtv1-ui wants to merge 1 commit into
Open
Add async command execution for long-running SetPackage operations#621sigabrtv1-ui wants to merge 1 commit into
sigabrtv1-ui wants to merge 1 commit into
Conversation
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Add RunHostCommandAsync to pkg/exec which starts commands decoupled from the caller's context. This is critical for SetPackage where sonic-installer can run for minutes - if the gRPC client disconnects, the installation continues safely in the background instead of being killed. Changes: - Add AsyncCommandHandle struct with Wait() and Done() methods - Add RunHostCommandAsync() using background context with timeout - Add NewCompletedAsyncHandle() helper for testing - Refactor installPackage/activatePackage to use async execution - Remove ctx parameter since the command manages its own lifecycle - Update all tests for the new signatures Related: sonic-net#619 Related: sonic-net/sonic-mgmt#23054 Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
sigabrtv1-ui
force-pushed
the
feat/async-command-exec
branch
from
13cfcd1 to
b3f7370
Compare
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
Added
RunHostCommandAsynctopkg/execand refactored theSetPackagehandler to use it, so that long-runningsonic-installercommands survive gRPC client disconnects.How I did it
pkg/exec/command.go: AddedAsyncCommandHandlestruct andRunHostCommandAsync()function that starts commands using a background context (decoupled from the caller). The timeout is still enforced via an internal context.pkg/gnoi/system/system.go: RefactoredinstallPackage()andactivatePackage()to useRunHostCommandAsync()instead ofRunHostCommand(). Removed thectxparameter since the command now manages its own lifecycle.NewCompletedAsyncHandle()helper for testing.Why I did it
During DPU image installation via
SetPackage,sonic-installer installcan take several minutes. If the gRPC connection drops (e.g., due to keepalive issues), the previous synchronousRunHostCommandwould be killed via context cancellation — potentially leaving the DPU in a broken state mid-install.With async execution, the
sonic-installerprocess continues to completion regardless of client disconnect.How to verify it
make -f pure.mk test PACKAGES="pkg/exec pkg/gnoi/system"(96.1% coverage on system, 82.9% on exec)Related: #619
Related: sonic-net/sonic-mgmt#23054