Indirect agent connection improvements

[sureshanaparti]

Description

Continued from #13028

This PR improves the Indirect agent connection handling, has the following improvements.

• Enhances the Host connecting logic to avoid connecting storm (where Agent opens multiple sockets against Management Server).
• Implements HostConnectProcess task where Host upon connection checks whether lock is available, traces Host connecting progress, status and timeout.
• Introduces AgentConnectStatusCommand, where Host checks whether lock for the Host is available (i.e. "previous" connect process is finished).
• Implementes logic to check whether Management Server has lock against Host (exposed MySQL DB lock presence via API)
• Removes synchronization on Host disconnect process, double-disconnect logic in clustered Management Server environment, added early removal from ping map (in case of combination ping timeout delay + synchronized disconnect process the Agent Manager submits more disconnect requests)
• Introduces parameterized connection and status check timeouts
• Implements backoff algorithm abstraction - can be used either constant backoff timeout or exponential with jitter to wait between connection Host attempts to Management Server
• Implements ServerAttache to be used on the Agent side of communication (similar to Attache on Management Server side)
• Enhances/Adds logs significantly to Host Agent and Agent Manager logic to trace Host connecting and disconnecting process, including ids, names, context UUIDs and timings (how much time took overall initialization/deinitialization)
• Adds logs to communication between Management Servers (PDU requests)
• Adds DB indexes to improve search performance, uses IDEMPOTENT_ADD_INDEX for safer DB schema updates
• Dedupes agent connection requests. Observed scenarios where on agent with old agent code send a storm of connection requests. In such a case, to ensure other agents are not impacted, deduping the connection requests.
• Remove IP address hard dependency for management server communication. It allows the management.server.address config to be a list of hostnames instead of static IPs. Rebalancing, cluster propagation, alerts, and stats collection all detect the format at runtime via ManagementServerAddressUtil and use the matching lookup (listNonUpStateMsHostnames vs listNonUpStateMsIPs) when computing the "avoid" list sent to agents. Hostnames survive MS restarts and IP reassignments where static IPs do not, which is required in deployment environments where management server instances can be rescheduled or replaced.
• Propagates instance (vm, volume, etc) uuid to all operations. The uuid is propagated from MS to agent communications and to all async jobs.

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
• Test (unit or integration test code)

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?

Checked the indirect agents connections (and re-connections) with KVM hosts, SSVM & CPVM.

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

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 3.71%. Comparing base (602d9ec) to head (e5dc90b).

❗ There is a different number of reports uploaded between BASE (602d9ec) and HEAD (e5dc90b). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (602d9ec)	HEAD (e5dc90b)
unittests	1	0

Additional details and impacted files

@@              Coverage Diff              @@
##               main   #13345       +/-   ##
=============================================
- Coverage     19.91%    3.71%   -16.21%     
=============================================
  Files          6373      487     -5886     
  Lines        577230    41992   -535238     
  Branches      70696     7942    -62754     
=============================================
- Hits         114974     1558   -113416     
+ Misses       449690    40208   -409482     
+ Partials      12566      226    -12340     

Flag	Coverage Δ
uitests	3.71% <ø> (ø)
unittests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

This PR is a broad refactor/enhancement of indirect agent connection handling across both Management Server and Agent sides. It introduces a host-side connect/status-check process, adds configurable backoff and host-status heuristics, and expands logging/lock-check mechanisms to reduce reconnect storms and improve observability.

Changes:

• Adds agent-side connection orchestration (status polling + startup submission) and a server-side AgentConnectStatus* command/answer pair to coordinate connect progress.
• Introduces configurable backoff (factory + exponential-with-jitter implementation) and propagates configuration from Management Server to Agent during startup.
• Refactors NIO connection lifecycle and adds supporting utilities/logging, plus DB lock availability checks and DB indexes for performance.

Reviewed changes

