[App Service] Fix SSH tunnel reliability, signal handling, and instance targeting#33070
Open
[App Service] Fix SSH tunnel reliability, signal handling, and instance targeting#33070
Conversation
…h`: tunnel reliability and instance targeting - Add WebSocket retry logic with exponential backoff for tunnel connections (Azure#8831) - Add keepalive pings to prevent idle disconnects (Azure#8831) - Register signal handlers (SIGINT/SIGTERM) and atexit for clean tunnel shutdown (Azure#13662) - Wrap main loops in try/finally to ensure cleanup on KeyboardInterrupt (Azure#13662) - Add close() method to TunnelServer for deterministic resource cleanup (Azure#13662) - Improve --instance and --timeout parameter help text (Azure#13008) - Add --instance examples to help text for webapp ssh and create-remote-connection (Azure#13008) - Add unit tests for tunnel retry, keepalive, close, and signal registration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
️✔️AzureCLI-FullTest
|
|
Hi @seligj95, |
Collaborator
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
️✔️AzureCLI-BreakingChangeTest
|
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
bot
requested review from
jsntcy,
wangzelin007,
yonzhan and
zhoxing-ms
microsoft-github-policy-service
bot
added
Web Apps
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the reliability and shutdown behavior of App Service tunneling used by az webapp ssh / az webapp create-remote-connection, and enhances instance-targeting guidance in CLI help.
Changes:
- Adds WebSocket reconnect (exponential backoff) plus periodic keepalive pings in the App Service tunnel server.
- Introduces explicit tunnel cleanup via
TunnelServer.close()and registers cleanup on Ctrl+C / termination paths. - Updates parameter help and command examples for
--instance/--timeout, and adds unit tests for the new tunnel behaviors.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/azure-cli/azure/cli/command_modules/appservice/tunnel.py |
Adds retry/backoff WebSocket creation, keepalive pings, shutdown event, and close() cleanup. |
src/azure-cli/azure/cli/command_modules/appservice/custom.py |
Registers signal/atexit cleanup and ensures tunnel close on KeyboardInterrupt via try/finally. |
src/azure-cli/azure/cli/command_modules/appservice/_params.py |
Improves --timeout / --instance argument descriptions for tunnel-related commands. |
src/azure-cli/azure/cli/command_modules/appservice/_help.py |
Adds examples demonstrating --instance usage for SSH and remote connection commands. |
src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py |
Adds unit tests for retry, keepalive, close() idempotency, and signal cleanup registration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
seligj95
changed the title
az webapp ssh: tunnel reliability and instance targeting… handler - Replace time.sleep(delay) with self._closing.wait(delay) in retry backoff so shutdown wakes immediately when closing event is set - Remove sys.exit(0) from signal handler; let close() set the event and the main loop exit naturally - Remove unused 'import time' from tunnel.py - Update tests to mock _closing.wait instead of time.sleep Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 3 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
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.
Description
Changes
Tunnel reliability (#8831):
TunnelServer._create_websocket_connection()Clean Ctrl+C shutdown (#13662):
tunnel_server.close()atexitfallback cleanupTunnelServer.close()method with idempotent, exception-safe resource cleanupInstance targeting docs (#13008):
--instanceand--timeoutparameter descriptions--instanceusage examples to help text for bothwebapp sshandwebapp create-remote-connectionTesting
test_webapp_commands_thru_mock.pycovering:TunnelServer.close()idempotency and resource cleanupFiles Changed
tunnel.py— retry logic, keepalive, close(), closing eventcustom.py— signal handlers, atexit, try/finally cleanup_params.py— improved parameter help text_help.py— added --instance examplestest_webapp_commands_thru_mock.py— new unit testsFixes #8831
Fixes #13662
Fixes #13008