-
Notifications
You must be signed in to change notification settings - Fork 1.2k
extension/proxmox: add console access for instances #11601
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
Conversation
Add console access support for the instances deployed using inbuilt Proxmox Orchestrator extension. Underlying CloudStack queries Proxmox API for console and then passes the ticket and host to the CPVM for the zone. During the flow it is assumed CPVM will be able to access the instance (Proxmox) host. Signed-off-by: Abhishek Kumar <[email protected]>
@blueorangutan package |
@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #11601 +/- ##
============================================
+ Coverage 17.39% 17.50% +0.10%
- Complexity 15285 15425 +140
============================================
Files 5889 5894 +5
Lines 526183 526836 +653
Branches 64242 64331 +89
============================================
+ Hits 91542 92228 +686
+ Misses 424297 424231 -66
- Partials 10344 10377 +33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14921 |
Signed-off-by: Abhishek Kumar <[email protected]>
Signed-off-by: Abhishek Kumar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Add console access support for instances deployed using the inbuilt Proxmox Orchestrator extension. CloudStack queries the Proxmox API for console credentials and passes them to the CPVM for the zone.
- Enables console access for External hypervisor instances by removing restriction
- Implements console command handling for External/Proxmox instances
- Adds proper authentication bypass for External hypervisors
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 7 comments.
Show a summary per file
File | Description |
---|---|
ui/src/components/view/ActionButton.vue | Removes External hypervisor restriction from console buttons |
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java | Adds console connection details handling for External hypervisors with proper fallback logic |
server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java | Adds authentication bypass for External hypervisor instances |
server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java | Adds authentication bypass for External hypervisor instances |
server/src/main/java/com/cloud/server/ManagementServerImpl.java | Implements getExternalVmConsole method |
extensions/Proxmox/proxmox.sh | Adds getconsole action for Proxmox VNC proxy support |
Multiple test files | Comprehensive test coverage for new console functionality |
Multiple API/framework files | New command classes and framework integration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Abhishek Kumar <[email protected]>
@blueorangutan package |
@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14936 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @shwstppr LGTM - left only some minor comments.
It would worth documenting what CloudStack expects from external extensions in order to display the console correctly.
.../src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java
Outdated
Show resolved
Hide resolved
...va/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java
Outdated
Show resolved
Hide resolved
Related: apache/cloudstack#11601 Signed-off-by: Abhishek Kumar <[email protected]>
Signed-off-by: Abhishek Kumar <[email protected]>
Signed-off-by: Abhishek Kumar <[email protected]>
@blueorangutan test |
@kiranchavala a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Test Case Execution | Result |
---|---|
Test the console access of a proxmox vm from the UI | Pass |
Test the listConsoleSessions api call | Pass |
Test the api createConsoleEndpoint | Pass |
Test the Direct Url based console | Pass |
Test from multiple management servers enabled environment | Pass |
Test from a ssl enabled cloudstack-management server | Pass |
[SF] Trillian test result (tid-14431)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes requested:
-
Improve spec & documentation on getconsole access types, vnc and url. This does not make sense to me: " the extension returns a console URL with the protocol set to direct, url, or link. The URL is then provided directly to the user." Please also update https://github.com/apache/cloudstack-documentation/pull/560/files
-
In the getconsole, can we pass some kind of account or role type information, which can be used by Extension authors to return different URLs for different types of roles. For example, return an internal URL if the caller is a root admin versus a normal/public URL if the caller is a public tenant.
Signed-off-by: Abhishek Kumar <[email protected]>
Signed-off-by: Abhishek Kumar <[email protected]>
@blueorangutan package |
@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
@rohityadavcloud @harikrishna-patnala addressed comments. Now all extension payloads will contain
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 15161 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, didn't test the changes but good to see my remarks have been addressed. Thanks Abhishek, great work.
On a second thought, I feel |
Signed-off-by: Abhishek Kumar <[email protected]>
@blueorangutan package |
@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15165 |
@blueorangutan test ol8 vmware-80u3 |
@shwstppr a [SL] Trillian-Jenkins test job (ol8 mgmt + vmware-80u3) has been kicked to run smoke tests |
[SF] Trillian test result (tid-14439)
|
Some VMware test failures unrelated. Similar results seen here in healthcheck, #11523 (comment) |
Notes about console access for Orchestrator extension instances. Related: apache/cloudstack#11601 --------- Signed-off-by: Abhishek Kumar <[email protected]>
Description
This PR introduces console access support for instances deployed using Orchestrator Extensions, available via either VNC or a direct URL.
direct
. The URL is then provided directly to the user.Also, adds changes to send caller details to the extension payload.
Documentation PR: apache/cloudstack-documentation#560
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
proxmox-console.mp4
How Has This Been Tested?
How did you try to break this feature and the system with this change?