You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "dheeraj12347 (via GitHub)" <gi...@apache.org> on 2026/04/22 18:49:37 UTC
[PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
dheeraj12347 opened a new pull request, #13058:
URL: https://github.com/apache/cloudstack/pull/13058
### Description
This PR backports the console proxy/noVNC timeout fix to the 4.20 branch.
It ensures that `consoleproxy.session.timeout` is honoured for noVNC console
sessions, so idle sessions are cleaned up correctly and do not linger
indefinitely.
Key points:
- Wire `consoleproxy.session.timeout` through the console proxy server for
noVNC-based console sessions.
- Apply the timeout in the WebSocket handler and GC thread so idle sessions
are closed after the configured period.
- Keep the change minimal and compatible with 4.20: do not introduce the
newer `sessionRequiresNewViewer` API on `ConsoleProxyClientParam`, only
remove the references that exist in later branches.
### Related work
- Backport of the console proxy timeout fix from the main branch.
- Intended to address the same behaviour as the upstream change that fixed
idle noVNC sessions not respecting `consoleproxy.session.timeout`.
### Testing
- Compiled the console proxy server module successfully:
```bash
mvn -pl services/console-proxy/server -am -DskipTests compile
```
- Verified that the code builds cleanly with checkstyle on the 4.20 branch.
Fix #12810
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4343471389
@blueorangutan test keepEnv
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "dheeraj12347 (via GitHub)" <gi...@apache.org>.
dheeraj12347 commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5967116851
Thanks @DaanHoogland. I added the missing propagation of consoleproxy.session.timeout to the CPVM boot arguments in ConsoleProxyManagerImpl, as suggested.
In the 4.20 branch, I used the existing proxySessionTimeoutValue, which is already initialized from the consoleproxy.session.timeout configuration, to keep the backport minimal and compatible.
I also verified that the console-proxy module compiles successfully and git diff --check passes.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6317948607
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4439230793
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17838
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4505458873
<b>[SF] Trillian test result (tid-16112)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 82301 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13058-t16112-kvm-ol8.zip
Smoke tests completed. 137 look OK, 4 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestSharedFSLifecycle>:setup | `Error` | 0.00 | test_sharedfs_lifecycle.py
test_10_attachAndDetach_iso | `Failure` | 607.14 | test_vm_life_cycle.py
test_01_create_vm_snapshots | `Failure` | 603.83 | test_vm_snapshots.py
test_02_revert_vm_snapshots | `Failure` | 600.66 | test_vm_snapshots.py
test_03_delete_vm_snapshots | `Failure` | 0.02 | test_vm_snapshots.py
test_01_create_volume | `Failure` | 609.32 | test_volumes.py
test_01_root_volume_encryption | `Failure` | 728.54 | test_volumes.py
test_02_data_volume_encryption | `Failure` | 669.86 | test_volumes.py
test_03_root_and_data_volume_encryption | `Failure` | 686.29 | test_volumes.py
test_02_attach_volume | `Failure` | 1267.08 | test_volumes.py
test_02_attach_volume | `Failure` | 1267.11 | test_volumes.py
test_03_download_attached_volume | `Failure` | 664.18 | test_volumes.py
test_04_delete_attached_volume | `Failure` | 664.35 | test_volumes.py
test_05_detach_volume | `Failure` | 752.24 | test_volumes.py
test_06_download_detached_volume | `Failure` | 843.42 | test_volumes.py
test_07_resize_fail | `Failure` | 659.12 | test_volumes.py
test_08_resize_volume | `Failure` | 663.38 | test_volumes.py
test_09_delete_detached_volume | `Failure` | 664.47 | test_volumes.py
test_10_list_volumes | `Failure` | 659.06 | test_volumes.py
test_11_attach_volume_with_unstarted_vm | `Failure` | 761.82 | test_volumes.py
test_12_resize_volume_with_only_size_parameter | `Failure` | 665.42 | test_volumes.py
test_13_migrate_volume_and_change_offering | `Failure` | 792.77 | test_volumes.py
test_14_delete_volume_delete_protection | `Failure` | 662.48 | test_volumes.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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4552773901
@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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5911504491
@dheeraj12347 : a ai generated comment to this change:
> Heads up: this fix won't actually take effect even once merged.
>
> I traced why a changed consoleproxy.session.timeout still logs 300000 ms after recreating the CPVM (as seen in the thread above (#)): the ConfigKey ConsoleProxySessionTimeout (server/.../ConsoleProxyManager.java:81) is never forwarded to the CPVM. The only place that reads it management-server-side is the unrelated hasPreviousSession() reassignment check — ConsoleProxyManagerImpl.finalizeVirtualMachineProfile(), which builds the CPVM's boot args, forwards ConsoleProxySessionReconnectionWindow (session_reconnection_window=...) but has no equivalent line for this setting. So conf.getProperty("consoleproxy.session.timeout") on the CPVM side always returns null, and sessionTimeoutMillis stays at its Java default of 300000 regardless of what's configured.
>
> This PR needs an additional hunk in ConsoleProxyManagerImpl.java, next to the sessionReconnectionWindow block:
>
> Integer sessionTimeoutMillis = ConsoleProxySessionTimeout.valueIn(datacenterId);
> if (sessionTimeoutMillis != null && sessionTimeoutMillis > 0) {
> buf.append(" consoleproxy.session.timeout=").append(sessionTimeoutMillis);
> }
>
> Without it, the WebSocket-idle-timeout and GC-thread changes in this PR are unreachable — the config setting will keep behaving exactly as it does today.
I think we (as in you ;) ) should add this to this PR.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6340096911
@dheeraj12347 0f26cc3 covers my thread, only key presses and mouse moves keep the session alive now. when the timer runs out, does the console just freeze until the second timeout closes it? looks like an idle tab can sit frozen for up to 5 more minutes before it disconnects
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4440574986
<b>[SF] Trillian test result (tid-16088)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 2316 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13058-t16088-kvm-ol8.zip
Smoke tests completed. 8 look OK, 133 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
runTest | `Error` | 0.00 | test_2fa.py
runTest | `Error` | 0.00 | test_account_access.py
runTest | `Error` | 0.00 | test_accounts.py
runTest | `Error` | 0.00 | test_affinity_groups_projects.py
runTest | `Error` | 0.00 | test_affinity_groups.py
runTest | `Error` | 0.00 | test_annotations.py
runTest | `Error` | 0.00 | test_async_job.py
runTest | `Error` | 0.00 | test_attach_multiple_volumes.py
runTest | `Error` | 0.00 | test_backup_recovery_dummy.py
runTest | `Error` | 0.00 | test_backup_recovery_veeam.py
runTest | `Error` | 0.00 | test_certauthority_root.py
runTest | `Error` | 0.00 | test_cluster_drs.py
runTest | `Error` | 0.00 | test_console_endpoint.py
runTest | `Error` | 0.00 | test_create_list_domain_account_project.py
runTest | `Error` | 0.00 | test_create_network.py
runTest | `Error` | 0.00 | test_deploy_vgpu_enabled_vm.py
runTest | `Error` | 0.00 | test_deploy_virtio_scsi_vm.py
runTest | `Error` | 0.00 | test_deploy_vm_extra_config_data.py
runTest | `Error` | 0.00 | test_deploy_vm_iso.py
runTest | `Error` | 0.00 | test_deploy_vm_iso_uefi.py
runTest | `Error` | 0.00 | test_deploy_vm_root_resize.py
runTest | `Error` | 0.00 | test_deploy_vms_in_parallel.py
runTest | `Error` | 0.00 | test_deploy_vms_with_varied_deploymentplanners.py
runTest | `Error` | 0.00 | test_deploy_vm_with_userdata.py
runTest | `Error` | 0.00 | test_diagnostics.py
runTest | `Error` | 0.00 | test_direct_download.py
runTest | `Error` | 0.00 | test_disk_offerings.py
runTest | `Error` | 0.00 | test_disk_provisioning_types.py
runTest | `Error` | 0.00 | test_domain_disk_offerings.py
runTest | `Error` | 0.00 | test_domain_network_offerings.py
runTest | `Error` | 0.00 | test_domain_service_offerings.py
runTest | `Error` | 0.00 | test_domain_vpc_offerings.py
runTest | `Error` | 0.00 | test_enable_account_settings_for_domain.py
runTest | `Error` | 0.00 | test_login.py
runTest | `Error` | 0.00 | test_events_resource.py
runTest | `Error` | 0.00 | test_gateway_on_shared_networks.py
runTest | `Error` | 0.00 | test_global_acls.py
runTest | `Error` | 0.00 | test_global_settings.py
runTest | `Error` | 0.00 | test_guest_os.py
runTest | `Error` | 0.00 | test_guest_vlan_range.py
runTest | `Error` | 0.00 | test_host_control_state.py
runTest | `Error` | 0.00 | test_hostha_simulator.py
runTest | `Error` | 0.00 | test_host_ping.py
runTest | `Error` | 0.00 | test_image_store_object_migration.py
runTest | `Error` | 0.00 | test_import_unmanage_volumes.py
runTest | `Error` | 0.00 | test_internal_lb.py
runTest | `Error` | 0.00 | test_ipv4_routing.py
runTest | `Error` | 0.00 | test_ipv6_infra.py
runTest | `Error` | 0.00 | test_iso.py
runTest | `Error` | 0.00 | test_kubernetes_clusters.py
runTest | `Error` | 0.00 | test_kubernetes_supported_versions.py
runTest | `Error` | 0.00 | test_list_accounts.py
runTest | `Error` | 0.00 | test_list_disk_offerings.py
runTest | `Error` | 0.00 | test_list_domains.py
runTest | `Error` | 0.00 | test_list_hosts.py
runTest | `Error` | 0.00 | test_list_ids_parameter.py
runTest | `Error` | 0.00 | test_list_service_offerings.py
runTest | `Error` | 0.00 | test_list_storage_pools.py
runTest | `Error` | 0.00 | test_list_volumes.py
runTest | `Error` | 0.00 | test_loadbalance.py
runTest | `Error` | 0.00 | test_metrics_api.py
runTest | `Error` | 0.00 | test_migration.py
runTest | `Error` | 0.00 | test_multipleips_per_nic.py
runTest | `Error` | 0.00 | test_nested_virtualization.py
runTest | `Error` | 0.00 | test_network_acl.py
runTest | `Error` | 0.00 | test_network_ipv6.py
runTest | `Error` | 0.00 | test_network_permissions.py
runTest | `Error` | 0.00 | test_network.py
runTest | `Error` | 0.00 | test_nic_adapter_type.py
runTest | `Error` | 0.00 | test_nic.py
runTest | `Error` | 0.00 | test_non_contigiousvlan.py
runTest | `Error` | 0.00 | test_nonstrict_affinity_group.py
runTest | `Error` | 0.00 | test_outofbandmanagement_nestedplugin.py
runTest | `Error` | 0.00 | test_outofbandmanagement.py
runTest | `Error` | 0.00 | test_over_provisioning.py
runTest | `Error` | 0.00 | test_password_server.py
runTest | `Error` | 0.00 | test_persistent_network.py
runTest | `Error` | 0.00 | test_portable_publicip.py
runTest | `Error` | 0.00 | test_portforwardingrules.py
runTest | `Error` | 0.00 | test_primary_storage.py
runTest | `Error` | 0.00 | test_primary_storage_scope.py
runTest | `Error` | 0.00 | test_privategw_acl_ovs_gre.py
runTest | `Error` | 0.00 | test_privategw_acl.py
runTest | `Error` | 0.00 | test_projects.py
runTest | `Error` | 0.00 | test_public_ip_range.py
runTest | `Error` | 0.00 | test_purge_expunged_vms.py
runTest | `Error` | 0.00 | test_pvlan.py
runTest | `Error` | 0.00 | test_quarantined_ips.py
runTest | `Error` | 0.00 | test_regions.py
runTest | `Error` | 0.00 | test_register_userdata.py
runTest | `Error` | 0.00 | test_reset_configuration_settings.py
runTest | `Error` | 0.00 | test_reset_vm_on_reboot.py
runTest | `Error` | 0.00 | test_resource_accounting.py
runTest | `Error` | 0.00 | test_resource_detail.py
runTest | `Error` | 0.00 | test_resource_names.py
runTest | `Error` | 0.00 | test_restore_vm.py
runTest | `Error` | 0.00 | test_router_dhcphosts.py
runTest | `Error` | 0.00 | test_router_dns.py
runTest | `Error` | 0.00 | test_router_dnsservice.py
runTest | `Error` | 0.00 | test_routers_iptables_default_policy.py
runTest | `Error` | 0.00 | test_routers_network_ops.py
runTest | `Error` | 0.00 | test_routers.py
runTest | `Error` | 0.00 | test_safe_shutdown.py
runTest | `Error` | 0.00 | test_scale_vm.py
runTest | `Error` | 0.00 | test_secondary_storage.py
runTest | `Error` | 0.00 | test_service_offerings.py
runTest | `Error` | 0.00 | test_set_sourcenat.py
runTest | `Error` | 0.00 | test_sharedfs_lifecycle.py
runTest | `Error` | 0.00 | test_snapshots.py
runTest | `Error` | 0.00 | test_ssvm.py
runTest | `Error` | 0.00 | test_storage_policy.py
runTest | `Error` | 0.00 | test_templates.py
runTest | `Error` | 0.00 | test_update_security_group.py
runTest | `Error` | 0.00 | test_usage_events.py
runTest | `Error` | 0.00 | test_usage.py
runTest | `Error` | 0.00 | test_vm_autoscaling.py
runTest | `Error` | 0.00 | test_vm_deployment_planner.py
runTest | `Error` | 0.00 | test_vm_life_cycle.py
runTest | `Error` | 0.00 | test_vm_lifecycle_unmanage_import.py
runTest | `Error` | 0.00 | test_vm_schedule.py
runTest | `Error` | 0.00 | test_vm_snapshot_kvm.py
runTest | `Error` | 0.00 | test_vm_snapshots.py
runTest | `Error` | 0.00 | test_vm_strict_host_tags.py
runTest | `Error` | 0.00 | test_volumes.py
runTest | `Error` | 0.00 | test_vnf_templates.py
runTest | `Error` | 0.00 | test_vpc_ipv6.py
runTest | `Error` | 0.00 | test_vpc_redundant.py
runTest | `Error` | 0.00 | test_vpc_router_nics.py
runTest | `Error` | 0.00 | test_vpc_vpn.py
runTest | `Error` | 0.00 | test_webhook_delivery.py
runTest | `Error` | 0.00 | test_webhook_lifecycle.py
runTest | `Error` | 0.00 | test_host_maintenance.py
runTest | `Error` | 0.00 | test_hostha_kvm.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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "dheeraj12347 (via GitHub)" <gi...@apache.org>.
dheeraj12347 commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4348019343
Thanks for the detailed review and suggestions from Copilot.
I’ve read and understood all of the comments, especially around the 4.20 authenticateConsoleAccess signature/invocation, idle timeout handling in the GC thread, noVNC viewer activity tracking, and the unused sessionRequiresNewViewer flag.
I’ll work through each point and push updates to address them as soon as possible.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4554847296
<b>[SF] Trillian Build Failed (tid-16198)<b/>
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3765547951
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java:
##########
Review Comment:
```suggestion
public void onConnect(final Session session) throws IOException, InterruptedException {
session.setIdleTimeout(ConsoleProxy.sessionTimeoutMillis);
logger.debug("Set WebSocket idle timeout to {} ms for session from {}.",
ConsoleProxy.sessionTimeoutMillis, session.getRemoteAddress());
String queries = session.getUpgradeRequest().getQueryString();
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6018539857
@kiranchavala can you have a second look?
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5990302820
@DaanHoogland 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.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "dheeraj12347 (via GitHub)" <gi...@apache.org>.
dheeraj12347 commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6322704293
Hi @DaanHoogland, @Damans227, and @kiranchavala,
I've pushed an additional test in `ConsoleProxyNoVncClientTest` to verify that the frontend activity timestamp advances when updated.
The targeted test passes locally (2 tests, 0 failures), and the console-proxy server module compiles successfully.
The activity-tracking changes are also pushed. I'll continue verifying that only actual keyboard and mouse interactions refresh the idle timer, while automatic screen updates do not.
Could you please review the latest changes when convenient? 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4342461800
@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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4553188567
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18054
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3765769893
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java:
##########
@@ -183,12 +190,20 @@ public void onClose(Session session, int statusCode, String reason) throws IOExc
@OnWebSocketFrame
public void onFrame(Frame f) throws IOException {
+ if (viewer == null) {
+ logger.debug("Ignoring WebSocket frame because viewer is not initialized yet.");
+ return;
+ }
logger.trace("Sending client [ID: {}] frame of {} bytes.", viewer.getClientId(), f.getPayloadLength());
Review Comment:
```suggestion
logger.trace("Sending client [ID: {}] frame of {} bytes.", viewer.getClientId(), f.getPayloadLength());
viewer.updateFrontEndActivityTime();
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5266358931
@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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5323965969
<b>[SF] Trillian test result (tid-16769)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 53695 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13058-t16769-kvm-ol8.zip
Smoke tests completed. 141 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5990283815
@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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5993406872
@blueorangutan test keepEnv
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3309508191
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java:
##########
@@ -93,15 +94,16 @@ public void onConnect(final Session session) throws IOException, InterruptedExce
String websocketUrl = queryMap.get("websocketUrl");
String sessionUuid = queryMap.get("sessionUuid");
String clientIp = session.getRemoteAddress().getAddress().getHostAddress();
+ boolean sessionRequiresNewViewer = Boolean.parseBoolean(queryMap.get("sessionRequiresNewViewer"));
- if (tag == null)
+ if (tag == null) {
tag = "";
+ }
Review Comment:
`sessionRequiresNewViewer` is parsed from the query string but never used. This is confusing (and conflicts with the PR description about removing newer-branch references). Please remove the unused variable/parameter, or wire it into the viewer selection logic if it is required on 4.20.
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java:
##########
@@ -16,98 +16,119 @@
// under the License.
package com.cloud.consoleproxy;
+
import java.io.File;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
+
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
+
/**
*
* ConsoleProxyGCThread does house-keeping work for the process, it helps cleanup log files,
* recycle idle client sessions without front-end activities and report client stats to external
* management software
*/
public class ConsoleProxyGCThread extends Thread {
- protected Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
+ private static final Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
- private final static int MAX_SESSION_IDLE_SECONDS = 180;
private final Map<String, ConsoleProxyClient> connMap;
private final Set<String> removedSessionsSet;
private long lastLogScan = 0;
+
public ConsoleProxyGCThread(Map<String, ConsoleProxyClient> connMap, Set<String> removedSet) {
this.connMap = connMap;
this.removedSessionsSet = removedSet;
}
+
private void cleanupLogging() {
- if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000)
+ if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000) {
return;
+ }
+
lastLogScan = System.currentTimeMillis();
+
File logDir = new File("./logs");
- File files[] = logDir.listFiles();
+ File[] files = logDir.listFiles();
if (files != null) {
for (File file : files) {
if (System.currentTimeMillis() - file.lastModified() >= 86400000L) {
try {
file.delete();
} catch (Throwable e) {
- logger.info("[ignored]"
- + "failed to delete file: " + e.getLocalizedMessage());
+ logger.info("[ignored] failed to delete file: " + e.getLocalizedMessage());
}
}
}
}
}
+
@Override
public void run() {
+
boolean bReportLoad = false;
long lastReportTick = System.currentTimeMillis();
+
while (true) {
cleanupLogging();
bReportLoad = false;
+
if (logger.isDebugEnabled()) {
- logger.debug(String.format("connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
+ logger.debug(String.format("ConsoleProxyGCThread loop: connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
}
- Set<String> e = connMap.keySet();
- Iterator<String> iterator = e.iterator();
+ Set<String> keys = connMap.keySet();
+ Iterator<String> iterator = keys.iterator();
while (iterator.hasNext()) {
String key;
ConsoleProxyClient client;
+
synchronized (connMap) {
key = iterator.next();
client = connMap.get(key);
}
- long seconds_unused = (System.currentTimeMillis() - client.getClientLastFrontEndActivityTime()) / 1000;
- if (seconds_unused < MAX_SESSION_IDLE_SECONDS) {
+
+ if (client == null) {
+ continue;
+ }
+
+
+ long millisecondsUnused = System.currentTimeMillis() - client.getClientLastFrontEndActivityTime();
+ if (millisecondsUnused < ConsoleProxy.sessionTimeoutMillis) {
continue;
}
+
synchronized (connMap) {
connMap.remove(key);
bReportLoad = true;
}
Review Comment:
The GC loop iterates `connMap.keySet().iterator()` but removes entries via `connMap.remove(key)` while still using the iterator. This can throw `ConcurrentModificationException` (and permanently stop the GC thread), especially since other threads also modify `connMap`. Please iterate over a snapshot of keys/entries or remove via `iterator.remove()` while holding the appropriate lock, and keep the close operation outside the lock.
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -53,23 +56,33 @@
public class ConsoleProxy {
protected static Logger LOGGER = LogManager.getLogger(ConsoleProxy.class);
+
public static final int KEYBOARD_RAW = 0;
public static final int KEYBOARD_COOKED = 1;
+
public static final int VIEWER_LINGER_SECONDS = 180;
+ // New: default and effective session timeout (milliseconds) honoured from consoleproxy.session.timeout
+ public static final int DEFAULT_SESSION_TIMEOUT_MILLIS = 300000;
+ public static volatile int sessionTimeoutMillis = DEFAULT_SESSION_TIMEOUT_MILLIS;
+
+
Review Comment:
The newly introduced `sessionTimeoutMillis` (ms) and the existing `VIEWER_LINGER_SECONDS` (s) now represent two different “idle” thresholds used in different places (e.g., `isFrontEndAlive()` vs GC cleanup). This can lead to inconsistent behaviour where sessions are considered dead/reused at one timeout but only cleaned up at another. Also, parsing/storing the timeout as an `int` risks overflow for larger millisecond values. Consider using a single long-based timeout source (e.g., `volatile long sessionTimeoutMillis`) and deriving any second-based thresholds from it where needed.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4554434177
@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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4562196913
<b>[SF] Trillian test result (tid-16205)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 56629 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13058-t16205-kvm-ol8.zip
Smoke tests completed. 140 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_04_deploy_vnf_appliance | `Error` | 311.62 | test_vnf_templates.py
test_05_delete_vnf_template | `Error` | 1.09 | test_vnf_templates.py
ContextSuite context=TestVnfTemplates>:teardown | `Error` | 2.19 | test_vnf_templates.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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3765607694
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java:
##########
@@ -73,6 +74,11 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
@OnWebSocketConnect
public void onConnect(final Session session) throws IOException, InterruptedException {
+ public void onConnect(final Session session) throws IOException, InterruptedException {
Review Comment:
```suggestion
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3765774217
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java:
##########
@@ -16,98 +16,119 @@
// under the License.
package com.cloud.consoleproxy;
+
import java.io.File;
import java.util.ArrayList;
import java.util.Iterator;
Review Comment:
```suggestion
import java.util.List;
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3765990687
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -144,36 +170,61 @@ private static void configProxy(Properties conf) {
factoryClzName = ConsoleProxyBaseServerFactoryImpl.class.getName();
}
- s = conf.getProperty("consoleproxy.httpCmdListenPort");
+
+ s = conf != null ? conf.getProperty("consoleproxy.httpCmdListenPort") : null;
if (s != null) {
httpCmdListenPort = Integer.parseInt(s);
LOGGER.info("Setting httpCmdListenPort=" + s);
}
- s = conf.getProperty("consoleproxy.reconnectMaxRetry");
+
+ s = conf != null ? conf.getProperty("consoleproxy.reconnectMaxRetry") : null;
if (s != null) {
reconnectMaxRetry = Integer.parseInt(s);
LOGGER.info("Setting reconnectMaxRetry=" + reconnectMaxRetry);
}
- s = conf.getProperty("consoleproxy.readTimeoutSeconds");
+
+ s = conf != null ? conf.getProperty("consoleproxy.readTimeoutSeconds") : null;
if (s != null) {
readTimeoutSeconds = Integer.parseInt(s);
LOGGER.info("Setting readTimeoutSeconds=" + readTimeoutSeconds);
}
- s = conf.getProperty("consoleproxy.defaultBufferSize");
+
+ s = conf != null ? conf.getProperty("consoleproxy.defaultBufferSize") : null;
if (s != null) {
defaultBufferSize = Integer.parseInt(s);
LOGGER.info("Setting defaultBufferSize=" + defaultBufferSize);
}
+
+ // New: read consoleproxy.session.timeout (milliseconds)
+ s = conf != null ? conf.getProperty("consoleproxy.session.timeout") : null;
+ if (s != null) {
+ try {
+ int value = Integer.parseInt(s);
Review Comment:
```suggestion
long value = Long.parseLong(s);
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3765986218
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -53,23 +56,33 @@
public class ConsoleProxy {
protected static Logger LOGGER = LogManager.getLogger(ConsoleProxy.class);
+
public static final int KEYBOARD_RAW = 0;
public static final int KEYBOARD_COOKED = 1;
+
public static final int VIEWER_LINGER_SECONDS = 180;
+ // New: default and effective session timeout (milliseconds) honoured from consoleproxy.session.timeout
+ public static final int DEFAULT_SESSION_TIMEOUT_MILLIS = 300000;
+ public static volatile int sessionTimeoutMillis = DEFAULT_SESSION_TIMEOUT_MILLIS;
Review Comment:
```suggestion
public static final long DEFAULT_SESSION_TIMEOUT_MILLIS = 300000;
public static volatile long sessionTimeoutMillis = DEFAULT_SESSION_TIMEOUT_MILLIS;
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5993425360
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4343482960
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3153177256
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -233,20 +234,21 @@ public static ConsoleProxyAuthenticationResult authenticateConsoleAccess(Console
Object result;
try {
result =
- authMethod.invoke(ConsoleProxy.context, param.getClientHostAddress(), String.valueOf(param.getClientHostPort()), param.getClientTag(),
- param.getClientHostPassword(), param.getTicket(), reauthentication, param.getSessionUuid());
+ authMethod.invoke(ConsoleProxy.context, param.getClientHostAddress(), String.valueOf(param.getClientHostPort()),
+ param.getClientTag(), param.getClientHostPassword(), param.getTicket(), reauthentication,
+ param.getSessionUuid(), param.getClientIp());
} catch (IllegalAccessException e) {
Review Comment:
authenticateConsoleAccess(...) now invokes authMethod with an extra param.getClientIp() argument. On 4.20, ConsoleProxyResource.authenticateConsoleAccess(...) only accepts 7 arguments, so this call will fail if/when authMethod is resolved. Keep the invocation arguments consistent with the reflected method signature (or implement a dual-signature invocation path).
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -308,7 +311,7 @@ public static void startWithContext(Properties conf, Object context, byte[] ksBi
final ClassLoader loader = Thread.currentThread().getContextClassLoader();
Class<?> contextClazz = loader.loadClass("com.cloud.agent.resource.consoleproxy.ConsoleProxyResource");
authMethod = contextClazz.getDeclaredMethod("authenticateConsoleAccess", String.class, String.class,
- String.class, String.class, String.class, Boolean.class, String.class);
+ String.class, String.class, String.class, Boolean.class, String.class, String.class);
Review Comment:
The reflective lookup of ConsoleProxyResource.authenticateConsoleAccess(...) now expects an extra String parameter, but in this branch ConsoleProxyResource defines authenticateConsoleAccess(host, port, vmId, sid, ticket, isReauthentication, sessionToken) with 7 params. This will trigger NoSuchMethodException at startup, leaving authMethod unset and causing authenticateConsoleAccess(...) to fall back to "offline mode" (success=true) which effectively bypasses management-server authentication. Align the getDeclaredMethod(...) signature (and corresponding invoke call) with the actual ConsoleProxyResource method in 4.20, or add a backwards-compatible lookup that tries both signatures.
```suggestion
String.class, String.class, String.class, Boolean.class, String.class);
```
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -596,50 +611,55 @@ public void execute(Runnable r) {
}
public static ConsoleProxyNoVncClient getNoVncViewer(ConsoleProxyClientParam param, String ajaxSession,
- Session session) throws AuthenticationException {
+ Session session) throws AuthenticationException {
boolean reportLoadChange = false;
String clientKey = param.getClientMapKey();
- synchronized (connectionMap) {
- ConsoleProxyClient viewer = connectionMap.get(clientKey);
- if (viewer == null || viewer.getClass() != ConsoleProxyNoVncClient.class) {
- authenticationExternally(param);
- viewer = new ConsoleProxyNoVncClient(session);
- viewer.initClient(param);
+ LOGGER.debug("Getting NoVNC viewer for {}. Client tag: {}. session UUID: {}",
+ clientKey, param.getClientTag(), param.getSessionUuid());
+synchronized (connectionMap) {
+ ConsoleProxyClient viewer = connectionMap.get(clientKey);
+ if (viewer == null || viewer.getClass() != ConsoleProxyNoVncClient.class) {
+ authenticationExternally(param);
+ viewer = new ConsoleProxyNoVncClient(session);
+ viewer.initClient(param);
+
+ connectionMap.put(clientKey, viewer);
+ reportLoadChange = true;
+ } else {
+ if (param.getClientHostPassword() == null || param.getClientHostPassword().isEmpty()
+ || !param.getClientHostPassword().equals(viewer.getClientHostPassword())) {
+ throw new AuthenticationException("Cannot use the existing viewer " + viewer + ": bad sid");
+ }
- connectionMap.put(clientKey, viewer);
- reportLoadChange = true;
- } else {
- if (param.getClientHostPassword() == null || param.getClientHostPassword().isEmpty() ||
- !param.getClientHostPassword().equals(viewer.getClientHostPassword()))
- throw new AuthenticationException("Cannot use the existing viewer " + viewer + ": bad sid");
+ try {
+ authenticationExternally(param);
+ } catch (Exception e) {
+ LOGGER.error("Authentication failed for param: " + param);
Review Comment:
In getNoVncViewer(...), the AuthenticationException path logs "Authentication failed" without including the caught exception, which loses the stack trace/context needed to debug auth failures. Log the exception (and preferably include key identifiers like sessionUuid/clientKey) when catching the failure.
```suggestion
LOGGER.error("Authentication failed for sessionUuid={}, clientKey={}, param={}",
param.getSessionUuid(), clientKey, param, e);
```
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java:
##########
@@ -93,15 +94,16 @@ public void onConnect(final Session session) throws IOException, InterruptedExce
String websocketUrl = queryMap.get("websocketUrl");
String sessionUuid = queryMap.get("sessionUuid");
String clientIp = session.getRemoteAddress().getAddress().getHostAddress();
+ boolean sessionRequiresNewViewer = Boolean.parseBoolean(queryMap.get("sessionRequiresNewViewer"));
Review Comment:
sessionRequiresNewViewer is parsed from the query string but never used. If this branch intentionally does not support that behavior (per the PR description), remove this local variable (and possibly the query param handling) to avoid confusion and dead code.
```suggestion
```
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java:
##########
@@ -32,9 +32,13 @@
* management software
*/
public class ConsoleProxyGCThread extends Thread {
- protected Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
+ private static final Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
- private final static int MAX_SESSION_IDLE_SECONDS = 180;
+ /**
+ * Maximum time (in seconds) a console session is allowed to be idle before it is closed.
+ * This value should be kept in sync with ConsoleProxy.VIEWER_LINGER_SECONDS.
+ */
+ private static final int MAX_SESSION_IDLE_SECONDS = 180;
Review Comment:
MAX_SESSION_IDLE_SECONDS is still hard-coded to 180s, which does not match the documented/expected behavior of honoring the global consoleproxy.session.timeout (default 300000ms) and is also duplicating ConsoleProxy.VIEWER_LINGER_SECONDS. To honor the setting and avoid drift, derive the idle timeout from the configured consoleproxy.session.timeout and use a single shared value for both GC and viewer liveness checks.
```suggestion
* Reuse the shared viewer linger timeout so GC honors the configured
* consoleproxy.session.timeout and does not drift from viewer liveness checks.
*/
private static final int MAX_SESSION_IDLE_SECONDS = ConsoleProxy.VIEWER_LINGER_SECONDS;
```
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java:
##########
@@ -183,12 +186,20 @@ public void onClose(Session session, int statusCode, String reason) throws IOExc
@OnWebSocketFrame
public void onFrame(Frame f) throws IOException {
+ if (viewer == null) {
+ logger.debug("Ignoring WebSocket frame because viewer is not initialized yet.");
+ return;
+ }
Review Comment:
NoVNC session idle tracking relies on getClientLastFrontEndActivityTime(), but this handler does not update the viewer's front-end activity timestamp when frames arrive from the browser. As a result, GC/linger logic may not reflect actual browser activity and can prevent consoleproxy.session.timeout from being enforced. Update the viewer's front-end activity time on each received WebSocket frame (and ensure the timestamp isn't continuously refreshed by backend reads alone).
```suggestion
}
viewer.setClientLastFrontEndActivityTime(System.currentTimeMillis());
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5176205980
@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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5186619784
<b>[SF] Trillian test result (tid-16700)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 55376 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13058-t16700-kvm-ol8.zip
Smoke tests completed. 141 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3765839777
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java:
##########
@@ -16,98 +16,120 @@
// under the License.
package com.cloud.consoleproxy;
+
import java.io.File;
import java.util.ArrayList;
-import java.util.Iterator;
+import java.util.List;
import java.util.Map;
import java.util.Set;
+
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
+
/**
*
* ConsoleProxyGCThread does house-keeping work for the process, it helps cleanup log files,
* recycle idle client sessions without front-end activities and report client stats to external
* management software
*/
public class ConsoleProxyGCThread extends Thread {
- protected Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
+ private static final Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
- private final static int MAX_SESSION_IDLE_SECONDS = 180;
private final Map<String, ConsoleProxyClient> connMap;
private final Set<String> removedSessionsSet;
private long lastLogScan = 0;
+
public ConsoleProxyGCThread(Map<String, ConsoleProxyClient> connMap, Set<String> removedSet) {
this.connMap = connMap;
this.removedSessionsSet = removedSet;
}
+
private void cleanupLogging() {
- if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000)
+ if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000) {
return;
+ }
+
lastLogScan = System.currentTimeMillis();
+
File logDir = new File("./logs");
- File files[] = logDir.listFiles();
+ File[] files = logDir.listFiles();
if (files != null) {
for (File file : files) {
if (System.currentTimeMillis() - file.lastModified() >= 86400000L) {
try {
file.delete();
} catch (Throwable e) {
- logger.info("[ignored]"
- + "failed to delete file: " + e.getLocalizedMessage());
+ logger.info("[ignored] failed to delete file: " + e.getLocalizedMessage());
}
}
}
}
}
+
@Override
public void run() {
+
boolean bReportLoad = false;
long lastReportTick = System.currentTimeMillis();
+
while (true) {
cleanupLogging();
bReportLoad = false;
+
if (logger.isDebugEnabled()) {
- logger.debug(String.format("connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
+ logger.debug(String.format("ConsoleProxyGCThread loop: connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
}
- Set<String> e = connMap.keySet();
- Iterator<String> iterator = e.iterator();
- while (iterator.hasNext()) {
+ List<String> keys;
+ synchronized (connMap) {
+ keys = new ArrayList<>(connMap.keySet());
+ }
+ for (String key : keys) {
String key;
Review Comment:
```suggestion
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r4258543050
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java:
##########
@@ -183,12 +188,21 @@ public void onClose(Session session, int statusCode, String reason) throws IOExc
@OnWebSocketFrame
public void onFrame(Frame f) throws IOException {
+ if (viewer == null) {
+ logger.debug("Ignoring WebSocket frame because viewer is not initialized yet.");
+ return;
+ }
logger.trace("Sending client [ID: {}] frame of {} bytes.", viewer.getClientId(), f.getPayloadLength());
+ viewer.updateFrontEndActivityTime();
Review Comment:
does an open console that nobody touches ever time out? the browser keeps asking for screen updates on its own, so every frame here looks like activity
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "dheeraj12347 (via GitHub)" <gi...@apache.org>.
dheeraj12347 commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6222109971
@DaanHoogland , @Pearl1594 and @Damans227
I found that the noVNC worker loop was refreshing the frontend activity timestamp repeatedly while the WebSocket connection remained open, even without user interaction. I've removed that refresh in commit `ce7f38a`.
I also checked the bundled noVNC implementation: continuous screen updates are enabled through a client message, while the resulting screen updates travel from the server to the browser. I'll verify the idle-session behavior and whether the activity refresh in `onFrame()` needs any further adjustment.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6334662743
<b>[SF] Trillian test result (tid-17181)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 56528 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13058-t17181-kvm-ol8.zip
Smoke tests completed. 141 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4554451621
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "dheeraj12347 (via GitHub)" <gi...@apache.org>.
dheeraj12347 commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4412381497
> @dheeraj12347 , I set consoleproxy.session.timeout to 30000, expecting my consoles to get lost in half a minute. I restarted the MS. The consoles keep working until about the half an hour, the default setting. (I didn’t measure exactly) Can you have another look at this?
Hi @DaanHoogland ,
I’ve updated the PR to address the consoleproxy.session.timeout behaviour you described:
In ConsoleProxy.java I now read consoleproxy.session.timeout from the configuration (milliseconds), validate it, and store the effective value in ConsoleProxy.sessionTimeoutMillis, with a log line on startup showing the effective timeout.
In ConsoleProxyGCThread.java I removed the hardcoded idle timeout and changed the GC logic to compute each viewer’s idle time in milliseconds and compare it against ConsoleProxy.sessionTimeoutMillis, logging the idle duration and configured timeout when a session is removed.
To verify my changes, I built the console proxy server module with:
bash
mvn -pl services/console-proxy/server -am clean compile
I’m open to any feedback on this approach, and I’m happy to further adjust or iterate based on your results when you re-test with consoleproxy.session.timeout=30000.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5177089723
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18758
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3789100777
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -109,33 +117,34 @@ private static void configLog4j() {
try {
File file = new File(configUrl.toURI());
+
Review Comment:
```suggestion
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5265846932
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 18835
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3765780945
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java:
##########
@@ -16,98 +16,119 @@
// under the License.
package com.cloud.consoleproxy;
+
import java.io.File;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
+
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
+
/**
*
* ConsoleProxyGCThread does house-keeping work for the process, it helps cleanup log files,
* recycle idle client sessions without front-end activities and report client stats to external
* management software
*/
public class ConsoleProxyGCThread extends Thread {
- protected Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
+ private static final Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
- private final static int MAX_SESSION_IDLE_SECONDS = 180;
private final Map<String, ConsoleProxyClient> connMap;
private final Set<String> removedSessionsSet;
private long lastLogScan = 0;
+
public ConsoleProxyGCThread(Map<String, ConsoleProxyClient> connMap, Set<String> removedSet) {
this.connMap = connMap;
this.removedSessionsSet = removedSet;
}
+
private void cleanupLogging() {
- if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000)
+ if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000) {
return;
+ }
+
lastLogScan = System.currentTimeMillis();
+
File logDir = new File("./logs");
- File files[] = logDir.listFiles();
+ File[] files = logDir.listFiles();
if (files != null) {
for (File file : files) {
if (System.currentTimeMillis() - file.lastModified() >= 86400000L) {
try {
file.delete();
} catch (Throwable e) {
- logger.info("[ignored]"
- + "failed to delete file: " + e.getLocalizedMessage());
+ logger.info("[ignored] failed to delete file: " + e.getLocalizedMessage());
}
}
}
}
}
+
@Override
public void run() {
+
boolean bReportLoad = false;
long lastReportTick = System.currentTimeMillis();
+
while (true) {
cleanupLogging();
bReportLoad = false;
+
if (logger.isDebugEnabled()) {
- logger.debug(String.format("connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
+ logger.debug(String.format("ConsoleProxyGCThread loop: connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
}
- Set<String> e = connMap.keySet();
- Iterator<String> iterator = e.iterator();
+ Set<String> keys = connMap.keySet();
+ Iterator<String> iterator = keys.iterator();
while (iterator.hasNext()) {
Review Comment:
```suggestion
List<String> keys;
synchronized (connMap) {
keys = new ArrayList<>(connMap.keySet());
}
for (String key : keys) {
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "dheeraj12347 (via GitHub)" <gi...@apache.org>.
dheeraj12347 commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6099660252
Hi @DaanHoogland and @Pearl1594, I noticed that the Codecov, Simulator CI and UI Build checks are currently failing on this PR. The Codecov log shows repeated request failures while creating the commit record at `ingest.codecov.io`. Are these failures known infrastructure issues, or should I investigate any specific failures on my side? 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6317922573
@blueorangutan test keepEnv
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6361485293
@Damans227, @kiranchavala did you test this?
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4397944085
@dheeraj12347
only one of #13002 and #13058 is needed.
once this is merged into 4.20, it will be included in main as well after forward-merge
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4389668457
@dheeraj12347 , I set consoleproxy.session.timeout to 30000, expecting my consoles to get lost in half a minute. I restarted the MS. The consoles keep working until about the half an hour, the default setting. (I didn’t measure exactly) Can you have another look at this?
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4349401315
<b>[SF] Trillian test result (tid-15983)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 50988 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13058-t15983-kvm-ol8.zip
Smoke tests completed. 141 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5266429169
#13858 opened for possible follow-up for non-noVNC consoles.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5177130526
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5177106170
@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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3773016419
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -81,23 +94,31 @@ public class ConsoleProxy {
static String factoryClzName;
static boolean standaloneStart = false;
+
static String encryptorPassword = "Dummy";
- static final String[] skipProperties = new String[]{"certificate", "cacertificate", "keystore_password", "privatekey"};
+ static final String[] skipProperties = new String[] {"certificate", "cacertificate", "keystore_password", "privatekey"};
+
static Set<String> allowedSessions = new HashSet<>();
+
public static void addAllowedSession(String sessionUuid) {
allowedSessions.add(sessionUuid);
}
+
private static void configLog4j() {
final ClassLoader loader = Thread.currentThread().getContextClassLoader();
URL configUrl = loader.getResource("/conf/log4j-cloud.xml");
- if (configUrl == null)
+ if (configUrl == null) {
configUrl = ClassLoader.getSystemResource("log4j-cloud.xml");
+ }
- if (configUrl == null)
+
+ if (configUrl == null) {
configUrl = ClassLoader.getSystemResource("conf/log4j-cloud.xml");
+ }
+
Review Comment:
```suggestion
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5301948956
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18869
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6339838257
Thanks for the fix @dheeraj12347, can you please reference the PR from which the fix is being backported from?
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4336513184
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/13058?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 4.15%. Comparing base ([`9f96c9d`](https://app.codecov.io/gh/apache/cloudstack/commit/9f96c9d5eb97f1836103918cf520d72cc3395c51?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`57356f8`](https://app.codecov.io/gh/apache/cloudstack/commit/57356f8329ed9838450abc2c472782ffd02f5f7b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
> :exclamation: There is a different number of reports uploaded between BASE (9f96c9d) and HEAD (57356f8). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (9f96c9d) | HEAD (57356f8) |
>|------|------|------|
>|unittests|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 4.20 #13058 +/- ##
=============================================
- Coverage 16.26% 4.15% -12.11%
=============================================
Files 5665 404 -5261
Lines 500556 32966 -467590
Branches 60790 5893 -54897
=============================================
- Hits 81416 1370 -80046
+ Misses 410036 31420 -378616
+ Partials 9104 176 -8928
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/13058/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/13058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.15% <ø> (ø)` | |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/13058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/13058?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4342471416
@DaanHoogland 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.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4342964795
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17652
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5176213242
@DaanHoogland 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.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "Pearl1594 (via GitHub)" <gi...@apache.org>.
Pearl1594 commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3767678980
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -144,36 +170,61 @@ private static void configProxy(Properties conf) {
factoryClzName = ConsoleProxyBaseServerFactoryImpl.class.getName();
}
- s = conf.getProperty("consoleproxy.httpCmdListenPort");
+
+ s = conf != null ? conf.getProperty("consoleproxy.httpCmdListenPort") : null;
if (s != null) {
httpCmdListenPort = Integer.parseInt(s);
LOGGER.info("Setting httpCmdListenPort=" + s);
}
- s = conf.getProperty("consoleproxy.reconnectMaxRetry");
+
+ s = conf != null ? conf.getProperty("consoleproxy.reconnectMaxRetry") : null;
if (s != null) {
reconnectMaxRetry = Integer.parseInt(s);
LOGGER.info("Setting reconnectMaxRetry=" + reconnectMaxRetry);
}
- s = conf.getProperty("consoleproxy.readTimeoutSeconds");
+
+ s = conf != null ? conf.getProperty("consoleproxy.readTimeoutSeconds") : null;
if (s != null) {
readTimeoutSeconds = Integer.parseInt(s);
LOGGER.info("Setting readTimeoutSeconds=" + readTimeoutSeconds);
}
- s = conf.getProperty("consoleproxy.defaultBufferSize");
+
+ s = conf != null ? conf.getProperty("consoleproxy.defaultBufferSize") : null;
if (s != null) {
defaultBufferSize = Integer.parseInt(s);
LOGGER.info("Setting defaultBufferSize=" + defaultBufferSize);
}
+
+ // New: read consoleproxy.session.timeout (milliseconds)
Review Comment:
nit - comment not required.
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -307,6 +376,7 @@ public static void startWithContext(Properties conf, Object context, byte[] ksBi
try {
final ClassLoader loader = Thread.currentThread().getContextClassLoader();
Class<?> contextClazz = loader.loadClass("com.cloud.agent.resource.consoleproxy.ConsoleProxyResource");
+ // 4.20 signature: 7 parameters
Review Comment:
nit: remove comment
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -220,33 +276,39 @@ public static ConsoleProxyAuthenticationResult authenticateConsoleAccess(Console
return authResult;
}
+
String websocketUrl = param.getWebsocketUrl();
if (StringUtils.isNotBlank(websocketUrl)) {
return authResult;
}
+
if (standaloneStart) {
return authResult;
}
+
if (authMethod != null) {
Object result;
try {
+ // 4.20: authenticateConsoleAccess(host, port, vmId, sid, ticket, isReauthentication, sessionToken)
Review Comment:
nit: can we remove this comment or remove the 4.20 reference?
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3765780945
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java:
##########
@@ -16,98 +16,119 @@
// under the License.
package com.cloud.consoleproxy;
+
import java.io.File;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
+
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
+
/**
*
* ConsoleProxyGCThread does house-keeping work for the process, it helps cleanup log files,
* recycle idle client sessions without front-end activities and report client stats to external
* management software
*/
public class ConsoleProxyGCThread extends Thread {
- protected Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
+ private static final Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
- private final static int MAX_SESSION_IDLE_SECONDS = 180;
private final Map<String, ConsoleProxyClient> connMap;
private final Set<String> removedSessionsSet;
private long lastLogScan = 0;
+
public ConsoleProxyGCThread(Map<String, ConsoleProxyClient> connMap, Set<String> removedSet) {
this.connMap = connMap;
this.removedSessionsSet = removedSet;
}
+
private void cleanupLogging() {
- if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000)
+ if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000) {
return;
+ }
+
lastLogScan = System.currentTimeMillis();
+
File logDir = new File("./logs");
- File files[] = logDir.listFiles();
+ File[] files = logDir.listFiles();
if (files != null) {
for (File file : files) {
if (System.currentTimeMillis() - file.lastModified() >= 86400000L) {
try {
file.delete();
} catch (Throwable e) {
- logger.info("[ignored]"
- + "failed to delete file: " + e.getLocalizedMessage());
+ logger.info("[ignored] failed to delete file: " + e.getLocalizedMessage());
}
}
}
}
}
+
@Override
public void run() {
+
boolean bReportLoad = false;
long lastReportTick = System.currentTimeMillis();
+
while (true) {
cleanupLogging();
bReportLoad = false;
+
if (logger.isDebugEnabled()) {
- logger.debug(String.format("connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
+ logger.debug(String.format("ConsoleProxyGCThread loop: connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
}
- Set<String> e = connMap.keySet();
- Iterator<String> iterator = e.iterator();
+ Set<String> keys = connMap.keySet();
+ Iterator<String> iterator = keys.iterator();
while (iterator.hasNext()) {
Review Comment:
```suggestion
List<String> keys;
synchronized (connMap) {
keys = new ArrayList<>(connMap.keySet());
}
for (String key : keys) {
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5265503968
@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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5266380848
@DaanHoogland 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.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6009834741
<b>[SF] Trillian test result (tid-17073)</b>
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 60429 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13058-t17073-kvm-ol8.zip
Smoke tests completed. 141 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6139664168
@dheeraj12347 leave the UI. codecov, I'm not sure of, but IU doubt it has anything to do with your code.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "kiranchavala (via GitHub)" <gi...@apache.org>.
kiranchavala commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5615131038
cc @nvazquez any relation to the pr https://github.com/apache/cloudstack/pull/13683
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-4552781919
@DaanHoogland 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.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5266869909
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18836
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5265513557
@DaanHoogland 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.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5266411007
@nvazquez , can you please review (and maybe 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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3765786124
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java:
##########
@@ -16,98 +16,119 @@
// under the License.
package com.cloud.consoleproxy;
+
import java.io.File;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
+
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
+
/**
*
* ConsoleProxyGCThread does house-keeping work for the process, it helps cleanup log files,
* recycle idle client sessions without front-end activities and report client stats to external
* management software
*/
public class ConsoleProxyGCThread extends Thread {
- protected Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
+ private static final Logger logger = LogManager.getLogger(ConsoleProxyGCThread.class);
- private final static int MAX_SESSION_IDLE_SECONDS = 180;
private final Map<String, ConsoleProxyClient> connMap;
private final Set<String> removedSessionsSet;
private long lastLogScan = 0;
+
public ConsoleProxyGCThread(Map<String, ConsoleProxyClient> connMap, Set<String> removedSet) {
this.connMap = connMap;
this.removedSessionsSet = removedSet;
}
+
private void cleanupLogging() {
- if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000)
+ if (lastLogScan != 0 && System.currentTimeMillis() - lastLogScan < 3600000) {
return;
+ }
+
lastLogScan = System.currentTimeMillis();
+
File logDir = new File("./logs");
- File files[] = logDir.listFiles();
+ File[] files = logDir.listFiles();
if (files != null) {
for (File file : files) {
if (System.currentTimeMillis() - file.lastModified() >= 86400000L) {
try {
file.delete();
} catch (Throwable e) {
- logger.info("[ignored]"
- + "failed to delete file: " + e.getLocalizedMessage());
+ logger.info("[ignored] failed to delete file: " + e.getLocalizedMessage());
}
}
}
}
}
+
@Override
public void run() {
+
boolean bReportLoad = false;
long lastReportTick = System.currentTimeMillis();
+
while (true) {
cleanupLogging();
bReportLoad = false;
+
if (logger.isDebugEnabled()) {
- logger.debug(String.format("connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
+ logger.debug(String.format("ConsoleProxyGCThread loop: connMap=%s, removedSessions=%s", connMap, removedSessionsSet));
}
- Set<String> e = connMap.keySet();
- Iterator<String> iterator = e.iterator();
+ Set<String> keys = connMap.keySet();
+ Iterator<String> iterator = keys.iterator();
while (iterator.hasNext()) {
String key;
ConsoleProxyClient client;
+
synchronized (connMap) {
key = iterator.next();
Review Comment:
```suggestion
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#discussion_r3773076621
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java:
##########
@@ -502,40 +603,49 @@ public static ConsoleProxyClient getAjaxVncViewer(ConsoleProxyClientParam param,
viewer = getClient(param);
viewer.initClient(param);
+
connectionMap.put(clientKey, viewer);
LOGGER.info("Added viewer object " + viewer);
reportLoadChange = true;
} else {
// protected against malicious attack by modifying URL content
if (ajaxSession != null) {
long ajaxSessionIdFromUrl = Long.parseLong(ajaxSession);
- if (ajaxSessionIdFromUrl != viewer.getAjaxSessionId())
+ if (ajaxSessionIdFromUrl != viewer.getAjaxSessionId()) {
throw new AuthenticationException("Cannot use the existing viewer " + viewer + ": modified AJAX session id");
+ }
}
- if (param.getClientHostPassword() == null || param.getClientHostPassword().isEmpty() ||
- !param.getClientHostPassword().equals(viewer.getClientHostPassword()))
+
+ if (param.getClientHostPassword() == null || param.getClientHostPassword().isEmpty()
+ || !param.getClientHostPassword().equals(viewer.getClientHostPassword())) {
throw new AuthenticationException("Cannot use the existing viewer " + viewer + ": bad sid");
+ }
+
Review Comment:
```suggestion
```
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5908781631
@dheeraj12347 are you still looking at this?
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-5991131348
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19399
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6288051724
@DaanHoogland 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.
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6288038287
@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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6289682624
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19625
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6222086273
> does an open console that nobody touches ever time out?
@dheeraj12347 still no. every frame from the browser still counts as activity, and so does every screen update sent back to it. can we count only key presses and mouse moves?
--
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
Re: [PR] Backport: Honour consoleproxy.session.timeout for noVNC console sessions [cloudstack]
Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on PR #13058:
URL: https://github.com/apache/cloudstack/pull/13058#issuecomment-6367491622
@DaanHoogland no, i havent run it on a lab, only read the code. still waiting on @dheeraj12347 for the frozen screen question above
--
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