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); + } +}