Copilot reviewed 47 out of 47 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
utils/src/test/java/com/cloud/utils/testcase/NioTest.java	Test logging cleanup (exception logging).
utils/src/test/java/com/cloud/utils/backoff/impl/ConstantTimeBackoffTest.java	Updates config key name for constant backoff.
utils/src/main/java/com/cloud/utils/nio/NioServer.java	NIO server init tweaks and docstring.
utils/src/main/java/com/cloud/utils/nio/NioConnection.java	Connection lifecycle refactor, selector loop/logging, reject logic.
utils/src/main/java/com/cloud/utils/nio/NioClient.java	Client connect/handshake logging and expanded cleanup.
utils/src/main/java/com/cloud/utils/nio/Link.java	Adds local-port tracking, richer toString, termination helpers/logging changes.
utils/src/main/java/com/cloud/utils/nio/HandlerFactory.java	Changes new-connection registration API to InetSocketAddress.
utils/src/main/java/com/cloud/utils/net/NetUtils.java	Null guard in hostname-to-IP resolution helper.
utils/src/main/java/com/cloud/utils/LogUtils.java	Adds host logging helper with optional reverse lookup.
utils/src/main/java/com/cloud/utils/exception/CSExceptionErrorCode.java	Adds error-code mapping for ConnectionException.
utils/src/main/java/com/cloud/utils/DateUtil.java	Adds formatMillis duration formatter.
utils/src/main/java/com/cloud/utils/backoff/impl/ExponentialWithJitterBackoffMBean.java	New MBean interface for exponential-jitter backoff.
utils/src/main/java/com/cloud/utils/backoff/impl/ExponentialWithJitterBackoff.java	New exponential-with-jitter backoff implementation.
utils/src/main/java/com/cloud/utils/backoff/impl/ConstantTimeBackoff.java	Migrates to namespaced config keys + exposes configuration.
utils/src/main/java/com/cloud/utils/backoff/BackoffFactory.java	New backoff factory for selecting/configuring algorithms.
utils/src/main/java/com/cloud/utils/backoff/BackoffAlgorithm.java	Adds getConfiguration() API for propagating settings.
server/src/main/java/org/apache/cloudstack/agent/lb/IndirectAgentLBServiceImpl.java	Adds notes/timeout tweaks around agent migration command dispatch.
framework/db/src/main/java/com/cloud/utils/db/GlobalLock.java	Refactors lock bookkeeping/logging and adds lock-availability query.
framework/db/src/main/java/com/cloud/utils/db/DbUtil.java	Adds IS_FREE_LOCK helper and improves logging around lock DB connections.
framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKeyUtil.java	New utility to parse key=value;... configuration strings.
framework/cluster/src/main/java/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java	Adds DAO method to list all MS hosts including removed.
framework/cluster/src/main/java/com/cloud/cluster/dao/ManagementServerHostDao.java	Adds DAO API for “including removed” listing.
framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletImpl.java	Adds request logging and more detailed RemoteException messages.
framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletHttpHandler.java	Adds debug logging of inbound request line/body.
engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql	Adds indexes via idempotent add-index procedure calls.
engine/schema/src/main/resources/META-INF/db/procedures/cloud.idempotent_add_index.sql	Adds/defines IDEMPOTENT_ADD_INDEX procedure.
engine/orchestration/src/test/java/com/cloud/agent/manager/ClusteredAgentManagerImplTest.java	Expands tests for disconnect broadcast behavior and lock usage.
engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java	Adds tests for config keys and deregister/disconnect behaviors.
engine/orchestration/src/main/java/com/cloud/agent/manager/ClusteredAgentManagerImpl.java	Refactors attache creation/removal and event handling behavior.
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java	Major connect/disconnect refactor, new config keys, status checks, backoff propagation.
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java	Cancels listener alarm futures on unregister/cancel paths.
core/src/main/java/org/apache/cloudstack/threadcontext/ThreadContextUtil.java	New helper for propagating Log4j ThreadContext across threads.
core/src/main/java/com/cloud/resource/ServerResource.java	Changes default isExitOnFailures() behavior.
core/src/main/java/com/cloud/agent/transport/Request.java	Improves JSON deserialization error logging.
core/src/main/java/com/cloud/agent/api/StartupAnswer.java	Adds params + agent-side status-check delay transport fields.
core/src/main/java/com/cloud/agent/api/AgentConnectStatusCommand.java	New command for host connect-status checks.
core/src/main/java/com/cloud/agent/api/AgentConnectStatusAnswer.java	New answer carrying lock availability and host status.
agent/src/test/java/com/cloud/agent/HostConnectProcessTest.java	New test around scheduling the agent connect process.
agent/src/test/java/com/cloud/agent/AgentTest.java	Updates tests for new link/logging and reconnect helpers.
agent/src/main/java/com/cloud/agent/SynchronousListener.java	New blocking listener for synchronous waits on answers.
agent/src/main/java/com/cloud/agent/ServerListener.java	New listener interface for agent-side ServerAttache callbacks.
agent/src/main/java/com/cloud/agent/ServerAttache.java	New agent-side counterpart to MS Attache for command/answer flow.
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java	Adds new agent properties for async timeouts and status-check delay.
agent/src/main/java/com/cloud/agent/IAgentShell.java	Adds setter for dynamically updated backoff algorithm.
agent/src/main/java/com/cloud/agent/HostConnectProcess.java	New connect/status-check orchestration loop on the agent side.
agent/src/main/java/com/cloud/agent/AgentShell.java	Uses backoff factory + persists configuration; relaxes version handling.
agent/src/main/java/com/cloud/agent/Agent.java	Major reconnect/startup refactor; integrates HostConnectProcess and ServerAttache.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[blueorangutan]

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

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

