Fix failure on agent reconnection by vishesh92 · Pull Request #8089 · apache/cloudstack

vishesh92 · 2023-10-12T20:07:40Z

Description

Depending on the agents' configuration, restarting a management server (preferred MS for the agent) will make the agent connect to another management server (non preferred MS). When the preferred MS comes back up, agent will try to disconnect with non-preferred MS and connect with the preferred MS. A race condition can happen during this process in which disconnection from non-preferred MS completes after the connection with preferred MS. This leads to agent to go into an Alert state. During this time, agent is still sending Ping to the preferred MS.

This PR solves this issue by:

Taking a lock in database which ensures that for a host, only connection or disconnection can happen across different MS.
While processing the Ping command if the Host is not in Up state, we request the agent to send a startup command again to the connection. If the startup is successful, the agent will come back in Up state.

To reproduce the issue,

apply the patch below.

diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
index b74c11cf..aec8a8dd 100644
--- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
+++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
@@ -836,6 +836,9 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
 
         removeAgent(attache, nextStatus);
         // update the DB
+        try {
+            Thread.sleep(5000L);
+        } catch (Exception e){}
         if (host != null && transitState) {
             disconnectAgent(host, event, _nodeId);
         }

Setup an environment with two management servers and below global configuration

agent.lb.enabled = true
indirect.agent.lb.algorithm = roundrobin
indirect.agent.lb.check.interval = 30

Restart the preferred management server.
Check status of host in database or in the UI

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)
build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Tried by applying the above patch.
Updated the state of a host to Alert state in database. After getting a ping, it gets a startup command after which it turns back to Up state.

How did you try to break this feature and the system with this change?

…e same time for a host

codecov · 2023-10-12T21:16:05Z

Codecov Report

Merging #8089 (0169fc9) into 4.18 (3e7f21a) will increase coverage by 0.00%.
Report is 14 commits behind head on 4.18.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##               4.18    #8089   +/-   ##
=========================================
  Coverage     13.06%   13.07%           
- Complexity     9109     9111    +2     
=========================================
  Files          2720     2720           
  Lines        257526   257566   +40     
  Branches      40150    40154    +4     
=========================================
+ Hits          33655    33666   +11     
- Misses       219644   219671   +27     
- Partials       4227     4229    +2

Files	Coverage Δ
...java/com/cloud/agent/manager/AgentManagerImpl.java	`4.82% <0.00%> (-0.12%)`	⬇️

... and 8 files with indirect coverage changes

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

vishesh92 · 2023-10-13T05:53:16Z

@blueorangutan package

blueorangutan · 2023-10-13T05:54:05Z

@vishesh92 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 · 2023-10-13T06:59:45Z

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

yadvr

LGTM - didn't test it though

yadvr · 2023-10-13T08:22:29Z

@blueorangutan test

blueorangutan · 2023-10-13T08:24:03Z

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

yadvr · 2023-10-13T08:24:13Z

@blueorangutan test alma8 kvm-alma8

blueorangutan · 2023-10-13T08:26:03Z

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

blueorangutan · 2023-10-13T21:04:09Z

[SF] Trillian test result (tid-7962)
Environment: kvm-alma8 (x2), Advanced Networking with Mgmt server a8
Total time taken: 42908 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8089-t7962-kvm-alma8.zip
Smoke tests completed. 107 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_03_deploy_vm_wrong_checksum	`Error`	43.84	test_templates.py
test_09_list_templates_download_details	`Failure`	0.06	test_templates.py
test_01_migrate_VM_and_root_volume	`Error`	83.08	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	50.71	test_vm_life_cycle.py
test_08_migrate_vm	`Error`	47.03	test_vm_life_cycle.py

blueorangutan · 2023-10-13T22:19:05Z

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

Test	Result	Time (s)	Test File
test_03_deploy_vm_wrong_checksum	`Error`	40.57	test_templates.py
test_09_list_templates_download_details	`Failure`	0.05	test_templates.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	3643.84	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	0.06	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	324.61	test_kubernetes_clusters.py
test_08_migrate_vm	`Error`	43.77	test_vm_life_cycle.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	303.79	test_hostha_kvm.py
test_hostha_kvm_host_degraded	`Error`	9.15	test_hostha_kvm.py

weizhouapache

code lgtm

yadvr · 2023-10-18T07:30:18Z

@blueorangutan package

blueorangutan · 2023-10-18T07:32:04Z

@rohityadavcloud 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 · 2023-10-18T08:34:47Z

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

DaanHoogland · 2023-10-18T10:08:28Z

