You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/11/24 01:34:44 UTC
[GitHub] [cloudstack] nvazquez opened a new pull request, #6919: Minor console access refactor and improvements
nvazquez opened a new pull request, #6919:
URL: https://github.com/apache/cloudstack/pull/6919
### Description
This PR adds a few improvements on the console access, by:
- Removing the `consoleproxy.extra.security.validation.enabled` configuration introduced on the main branch (not yet on any release), in favour of the optional parameter `token` on the `createConsoleEndpoint`.
- Improving the error message when trying to access the console of a virtual machine that is not running
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [x] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
- Access console for virtual machines on the UI
- Using cmk, generate a console endpoint without the token parameter -> verify connection to the URL
- Using cmk, generate a console endpoint with the token parameter -> verify connection to the URL
- Using cmk, generate a console endpoint for a stopped VM -> check descriptive error message
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on a diff in pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on code in PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#discussion_r1031613299
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -207,15 +199,22 @@ private ConsoleEndpoint generateAccessEndpoint(Long vmId, String sessionUuid, St
throw new CloudRuntimeException(msg);
}
+ String vmUuid = vm.getUuid();
+ if (vm.getState() != VirtualMachine.State.Running) {
Review Comment:
For Running it is, do you think Starting, Stopping and Migrating should be required as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on a diff in pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#discussion_r1033463834
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -207,15 +199,22 @@ private ConsoleEndpoint generateAccessEndpoint(Long vmId, String sessionUuid, St
throw new CloudRuntimeException(msg);
}
+ String vmUuid = vm.getUuid();
+ if (vm.getState() != VirtualMachine.State.Running) {
Review Comment:
https://github.com/apache/cloudstack/blob/main/ui/src/components/widgets/Console.vue#L22 we show the console when VM state is not in error, destroyed or stopped ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1331409952
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1326933918
<b>Trillian test result (tid-5257)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 46417 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6919-t5257-kvm-centos7.zip
Smoke tests completed. 103 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 463.80 | test_vpc_redundant.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1331404927
Packaging result: :heavy_check_mark: el7 :heavy_multiplication_x: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4687
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#discussion_r1044416772
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -207,15 +205,22 @@ private ConsoleEndpoint generateAccessEndpoint(Long vmId, String sessionUuid, St
throw new CloudRuntimeException(msg);
}
+ String vmUuid = vm.getUuid();
+ if (unsupportedConsoleVMState.contains(vm.getState())) {
+ msg = "VM " + vmUuid + " must be running to connect console, sending blank response for console access request";
+ s_logger.warn(msg);
+ throw new CloudRuntimeException(msg);
+ }
+
if (vm.getHostId() == null) {
- msg = "VM " + vmId + " lost host info, sending blank response for console access request";
+ msg = "VM " + vmUuid + " lost host info, sending blank response for console access request";
s_logger.warn(msg);
throw new CloudRuntimeException(msg);
}
HostVO host = managementServer.getHostBy(vm.getHostId());
if (host == null) {
- msg = "VM " + vmId + "'s host does not exist, sending blank response for console access request";
+ msg = "VM " + vmUuid + "'s host does not exist, sending blank response for console access request";
Review Comment:
if the vm state is Migrating, use vm.getLastHostId instead of vm.getHostId
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1348681762
<b>Trillian test result (tid-5484)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40038 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6919-t5484-kvm-centos7.zip
Smoke tests completed. 105 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1326364977
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1345982944
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1326328364
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4639
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1331364495
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland merged pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1326366119
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#discussion_r1044414857
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -489,6 +494,6 @@ public String getConfigComponentName() {
@Override
public ConfigKey<?>[] getConfigKeys() {
- return new ConfigKey[] { ConsoleProxyExtraSecurityValidationEnabled };
+ return new ConfigKey[] { };
Review Comment:
if so, we can remove `Configurable` from ConsoleAccessManager
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1326280647
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] codecov[bot] commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1325909089
# [Codecov](https://codecov.io/gh/apache/cloudstack/pull/6919?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6919](https://codecov.io/gh/apache/cloudstack/pull/6919?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16034f8) into [main](https://codecov.io/gh/apache/cloudstack/commit/c5e657ddd80faa93585aef1381e30b7e2d9a9eaa?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c5e657d) will **decrease** coverage by `0.00%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## main #6919 +/- ##
============================================
- Coverage 10.87% 10.87% -0.01%
+ Complexity 7117 7115 -2
============================================
Files 2485 2485
Lines 245507 245507
Branches 38334 38333 -1
============================================
- Hits 26699 26695 -4
- Misses 215538 215543 +5
+ Partials 3270 3269 -1
```
| [Impacted Files](https://codecov.io/gh/apache/cloudstack/pull/6919?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...udstack/consoleproxy/ConsoleAccessManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6919/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2NvbnNvbGVwcm94eS9Db25zb2xlQWNjZXNzTWFuYWdlckltcGwuamF2YQ==) | `5.09% <0.00%> (ø)` | |
| [...dstack/network/contrail/model/ModelObjectBase.java](https://codecov.io/gh/apache/cloudstack/pull/6919/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9uZXR3b3JrLWVsZW1lbnRzL2p1bmlwZXItY29udHJhaWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Nsb3Vkc3RhY2svbmV0d29yay9jb250cmFpbC9tb2RlbC9Nb2RlbE9iamVjdEJhc2UuamF2YQ==) | `21.15% <0.00%> (-7.70%)` | :arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#discussion_r1031511782
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -207,15 +199,22 @@ private ConsoleEndpoint generateAccessEndpoint(Long vmId, String sessionUuid, St
throw new CloudRuntimeException(msg);
}
+ String vmUuid = vm.getUuid();
+ if (vm.getState() != VirtualMachine.State.Running) {
Review Comment:
@nvazquez
is it possible to allow the console for vms in Running/Starting/Stopping/Migrating states ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on a diff in pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on code in PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#discussion_r1033857519
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -207,15 +199,22 @@ private ConsoleEndpoint generateAccessEndpoint(Long vmId, String sessionUuid, St
throw new CloudRuntimeException(msg);
}
+ String vmUuid = vm.getUuid();
+ if (vm.getState() != VirtualMachine.State.Running) {
Review Comment:
Thanks @weizhouapache @rohityadavcloud, I'll update accordingly
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1347673610
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1332100480
<b>Trillian test result (tid-5297)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 50069 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6919-t5297-kvm-centos7.zip
Smoke tests completed. 102 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_07_deploy_kubernetes_ha_cluster | `Failure` | 3606.27 | test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster | `Failure` | 0.06 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 67.69 | test_kubernetes_clusters.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 456.44 | test_vpc_redundant.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1344124508
@vladimirpetrov a 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1344216712
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_multiplication_x: suse15. SL-JID 4827
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on a diff in pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#discussion_r1045460518
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -489,6 +494,6 @@ public String getConfigComponentName() {
@Override
public ConfigKey<?>[] getConfigKeys() {
- return new ConfigKey[] { ConsoleProxyExtraSecurityValidationEnabled };
+ return new ConfigKey[] { };
Review Comment:
agree @nvazquez if there's no need for Configurable in this class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1346466223
@nvazquez can you answer @weizhouapache 's comments?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1352973762
@vladimirpetrov I have addressed @weizhouapache's comments, could you review again please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1347601700
Thanks for the review @weizhouapache, comments addressed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1347674066
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] vladimirpetrov commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
vladimirpetrov commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1344123808
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#discussion_r1032116495
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -207,15 +199,22 @@ private ConsoleEndpoint generateAccessEndpoint(Long vmId, String sessionUuid, St
throw new CloudRuntimeException(msg);
}
+ String vmUuid = vm.getUuid();
+ if (vm.getState() != VirtualMachine.State.Running) {
Review Comment:
@nvazquez
I think so.
the console icon is previously displayed for Starting/Stopping/Migrating VMs on UI.(I do not know the current behavior)
for Stopping/Migrating VMs, console works mostly (please notice the Migrating vm is running on the last host, not current host)
for Starting VMs, sometimes console works, sometimes console does not work (depend on the VM state on hypervisor).
You can test with 4.17
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1331256833
@nvazquez a 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1331256376
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1326281791
@nvazquez a 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1345983335
@rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1347086433
<b>Trillian test result (tid-5463)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41718 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6919-t5463-kvm-centos7.zip
Smoke tests completed. 105 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on a diff in pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on code in PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#discussion_r1046564256
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -489,6 +494,6 @@ public String getConfigComponentName() {
@Override
public ConfigKey<?>[] getConfigKeys() {
- return new ConfigKey[] { ConsoleProxyExtraSecurityValidationEnabled };
+ return new ConfigKey[] { };
Review Comment:
Done, thanks
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -207,15 +205,22 @@ private ConsoleEndpoint generateAccessEndpoint(Long vmId, String sessionUuid, St
throw new CloudRuntimeException(msg);
}
+ String vmUuid = vm.getUuid();
+ if (unsupportedConsoleVMState.contains(vm.getState())) {
+ msg = "VM " + vmUuid + " must be running to connect console, sending blank response for console access request";
+ s_logger.warn(msg);
+ throw new CloudRuntimeException(msg);
+ }
+
if (vm.getHostId() == null) {
- msg = "VM " + vmId + " lost host info, sending blank response for console access request";
+ msg = "VM " + vmUuid + " lost host info, sending blank response for console access request";
s_logger.warn(msg);
throw new CloudRuntimeException(msg);
}
HostVO host = managementServer.getHostBy(vm.getHostId());
if (host == null) {
- msg = "VM " + vmId + "'s host does not exist, sending blank response for console access request";
+ msg = "VM " + vmUuid + "'s host does not exist, sending blank response for console access request";
Review Comment:
Done, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1347602555
@nvazquez a 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1347601834
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1347633404
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4868
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1331322973
Packaging result: :heavy_check_mark: el7 :heavy_multiplication_x: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4685
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1331365068
@nvazquez a 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #6919: Minor console access refactor and improvements
Posted by GitBox <gi...@apache.org>.
nvazquez commented on PR #6919:
URL: https://github.com/apache/cloudstack/pull/6919#issuecomment-1331409658
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org