[Copilot]

Pull request overview

Copilot reviewed 47 out of 47 changed files in this pull request and generated 7 comments.

[blueorangutan]

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

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

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

[bernardodemarco]

These @author fields are not necessary, right?

Suggested change

	/**
	* Command to check status of {@link StartupCommand} from the Agent.
	*
	* @author mprokopchuk
	*/
	/**
	* Command to check status of {@link StartupCommand} from the Agent.
	*
	*/

[sureshanaparti]

@bernardodemarco I think, it's ok to have it. we've author mentioned in few other classes as well. (check with '@author' in the code base).

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[Copilot]

🟡 Changes recommended

It introduces a real configuration bug in default backoff creation and adds new logging that can expose sensitive configuration/request data.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

agent/src/main/java/com/cloud/agent/HostConnectProcess.java:228

• sendStartupCommand receives connectionTransfer but calls serverResource.initialize() instead of initialize(connectionTransfer). This prevents resources from tailoring their StartupCommand content/behavior for transferred connections, even though the ServerResource interface now supports it.

Call the boolean overload so implementations can react to connection transfers.

            ServerResource serverResource = _agent.getResource();
            StartupCommand[] startup = serverResource.initialize();
            if (ArrayUtils.isEmpty(startup)) {

• Files reviewed: 63/63 changed files
• Comments generated: 3
• Review effort level: Lite

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain, including credential exposure, connection-flow failures, compatibility issues, and NIO lifecycle risk.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (19)

Previously missed (3) — in code that hasn't changed since the last review.

core/src/main/java/org/apache/cloudstack/threadcontext/ThreadContextCommandUtil.java:39

• These setters ignore absent values, but agent request tasks reuse executor threads and call this method for every command. A command without trace context therefore retains the previous command's UUID/log ID and is logged under the wrong resource. Clear each MDC key when its command value is absent, or run command processing in a context scope that resets the map.
  framework/cluster/src/main/java/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java:70
• ActiveSearch includes removed IS NULL and a recent lastUpdateTime condition, so this method does not return the “all ... including down and removed” rows promised by its contract. MsCache relies on these rows to recognize old Management Server IPs; omitting stale/removed entries can make them look like new agent connections. Use the unfiltered listAllIncludingRemoved() here.
  utils/src/main/java/com/cloud/utils/backoff/impl/ExponentialWithJitterBackoff.java:166
• The MBean exposes setTimeToWait, but this implementation silently ignores every JMX update. Operators cannot tune or disable the exponential backoff through the advertised management interface, unlike ConstantTimeBackoff; either implement the setter or remove it from the MBean contract.

agent/src/main/java/com/cloud/agent/AgentShell.java:423

• This debug log now dumps every agent property value. The property set includes keystore.passphrase (AgentProperties.java:903) and may contain operator-supplied secrets, so enabling debug logging exposes credentials in the agent log. Keep the previous name-only log or redact sensitive keys.

                LOGGER.debug("Found property: {}, value: {}", property, _properties.getProperty(property));

agent/src/main/java/com/cloud/agent/HostConnectProcess.java:109

• ScheduledExecutorService.scheduleWithFixedDelay rejects a non-positive delay. The management-server setting is an unrestricted integer, so configuring agent.host.status.check.delay.sec as 0 or negative makes every new connection fail while scheduling this task instead of falling back safely.

        var future = hostStatusExecutor.scheduleWithFixedDelay(ThreadContextUtil.wrapThreadContext(task),
                HOST_STATUS_CHECK_INITIAL_DELAY_SEC,
                hostStatusCheckDelaySec, TimeUnit.SECONDS);

agent/src/main/java/com/cloud/agent/HostConnectProcess.java:180

• These reconnect paths pass a null preferred host. The agent's host selection then falls back to InetSocketAddress.getAddress().getHostAddress(), so a connection made using a hostname is retried against its currently resolved IP. After a management-server IP change, reconnection still targets the stale address, defeating the hostname support described by this PR; preserve the original host string.

                _agent.getRequestHandler().submit(() -> _agent.reconnect(_link, null, _forceConnect));

agent/src/main/java/com/cloud/agent/HostConnectProcess.java:274

• When the startup request timed out, Boolean.FALSE means the Management Server still holds the host lock. This branch only logs and returns; runInternal() then unconditionally cancels the periodic task, so the agent stops checking while the previous connect is still in progress and can remain connected but never initialize. Keep the status task alive/retry (or explicitly reconnect) until the lock state is resolved.

                } else if (Boolean.FALSE.equals(lockAvailable)) {
                    logger.info("Host is locked and has state {} on {}", status, _link);

agent/src/main/java/com/cloud/agent/HostConnectProcess.java:247

• StartupAnswer responses are handled by Agent.processResponse via processStartupAnswer and are not forwarded to ServerAttache.processAnswers. Consequently this synchronous send cannot receive the normal startup response and can wait up to the 300-second timeout; successful startup handling also stops/interrupts this task while it is waiting. Send the startup command through the existing startup-response path, or explicitly deliver the answer to this listener without double-processing it.

                var answer = send(attache, commands, StartupAnswer.class, DEFAULT_ASYNC_STARTUP_COMMAND_TIMEOUT_SEC);

agent/src/main/java/com/cloud/agent/ServerAttache.java:302

• Interrupting the scheduled connect task does not stop an in-flight status request here: waitFor interruption is caught and the loop waits again. When a new connection process replaces this one, the old task can therefore resume and submit another StartupCommand, recreating the connection storm this process is intended to prevent. Preserve interruption and abort the request.

                    answers = sl.waitFor(wait);
                } catch (InterruptedException e) {
                    logger.debug(log(seq, "Interrupted"));
                }