-        // update the DB
-        if (host != null && transitState) {
-            disconnectAgent(host, event, _nodeId);
+                // update the DB


Suggested change

// update the DB

It was part of the initial code and we are updating the state on disconnecting the agent.

do you mean this comment should stay there @vishesh92 ?

IMO, yes. The comment should be more detailed instead.

shwstppr

Code LGTM

DaanHoogland

Note the use of host.getUuid() instead of host.getId() in my suggestions!?

vishesh92 · 2023-10-23T07:26:34Z

@blueorangutan package

blueorangutan · 2023-10-23T07:28:03Z

@vishesh92 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.

blueorangutan · 2023-10-23T08:27:32Z

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

vishesh92 · 2023-10-23T08:30:03Z

@blueorangutan test

blueorangutan · 2023-10-23T08:32:03Z

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

DaanHoogland · 2023-10-24T14:18:53Z

@blueorangutan test

blueorangutan · 2023-10-24T14:20:04Z

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

blueorangutan · 2023-10-25T01:33:20Z

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

Test	Result	Time (s)	Test File
test_08_migrate_vm	`Error`	45.83	test_vm_life_cycle.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	305.88	test_hostha_kvm.py

DaanHoogland · 2023-10-25T11:04:12Z

@blueorangutan package

blueorangutan · 2023-10-25T11:06:03Z

@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.

blueorangutan · 2023-10-25T12:08:00Z

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

DaanHoogland · 2023-10-25T12:29:48Z

@blueorangutan test

blueorangutan · 2023-10-25T12:30:04Z

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

DaanHoogland · 2023-10-25T12:50:13Z

tried to break this by setting a breakpoint on the host check in the disconnect handler on the secondary MS/host and could see the host getting back up and connected to its original MS/host.

blueorangutan · 2023-10-26T07:27:10Z

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

Test	Result	Time (s)	Test File

vishesh92 added 3 commits October 12, 2023 19:08

Add locks to ensure connection and disconnection doesn't happen at th…

ae7c36e

…e same time for a host

On ping, if agent is not in Up state, request startup command

79100ef

Add smoketest to check host's status update on ping

b593a20

boring-cyborg bot added component:agent component:api component:integration-test component:orchestration Python Warning... Python code Ahead! labels Oct 12, 2023

yadvr added this to the 4.18.2.0 milestone Oct 13, 2023

yadvr approved these changes Oct 13, 2023

View reviewed changes

yadvr requested review from shwstppr and weizhouapache October 13, 2023 08:22

DaanHoogland reviewed Oct 13, 2023

View reviewed changes

weizhouapache approved these changes Oct 17, 2023

View reviewed changes

vishesh92 force-pushed the fix-failure-on-agent-reconnection branch from 04ef4c2 to 17d6385 Compare October 18, 2023 06:58

Address comments

39b9cfe

vishesh92 force-pushed the fix-failure-on-agent-reconnection branch from 17d6385 to 39b9cfe Compare October 18, 2023 07:16

DaanHoogland reviewed Oct 18, 2023

View reviewed changes

shwstppr approved these changes Oct 19, 2023

View reviewed changes

DaanHoogland approved these changes Oct 20, 2023

View reviewed changes

Resolve comments

c8dd68f

vishesh92 force-pushed the fix-failure-on-agent-reconnection branch from 7bab9b3 to c8dd68f Compare October 23, 2023 07:24

Handle case when MS is unable to acquire lock on host

0169fc9

DaanHoogland merged commit 3b11663 into apache:4.18 Oct 26, 2023

DaanHoogland deleted the fix-failure-on-agent-reconnection branch October 26, 2023 14:54

Conversation

vishesh92 commented Oct 12, 2023

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

codecov bot commented Oct 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vishesh92 commented Oct 13, 2023

Uh oh!

blueorangutan commented Oct 13, 2023

Uh oh!

blueorangutan commented Oct 13, 2023

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Oct 13, 2023

Uh oh!

blueorangutan commented Oct 13, 2023

Uh oh!

yadvr commented Oct 13, 2023

Uh oh!

blueorangutan commented Oct 13, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

blueorangutan commented Oct 13, 2023

Uh oh!

blueorangutan commented Oct 13, 2023

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Oct 18, 2023

Uh oh!

blueorangutan commented Oct 18, 2023

Uh oh!

blueorangutan commented Oct 18, 2023

Uh oh!

DaanHoogland Oct 18, 2023

Choose a reason for hiding this comment

Uh oh!

vishesh92 Oct 18, 2023

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Oct 19, 2023

Choose a reason for hiding this comment

Uh oh!

vishesh92 Oct 23, 2023

Choose a reason for hiding this comment

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vishesh92 commented Oct 23, 2023

Uh oh!

blueorangutan commented Oct 23, 2023

Uh oh!

blueorangutan commented Oct 23, 2023

Uh oh!

vishesh92 commented Oct 23, 2023

codecov bot commented Oct 12, 2023 •

edited

Loading