Damans227 commented on code in PR #13662:
URL: https://github.com/apache/cloudstack/pull/13662#discussion_r3974977244
##########
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();
Review Comment:
tested this on a lab. works and the hash matches, but the id follows the
resolver rather than the node, so it isnt stable in the way the pr describes.
`getCanonicalHostName()` doesnt throw when dns is down, it just gives back a
different name. pointed resolv.conf at a dead server and the lookup dropped to
the short name:
```
$ getent hosts 10.0.35.66
10.0.35.66 pr13662-t16966-kvm-ol8-mgmt1
```
restarted and the msid moved on its own:
```
00:51:22 Management server node id: 191603954322557 (source:
fqdn:pr13662-t16966-kvm-ol8-mgmt1.sofia.shapeblue.com)
00:53:35 Management server node id: 163047240293385 (source:
fqdn:pr13662-t16966-kvm-ol8-mgmt1)
```
third mshost row, previous one down:
```
1 32987445986261 ...mgmt1.sofia.shapeblue.com Down
2 191603954322557 ...mgmt1.sofia.shapeblue.com Down
3 163047240293385 pr13662-t16966-kvm-ol8-mgmt1 Up
```
no error, no warning. restoring resolv.conf and restarting brought the old
id back, so it tracks dns not the machine.
thats the same orphaned mshost problem the pr is fixing, just triggered by a
dns blip or a changed search domain instead of a mac change.
the catch below handles the lookup throwing, but the common case is a lookup
that succeeds and returns something else, and theres no guard for that.
could we reject a name with no dot in it and fail the check instead? or let
the operator pin the name in a setting rather than looking it up?
--
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]
