From f9e24fa387e765710452109359c77ccdc55f901f Mon Sep 17 00:00:00 2001 From: poddm <8801231+poddm@users.noreply.github.com> Date: Tue, 21 Jul 2026 08:58:30 -0700 Subject: [PATCH] identity: derive stable management server id (msid) from FQDN By default the management server node id (msid) is derived from the host MAC address. When the MAC address is not stable across restarts the msid changes, orphaning the mshost row and breaking async jobs, HA work (fk_op_ha_work__mgmt_server_id), and router/stats ownership. This adds an opt-in mode that derives the msid from a SHA-256 hash of the node FQDN, which stays stable across restarts. Enable it with the environment variable CLOUDSTACK_MSID_FROM_FQDN=true or the system property cloudstack.msid.from.fqdn=true. On failure to derive the FQDN-based id, ManagementServerNode records the cause and returns an invalid id (0) rather than silently reverting to the unstable MAC-based id, so the system-integrity check fails startup cleanly instead of raising an ExceptionInInitializerError. All node-identity consumers (StatsCollector, ManagementServerImpl, NetworkUsageManagerImpl, VirtualNetworkApplianceManagerImpl, CloudZonesStartupProcessor) now obtain the id from ManagementServerNode.getManagementServerId() so they agree on the same value. Adds ManagementServerNodeTest covering both derivation modes. --- .../CloudZonesStartupProcessor.java | 5 +- .../network/NetworkUsageManagerImpl.java | 4 +- .../VirtualNetworkApplianceManagerImpl.java | 3 +- .../cloud/server/ManagementServerImpl.java | 3 +- .../java/com/cloud/server/StatsCollector.java | 3 +- .../utils/identity/ManagementServerNode.java | 87 ++++++++++- .../identity/ManagementServerNodeTest.java | 137 ++++++++++++++++++ 7 files changed, 226 insertions(+), 16 deletions(-) create mode 100644 utils/src/test/java/org/apache/cloudstack/utils/identity/ManagementServerNodeTest.java diff --git a/server/src/main/java/com/cloud/hypervisor/CloudZonesStartupProcessor.java b/server/src/main/java/com/cloud/hypervisor/CloudZonesStartupProcessor.java index f2704f35fcdd..35b3a0498cfa 100644 --- a/server/src/main/java/com/cloud/hypervisor/CloudZonesStartupProcessor.java +++ b/server/src/main/java/com/cloud/hypervisor/CloudZonesStartupProcessor.java @@ -24,6 +24,8 @@ import org.springframework.stereotype.Component; +import org.apache.cloudstack.utils.identity.ManagementServerNode; + import com.cloud.agent.AgentManager; import com.cloud.agent.StartupCommandProcessor; import com.cloud.agent.api.StartupCommand; @@ -38,7 +40,6 @@ import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.utils.component.AdapterBase; -import com.cloud.utils.net.MacAddress; import com.cloud.utils.net.NetUtils; /** @@ -62,7 +63,7 @@ public boolean configure(String name, Map params) throws Configu if (_nodeId == -1) { // FIXME: We really should not do this like this. It should be done // at config time and is stored as a config variable. - _nodeId = MacAddress.getMacAddress().toLong(); + _nodeId = ManagementServerNode.getManagementServerId(); } return true; } diff --git a/server/src/main/java/com/cloud/network/NetworkUsageManagerImpl.java b/server/src/main/java/com/cloud/network/NetworkUsageManagerImpl.java index 0aee5f234969..3d512be0d31e 100644 --- a/server/src/main/java/com/cloud/network/NetworkUsageManagerImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkUsageManagerImpl.java @@ -34,6 +34,7 @@ import org.apache.cloudstack.api.command.admin.usage.DeleteTrafficMonitorCmd; import org.apache.cloudstack.api.command.admin.usage.ListTrafficMonitorsCmd; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.utils.identity.ManagementServerNode; import com.cloud.agent.AgentManager; import com.cloud.agent.Listener; @@ -88,7 +89,6 @@ import com.cloud.utils.db.TransactionCallbackNoReturn; import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.net.MacAddress; @Component public class NetworkUsageManagerImpl extends ManagerBase implements NetworkUsageService, NetworkUsageManager, ResourceStateAdapter { @@ -251,7 +251,7 @@ protected class DirectNetworkStatsListener implements Listener { private int _interval; - private final long mgmtSrvrId = MacAddress.getMacAddress().toLong(); + private final long mgmtSrvrId = ManagementServerNode.getManagementServerId(); protected DirectNetworkStatsListener(int interval) { _interval = interval; diff --git a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index a166e894be1b..6c43ccf419f8 100644 --- a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -252,7 +252,6 @@ import com.cloud.utils.fsm.StateListener; import com.cloud.utils.fsm.StateMachine2; import com.cloud.utils.net.Ip; -import com.cloud.utils.net.MacAddress; import com.cloud.utils.net.NetUtils; import com.cloud.vm.DomainRouterVO; import com.cloud.vm.Nic; @@ -367,7 +366,7 @@ public class VirtualNetworkApplianceManagerImpl extends ManagerBase implements V private int _routerExtraPublicNics = 2; private int _usageAggregationRange = 1440; private String _usageTimeZone = "GMT"; - private final long mgmtSrvrId = MacAddress.getMacAddress().toLong(); + private final long mgmtSrvrId = ManagementServerNode.getManagementServerId(); private static final int ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_COOPERATION = 5; // 5 seconds private boolean _dailyOrHourly = false; diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index f32857d7cf04..ba7ebc196fb0 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -858,7 +858,6 @@ import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.fsm.StateMachine2; -import com.cloud.utils.net.MacAddress; import com.cloud.utils.net.NetUtils; import com.cloud.utils.security.CertificateHelper; import com.cloud.utils.ssh.SSHKeysHelper; @@ -1207,7 +1206,7 @@ public DetailVO findDetail(final long hostId, final String name) { @Override public long getId() { - return MacAddress.getMacAddress().toLong(); + return ManagementServerNode.getManagementServerId(); } protected void checkPortParameters(final String publicPort, final String privatePort, final String privateIp, final String proto) { diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 293b28c6cb46..69a93107d56d 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -147,7 +147,6 @@ import com.cloud.utils.db.TransactionCallbackNoReturn; import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.net.MacAddress; import com.cloud.utils.script.Script; import com.cloud.vm.NicVO; import com.cloud.vm.UserVmManager; @@ -383,7 +382,7 @@ public String toString() { private ScheduledExecutorService _diskStatsUpdateExecutor; private int _usageAggregationRange = 1440; private String _usageTimeZone = "GMT"; - private final long mgmtSrvrId = MacAddress.getMacAddress().toLong(); + private final long mgmtSrvrId = ManagementServerNode.getManagementServerId(); private static final int ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_COOPERATION = 5; // 5 seconds private boolean _dailyOrHourly = false; protected long managementServerNodeId = ManagementServerNode.getManagementServerId(); diff --git a/utils/src/main/java/org/apache/cloudstack/utils/identity/ManagementServerNode.java b/utils/src/main/java/org/apache/cloudstack/utils/identity/ManagementServerNode.java index 32989c3497c5..d48f300ab59e 100644 --- a/utils/src/main/java/org/apache/cloudstack/utils/identity/ManagementServerNode.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/identity/ManagementServerNode.java @@ -19,7 +19,9 @@ package org.apache.cloudstack.utils.identity; - +import java.net.InetAddress; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.component.ComponentLifecycle; @@ -27,9 +29,80 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.MacAddress; +/** + * Canonical source of the management-server node id ({@code msid}). + * + *

By default the id is derived from the host hardware MAC address. When the MAC address is + * not stable across restarts, the {@code msid} changes, which orphans the {@code mshost} row + * and breaks async jobs, HA work ({@code fk_op_ha_work__mgmt_server_id}), and router/stats + * ownership. + * + *

Setting the environment variable {@code CLOUDSTACK_MSID_FROM_FQDN=true} (or the system + * property {@code cloudstack.msid.from.fqdn=true}) instead derives the id from a SHA-256 hash + * of the node FQDN, which stays stable across restarts. All node-identity consumers must + * obtain the id from {@link #getManagementServerId()} so they agree on the same value. + */ public class ManagementServerNode extends AdapterBase implements SystemIntegrityChecker { - private static final long s_nodeId = MacAddress.getMacAddress().toLong(); + private static final String FQDN_ENV_VAR = "CLOUDSTACK_MSID_FROM_FQDN"; + private static final String FQDN_SYS_PROP = "cloudstack.msid.from.fqdn"; + + private static String s_nodeIdSource; + private static Exception s_initError; + private static final long s_nodeId = initNodeId(); + + private static long initNodeId() { + if (isFqdnModeEnabled()) { + return generateIdFromFqdn(); + } + s_nodeIdSource = "mac-address"; + return MacAddress.getMacAddress().toLong(); + } + + private static boolean isFqdnModeEnabled() { + return isTruthy(System.getenv(FQDN_ENV_VAR)) || isTruthy(System.getProperty(FQDN_SYS_PROP)); + } + + private static boolean isTruthy(String value) { + if (value == null) { + return false; + } + String trimmed = value.trim(); + return "true".equalsIgnoreCase(trimmed) || "1".equals(trimmed) || "yes".equalsIgnoreCase(trimmed); + } + + /** + * Derives a stable node id from a SHA-256 hash of the local FQDN. + * + *

On failure it records the cause and returns {@code 0} (an invalid id) rather than + * silently reverting to an unstable MAC-based id. The invalid id makes {@link #check()} + * fail the system-integrity check, which stops startup cleanly via {@link #start()} + * instead of raising an {@code ExceptionInInitializerError} from static initialization. + * + * @return a positive, non-zero 48-bit id, or {@code 0} if it cannot be derived + */ + private static long generateIdFromFqdn() { + try { + String fqdn = InetAddress.getLocalHost().getCanonicalHostName(); + s_nodeIdSource = "fqdn:" + fqdn; + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + byte[] hash = digest.digest(fqdn.getBytes(StandardCharsets.UTF_8)); + long id = 0; + for (int i = 0; i < 6; i++) { + id = (id << 8) | (hash[i] & 0xFF); + } + // Ensure positive and non-zero + id = id & 0x7FFFFFFFFFFFFFFFL; + if (id == 0) { + id = 1; + } + return id; + } catch (Exception e) { + s_nodeIdSource = "fqdn-error"; + s_initError = e; + return 0; + } + } public ManagementServerNode() { setRunLevel(ComponentLifecycle.RUN_LEVEL_FRAMEWORK_BOOTSTRAP); @@ -38,7 +111,8 @@ public ManagementServerNode() { @Override public void check() { if (s_nodeId <= 0) { - throw new CloudRuntimeException("Unable to get the management server node id"); + throw new CloudRuntimeException( + "Unable to derive the management server node id (source: " + s_nodeIdSource + ")", s_initError); } } @@ -50,10 +124,11 @@ public static long getManagementServerId() { public boolean start() { try { check(); - } catch (Exception e) { - logger.error("System integrity check exception", e); - System.exit(1); + } catch (CloudRuntimeException e) { + logger.error("System integrity check failed for the management server node id", e); + throw e; } + logger.info("Management server node id: {} (source: {})", s_nodeId, s_nodeIdSource); return true; } } diff --git a/utils/src/test/java/org/apache/cloudstack/utils/identity/ManagementServerNodeTest.java b/utils/src/test/java/org/apache/cloudstack/utils/identity/ManagementServerNodeTest.java new file mode 100644 index 000000000000..b6b1f09a5e86 --- /dev/null +++ b/utils/src/test/java/org/apache/cloudstack/utils/identity/ManagementServerNodeTest.java @@ -0,0 +1,137 @@ +// 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.utils.identity; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.lang.reflect.Method; + +import org.junit.After; +import org.junit.Test; + +public class ManagementServerNodeTest { + + private static final String FQDN_SYS_PROP = "cloudstack.msid.from.fqdn"; + + @After + public void tearDown() { + System.clearProperty(FQDN_SYS_PROP); + } + + private static boolean invokeIsTruthy(String value) throws Exception { + Method m = ManagementServerNode.class.getDeclaredMethod("isTruthy", String.class); + m.setAccessible(true); + return (boolean) m.invoke(null, value); + } + + private static boolean invokeIsFqdnModeEnabled() throws Exception { + Method m = ManagementServerNode.class.getDeclaredMethod("isFqdnModeEnabled"); + m.setAccessible(true); + return (boolean) m.invoke(null); + } + + private static long invokeGenerateIdFromFqdn() throws Exception { + Method m = ManagementServerNode.class.getDeclaredMethod("generateIdFromFqdn"); + m.setAccessible(true); + return (long) m.invoke(null); + } + + @Test + public void testGetManagementServerIdIsPositive() { + assertTrue("Node id must be a positive, non-zero value", ManagementServerNode.getManagementServerId() > 0); + } + + @Test + public void testCheckPassesWithValidNodeId() { + // Node id is derived at class-load time and should be valid, so check() must not throw. + new ManagementServerNode().check(); + } + + @Test + public void testStartReturnsTrueWithValidNodeId() { + // With a valid node id, start() must succeed and return true without terminating the JVM. + assertTrue(new ManagementServerNode().start()); + } + + @Test + public void testIsTruthyRecognizesTrueValues() throws Exception { + assertTrue(invokeIsTruthy("true")); + assertTrue(invokeIsTruthy("TRUE")); + assertTrue(invokeIsTruthy("True")); + assertTrue(invokeIsTruthy("1")); + assertTrue(invokeIsTruthy("yes")); + assertTrue(invokeIsTruthy("YES")); + } + + @Test + public void testIsTruthyTrimsWhitespace() throws Exception { + assertTrue(invokeIsTruthy(" true ")); + assertTrue(invokeIsTruthy("\t1\n")); + assertTrue(invokeIsTruthy(" yes ")); + } + + @Test + public void testIsTruthyRejectsFalseValues() throws Exception { + assertFalse(invokeIsTruthy(null)); + assertFalse(invokeIsTruthy("")); + assertFalse(invokeIsTruthy(" ")); + assertFalse(invokeIsTruthy("false")); + assertFalse(invokeIsTruthy("0")); + assertFalse(invokeIsTruthy("no")); + assertFalse(invokeIsTruthy("enabled")); + assertFalse(invokeIsTruthy("2")); + } + + @Test + public void testIsFqdnModeEnabledDefaultsFalse() throws Exception { + System.clearProperty(FQDN_SYS_PROP); + assertFalse(invokeIsFqdnModeEnabled()); + } + + @Test + public void testIsFqdnModeEnabledWhenSystemPropertyTruthy() throws Exception { + System.setProperty(FQDN_SYS_PROP, "true"); + assertTrue(invokeIsFqdnModeEnabled()); + } + + @Test + public void testIsFqdnModeEnabledWhenSystemPropertyFalsy() throws Exception { + System.setProperty(FQDN_SYS_PROP, "false"); + assertFalse(invokeIsFqdnModeEnabled()); + } + + @Test + public void testGenerateIdFromFqdnIsPositiveAndNonZero() throws Exception { + long id = invokeGenerateIdFromFqdn(); + assertTrue("Generated id must be positive and non-zero", id > 0); + } + + @Test + public void testGenerateIdFromFqdnFitsIn48Bits() throws Exception { + long id = invokeGenerateIdFromFqdn(); + assertTrue("Generated id must fit within 48 bits", id <= 0xFFFFFFFFFFFFL); + } + + @Test + public void testGenerateIdFromFqdnIsDeterministic() throws Exception { + long first = invokeGenerateIdFromFqdn(); + long second = invokeGenerateIdFromFqdn(); + assertEquals("Generated id must be stable across calls for the same FQDN", first, second); + } +}