Copilot commented on code in PR #13345:
URL: https://github.com/apache/cloudstack/pull/13345#discussion_r4398568028
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/ClusteredAgentManagerImpl.java:
##########
@@ -510,14 +542,9 @@ public SocketChannel connectToPeer(final String peerName,
final SocketChannel pr
logger.info("Unable to find peer: {}", peerName);
return null;
}
- final String ip = ms.getServiceIP();
- InetAddress addr;
- int port = Port.value();
- try {
- addr = InetAddress.getByName(ip);
- } catch (final UnknownHostException e) {
- throw new CloudRuntimeException("Unable to resolve " + ip);
- }
+ final String hostName = getHostIdentifier(ms);
+ final int port = Port.value();
+ final InetAddress addr = resolveManagementServerAddress(ms);
Review Comment:
`getHostIdentifier` always prefers `ManagementServerHost.getName()`, but
`ClusterManagerImpl` stores that field as the node's canonical hostname even
when `management.server.address` is configured with IPs. In an IP-configured
deployment without DNS for that hostname, this replaces the previously working
`serviceIP` and makes clustered peer connections fail. Select the identifier
according to the configured address format (or fall back to `serviceIP`) rather
than unconditionally preferring the persisted name.
##########
core/src/main/java/org/apache/cloudstack/threadcontext/ThreadContextCommandUtil.java:
##########
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.threadcontext;
+
+import com.cloud.agent.api.Command;
+import com.cloud.utils.StringUtils;
+import org.apache.logging.log4j.ThreadContext;
+
+/**
+ * Utility class for Command-specific MDC operations.
+ * This class handles propagation of MDC values to and from Command objects.
+ *
+ * @author mprokopchuk
+ */
+public class ThreadContextCommandUtil {
+
+ /**
+ * Propagate UUID and log context ID from Command trace context to MDC.
+ * A key absent from the command is removed from MDC so that a worker
thread does not
+ * carry over the context of a previously processed command.
+ *
+ * @param cmd the command containing trace context parameters
+ */
+ public static void propagateContextFromCommand(Command cmd) {
+ if (cmd == null) {
+ return;
+ }
+ putOrRemove(ThreadContextUtil.MDC_LOG_CONTEXT_ID_KEY,
cmd.getTraceContextParam(ThreadContextUtil.CONTEXT_LOG_ID_KEY));
+ putOrRemove(ThreadContextUtil.MDC_UUID_KEY,
cmd.getTraceContextParam(ThreadContextUtil.CONTEXT_UUID_KEY));
+ }
+
+ private static void putOrRemove(String mdcKey, String value) {
+ if (StringUtils.isNotEmpty(value)) {
+ ThreadContext.put(mdcKey, value);
+ } else {
+ ThreadContext.remove(mdcKey);
+ }
+ }
+
+ /**
+ * Set UUID and log context ID in Command trace context from current MDC
values.
+ * MDC values are authoritative for the current thread; any pre-existing
trace context
+ * on the command is intentionally overwritten.
+ *
+ * @param cmd the command to set trace context parameters on
+ */
+ public static void setContextInCommand(Command cmd) {
+ if (cmd != null) {
+ String logContextId = (String)
ThreadContext.get(ThreadContextUtil.MDC_LOG_CONTEXT_ID_KEY);
+ if (StringUtils.isNotEmpty(logContextId)) {
+ cmd.setTraceContextParam(ThreadContextUtil.CONTEXT_LOG_ID_KEY,
logContextId);
+ }
+
+ String uuid = (String)
ThreadContext.get(ThreadContextUtil.MDC_UUID_KEY);
+ if (StringUtils.isNotEmpty(uuid)) {
+ cmd.setTraceContextParam(ThreadContextUtil.CONTEXT_UUID_KEY,
uuid);
Review Comment:
This method claims MDC values are authoritative and that existing trace
context is overwritten, but it only writes keys when the current MDC value is
non-empty. Reusing a `Command` that already carries a UUID or log ID on a
thread without that value leaves the old trace context attached and propagates
it to the next operation. Clear the command entries when the corresponding MDC
value is absent, rather than leaving the previous value in place.
##########
server/src/main/java/org/apache/cloudstack/agent/lb/IndirectAgentLBServiceImpl.java:
##########
@@ -595,16 +626,19 @@ protected void runInContext() {
msList = getManagementServerList(hostId, dcId,
orderedHostIdList, lbAlgorithm);
}
+ // ask Host to reconnect to another Management Server
final MigrateAgentConnectionCommand cmd = new
MigrateAgentConnectionCommand(msList, avoidMsList, lbAlgorithm,
lbCheckInterval);
- cmd.setWait(60);
+ cmd.setWait(IndirectAgentLBMigrateCommandWait.value());
final Answer answer = agentManager.easySend(hostId, cmd);
//may not receive answer when the agent disconnects immediately and try
reconnecting to other ms host
if (answer == null) {
logger.warn(String.format("Got empty answer while
initiating migration of agent connection for host agent ID: %d", hostId));
} else if (!answer.getResult()) {
Review Comment:
The null-answer branch immediately above records no failure, while this new
`failedHostIds` tracking only covers unsuccessful answers and exceptions. A
migration with no response is therefore omitted from the final failure warning.
Add the host to `failedHostIds` in the null-answer branch before logging it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
