Skip to content
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.

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

[SerialConsole] Add new extension to support Serial Console #3529

Merged
merged 30 commits into from
Jul 28, 2021

Conversation

adrianabedon
Copy link
Contributor

@adrianabedon adrianabedon commented Jun 22, 2021

add serial-console extension
Cli commands:
az serial-console connect
az serial-console send nmi/sysrq/reset


This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

Made send nmi/sysrq/reset not use virtual terminal. Reformatted code to
improve styling. Now Cli errors are used to report errors. When
configuring terminal, errors reporting is improved.
Now a resource is checked to see whether it exists, it is running, and
custom boot diagnostics is enabled, all before a call to the serial
console resource provider is made.
Now AzureConnectionError is used and serial console can be launched as
long as VM is not deallocating or deallocated.
Testing for sending SysRq and NMI to VM's
Modified previous tests and added tests for VMSS's
Loading bug where pressing enter while loading would attempt to make a
new connection to the serial console is now fixed. Also now check the
OS version of the connected VM and change the loading screen if it is
Windows and remove the SysRq option.
Now SysRq checks to make sure the connection is to a Linux VM. Removed
unnecessary storage create command from tests.
Changed parts of check resource to fix some edge case errors
Fixed problem were testing would fail for reset vm command. Updated
test recording files, readme, metedata to remove upper cli version
limit, and fixed style in some files.
Added dependencies and changed readme file location
@ghost ghost added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Jun 22, 2021
@ghost
Copy link

ghost commented Jun 22, 2021

Thank you for your contribution adrianabedon! We will review the pull request and get back to you soon.

@ghost
Copy link

ghost commented Jun 22, 2021

CLA assistant check
All CLA requirements met.

@adrianabedon adrianabedon marked this pull request as ready for review June 22, 2021 00:34
@adrianabedon adrianabedon marked this pull request as draft June 22, 2021 00:36
@yonzhan
Copy link
Collaborator

yonzhan commented Jun 22, 2021

SerialConsole

@yonzhan yonzhan requested review from kairu-ms and jsntcy June 22, 2021 01:16
@yonzhan yonzhan added this to the S189 milestone Jun 22, 2021
These commands enable or disable serial console across a subscription.
Also updated help docs, README, and tests for new commands.
Comment on lines 14 to 15
import numpy # pylint: disable=unused-import
import wsaccel # pylint: disable=unused-import
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you use these library?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The websocket client recommended that numpy and wsaccel be imported to improve the speed of sending data. However, this advice has changed in the last couple of weeks and now only wsaccel is recommended. I will remove the numpy import.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, upon further investigation, wsaccel is not needed if utf8 validation is disabled. I will remove it as well.

Comment on lines 30 to 38
self.webSocket = None
self.terminalInstance = None
self.serialConsoleInstance = None
self.terminatingApp = False
self.loading = True
self.firstMessage = True
self.blockPrint = False
self.trycount = 0
self.OSIsWindows = False
Copy link
Contributor

@kairu-ms kairu-ms Jun 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use snake case instead of camel case for the names of property, variable and function in python. #Resolved

@kairu-ms kairu-ms requested a review from jiasli June 28, 2021 01:43
Code no longer needs numpy/wsaccel dependecy to run quickly so they were
removed.


ARM_ENDPOINT = "https://management.azure.com"
RP_PROVIDER = "Microsoft.SerialConsole"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARM ENDPOINT will be changed when the could is switched. So this value should be read from cli_ctx.cloud property. It's defined here https://github.com/Azure/azure-cli/blob/3feea02888ea67f033f407174a3a7a340158b81a/src/azure-cli-core/azure/cli/core/cloud.py#L297-L332

@yonzhan yonzhan modified the milestones: S189, Jul 2021 (2021-08-03) Jul 5, 2021
This client code was generated using the autorest.az code generator.
Some small changes to error checking and messaging.
No longer need custom boot diagnostics for VM. Changed testing to reflect
changes.
The Cli would automatically initialize colorama breaking a lot of the
Virtual Terminal Sequences that were used in the program
@adrianabedon adrianabedon marked this pull request as ready for review July 8, 2021 05:20
@kairu-ms
Copy link
Contributor

@adrianabedon Please let me know when it's ready to release.

@kairu-ms kairu-ms merged commit 1fad1dd into Azure:main Jul 28, 2021
@adrianabedon adrianabedon deleted the serialconsole-dev branch July 28, 2021 21:58
@adrianabedon adrianabedon restored the serialconsole-dev branch July 28, 2021 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants