From 2875edc4d1f48e51ed905601a0c0af91d70d5a80 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 25 Jan 2023 09:43:44 -0300 Subject: [PATCH 1/9] Add console session cleanup task --- .../consoleproxy/ConsoleAccessManager.java | 22 ++++++++ .../com/cloud/vm/dao/ConsoleSessionDao.java | 4 ++ .../cloud/vm/dao/ConsoleSessionDaoImpl.java | 20 +++++++ .../ConsoleAccessManagerImpl.java | 55 +++++++++++++++++++ 4 files changed, 101 insertions(+) diff --git a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java index c763730a952b..5dbf70268762 100644 --- a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java +++ b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java @@ -18,9 +18,31 @@ import com.cloud.utils.component.Manager; import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint; +import org.apache.cloudstack.framework.config.ConfigKey; public interface ConsoleAccessManager extends Manager { + ConfigKey ConsoleSessionCleanupEnabled = new ConfigKey<>("Advanced", Boolean.class, + "console.session.cleanup.enabled", + "true", + "Enables/disables the console session cleanup thread.", + true, + ConfigKey.Scope.Global); + + ConfigKey ConsoleSessionCleanupRetentionDays = new ConfigKey<>("Advanced", Integer.class, + "console.session.cleanup.retention.days", + "7", + "Determines the days to keep removed console session records before expunging them", + true, + ConfigKey.Scope.Global); + + ConfigKey ConsoleSessionCleanupInterval = new ConfigKey<>("Advanced", Integer.class, + "console.session.cleanup.interval", + "180", + "Determines how long (in hours) to wait before actually expunging destroyed console session records", + true, + ConfigKey.Scope.Global); + ConsoleEndpoint generateConsoleEndpoint(Long vmId, String extraSecurityToken, String clientAddress); boolean isSessionAllowed(String sessionUuid); diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java index 43f30970befe..41df126cad5d 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java @@ -22,10 +22,14 @@ import com.cloud.vm.ConsoleSessionVO; import com.cloud.utils.db.GenericDao; +import java.util.Date; +import java.util.List; + public interface ConsoleSessionDao extends GenericDao { void removeSession(String sessionUuid); boolean isSessionAllowed(String sessionUuid); + List listRemovedSessionsOlderThanDate(Date date); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java index 2ac41918c2cb..38ce87c69467 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java @@ -19,11 +19,24 @@ package com.cloud.vm.dao; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; import com.cloud.vm.ConsoleSessionVO; import com.cloud.utils.db.GenericDaoBase; +import java.util.Date; +import java.util.List; + public class ConsoleSessionDaoImpl extends GenericDaoBase implements ConsoleSessionDao { + private final SearchBuilder SearchByRemovedDate; + + public ConsoleSessionDaoImpl() { + SearchByRemovedDate = createSearchBuilder(); + SearchByRemovedDate.and("removed", SearchByRemovedDate.entity().getRemoved(), SearchCriteria.Op.NNULL); + SearchByRemovedDate.and("removed", SearchByRemovedDate.entity().getRemoved(), SearchCriteria.Op.LTEQ); + } + @Override public void removeSession(String sessionUuid) { ConsoleSessionVO session = findByUuid(sessionUuid); @@ -34,4 +47,11 @@ public void removeSession(String sessionUuid) { public boolean isSessionAllowed(String sessionUuid) { return findByUuid(sessionUuid) != null; } + + @Override + public List listRemovedSessionsOlderThanDate(Date date) { + SearchCriteria searchCriteria = SearchByRemovedDate.create(); + searchCriteria.setParameters("removed", date); + return listBy(searchCriteria); + } } diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index 0e3a0f822f60..bd8538833b08 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -36,7 +36,9 @@ import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.component.ManagerBase; +import com.cloud.utils.concurrency.NamedThreadFactory; import com.cloud.utils.db.EntityManager; +import com.cloud.utils.db.GlobalLock; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.ConsoleSessionVO; import com.cloud.vm.UserVmDetailVO; @@ -50,8 +52,11 @@ import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.security.keys.KeysManager; +import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.commons.codec.binary.Base64; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.ArrayUtils; +import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; @@ -63,9 +68,13 @@ import java.util.Arrays; import java.util.Date; +import java.util.GregorianCalendar; import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAccessManager { @@ -88,6 +97,8 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce @Inject private ConsoleSessionDao consoleSessionDao; + private ScheduledExecutorService executorService = null; + private static KeysManager secretKeysManager; private final Gson gson = new GsonBuilder().create(); @@ -100,9 +111,53 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce @Override public boolean configure(String name, Map params) throws ConfigurationException { ConsoleAccessManagerImpl.secretKeysManager = keysManager; + executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger")); return super.configure(name, params); } + @Override + public boolean start() { + Integer consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value(); + Boolean consoleCleanupEnabled = ConsoleAccessManager.ConsoleSessionCleanupEnabled.value(); + if (BooleanUtils.isTrue(consoleCleanupEnabled)) { + s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval)); + executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS); + } + return true; + } + + public class ConsoleSessionCleanupTask extends ManagedContextRunnable { + @Override + protected void runInContext() { + final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock"); + try { + if (gcLock.lock(3)) { + try { + reallyRun(); + } finally { + gcLock.unlock(); + } + } + } finally { + gcLock.releaseRef(); + } + } + + private void reallyRun() { + s_logger.info("Starting ConsoleSessionCleanupTask..."); + Integer retentionDays = ConsoleAccessManager.ConsoleSessionCleanupRetentionDays.value(); + Date date = GregorianCalendar.getInstance().getTime(); + Date dateBefore = new Date(date.getTime() - retentionDays * 24 * 3600 * 1000); + List sessionsToExpunge = consoleSessionDao.listRemovedSessionsOlderThanDate(dateBefore); + if (CollectionUtils.isNotEmpty(sessionsToExpunge)) { + s_logger.info(String.format("Expunging %s removed console session records")); + for (ConsoleSessionVO sessionVO : sessionsToExpunge) { + consoleSessionDao.expunge(sessionVO.getId()); + } + } + } + } + @Override public ConsoleEndpoint generateConsoleEndpoint(Long vmId, String extraSecurityToken, String clientAddress) { try { From 2bb9d557323cfbf3482a6676a33123b2db629a5c Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 25 Jan 2023 12:49:03 -0300 Subject: [PATCH 2/9] Address review --- .../consoleproxy/ConsoleAccessManager.java | 19 +++++-------- .../com/cloud/vm/dao/ConsoleSessionDao.java | 3 +-- .../cloud/vm/dao/ConsoleSessionDaoImpl.java | 9 ++++--- .../ConsoleAccessManagerImpl.java | 27 +++++++++---------- 4 files changed, 24 insertions(+), 34 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java index 5dbf70268762..2e6c4de69e3c 100644 --- a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java +++ b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java @@ -22,25 +22,18 @@ public interface ConsoleAccessManager extends Manager { - ConfigKey ConsoleSessionCleanupEnabled = new ConfigKey<>("Advanced", Boolean.class, - "console.session.cleanup.enabled", - "true", - "Enables/disables the console session cleanup thread.", - true, - ConfigKey.Scope.Global); - - ConfigKey ConsoleSessionCleanupRetentionDays = new ConfigKey<>("Advanced", Integer.class, - "console.session.cleanup.retention.days", - "7", - "Determines the days to keep removed console session records before expunging them", - true, + ConfigKey ConsoleSessionCleanupRetentionHours = new ConfigKey<>("Advanced", Integer.class, + "console.session.cleanup.retention.hours", + "240", + "Determines the hours to keep removed console session records before expunging them", + false, ConfigKey.Scope.Global); ConfigKey ConsoleSessionCleanupInterval = new ConfigKey<>("Advanced", Integer.class, "console.session.cleanup.interval", "180", "Determines how long (in hours) to wait before actually expunging destroyed console session records", - true, + false, ConfigKey.Scope.Global); ConsoleEndpoint generateConsoleEndpoint(Long vmId, String extraSecurityToken, String clientAddress); diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java index 41df126cad5d..97a6148e97d0 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java @@ -23,7 +23,6 @@ import com.cloud.utils.db.GenericDao; import java.util.Date; -import java.util.List; public interface ConsoleSessionDao extends GenericDao { @@ -31,5 +30,5 @@ public interface ConsoleSessionDao extends GenericDao { boolean isSessionAllowed(String sessionUuid); - List listRemovedSessionsOlderThanDate(Date date); + int expungeSessionsOlderThanDate(Date date); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java index 38ce87c69467..3badbfcfb518 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java @@ -25,7 +25,6 @@ import com.cloud.utils.db.GenericDaoBase; import java.util.Date; -import java.util.List; public class ConsoleSessionDaoImpl extends GenericDaoBase implements ConsoleSessionDao { @@ -33,7 +32,7 @@ public class ConsoleSessionDaoImpl extends GenericDaoBase listRemovedSessionsOlderThanDate(Date date) { + public int expungeSessionsOlderThanDate(Date date) { SearchCriteria searchCriteria = SearchByRemovedDate.create(); searchCriteria.setParameters("removed", date); - return listBy(searchCriteria); + return expunge(searchCriteria); } + + } diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index bd8538833b08..f5580b83069e 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -54,12 +54,11 @@ import org.apache.cloudstack.framework.security.keys.KeysManager; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.commons.codec.binary.Base64; -import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.ArrayUtils; -import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; +import org.joda.time.DateTime; import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; @@ -68,7 +67,6 @@ import java.util.Arrays; import java.util.Date; -import java.util.GregorianCalendar; import java.util.List; import java.util.Map; import java.util.UUID; @@ -117,9 +115,8 @@ public boolean configure(String name, Map params) throws Configu @Override public boolean start() { - Integer consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value(); - Boolean consoleCleanupEnabled = ConsoleAccessManager.ConsoleSessionCleanupEnabled.value(); - if (BooleanUtils.isTrue(consoleCleanupEnabled)) { + int consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value(); + if (consoleCleanupInterval > 0) { s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval)); executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS); } @@ -144,15 +141,15 @@ protected void runInContext() { } private void reallyRun() { - s_logger.info("Starting ConsoleSessionCleanupTask..."); - Integer retentionDays = ConsoleAccessManager.ConsoleSessionCleanupRetentionDays.value(); - Date date = GregorianCalendar.getInstance().getTime(); - Date dateBefore = new Date(date.getTime() - retentionDays * 24 * 3600 * 1000); - List sessionsToExpunge = consoleSessionDao.listRemovedSessionsOlderThanDate(dateBefore); - if (CollectionUtils.isNotEmpty(sessionsToExpunge)) { - s_logger.info(String.format("Expunging %s removed console session records")); - for (ConsoleSessionVO sessionVO : sessionsToExpunge) { - consoleSessionDao.expunge(sessionVO.getId()); + if (s_logger.isDebugEnabled()) { + s_logger.debug("Starting ConsoleSessionCleanupTask..."); + } + Integer retentionHours = ConsoleAccessManager.ConsoleSessionCleanupRetentionHours.value(); + Date dateBefore = DateTime.now().minusHours(retentionHours).toDate(); + int sessionsExpunged = consoleSessionDao.expungeSessionsOlderThanDate(dateBefore); + if (sessionsExpunged > 0) { + if (s_logger.isDebugEnabled()) { + s_logger.info(String.format("Expunged %s removed console session records", sessionsExpunged)); } } } From 9e103c9131ab79c9cc94e7122a6991af9db66409 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 25 Jan 2023 14:00:21 -0300 Subject: [PATCH 3/9] Add missing config keys --- .../consoleproxy/ConsoleAccessManager.java | 3 ++- .../consoleproxy/ConsoleAccessManagerImpl.java | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java index 2e6c4de69e3c..55e7e40640fc 100644 --- a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java +++ b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java @@ -19,8 +19,9 @@ import com.cloud.utils.component.Manager; import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint; import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; -public interface ConsoleAccessManager extends Manager { +public interface ConsoleAccessManager extends Manager, Configurable { ConfigKey ConsoleSessionCleanupRetentionHours = new ConfigKey<>("Advanced", Integer.class, "console.session.cleanup.retention.hours", diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index f5580b83069e..e91ef2ec9546 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -51,6 +51,7 @@ import com.google.gson.GsonBuilder; import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.security.keys.KeysManager; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.commons.codec.binary.Base64; @@ -123,6 +124,19 @@ public boolean start() { return true; } + @Override + public String getConfigComponentName() { + return ConsoleAccessManager.class.getName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[] { + ConsoleAccessManager.ConsoleSessionCleanupInterval, + ConsoleAccessManager.ConsoleSessionCleanupRetentionHours + }; + } + public class ConsoleSessionCleanupTask extends ManagedContextRunnable { @Override protected void runInContext() { From 3e24f81775a2cc7cc4bbe70ec74c4003c2e190f4 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 25 Jan 2023 15:33:54 -0300 Subject: [PATCH 4/9] Fix sonar cloud reports --- .../java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java | 10 +++++----- .../consoleproxy/ConsoleAccessManagerImpl.java | 6 ++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java index 3badbfcfb518..9de1a5fe4490 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java @@ -28,12 +28,12 @@ public class ConsoleSessionDaoImpl extends GenericDaoBase implements ConsoleSessionDao { - private final SearchBuilder SearchByRemovedDate; + private final SearchBuilder searchByRemovedDate; public ConsoleSessionDaoImpl() { - SearchByRemovedDate = createSearchBuilder(); - SearchByRemovedDate.and("removedNotNull", SearchByRemovedDate.entity().getRemoved(), SearchCriteria.Op.NNULL); - SearchByRemovedDate.and("removed", SearchByRemovedDate.entity().getRemoved(), SearchCriteria.Op.LTEQ); + searchByRemovedDate = createSearchBuilder(); + searchByRemovedDate.and("removedNotNull", searchByRemovedDate.entity().getRemoved(), SearchCriteria.Op.NNULL); + searchByRemovedDate.and("removed", searchByRemovedDate.entity().getRemoved(), SearchCriteria.Op.LTEQ); } @Override @@ -49,7 +49,7 @@ public boolean isSessionAllowed(String sessionUuid) { @Override public int expungeSessionsOlderThanDate(Date date) { - SearchCriteria searchCriteria = SearchByRemovedDate.create(); + SearchCriteria searchCriteria = searchByRemovedDate.create(); searchCriteria.setParameters("removed", date); return expunge(searchCriteria); } diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index e91ef2ec9546..c03ddefcf616 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -161,10 +161,8 @@ private void reallyRun() { Integer retentionHours = ConsoleAccessManager.ConsoleSessionCleanupRetentionHours.value(); Date dateBefore = DateTime.now().minusHours(retentionHours).toDate(); int sessionsExpunged = consoleSessionDao.expungeSessionsOlderThanDate(dateBefore); - if (sessionsExpunged > 0) { - if (s_logger.isDebugEnabled()) { - s_logger.info(String.format("Expunged %s removed console session records", sessionsExpunged)); - } + if (sessionsExpunged > 0 && s_logger.isDebugEnabled()) { + s_logger.info(String.format("Expunged %s removed console session records", sessionsExpunged)); } } } From 1b50ed7a0afd9bda329cd6db0d42361afe83690b Mon Sep 17 00:00:00 2001 From: nvazquez Date: Thu, 26 Jan 2023 01:32:51 -0300 Subject: [PATCH 5/9] In progress fix console load report --- .../consoleproxy/ConsoleAccessManager.java | 2 +- .../main/java/com/cloud/vm/ConsoleSessionVO.java | 11 +++++++++++ .../java/com/cloud/vm/dao/ConsoleSessionDao.java | 2 ++ .../com/cloud/vm/dao/ConsoleSessionDaoImpl.java | 13 ++++++++++++- .../resources/META-INF/db/schema-41720to41800.sql | 1 + .../java/com/cloud/consoleproxy/AgentHookBase.java | 2 +- .../consoleproxy/ConsoleAccessManagerImpl.java | 8 ++++++-- .../cloud/consoleproxy/ConsoleProxyGCThread.java | 8 +++++--- 8 files changed, 39 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java index 55e7e40640fc..30d407b0aeec 100644 --- a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java +++ b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java @@ -43,5 +43,5 @@ public interface ConsoleAccessManager extends Manager, Configurable { void removeSessions(String[] sessionUuids); - void removeSession(String sessionUuid); + void acquireSession(String sessionUuid); } diff --git a/engine/schema/src/main/java/com/cloud/vm/ConsoleSessionVO.java b/engine/schema/src/main/java/com/cloud/vm/ConsoleSessionVO.java index 2373896b7274..4b476af463cf 100644 --- a/engine/schema/src/main/java/com/cloud/vm/ConsoleSessionVO.java +++ b/engine/schema/src/main/java/com/cloud/vm/ConsoleSessionVO.java @@ -54,6 +54,9 @@ public class ConsoleSessionVO { @Column(name = "host_id") private long hostId; + @Column(name = "acquired") + private boolean acquired; + @Column(name = "removed") private Date removed; @@ -120,4 +123,12 @@ public Date getRemoved() { public void setRemoved(Date removed) { this.removed = removed; } + + public boolean isAcquired() { + return acquired; + } + + public void setAcquired(boolean acquired) { + this.acquired = acquired; + } } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java index 97a6148e97d0..71b1aed1938d 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDao.java @@ -31,4 +31,6 @@ public interface ConsoleSessionDao extends GenericDao { boolean isSessionAllowed(String sessionUuid); int expungeSessionsOlderThanDate(Date date); + + void acquireSession(String sessionUuid); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java index 9de1a5fe4490..f2f4703a2a2c 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java @@ -44,7 +44,11 @@ public void removeSession(String sessionUuid) { @Override public boolean isSessionAllowed(String sessionUuid) { - return findByUuid(sessionUuid) != null; + ConsoleSessionVO consoleSessionVO = findByUuid(sessionUuid); + if (consoleSessionVO == null) { + return false; + } + return !consoleSessionVO.isAcquired(); } @Override @@ -54,5 +58,12 @@ public int expungeSessionsOlderThanDate(Date date) { return expunge(searchCriteria); } + @Override + public void acquireSession(String sessionUuid) { + ConsoleSessionVO consoleSessionVO = findByUuid(sessionUuid); + consoleSessionVO.setAcquired(true); + update(consoleSessionVO.getId(), consoleSessionVO); + } + } diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41720to41800.sql b/engine/schema/src/main/resources/META-INF/db/schema-41720to41800.sql index ae6fcfc57d73..00897ea69c92 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41720to41800.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41720to41800.sql @@ -1056,6 +1056,7 @@ CREATE TABLE IF NOT EXISTS `cloud`.`console_session` ( `user_id` bigint(20) unsigned NOT NULL COMMENT 'User who generated the session', `instance_id` bigint(20) unsigned NOT NULL COMMENT 'VM for which the session was generated', `host_id` bigint(20) unsigned NOT NULL COMMENT 'Host where the VM was when the session was generated', + `acquired` int(1) NOT NULL DEFAULT 0 COMMENT 'True if the session was already used', `removed` datetime COMMENT 'When the session was removed/used', CONSTRAINT `fk_consolesession__account_id` FOREIGN KEY(`account_id`) REFERENCES `cloud`.`account` (`id`), CONSTRAINT `fk_consolesession__user_id` FOREIGN KEY(`user_id`) REFERENCES `cloud`.`user`(`id`), diff --git a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java index 0bac76923cf9..f200217e4081 100644 --- a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java +++ b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java @@ -111,7 +111,7 @@ public AgentControlAnswer onConsoleAccessAuthentication(ConsoleAccessAuthenticat } s_logger.debug(String.format("Removing session [%s] as it was just used.", sessionUuid)); - consoleAccessManager.removeSession(sessionUuid); + consoleAccessManager.acquireSession(sessionUuid); if (!ticket.equals(ticketInUrl)) { Date now = new Date(); diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index c03ddefcf616..c95f2d193332 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -222,11 +222,15 @@ public void removeSessions(String[] sessionUuids) { } } - @Override - public void removeSession(String sessionUuid) { + protected void removeSession(String sessionUuid) { consoleSessionDao.removeSession(sessionUuid); } + @Override + public void acquireSession(String sessionUuid) { + consoleSessionDao.acquireSession(sessionUuid); + } + protected boolean checkSessionPermission(VirtualMachine vm, Account account) { if (accountManager.isRootAdmin(account.getId())) { return true; diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java index 7965a2083504..dd450226df4d 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java @@ -74,10 +74,10 @@ public void run() { while (true) { cleanupLogging(); bReportLoad = false; - removedSessions.clear(); - if (s_logger.isDebugEnabled()) + if (s_logger.isDebugEnabled()) { s_logger.debug("connMap=" + connMap); + } Enumeration e = connMap.keys(); while (e.hasMoreElements()) { String key; @@ -111,9 +111,11 @@ public void run() { String loadInfo = collector.getStatsReport(); ConsoleProxy.reportLoadInfo(loadInfo); lastReportTick = System.currentTimeMillis(); + removedSessions.clear(); - if (s_logger.isDebugEnabled()) + if (s_logger.isDebugEnabled()) { s_logger.debug("Report load change : " + loadInfo); + } } try { From d3729382a2a0b0210e5e78a78fb9a7e6f4151145 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Thu, 26 Jan 2023 20:38:02 -0300 Subject: [PATCH 6/9] Fix remove console when session ends --- .../com/cloud/consoleproxy/AgentHookBase.java | 2 +- .../com/cloud/consoleproxy/ConsoleProxy.java | 5 ++++- .../cloud/consoleproxy/ConsoleProxyGCThread.java | 16 +++++++++------- .../consoleproxy/ConsoleProxyNoVncClient.java | 9 +++++++-- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java index f200217e4081..efc5a1b5d84b 100644 --- a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java +++ b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java @@ -110,7 +110,7 @@ public AgentControlAnswer onConsoleAccessAuthentication(ConsoleAccessAuthenticat return new ConsoleAccessAuthenticationAnswer(cmd, false); } - s_logger.debug(String.format("Removing session [%s] as it was just used.", sessionUuid)); + s_logger.debug(String.format("Acquiring session [%s] as it was just used.", sessionUuid)); consoleAccessManager.acquireSession(sessionUuid); if (!ticket.equals(ticketInUrl)) { diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java index c2b54e991909..2753d9fcb654 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java @@ -31,6 +31,7 @@ import java.util.Map; import java.util.Properties; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; import org.apache.commons.lang3.ArrayUtils; @@ -68,6 +69,7 @@ public class ConsoleProxy { public static Method ensureRouteMethod; static Hashtable connectionMap = new Hashtable(); + static Set removedSessionsSet = ConcurrentHashMap.newKeySet(); static int httpListenPort = 80; static int httpCmdListenPort = 8001; static int reconnectMaxRetry = 5; @@ -372,7 +374,7 @@ public static void start(Properties conf) { s_logger.info("HTTP command port is disabled"); } - ConsoleProxyGCThread cthread = new ConsoleProxyGCThread(connectionMap); + ConsoleProxyGCThread cthread = new ConsoleProxyGCThread(connectionMap, removedSessionsSet); cthread.setName("Console Proxy GC Thread"); cthread.start(); } @@ -540,6 +542,7 @@ public static void removeViewer(ConsoleProxyClient viewer) { for (Map.Entry entry : connectionMap.entrySet()) { if (entry.getValue() == viewer) { connectionMap.remove(entry.getKey()); + removedSessionsSet.add(viewer.getSessionUuid()); return; } } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java index dd450226df4d..c4cbec5c4394 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java @@ -20,7 +20,7 @@ import java.util.ArrayList; import java.util.Enumeration; import java.util.Hashtable; -import java.util.List; +import java.util.Set; import org.apache.log4j.Logger; @@ -36,10 +36,12 @@ public class ConsoleProxyGCThread extends Thread { private final static int MAX_SESSION_IDLE_SECONDS = 180; private final Hashtable connMap; + private final Set removedSessionsSet; private long lastLogScan = 0; - public ConsoleProxyGCThread(Hashtable connMap) { + public ConsoleProxyGCThread(Hashtable connMap, Set removedSet) { this.connMap = connMap; + this.removedSessionsSet = removedSet; } private void cleanupLogging() { @@ -69,14 +71,13 @@ public void run() { boolean bReportLoad = false; long lastReportTick = System.currentTimeMillis(); - List removedSessions = new ArrayList<>(); while (true) { cleanupLogging(); bReportLoad = false; if (s_logger.isDebugEnabled()) { - s_logger.debug("connMap=" + connMap); + s_logger.debug(String.format("connMap=%s, removedSessions=%s", connMap, removedSessionsSet)); } Enumeration e = connMap.keys(); while (e.hasMoreElements()) { @@ -94,7 +95,6 @@ public void run() { } synchronized (connMap) { - removedSessions.add(client.getSessionUuid()); connMap.remove(key); bReportLoad = true; } @@ -107,11 +107,13 @@ public void run() { if (bReportLoad || System.currentTimeMillis() - lastReportTick > 5000) { // report load changes ConsoleProxyClientStatsCollector collector = new ConsoleProxyClientStatsCollector(connMap); - collector.setRemovedSessions(removedSessions); + collector.setRemovedSessions(new ArrayList<>(removedSessionsSet)); String loadInfo = collector.getStatsReport(); ConsoleProxy.reportLoadInfo(loadInfo); lastReportTick = System.currentTimeMillis(); - removedSessions.clear(); + synchronized (removedSessionsSet) { + removedSessionsSet.clear(); + } if (s_logger.isDebugEnabled()) { s_logger.debug("Report load change : " + loadInfo); diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java index f1c195913038..a2add1b09de0 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java @@ -19,6 +19,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.eclipse.jetty.websocket.api.Session; +import org.eclipse.jetty.websocket.api.WebSocketException; import org.eclipse.jetty.websocket.api.extensions.Frame; import java.awt.Image; @@ -120,8 +121,12 @@ public void run() { break; } if (readBytes > 0) { - session.getRemote().sendBytes(ByteBuffer.wrap(b, 0, readBytes)); - updateFrontEndActivityTime(); + try { + session.getRemote().sendBytes(ByteBuffer.wrap(b, 0, readBytes)); + updateFrontEndActivityTime(); + } catch (WebSocketException e) { + connectionAlive = false; + } } } } From 2e865e57919326881967c2edee6910d72ecbc33a Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 27 Jan 2023 11:34:42 -0300 Subject: [PATCH 7/9] Improve setting description --- .../apache/cloudstack/consoleproxy/ConsoleAccessManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java index 30d407b0aeec..f19b5398dd9e 100644 --- a/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java +++ b/api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java @@ -33,7 +33,7 @@ public interface ConsoleAccessManager extends Manager, Configurable { ConfigKey ConsoleSessionCleanupInterval = new ConfigKey<>("Advanced", Integer.class, "console.session.cleanup.interval", "180", - "Determines how long (in hours) to wait before actually expunging destroyed console session records", + "Determines the interval (in hours) to wait between the console session cleanup tasks", false, ConfigKey.Scope.Global); From 6a41242aeae07aa80ef1a32de4f4ad1a8707a5f3 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 31 Jan 2023 07:34:05 -0300 Subject: [PATCH 8/9] Improve logs --- .../consoleproxy/ConsoleAccessManagerImpl.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index e29b6d3aac7e..31a7a8d20efd 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -167,9 +167,15 @@ private void reallyRun() { } Integer retentionHours = ConsoleAccessManager.ConsoleSessionCleanupRetentionHours.value(); Date dateBefore = DateTime.now().minusHours(retentionHours).toDate(); + if (s_logger.isDebugEnabled()) { + s_logger.debug(String.format("Retention hours: %s, checking for removed console session " + + "records to expunge older than: %s", retentionHours, dateBefore)); + } int sessionsExpunged = consoleSessionDao.expungeSessionsOlderThanDate(dateBefore); - if (sessionsExpunged > 0 && s_logger.isDebugEnabled()) { - s_logger.info(String.format("Expunged %s removed console session records", sessionsExpunged)); + if (s_logger.isDebugEnabled()) { + s_logger.debug(sessionsExpunged > 0 ? + String.format("Expunged %s removed console session records", sessionsExpunged) : + "No removed console session records expunged on this cleanup task run"); } } } From 30da7a913af9c19c91b595bc43bade8904400c23 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 31 Jan 2023 12:42:35 -0300 Subject: [PATCH 9/9] Fix code smells from sonarcloud --- .../ConsoleProxyClientStatsCollector.java | 16 +++++++------ .../consoleproxy/ConsoleProxyGCThread.java | 15 ++++++------ .../consoleproxy/ConsoleProxyNoVncClient.java | 23 +++++++++++-------- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientStatsCollector.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientStatsCollector.java index f82bfdfebb58..6b144a3def3d 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientStatsCollector.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientStatsCollector.java @@ -18,9 +18,10 @@ import java.io.OutputStreamWriter; import java.util.ArrayList; -import java.util.Enumeration; -import java.util.Hashtable; +import java.util.Iterator; import java.util.List; +import java.util.Map; +import java.util.Set; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -42,7 +43,7 @@ public void setRemovedSessions(List removed) { removedSessions.addAll(removed); } - public ConsoleProxyClientStatsCollector(Hashtable connMap) { + public ConsoleProxyClientStatsCollector(Map connMap) { setConnections(connMap); } @@ -56,13 +57,14 @@ public void getStatsReport(OutputStreamWriter os) { gson.toJson(this, os); } - private void setConnections(Hashtable connMap) { + private void setConnections(Map connMap) { ArrayList conns = new ArrayList(); - Enumeration e = connMap.keys(); - while (e.hasMoreElements()) { + Set e = connMap.keySet(); + Iterator iterator = e.iterator(); + while (iterator.hasNext()) { synchronized (connMap) { - String key = e.nextElement(); + String key = iterator.next(); ConsoleProxyClient client = connMap.get(key); ConsoleProxyConnection conn = new ConsoleProxyConnection(); diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java index c4cbec5c4394..16046abc7eb0 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java @@ -18,8 +18,8 @@ import java.io.File; import java.util.ArrayList; -import java.util.Enumeration; -import java.util.Hashtable; +import java.util.Iterator; +import java.util.Map; import java.util.Set; import org.apache.log4j.Logger; @@ -35,11 +35,11 @@ public class ConsoleProxyGCThread extends Thread { private final static int MAX_SESSION_IDLE_SECONDS = 180; - private final Hashtable connMap; + private final Map connMap; private final Set removedSessionsSet; private long lastLogScan = 0; - public ConsoleProxyGCThread(Hashtable connMap, Set removedSet) { + public ConsoleProxyGCThread(Map connMap, Set removedSet) { this.connMap = connMap; this.removedSessionsSet = removedSet; } @@ -79,13 +79,14 @@ public void run() { if (s_logger.isDebugEnabled()) { s_logger.debug(String.format("connMap=%s, removedSessions=%s", connMap, removedSessionsSet)); } - Enumeration e = connMap.keys(); - while (e.hasMoreElements()) { + Set e = connMap.keySet(); + Iterator iterator = e.iterator(); + while (iterator.hasNext()) { String key; ConsoleProxyClient client; synchronized (connMap) { - key = e.nextElement(); + key = iterator.next(); client = connMap.get(key); } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java index 36ba2a7b26bd..ae9573c3aea9 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java @@ -126,16 +126,8 @@ public void run() { } else { b = new byte[100]; readBytes = client.read(b); - if (readBytes == -1) { - break; - } - if (readBytes > 0) { - try { - session.getRemote().sendBytes(ByteBuffer.wrap(b, 0, readBytes)); - updateFrontEndActivityTime(); - } catch (WebSocketException e) { - connectionAlive = false; - } + if (readBytes == -1 || (readBytes > 0 && !sendReadBytesToNoVNC(b, readBytes))) { + connectionAlive = false; } } } @@ -149,6 +141,17 @@ public void run() { worker.start(); } + private boolean sendReadBytesToNoVNC(byte[] b, int readBytes) { + try { + session.getRemote().sendBytes(ByteBuffer.wrap(b, 0, readBytes)); + updateFrontEndActivityTime(); + } catch (WebSocketException | IOException e) { + s_logger.debug("Connection exception", e); + return false; + } + return true; + } + /** * Authenticate to VNC server when not using websockets *