Mp/flxcld 2802

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<String, Object> 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;
     }

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;

server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java

@@ -250,7 +250,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;
@@ -365,7 +364,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;
 

server/src/main/java/com/cloud/server/ManagementServerImpl.java

@@ -843,7 +843,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;
@@ -1196,7 +1195,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) {

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;
@@ -382,7 +381,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();

utils/src/main/java/org/apache/cloudstack/utils/identity/ManagementServerNode.java

@@ -19,17 +19,90 @@
 
 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;
 import com.cloud.utils.component.SystemIntegrityChecker;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.net.MacAddress;
 
+/**
+ * Canonical source of the management-server node id ({@code msid}).
+ *
+ * <p>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.
+ *
+ * <p>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.
+     *
+     * <p>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;
     }
 }

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