agent/src/main/java/com/cloud/agent/ServerAttache.java:364

• If sending the next queued request fails, the unconditional assignment still marks that failed sequence as _currentSequence. Subsequent execute-in-sequence requests will remain queued behind a request that can never receive a response until the link is disconnected. Set the current sequence only after a successful send.

        try {
            send(req);
        } catch (CloudException e) {
            logger.debug(log(req.getSequence(), "Unable to send the next sequence"));
            cancel(req.getSequence());
        }
        _currentSequence = req.getSequence();

core/src/main/java/com/cloud/agent/api/AgentConnectStatusAnswer.java:35

• The description is inverted: lockAvailable is set from GlobalLock.isLockAvailable() and callers treat TRUE as “no lock”, but this Javadoc says TRUE means the lock is acquired. Document TRUE as the lock being available/free so implementations do not follow the wrong contract.

     * {@link Boolean#TRUE} means host has {@link GlobalLock#lock(int)} acquired, otherwise {@link Boolean#FALSE},
     * and null if there is an error during executing {@link AgentConnectStatusCommand}.

engine/orchestration/src/main/java/com/cloud/agent/manager/ClusteredAgentManagerImpl.java:602

• This helper unconditionally prefers ManagementServerHost.name. That column is populated with each node's canonical hostname independently of whether management.server.address uses IPs, so connectToPeer now breaks IP-only deployments when canonical names are not resolvable or reachable. Select the name versus serviceIP according to the configured address format, preserving the other only as fallback.

        final String hostName = host.getName();
        if (StringUtils.isNotBlank(hostName)) {
            return hostName;

engine/orchestration/src/main/java/com/cloud/agent/manager/ClusteredAgentManagerImpl.java:296

• When old != null, _agents.put(host.getId(), attache) has already installed the new attachment. removeAgent(old.getId(), attache) then removes that same new attachment; the helper only restores the removed entry when it differs from its second argument. The host is therefore left absent from _agents after a replacement. Pass old to this helper (or remove the old entry before installing the new one) so the new attachment remains registered.

            removeAgent(old.getId(), attache);

framework/cluster/src/main/java/com/cloud/cluster/ClusterManagerImpl.java:1265

• This hostname self-check makes the new hostname-conflict detection ineffective: checkConflicts() calls pingManagementNode on a peer with the same hostname, and this branch returns false before probing it. A live second management server that reused the hostname is therefore treated as non-pingable and initialization continues. Identify self by the local management-server id/bind identity, not by the peer hostname alone.

        if (currentHostName != null && currentHostName.equals(targetHostName)) {
            logger.info("ping management node cluster service can not be performed on self (hostname match: {})", currentHostName);
            return false;

framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletAdapter.java:98

• This now prefers mshost.name for every peer, but ClusterManagerImpl stores the canonical hostname there even when management.server.address is configured with IPs. Existing IP-based deployments will silently switch cluster RPC to DNS and can fail when that hostname is not reachable; choose the identifier using the configured address format, with the other field only as fallback.

        final String hostName = msHost.getName();
        final String identifier = hostName != null ? hostName : msHost.getServiceIP();

framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletHttpHandler.java:125

• The new HTTP-handler log writes the complete request body, including the serialized PDU payload, to the debug log. That payload is not guaranteed to be free of credentials or other sensitive command data; log only the request line or a sanitized payload instead.
  framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java:684
• cmdInfo is not uniformly JSON: VM work jobs are written by VmWorkSerializer.serialize using object serialization. Parsing those payloads as Map<String, String> fails, so UUID propagation is skipped for VM/volume-related async jobs. Derive the UUID from the job/entity or persist it in a format shared by these jobs.
  plugins/maintenance/src/main/java/org/apache/cloudstack/maintenance/ManagementServerMaintenanceManagerImpl.java:474
• Removing only msHost.getName() does not reliably remove the current configured address: ClusterManagerImpl stores the canonical hostname, while management.server.address may contain a CNAME/alias. In that deployment the server entering maintenance remains in indirectAgentMsList, so agents can be told to reconnect to it. Resolve the row to the exact configured address before removing it.
  utils/src/main/java/com/cloud/utils/backoff/impl/ExponentialWithJitterBackoff.java:68
• This backoff object is shared through the single AgentShell instance, while reconnect and command-handling tasks can call waitBeforeRetry() and reset() concurrently. attemptNumber and increasing are unsynchronized, so retries can lose each other's updates or reset another retry's state, producing an unpredictable backoff. Protect the state transition or keep attempt state per retry process.

• Files reviewed: 64/64 changed files
• Comments generated: 4
• Review effort level: Lite

[nvazquez]

Hi @sureshanaparti can you please fix the merge conflicts?

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

[Copilot]

🟡 Changes recommended

Unresolved credential-exposure and compilation issues, along with connection, hostname, context, and retry defects, block approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (19)

agent/src/main/java/com/cloud/agent/AgentShell.java:423

• This now logs every agent.properties value at DEBUG. The file can contain the keystore passphrase persisted under KeyStoreUtils.KS_PASSPHRASE_PROPERTY, so enabling debug logging exposes a credential; log only property names or redact sensitive values.

                LOGGER.debug("Found property: {}, value: {}", property, _properties.getProperty(property));

agent/src/main/java/com/cloud/agent/HostConnectProcess.java:133

• After a startup timeout, this set is used to decide whether the server already accepted the connection. It omits Status.Creating and Status.Alert, although the management-side connected/alive status contract includes them; a host in either state is treated as failed and reconnected, which can recreate the connection storm this process is meant to prevent. Share the connected-status set or include those states here.

        private final Set<Status> operationalStatuses = Set.of(Status.Connecting, Status.Up, Status.Rebalancing);

agent/src/main/java/com/cloud/agent/SynchronousListener.java:90

• An empty answer array can reach this listener (the caller explicitly checks for empty answers after send returns), but indexing _answers[0] throws ArrayIndexOutOfBoundsException. That converts an empty response into a timeout/transport failure instead of the intended empty-response error; guard the array length before indexing.

                profiler.getDurationInMillis(), _answers != null ? _answers[0].toString() : "null");

engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java:249

• The same future is added in this change, but AgentAttache.cancel(seq) still removes only _waitForList; it does not cancel/remove _alarmFutures. Every send failure can therefore leave a scheduled alarm alive until its timeout, adding timer work during the connection storm this PR is intended to prevent.

            java.util.concurrent.ScheduledFuture<?> alarmFuture =
                    s_listenerExecutor.schedule(new Alarm(seq), listener.getTimeout(), TimeUnit.SECONDS);
            _alarmFutures.put(seq, alarmFuture);

engine/orchestration/src/main/java/com/cloud/agent/manager/ClusteredAgentManagerImpl.java:296

• attache is the new object just inserted into _agents above. removeAgent removes the map entry and only restores it when the removed object differs from its second argument; passing this same new attache therefore leaves _agents without the active forward attache. Pass old as the second argument (or remove only the stale object) so subsequent findAttache calls still find the replacement.

            removeAgent(old.getId(), attache);

framework/cluster/src/main/java/com/cloud/cluster/ClusterManagerImpl.java:1263

• Hostnames are case-insensitive, but self-detection uses equals here while the conflict check below correctly uses equalsIgnoreCase. If the canonical and persisted names differ only by case, this node will be probed as a peer and can be reported as a duplicate of itself; use a case-insensitive comparison.

        if (currentHostName != null && currentHostName.equals(targetHostName)) {

framework/cluster/src/main/java/com/cloud/cluster/ClusterServiceServletImpl.java:162

• The outbound debug log includes both the serialized PDU request and its response. The request's gsonPackage can contain password-bearing agent commands, so this duplicates sensitive command data into management-server logs; retain only URL/timing/status metadata or redact both bodies.
  framework/cluster/src/test/java/com/cloud/cluster/ManagementServerAddressUtilTest.java:35
• The test mutates the singleton ApiServiceConfiguration.ManagementServerAddresses object's private _defaultValue and never restores it. After this class runs, the value remains whatever the last test used (currently ::1), contaminating unrelated tests that read the global configuration. Save the original value and restore it in an @After method.
  framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java:684
• cmdInfo is not uniformly JSON: VM work jobs serialize through VmWorkSerializer/JobSerializerHelper, so parsing those values as Map<String,String> fails and leaves the MDC UUID unset. This means the new propagation does not cover VM/volume async jobs; extract the UUID from the existing serialized work object or persist/read it separately.
  framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java:679
• The comment says the parent fallback applies when the current job has no cmdInfo, but this condition only checks related. A child that has its own cmdInfo will therefore be replaced by its parent's data and receive the wrong UUID; guard this lookup with StringUtils.isBlank(job.getCmdInfo()).
  plugins/maintenance/src/main/java/org/apache/cloudstack/maintenance/ManagementServerMaintenanceManagerImpl.java:481
• The same namespace mismatch applies here: listNonUpStateMsHostnames() yields canonical mshost.name values, but indirectAgentMsList is built from configured aliases. Removing these canonical values can leave down management servers in the maintenance candidate list.
  server/src/main/java/org/apache/cloudstack/agent/lb/IndirectAgentLBServiceImpl.java:547
• This source address is added from ManagementServerHostVO.getName(), which is the canonical hostname, not necessarily the hostname/alias in the configured host list. With an alias-based configuration, adding this value does not actually exclude the source management server during migration.
  utils/src/main/java/com/cloud/utils/LogUtils.java:127
• getHostName() can perform a blocking reverse-DNS lookup for a literal IP. This method is called before each NioClient connection and from the reconnect loop, so a slow/unavailable DNS service can stall connection attempts and defeat the intended bounded backoff behavior. Avoid reverse resolution on this hot path or use a non-blocking/cached value.
  utils/src/main/java/com/cloud/utils/backoff/impl/ExponentialWithJitterBackoff.java:107
• When the decreasing phase reaches attempt 0, this flips increasing but leaves attemptNumber at 1. The next cycle therefore starts at the second delay instead of the configured minimum, producing a max→...→second-delay→second-delay cycle rather than returning to the base delay. Assign attemptNumber = nextAttemptNumber in this branch.
  utils/src/main/java/com/cloud/utils/backoff/impl/ExponentialWithJitterBackoff.java:161
• getNextDelay() is capped at maxDelayMs, but adding jitter here returns values up to 1.5× that configured maximum. This violates the documented maximum and can make retry delays longer than the operator's bound; cap the final value after applying jitter.
  utils/src/main/java/com/cloud/utils/backoff/impl/ExponentialWithJitterBackoff.java:166
• The MBean exposes setTimeToWait, but this implementation silently ignores every update. An operator can therefore change the advertised backoff delay and observe no effect; implement a defined mapping to the exponential configuration or remove the setter from this MBean.
  utils/src/main/java/com/cloud/utils/backoff/impl/ExponentialWithJitterBackoff.java:68
• The backoff object is shared by the agent shell, and waitBeforeRetry() is invoked from multiple reconnect tasks, but attemptNumber and increasing are plain fields and calculateNextAttempt() is unsynchronized. Concurrent retries can read and overwrite the same state, causing several connections to use the same delay and defeating the storm-prevention backoff; protect the state transition or use an atomic state representation.
  utils/src/main/java/com/cloud/utils/nio/Link.java:396
• terminated() writes _key under synchronization, but this new reader is neither synchronized nor backed by a volatile field. The connection-storm guard and manager read it from other threads, so they can observe a stale false after termination and skip a needed reconnect; make this accessor synchronized or make _key volatile.
  utils/src/main/java/org/apache/cloudstack/threadcontext/ThreadContextUtil.java:107
• This treats every async-job cmdInfo as a JSON object map, but VM work jobs are serialized through JobSerializerHelper by VmWorkSerializer, not as JSON maps. Gson parsing therefore fails for those jobs and leaves the UUID unset, so the new propagation does not cover VM async operations; extract the UUID from the typed job/work data or pass it separately.

• Files reviewed: 63/63 changed files
• Comments generated: 10
• Review effort level: Lite

[blueorangutan]

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 19282