Skip to content

Fix acquire console session issue - #7558

Merged
yadvr merged 2 commits into
apache:mainfrom
shapeblue:main-fix-console-acquire-session
May 26, 2023
Merged

Fix acquire console session issue#7558
yadvr merged 2 commits into
apache:mainfrom
shapeblue:main-fix-console-acquire-session

Conversation

@nvazquez

Copy link
Copy Markdown
Contributor

Description

This PR fixes the VM console display on the main branch

Fixes: #7550

Before the fix:

2023-05-24 19:03:06,005 DEBUG [c.c.c.AgentHookBase] (AgentManager-Handler-8:null) (logid:) Acquiring session [cbc024b7-209a-4518-b582-7cc45272a29b] as it was just used.
2023-05-24 19:03:06,007 DEBUG [c.c.u.d.T.Transaction] (AgentManager-Handler-8:null) (logid:) Rolling back the transaction: Time = 1 Name = -ClusteredAgentManagerImpl$ClusteredAgentHandler.doTask:624-Task.call:83-Task.call:29-FutureTask.run:264-ThreadPoolExecutor.runWorker:1128-ThreadPoolExecutor$Worker.run:628-Thread.run:829; called by -TransactionLegacy.rollback:888-TransactionLegacy.removeUpTo:831-TransactionLegacy.close:655-TransactionContextInterceptor.invoke:36-ReflectiveMethodInvocation.proceed:175-ExposeInvocationInterceptor.invoke:97-ReflectiveMethodInvocation.proceed:186-JdkDynamicAopProxy.invoke:215-$Proxy251.acquireSession:-1-ConsoleAccessManagerImpl.acquireSession:251-AgentHookBase.onConsoleAccessAuthentication:114-ConsoleProxyListener.processControlCommand:61
2023-05-24 19:03:06,009 WARN [c.c.a.m.AgentManagerImpl] (AgentManager-Handler-8:null) (logid:) Caught: com.cloud.utils.exception.CloudRuntimeException: DB Exception on: com.mysql.cj.jdbc.ClientPreparedStatement: UPDATE console_session SET console_session.acquired=** NOT SPECIFIED ** WHERE console_session.id = 13 at com.cloud.utils.db.GenericDaoBase.update(GenericDaoBase.java:847)
at com.cloud.utils.db.GenericDaoBase.update(GenericDaoBase.java:802)
at com.cloud.utils.db.GenericDaoBase.update(GenericDaoBase.java:1399)
at com.cloud.vm.dao.ConsoleSessionDaoImpl.acquireSession(ConsoleSessionDaoImpl.java:65)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:344)
at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:198)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
at com.cloud.utils.db.TransactionContextInterceptor.invoke(TransactionContextInterceptor.java:34)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:175)
at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:97)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:215)
at com.sun.proxy.$Proxy251.acquireSession(Unknown Source)
at org.apache.cloudstack.consoleproxy.ConsoleAccessManagerImpl.acquireSession(ConsoleAccessManagerImpl.java:251)
at com.cloud.consoleproxy.AgentHookBase.onConsoleAccessAuthentication(AgentHookBase.java:114)
at com.cloud.consoleproxy.ConsoleProxyListener.processControlCommand(ConsoleProxyListener.java:61)
at com.cloud.agent.manager.AgentManagerImpl.handleControlCommand(AgentManagerImpl.java:300)
at com.cloud.agent.manager.AgentManagerImpl$AgentHandler.processRequest(AgentManagerImpl.java:1364)
at com.cloud.agent.manager.AgentManagerImpl$AgentHandler.doTask(AgentManagerImpl.java:1450)
at com.cloud.agent.manager.ClusteredAgentManagerImpl$ClusteredAgentHandler.doTask(ClusteredAgentManagerImpl.java:709)
at com.cloud.utils.nio.Task.call(Task.java:83)
at com.cloud.utils.nio.Task.call(Task.java:29)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: java.sql.SQLException: No value specified for parameter 1
at com.mysql.cj.jdbc.exceptions.SQLError.createSQLException(SQLError.java:129)
at com.mysql.cj.jdbc.exceptions.SQLError.createSQLException(SQLError.java:97)
at com.mysql.cj.jdbc.exceptions.SQLExceptionsMapping.translateException(SQLExceptionsMapping.java:122)
... 36 more
2023-05-24 19:03:06,011 DEBUG [c.c.a.m.AgentManagerImpl] (AgentManager-Handler-8:null) (logid:) SeqA 3-126: Sending Seq 3-126: { Ans: , MgmtId: 167781138, via: 3, Ver: v1, Flags: 100010, [{"com.cloud.agent.api.Answer":{"result":"false","details":"DB Exception on: com.mysql.cj.jdbc.ClientPreparedStatement: UPDATE console_session SET console_session.acquired=** NOT SPECIFIED ** WHERE console_session.id = 13 ","wait":"0","bypassHostMaintenance":"false"}}] }

