Skip to content

Commit f9e24fa

Browse files
committed
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.
1 parent 0da3740 commit f9e24fa

7 files changed

Lines changed: 226 additions & 16 deletions

File tree

server/src/main/java/com/cloud/hypervisor/CloudZonesStartupProcessor.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
import org.springframework.stereotype.Component;
2626

27+
import org.apache.cloudstack.utils.identity.ManagementServerNode;
28+
2729
import com.cloud.agent.AgentManager;
2830
import com.cloud.agent.StartupCommandProcessor;
2931
import com.cloud.agent.api.StartupCommand;
@@ -38,7 +40,6 @@
3840
import com.cloud.host.HostVO;
3941
import com.cloud.hypervisor.Hypervisor.HypervisorType;
4042
import com.cloud.utils.component.AdapterBase;
41-
import com.cloud.utils.net.MacAddress;
4243
import com.cloud.utils.net.NetUtils;
4344

4445
/**
@@ -62,7 +63,7 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
6263
if (_nodeId == -1) {
6364
// FIXME: We really should not do this like this. It should be done
6465
// at config time and is stored as a config variable.
65-
_nodeId = MacAddress.getMacAddress().toLong();
66+
_nodeId = ManagementServerNode.getManagementServerId();
6667
}
6768
return true;
6869
}

server/src/main/java/com/cloud/network/NetworkUsageManagerImpl.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.cloudstack.api.command.admin.usage.DeleteTrafficMonitorCmd;
3535
import org.apache.cloudstack.api.command.admin.usage.ListTrafficMonitorsCmd;
3636
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
37+
import org.apache.cloudstack.utils.identity.ManagementServerNode;
3738

3839
import com.cloud.agent.AgentManager;
3940
import com.cloud.agent.Listener;
@@ -88,7 +89,6 @@
8889
import com.cloud.utils.db.TransactionCallbackNoReturn;
8990
import com.cloud.utils.db.TransactionStatus;
9091
import com.cloud.utils.exception.CloudRuntimeException;
91-
import com.cloud.utils.net.MacAddress;
9292

9393
@Component
9494
public class NetworkUsageManagerImpl extends ManagerBase implements NetworkUsageService, NetworkUsageManager, ResourceStateAdapter {
@@ -251,7 +251,7 @@ protected class DirectNetworkStatsListener implements Listener {
251251

252252
private int _interval;
253253

254-
private final long mgmtSrvrId = MacAddress.getMacAddress().toLong();
254+
private final long mgmtSrvrId = ManagementServerNode.getManagementServerId();
255255

256256
protected DirectNetworkStatsListener(int interval) {
257257
_interval = interval;

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,6 @@
252252
import com.cloud.utils.fsm.StateListener;
253253
import com.cloud.utils.fsm.StateMachine2;
254254
import com.cloud.utils.net.Ip;
255-
import com.cloud.utils.net.MacAddress;
256255
import com.cloud.utils.net.NetUtils;
257256
import com.cloud.vm.DomainRouterVO;
258257
import com.cloud.vm.Nic;
@@ -367,7 +366,7 @@ public class VirtualNetworkApplianceManagerImpl extends ManagerBase implements V
367366
private int _routerExtraPublicNics = 2;
368367
private int _usageAggregationRange = 1440;
369368
private String _usageTimeZone = "GMT";
370-
private final long mgmtSrvrId = MacAddress.getMacAddress().toLong();
369+
private final long mgmtSrvrId = ManagementServerNode.getManagementServerId();
371370
private static final int ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_COOPERATION = 5; // 5 seconds
372371
private boolean _dailyOrHourly = false;
373372

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,6 @@
858858
import com.cloud.utils.db.TransactionStatus;
859859
import com.cloud.utils.exception.CloudRuntimeException;
860860
import com.cloud.utils.fsm.StateMachine2;
861-
import com.cloud.utils.net.MacAddress;
862861
import com.cloud.utils.net.NetUtils;
863862
import com.cloud.utils.security.CertificateHelper;
864863
import com.cloud.utils.ssh.SSHKeysHelper;
@@ -1207,7 +1206,7 @@ public DetailVO findDetail(final long hostId, final String name) {
12071206

12081207
@Override
12091208
public long getId() {
1210-
return MacAddress.getMacAddress().toLong();
1209+
return ManagementServerNode.getManagementServerId();
12111210
}
12121211

12131212
protected void checkPortParameters(final String publicPort, final String privatePort, final String privateIp, final String proto) {

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@
147147
import com.cloud.utils.db.TransactionCallbackNoReturn;
148148
import com.cloud.utils.db.TransactionStatus;
149149
import com.cloud.utils.exception.CloudRuntimeException;
150-
import com.cloud.utils.net.MacAddress;
151150
import com.cloud.utils.script.Script;
152151
import com.cloud.vm.NicVO;
153152
import com.cloud.vm.UserVmManager;
@@ -383,7 +382,7 @@ public String toString() {
383382
private ScheduledExecutorService _diskStatsUpdateExecutor;
384383
private int _usageAggregationRange = 1440;
385384
private String _usageTimeZone = "GMT";
386-
private final long mgmtSrvrId = MacAddress.getMacAddress().toLong();
385+
private final long mgmtSrvrId = ManagementServerNode.getManagementServerId();
387386
private static final int ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_COOPERATION = 5; // 5 seconds
388387
private boolean _dailyOrHourly = false;
389388
protected long managementServerNodeId = ManagementServerNode.getManagementServerId();

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

Lines changed: 81 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,90 @@
1919

2020
package org.apache.cloudstack.utils.identity;
2121

22-
22+
import java.net.InetAddress;
23+
import java.nio.charset.StandardCharsets;
24+
import java.security.MessageDigest;
2325

2426
import com.cloud.utils.component.AdapterBase;
2527
import com.cloud.utils.component.ComponentLifecycle;
2628
import com.cloud.utils.component.SystemIntegrityChecker;
2729
import com.cloud.utils.exception.CloudRuntimeException;
2830
import com.cloud.utils.net.MacAddress;
2931

32+
/**
33+
* Canonical source of the management-server node id ({@code msid}).
34+
*
35+
* <p>By default the id is derived from the host hardware MAC address. When the MAC address is
36+
* not stable across restarts, the {@code msid} changes, which orphans the {@code mshost} row
37+
* and breaks async jobs, HA work ({@code fk_op_ha_work__mgmt_server_id}), and router/stats
38+
* ownership.
39+
*
40+
* <p>Setting the environment variable {@code CLOUDSTACK_MSID_FROM_FQDN=true} (or the system
41+
* property {@code cloudstack.msid.from.fqdn=true}) instead derives the id from a SHA-256 hash
42+
* of the node FQDN, which stays stable across restarts. All node-identity consumers must
43+
* obtain the id from {@link #getManagementServerId()} so they agree on the same value.
44+
*/
3045
public class ManagementServerNode extends AdapterBase implements SystemIntegrityChecker {
3146

32-
private static final long s_nodeId = MacAddress.getMacAddress().toLong();
47+
private static final String FQDN_ENV_VAR = "CLOUDSTACK_MSID_FROM_FQDN";
48+
private static final String FQDN_SYS_PROP = "cloudstack.msid.from.fqdn";
49+
50+
private static String s_nodeIdSource;
51+
private static Exception s_initError;
52+
private static final long s_nodeId = initNodeId();
53+
54+
private static long initNodeId() {
55+
if (isFqdnModeEnabled()) {
56+
return generateIdFromFqdn();
57+
}
58+
s_nodeIdSource = "mac-address";
59+
return MacAddress.getMacAddress().toLong();
60+
}
61+
62+
private static boolean isFqdnModeEnabled() {
63+
return isTruthy(System.getenv(FQDN_ENV_VAR)) || isTruthy(System.getProperty(FQDN_SYS_PROP));
64+
}
65+
66+
private static boolean isTruthy(String value) {
67+
if (value == null) {
68+
return false;
69+
}
70+
String trimmed = value.trim();
71+
return "true".equalsIgnoreCase(trimmed) || "1".equals(trimmed) || "yes".equalsIgnoreCase(trimmed);
72+
}
73+
74+
/**
75+
* Derives a stable node id from a SHA-256 hash of the local FQDN.
76+
*
77+
* <p>On failure it records the cause and returns {@code 0} (an invalid id) rather than
78+
* silently reverting to an unstable MAC-based id. The invalid id makes {@link #check()}
79+
* fail the system-integrity check, which stops startup cleanly via {@link #start()}
80+
* instead of raising an {@code ExceptionInInitializerError} from static initialization.
81+
*
82+
* @return a positive, non-zero 48-bit id, or {@code 0} if it cannot be derived
83+
*/
84+
private static long generateIdFromFqdn() {
85+
try {
86+
String fqdn = InetAddress.getLocalHost().getCanonicalHostName();
87+
s_nodeIdSource = "fqdn:" + fqdn;
88+
MessageDigest digest = MessageDigest.getInstance("SHA-256");
89+
byte[] hash = digest.digest(fqdn.getBytes(StandardCharsets.UTF_8));
90+
long id = 0;
91+
for (int i = 0; i < 6; i++) {
92+
id = (id << 8) | (hash[i] & 0xFF);
93+
}
94+
// Ensure positive and non-zero
95+
id = id & 0x7FFFFFFFFFFFFFFFL;
96+
if (id == 0) {
97+
id = 1;
98+
}
99+
return id;
100+
} catch (Exception e) {
101+
s_nodeIdSource = "fqdn-error";
102+
s_initError = e;
103+
return 0;
104+
}
105+
}
33106

34107
public ManagementServerNode() {
35108
setRunLevel(ComponentLifecycle.RUN_LEVEL_FRAMEWORK_BOOTSTRAP);
@@ -38,7 +111,8 @@ public ManagementServerNode() {
38111
@Override
39112
public void check() {
40113
if (s_nodeId <= 0) {
41-
throw new CloudRuntimeException("Unable to get the management server node id");
114+
throw new CloudRuntimeException(
115+
"Unable to derive the management server node id (source: " + s_nodeIdSource + ")", s_initError);
42116
}
43117
}
44118

@@ -50,10 +124,11 @@ public static long getManagementServerId() {
50124
public boolean start() {
51125
try {
52126
check();
53-
} catch (Exception e) {
54-
logger.error("System integrity check exception", e);
55-
System.exit(1);
127+
} catch (CloudRuntimeException e) {
128+
logger.error("System integrity check failed for the management server node id", e);
129+
throw e;
56130
}
131+
logger.info("Management server node id: {} (source: {})", s_nodeId, s_nodeIdSource);
57132
return true;
58133
}
59134
}
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.utils.identity;
18+
19+
import static org.junit.Assert.assertEquals;
20+
import static org.junit.Assert.assertFalse;
21+
import static org.junit.Assert.assertTrue;
22+
23+
import java.lang.reflect.Method;
24+
25+
import org.junit.After;
26+
import org.junit.Test;
27+
28+
public class ManagementServerNodeTest {
29+
30+
private static final String FQDN_SYS_PROP = "cloudstack.msid.from.fqdn";
31+
32+
@After
33+
public void tearDown() {
34+
System.clearProperty(FQDN_SYS_PROP);
35+
}
36+
37+
private static boolean invokeIsTruthy(String value) throws Exception {
38+
Method m = ManagementServerNode.class.getDeclaredMethod("isTruthy", String.class);
39+
m.setAccessible(true);
40+
return (boolean) m.invoke(null, value);
41+
}
42+
43+
private static boolean invokeIsFqdnModeEnabled() throws Exception {
44+
Method m = ManagementServerNode.class.getDeclaredMethod("isFqdnModeEnabled");
45+
m.setAccessible(true);
46+
return (boolean) m.invoke(null);
47+
}
48+
49+
private static long invokeGenerateIdFromFqdn() throws Exception {
50+
Method m = ManagementServerNode.class.getDeclaredMethod("generateIdFromFqdn");
51+
m.setAccessible(true);
52+
return (long) m.invoke(null);
53+
}
54+
55+
@Test
56+
public void testGetManagementServerIdIsPositive() {
57+
assertTrue("Node id must be a positive, non-zero value", ManagementServerNode.getManagementServerId() > 0);
58+
}
59+
60+
@Test
61+
public void testCheckPassesWithValidNodeId() {
62+
// Node id is derived at class-load time and should be valid, so check() must not throw.
63+
new ManagementServerNode().check();
64+
}
65+
66+
@Test
67+
public void testStartReturnsTrueWithValidNodeId() {
68+
// With a valid node id, start() must succeed and return true without terminating the JVM.
69+
assertTrue(new ManagementServerNode().start());
70+
}
71+
72+
@Test
73+
public void testIsTruthyRecognizesTrueValues() throws Exception {
74+
assertTrue(invokeIsTruthy("true"));
75+
assertTrue(invokeIsTruthy("TRUE"));
76+
assertTrue(invokeIsTruthy("True"));
77+
assertTrue(invokeIsTruthy("1"));
78+
assertTrue(invokeIsTruthy("yes"));
79+
assertTrue(invokeIsTruthy("YES"));
80+
}
81+
82+
@Test
83+
public void testIsTruthyTrimsWhitespace() throws Exception {
84+
assertTrue(invokeIsTruthy(" true "));
85+
assertTrue(invokeIsTruthy("\t1\n"));
86+
assertTrue(invokeIsTruthy(" yes "));
87+
}
88+
89+
@Test
90+
public void testIsTruthyRejectsFalseValues() throws Exception {
91+
assertFalse(invokeIsTruthy(null));
92+
assertFalse(invokeIsTruthy(""));
93+
assertFalse(invokeIsTruthy(" "));
94+
assertFalse(invokeIsTruthy("false"));
95+
assertFalse(invokeIsTruthy("0"));
96+
assertFalse(invokeIsTruthy("no"));
97+
assertFalse(invokeIsTruthy("enabled"));
98+
assertFalse(invokeIsTruthy("2"));
99+
}
100+
101+
@Test
102+
public void testIsFqdnModeEnabledDefaultsFalse() throws Exception {
103+
System.clearProperty(FQDN_SYS_PROP);
104+
assertFalse(invokeIsFqdnModeEnabled());
105+
}
106+
107+
@Test
108+
public void testIsFqdnModeEnabledWhenSystemPropertyTruthy() throws Exception {
109+
System.setProperty(FQDN_SYS_PROP, "true");
110+
assertTrue(invokeIsFqdnModeEnabled());
111+
}
112+
113+
@Test
114+
public void testIsFqdnModeEnabledWhenSystemPropertyFalsy() throws Exception {
115+
System.setProperty(FQDN_SYS_PROP, "false");
116+
assertFalse(invokeIsFqdnModeEnabled());
117+
}
118+
119+
@Test
120+
public void testGenerateIdFromFqdnIsPositiveAndNonZero() throws Exception {
121+
long id = invokeGenerateIdFromFqdn();
122+
assertTrue("Generated id must be positive and non-zero", id > 0);
123+
}
124+
125+
@Test
126+
public void testGenerateIdFromFqdnFitsIn48Bits() throws Exception {
127+
long id = invokeGenerateIdFromFqdn();
128+
assertTrue("Generated id must fit within 48 bits", id <= 0xFFFFFFFFFFFFL);
129+
}
130+
131+
@Test
132+
public void testGenerateIdFromFqdnIsDeterministic() throws Exception {
133+
long first = invokeGenerateIdFromFqdn();
134+
long second = invokeGenerateIdFromFqdn();
135+
assertEquals("Generated id must be stable across calls for the same FQDN", first, second);
136+
}
137+
}

0 commit comments

Comments
 (0)