diff --git a/INSTALL.md b/INSTALL.md index 0619b97f21c1..76e2a389eee8 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -188,6 +188,26 @@ Then: To create packages for noredist add the `-p noredist` option to the package script. All the rpm packages will be created in `dist/rpmbuild/RPMS/x86_64` directory. +## Management server identity + +The server saves its ID in `${user.home}/.cloudstack/management-server.id` +and reuses it after restarts. Keep this file when upgrading or restoring the +same server. Do not copy it to a different management server. + +The service account needs write access to the directory and its `.lock` file. +Startup fails if the ID cannot be read, validated, or saved. + +These options can be added to `JAVA_OPTS` in `/etc/default/cloudstack-management`: + +* `-Dmanagement.server.id.file=/path/to/management-server.id` chooses a different + persistent local file location. +* `-Dmanagement.server.id=123456789` overrides the ID with a positive number + unique to this server. It does not change the saved file. + +If interface order already changed before upgrading, set `management.server.id` +to this server's original `msid` from `cloud.mshost` before the first start. +Keep that override configured on later starts. + ## Notes If you will be using Xen as your hypervisor, please download [vhd-util](https://download.cloudstack.org/tools/vhd-util) diff --git a/PendingReleaseNotes b/PendingReleaseNotes index 02d63811e36e..3a0ee3a836c4 100644 --- a/PendingReleaseNotes +++ b/PendingReleaseNotes @@ -40,6 +40,13 @@ example.ver.1 > example.ver.2: Storage to grow to enormous sizes as Linux Distributions keep growing in size while a stripped down Linux should fit on a 2.88MB floppy. +4.22.1.0 > 4.22.2.0: + * Management servers now reuse a saved ID after restarts, so network interface + reordering does not change their identity. Keep the ID file during upgrades + and restores. See INSTALL.md for the file location and ID settings. + * Old non-Up management server records no longer cause agents to avoid an IP + that also has an Up record. + 4.22.0.0 > 4.22.0.1: * Disk-only instance snapshots for KVM UEFI VMs now include a sidecar copy of the active NVRAM state so revert operations restore both disk and firmware diff --git a/framework/cluster/src/main/java/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java b/framework/cluster/src/main/java/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java index ec943a9c26be..99610747ab97 100644 --- a/framework/cluster/src/main/java/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java +++ b/framework/cluster/src/main/java/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java @@ -21,7 +21,10 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.Date; +import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.TimeZone; @@ -35,7 +38,6 @@ import com.cloud.utils.db.DB; import com.cloud.utils.db.Filter; import com.cloud.utils.db.GenericDaoBase; -import com.cloud.utils.db.GenericSearchBuilder; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.db.TransactionLegacy; @@ -47,7 +49,6 @@ public class ManagementServerHostDaoImpl extends GenericDaoBase ActiveSearch; private final SearchBuilder InactiveSearch; private final SearchBuilder StateSearch; - protected GenericSearchBuilder NonUpStateMsSearch; @Override public void invalidateRunSession(long id, long runid) { @@ -229,11 +230,6 @@ protected ManagementServerHostDaoImpl() { StateSearch.and("state", StateSearch.entity().getState(), SearchCriteria.Op.IN); StateSearch.and("runid", StateSearch.entity().getRunid(), SearchCriteria.Op.GT); StateSearch.done(); - - NonUpStateMsSearch = createSearchBuilder(String.class); - NonUpStateMsSearch.selectFields(NonUpStateMsSearch.entity().getServiceIP()); - NonUpStateMsSearch.and("state", NonUpStateMsSearch.entity().getState(), SearchCriteria.Op.NLIKE); - NonUpStateMsSearch.done(); } @Override @@ -267,11 +263,27 @@ public List listBy(ManagementServerHost.State... states) return listBy(sc); } + // The saved ID stops future ID changes but leaves old records behind + // This method handles duplicate records that already exist + // Old Down records must not make agents skip an address that also has an Up record @Override public List listNonUpStateMsIPs() { - SearchCriteria sc = NonUpStateMsSearch.create(); - sc.addAnd("state", SearchCriteria.Op.NLIKE, State.Up); - return customSearch(sc, null); + Set upIPs = new HashSet<>(); + Set nonUpIPs = new LinkedHashSet<>(); + // Read server records that have not been removed + for (ManagementServerHostVO host : listAll()) { + if (State.Up.equals(host.getState())) { + // Collect addresses marked Up + upIPs.add(host.getServiceIP()); + } else { + // Collect addresses in other states + nonUpIPs.add(host.getServiceIP()); + } + } + // Keep every Up address out of the avoid list + nonUpIPs.removeAll(upIPs); + // Give agents the remaining addresses to skip + return new ArrayList<>(nonUpIPs); } @Override diff --git a/framework/cluster/src/test/java/com/cloud/cluster/dao/ManagementServerHostDaoImplTest.java b/framework/cluster/src/test/java/com/cloud/cluster/dao/ManagementServerHostDaoImplTest.java new file mode 100644 index 000000000000..7a2ea02877e1 --- /dev/null +++ b/framework/cluster/src/test/java/com/cloud/cluster/dao/ManagementServerHostDaoImplTest.java @@ -0,0 +1,78 @@ +// 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 com.cloud.cluster.dao; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.apache.cloudstack.management.ManagementServerHost.State; +import org.junit.Test; + +import com.cloud.cluster.ManagementServerHostVO; + +// Checks that old server records do not make agents skip an available address +public class ManagementServerHostDaoImplTest { + private final ManagementServerHostDaoImpl dao = spy(new ManagementServerHostDaoImpl()); + + // Do not avoid an address if one record says Up and an old record says Down + @Test + public void staleDownIdentityDoesNotBlockAnUpServerAtTheSameIp() { + ManagementServerHostVO down = host("10.0.0.1", State.Down); + ManagementServerHostVO up = host("10.0.0.1", State.Up); + assertAvoided(Arrays.asList(down, up)); + assertAvoided(Arrays.asList(up, down)); + } + + // Keep unavailable addresses in the avoid list and include each address only once + @Test + public void unavailableAddressesRemainAvoidedWithoutDuplicates() { + assertAvoided(Arrays.asList(host("10.0.0.1", State.Down), host("10.0.0.1", State.Down), + host("10.0.0.2", State.Maintenance), host("10.0.0.3", State.Up)), "10.0.0.1", "10.0.0.2"); + } + + // Allow an address marked Up even if an old record says it is in maintenance + @Test + public void upServerTakesPrecedenceOverAnOldMaintenanceRecord() { + assertAvoided(Arrays.asList(host("10.0.0.1", State.Maintenance), host("10.0.0.1", State.Up))); + } + + // No server records means there are no addresses to avoid + @Test + public void emptyServerListHasNoAvoidedAddresses() { + assertAvoided(Collections.emptyList()); + } + + // Run the avoid-list code with pretend records and check the returned addresses + private void assertAvoided(List hosts, String... expected) { + doReturn(hosts).when(dao).listAll(); + assertEquals(Arrays.asList(expected), dao.listNonUpStateMsIPs()); + } + + // Make a pretend server record with an address and a state + private ManagementServerHostVO host(String ip, State state) { + ManagementServerHostVO host = new ManagementServerHostVO(); + host.setServiceIP(ip); + host.setState(state); + return host; + } +} diff --git a/server/src/main/java/com/cloud/network/NetworkUsageManagerImpl.java b/server/src/main/java/com/cloud/network/NetworkUsageManagerImpl.java index 0aee5f234969..fa669acac024 100644 --- a/server/src/main/java/com/cloud/network/NetworkUsageManagerImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkUsageManagerImpl.java @@ -28,6 +28,8 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import org.apache.cloudstack.utils.identity.ManagementServerNode; + import org.springframework.stereotype.Component; import org.apache.cloudstack.api.command.admin.usage.AddTrafficMonitorCmd; @@ -88,7 +90,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 +252,8 @@ protected class DirectNetworkStatsListener implements Listener { private int _interval; - private final long mgmtSrvrId = MacAddress.getMacAddress().toLong(); + // Use the saved ID to check which traffic monitors this server is in charge of + 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 e2415e200768..b057d53f01b0 100644 --- a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/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,8 @@ 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(); + // Use the saved ID to find the routers this server is in charge of + 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 95e688b5d8cb..a417ff84a3a3 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -847,7 +847,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; @@ -1198,9 +1197,10 @@ public DetailVO findDetail(final long hostId, final String name) { return _detailsDao.findDetail(hostId, name); } + // Return the persistent server ID shared by the other CloudStack components @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 3eec27d4f54d..ee606fb838f8 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -150,7 +150,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; @@ -391,7 +390,8 @@ public String toString() { private ScheduledExecutorService _diskStatsUpdateExecutor; private int _usageAggregationRange = 1440; private String _usageTimeZone = "GMT"; - private final long mgmtSrvrId = MacAddress.getMacAddress().toLong(); + // Use the saved ID to check which stats jobs this server should do + 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/server/src/test/java/com/cloud/server/ManagementServerIdentityTest.java b/server/src/test/java/com/cloud/server/ManagementServerIdentityTest.java new file mode 100644 index 000000000000..00c3d9373ebb --- /dev/null +++ b/server/src/test/java/com/cloud/server/ManagementServerIdentityTest.java @@ -0,0 +1,89 @@ +// 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 com.cloud.server; + +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +import org.apache.cloudstack.utils.identity.ManagementServerNode; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.cluster.ManagementServerHostVO; +import com.cloud.cluster.dao.ManagementServerHostDao; +import com.cloud.utils.db.Filter; +import com.cloud.utils.db.GlobalLock; +import com.cloud.utils.db.Transaction; +import com.cloud.utils.db.TransactionCallbackNoReturn; +import com.cloud.utils.net.MacAddress; + +// Check that the server and its stats jobs use the saved ID +public class ManagementServerIdentityTest { + // Use different values for the saved ID and the ID from the network card + private static final long SAVED_ID = 123L; + private static final long CHANGED_MAC_ID = 456L; + + // Return the saved ID even when the network card gives a different ID + @Test + public void managementServiceReportsSavedIdAfterMacChanges() { + try (MockedStatic node = mockStatic(ManagementServerNode.class); + MockedStatic mac = mockStatic(MacAddress.class)) { + node.when(ManagementServerNode::getManagementServerId).thenReturn(SAVED_ID); + mac.when(MacAddress::getMacAddress).thenReturn(new MacAddress(CHANGED_MAC_ID)); + + ManagementServerImpl server = mock(ManagementServerImpl.class, Mockito.CALLS_REAL_METHODS); + assertEquals(SAVED_ID, server.getId()); + } + } + + // Still start the disk stats job when the saved ID says this server is in charge + @Test + public void owningServerStillUpdatesDiskStatsAfterMacChanges() { + StatsCollector previous = StatsCollector.getInstance(); + try (MockedStatic node = mockStatic(ManagementServerNode.class); + MockedStatic mac = mockStatic(MacAddress.class); + MockedStatic locks = mockStatic(GlobalLock.class); + MockedStatic transactions = mockStatic(Transaction.class)) { + node.when(ManagementServerNode::getManagementServerId).thenReturn(SAVED_ID); + mac.when(MacAddress::getMacAddress).thenReturn(new MacAddress(CHANGED_MAC_ID)); + GlobalLock lock = mock(GlobalLock.class); + locks.when(() -> GlobalLock.getInternLock("vm.disk.stats")).thenReturn(lock); + when(lock.lock(5)).thenReturn(true); + + // Pretend the database says this server should do the job + ManagementServerHostVO host = new ManagementServerHostVO(); + host.setMsid(SAVED_ID); + ManagementServerHostDao dao = mock(ManagementServerHostDao.class); + when(dao.findOneInUpState(any(Filter.class))).thenReturn(host); + StatsCollector collector = new StatsCollector(); + ReflectionTestUtils.setField(collector, "managementServerHostDao", dao); + + collector.new VmDiskStatsUpdaterTask().runInContext(); + + transactions.verify(() -> Transaction.execute(any(TransactionCallbackNoReturn.class))); + } finally { + // Put the old stats collector back so other tests are not affected + ReflectionTestUtils.setField(StatsCollector.class, "s_instance", previous); + } + } +} diff --git a/utils/src/main/java/org/apache/cloudstack/utils/identity/ManagementServerId.java b/utils/src/main/java/org/apache/cloudstack/utils/identity/ManagementServerId.java new file mode 100644 index 000000000000..edeb78e2f431 --- /dev/null +++ b/utils/src/main/java/org/apache/cloudstack/utils/identity/ManagementServerId.java @@ -0,0 +1,108 @@ +// 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 java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.util.function.LongSupplier; + +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.net.MacAddress; + +// Keeps the management server ID the same across restarts +final class ManagementServerId { + + private ManagementServerId() { + } + + // Find the ID file and load the ID + static long get() { + Path path = Paths.get(System.getProperty("management.server.id.file", + Paths.get(System.getProperty("user.home"), ".cloudstack", "management-server.id").toString())); + try { + // Use the ID from management.server.id if it is set + // Otherwise, read the saved ID from the file at path + // If the file does not exist, create an ID from the MAC address and save it + return load(path, System.getProperty("management.server.id"), () -> MacAddress.getMacAddress().toLong()); + } catch (IOException | IllegalArgumentException e) { + // Stop with a clear error instead of silently choosing a different identity + throw new CloudRuntimeException("Unable to load the management server ID from " + path, e); + } + } + + static long load(Path path, String configuredId, LongSupplier initialId) throws IOException { + // An operator-provided ID takes priority without overwriting the saved file + if (configuredId != null) { + return parse(configuredId); + } + + // Ensure the directory exists, then lock the neighboring .lock file so + // two processes cannot initialize the same identity file at the same time + Files.createDirectories(path.toAbsolutePath().getParent()); + try (FileChannel lockChannel = FileChannel.open(path.resolveSibling(path.getFileName() + ".lock"), + StandardOpenOption.CREATE, StandardOpenOption.WRITE); + FileLock lock = lockChannel.lock()) { + // Reuse the saved ID without checking network interfaces again + if (Files.exists(path)) { + // An ID is a short number; reject an unexpectedly large file + if (Files.size(path) > 64) { + throw new IllegalArgumentException("Invalid management server ID file"); + } + return parse(new String(Files.readAllBytes(path), StandardCharsets.UTF_8)); + } + + // First startup: obtain the initial MAC-derived ID and validate it + long id = parse(Long.toString(initialId.getAsLong())); + // Write in the same directory so the completed file can be renamed atomically + Path temporary = Files.createTempFile(path.toAbsolutePath().getParent(), "management-server-id-", ".tmp"); + try { + try (FileChannel channel = FileChannel.open(temporary, StandardOpenOption.WRITE)) { + ByteBuffer buffer = StandardCharsets.UTF_8.encode(id + "\n"); + while (buffer.hasRemaining()) { + channel.write(buffer); + } + // Flush the complete ID to storage before publishing it + channel.force(true); + } + // Make the completed file appear under its final name in one operation + Files.move(temporary, path, StandardCopyOption.ATOMIC_MOVE); + } finally { + // Clean up after a failed save; a successful move already removed this name + Files.deleteIfExists(temporary); + } + return id; + } + } + + // Ignore surrounding whitespace and require a positive whole number that fits in a Java long + private static long parse(String value) { + long id = Long.parseLong(value.trim()); + if (id <= 0) { + throw new IllegalArgumentException("Management server ID must be a positive integer"); + } + return id; + } +} 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..e9df0f991573 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 @@ -25,16 +25,19 @@ import com.cloud.utils.component.ComponentLifecycle; import com.cloud.utils.component.SystemIntegrityChecker; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.net.MacAddress; +// Holds the management server ID and shares it with other CloudStack components public class ManagementServerNode extends AdapterBase implements SystemIntegrityChecker { - private static final long s_nodeId = MacAddress.getMacAddress().toLong(); + // Load the ID once and keep the same value for the life of this process + private static final long s_nodeId = ManagementServerId.get(); + // Register this component to run early during CloudStack startup public ManagementServerNode() { setRunLevel(ComponentLifecycle.RUN_LEVEL_FRAMEWORK_BOOTSTRAP); } + // A valid server ID must be greater than zero @Override public void check() { if (s_nodeId <= 0) { @@ -42,10 +45,12 @@ public void check() { } } + // Other components call this to get the same server ID public static long getManagementServerId() { return s_nodeId; } + // Check the ID during startup. Log an error and stop if the check fails @Override public boolean start() { try { diff --git a/utils/src/test/java/org/apache/cloudstack/utils/identity/ManagementServerIdTest.java b/utils/src/test/java/org/apache/cloudstack/utils/identity/ManagementServerIdTest.java new file mode 100644 index 000000000000..8962150ec4ff --- /dev/null +++ b/utils/src/test/java/org/apache/cloudstack/utils/identity/ManagementServerIdTest.java @@ -0,0 +1,232 @@ +// 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.assertThrows; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.LongSupplier; +import java.util.stream.Stream; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import com.cloud.utils.exception.CloudRuntimeException; + +public class ManagementServerIdTest { + + // Give each test its own temporary folder + @Rule + public TemporaryFolder folder = new TemporaryFolder(); + + // Choose where this test will save its ID + private Path idFile() { + return folder.getRoot().toPath().resolve("identity/management-server.id"); + } + + // Fail the test if the code asks for a new ID when it should not + private LongSupplier unexpectedLookup() { + return () -> { + throw new AssertionError("Network interfaces must not be consulted"); + }; + } + + // Keep the saved ID even when the network card would give a different ID + @Test + public void preservesInitialIdWhenNetworkInterfacesChange() throws IOException { + Path path = idFile(); + assertEquals(130014950130305L, ManagementServerId.load(path, null, () -> 130014950130305L)); + assertEquals(130014950130305L, ManagementServerId.load(path, null, () -> 42L)); + assertEquals(130014950130305L, ManagementServerId.load(path, null, unexpectedLookup())); + } + + // Use the chosen ID first and keep the saved ID for later + @Test + public void configuredIdTakesPrecedenceOverSavedId() throws IOException { + Path path = idFile(); + ManagementServerId.load(path, null, () -> 42L); + assertEquals(123L, ManagementServerId.load(path, "123", unexpectedLookup())); + assertEquals(42L, ManagementServerId.load(path, null, unexpectedLookup())); + } + + // A chosen ID still works when we cannot save a file + @Test + public void configuredIdDoesNotRequireWritableStorage() throws IOException { + Path path = folder.newFile("not-a-directory").toPath().resolve("id"); + assertEquals(123L, ManagementServerId.load(path, "123", unexpectedLookup())); + } + + // Reject bad IDs from the settings and do not save them + @Test + public void rejectsInvalidConfiguredIds() { + for (String value : new String[] {"", "0", "-1", "invalid", "9223372036854775808"}) { + assertThrows(IllegalArgumentException.class, + () -> ManagementServerId.load(idFile(), value, unexpectedLookup())); + } + assertFalse(Files.exists(idFile())); + } + + // Reject a bad saved ID without changing its file + @Test + public void rejectsInvalidSavedIdsWithoutReplacingThem() throws IOException { + Path path = folder.newFile("id").toPath(); + for (String value : new String[] {"", "0", "-1", "invalid", "9223372036854775808", "1".repeat(65)}) { + Files.write(path, value.getBytes(StandardCharsets.UTF_8)); + assertThrows(IllegalArgumentException.class, + () -> ManagementServerId.load(path, null, unexpectedLookup())); + assertEquals(value, Files.readString(path)); + } + } + + // Do not save a new ID if it is zero or negative + @Test + public void rejectsInvalidInitialIdsWithoutSavingThem() { + for (long value : new long[] {0L, -1L}) { + assertThrows(IllegalArgumentException.class, + () -> ManagementServerId.load(idFile(), null, () -> value)); + } + assertFalse(Files.exists(idFile())); + } + + // Report an error when the ID file cannot be saved + @Test + public void failsWhenIdentityCannotBeSaved() throws IOException { + Path path = folder.newFile("not-a-directory").toPath().resolve("id"); + assertThrows(IOException.class, () -> ManagementServerId.load(path, null, unexpectedLookup())); + } + + // A failed write leaves no broken ID file and a later try can succeed + @Test + public void failedInitialWriteDoesNotPublishAnInvalidIdentity() throws IOException { + Path path = idFile(); + AtomicReference underlyingWriter = new AtomicReference<>(); + // Make the file writer fail on purpose + FileChannel failedWriter = Mockito.mock(FileChannel.class); + Mockito.when(failedWriter.write(Mockito.any(ByteBuffer.class))).thenThrow(new IOException("Interrupted write")); + Mockito.doAnswer(invocation -> { + underlyingWriter.get().close(); + return null; + }).when(failedWriter).close(); + + try (MockedStatic channels = Mockito.mockStatic(FileChannel.class, invocation -> { + FileChannel channel = (FileChannel) invocation.callRealMethod(); + Path openedPath = invocation.getArgument(0); + if (openedPath.getFileName().toString().endsWith(".lock")) { + return channel; + } + underlyingWriter.compareAndSet(null, channel); + return failedWriter; + })) { + assertThrows(IOException.class, () -> ManagementServerId.load(path, null, () -> 42L)); + } + + assertFalse(Files.exists(path)); + try (Stream files = Files.list(path.getParent())) { + assertEquals(1L, files.count()); // Only the interprocess lock remains + } + assertEquals(43L, ManagementServerId.load(path, null, () -> 43L)); + } + + // Allow spaces around a valid ID and accept the biggest allowed number + @Test + public void readsWhitespaceAndFullLongIds() throws IOException { + Path path = folder.newFile("id").toPath(); + Files.writeString(path, " " + Long.MAX_VALUE + "\n", StandardCharsets.UTF_8); + assertEquals(Long.MAX_VALUE, ManagementServerId.load(path, null, unexpectedLookup())); + } + + // Use the chosen file and ID settings and report a bad saved file + @Test + public void usesJvmSettingsAndReportsInvalidStorage() throws IOException { + String previousFile = System.getProperty("management.server.id.file"); + String previousId = System.getProperty("management.server.id"); + Path path = folder.newFile("id").toPath(); + try { + System.setProperty("management.server.id.file", path.toString()); + System.clearProperty("management.server.id"); + Files.writeString(path, "42\n", StandardCharsets.UTF_8); + assertEquals(42L, ManagementServerId.get()); + + System.setProperty("management.server.id", "123"); + assertEquals(123L, ManagementServerId.get()); + + System.clearProperty("management.server.id"); + Files.writeString(path, "invalid", StandardCharsets.UTF_8); + assertThrows(CloudRuntimeException.class, ManagementServerId::get); + } finally { + restoreProperty("management.server.id.file", previousFile); + restoreProperty("management.server.id", previousId); + } + } + + // Three programs using the same file must all get the same ID + @Test + public void concurrentProcessesUseTheSameId() throws Exception { + Path path = idFile(); + List processes = new ArrayList<>(); + try { + for (int i = 1; i <= 3; i++) { + processes.add(new ProcessBuilder(Paths.get(System.getProperty("java.home"), "bin", "java").toString(), + "-cp", System.getProperty("java.class.path"), IdWriter.class.getName(), path.toString(), Integer.toString(i)) + .redirectErrorStream(true).start()); + } + for (Process process : processes) { + assertTrue("ID writer timed out", process.waitFor(30, TimeUnit.SECONDS)); + String output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8).trim(); + assertEquals(output, 0, process.exitValue()); + assertEquals(Files.readString(path).trim(), output); + } + } finally { + for (Process process : processes) { + process.destroyForcibly(); + } + } + } + + // A small helper program that loads an ID and prints it + public static class IdWriter { + public static void main(String[] args) throws IOException { + System.out.println(ManagementServerId.load(Paths.get(args[0]), null, () -> Long.parseLong(args[1]))); + } + } + + // Put the settings back so one test does not affect another + private void restoreProperty(String name, String value) { + if (value == null) { + System.clearProperty(name); + } else { + System.setProperty(name, value); + } + } +}