After the fix: VM console displayed correctly

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

View VM console

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@nvazquez a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@codecov

codecovBot commented May 24, 2023

Copy link
Copy Markdown

Codecov Report

Merging #7558 (db647f6) into main (0024cb0) will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@## main #7558 +/- ##
=========================================
Coverage 12.96% 12.96% Complexity 8993 8993 =========================================
Files 2728 2728 Lines 256674 256674 Branches 40031 40031 =========================================
Hits 33269 33269 - Misses 219224 219225 +1 + Partials 4181 4180 -1 
Impacted FilesCoverage Δ
...a/src/main/java/com/cloud/vm/ConsoleSessionVO.java0.00% <ø> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6143

@shwstpprshwstppr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nvazquez this also fixes the issue,

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 53e98226db..81a11241e4 100644
--- a/engine/schema/src/main/java/com/cloud/vm/ConsoleSessionVO.java
+++ b/engine/schema/src/main/java/com/cloud/vm/ConsoleSessionVO.java
@@ -27,6 +27,8 @@ import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.Id;
import javax.persistence.Table;
+import javax.persistence.Temporal;
+import javax.persistence.TemporalType;
@Entity
@Table(name = "console_session")
@@ -56,6 +58,7 @@ public class ConsoleSessionVO {
private long hostId;
@Column(name = "acquired")
+ @Temporal(value = TemporalType.TIMESTAMP)
private Date acquired;
@Column(name = "removed")

@yadvr

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@rohityadavcloud a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

Comment on lines +71 to +81
try {
txn.start();
PreparedStatement preparedStatement = txn.prepareAutoCloseStatement(ACQUIRE_CONSOLE_SESSION);
preparedStatement.setDate(1, new java.sql.Date(-1L));
preparedStatement.setLong(2, consoleSessionId);
preparedStatement.executeUpdate();
txn.commit();
} catch (Exception e) {
txn.rollback();
LOGGER.warn(String.format("Failed acquiring console session id = %s: %s", consoleSessionId, e.getMessage()), e);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the logic of this is the same as before. If this fixes the problem fine, but shouldn't we fix the update method instead, @nvazquez ?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I've refactored after @shwstppr's suggestion

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan package

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

Thanks @shwstppr - I have refactored the logic to add the missing annotation

@weizhouapacheweizhouapache left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

code lgtm

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@nvazquez a [SF] 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.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6151

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-6578)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 58701 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7558-t6578-kvm-centos7.zip
Smoke tests completed. 109 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

TestResultTime (s)Test File
test_02_upgrade_kubernetes_clusterError3969.42test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_clusterError4038.74test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_clusterFailure0.04test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardownError113.03test_kubernetes_clusters.py

@yadvr
yadvr merged commit 2457d20 into apache:mainMay 26, 2023
@yadvr
yadvr deleted the main-fix-console-acquire-session branch May 26, 2023 01:50
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Console proxy doesn't work since the beginning of May

6 participants

@nvazquez@blueorangutan@yadvr@shwstppr@DaanHoogland@weizhouapache