Add unit tests for snapshot overlay fix

Author: sidshas03

agent/src/test/java/com/cloud/agent/AgentTest.java

@@ -22,10 +22,12 @@
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -254,4 +256,43 @@ public void testStopAndCleanupConnectionValidConnectionWaitForStopWaitsForStartu
         verify(mockConnection).cleanUp();
         verify(mockBackoff, times(3)).waitBeforeRetry();
     }
+
+    @Test
+    public void testWaitForCommandsAndReconnectNoCommandsInProgress() throws ConfigurationException {
+        Agent spyAgent = spy(new Agent(shell, 1, serverResource));
+        ReflectionTestUtils.setField(spyAgent, "logger", logger);
+        doNothing().when(spyAgent).reconnect(any(Link.class));
+
+        Link mockLink = mock(Link.class);
+        spyAgent.commandsInProgress.set(0);
+        spyAgent.waitForCommandsAndReconnect(mockLink);
+
+        verify(spyAgent).reconnect(mockLink);
+        verify(logger).info(eq("Reconnecting instead of exiting to avoid losing in-flight work (e.g. snapshot overlays)."));
+    }
+
+    @Test
+    public void testWaitForCommandsAndReconnectCommandsCompleteBeforeTimeout() throws ConfigurationException {
+        Agent spyAgent = spy(new Agent(shell, 1, serverResource));
+        ReflectionTestUtils.setField(spyAgent, "logger", logger);
+        doNothing().when(spyAgent).reconnect(any(Link.class));
+
+        Link mockLink = mock(Link.class);
+        spyAgent.commandsInProgress.set(1);
+
+        Thread decrementThread = new Thread(() -> {
+            try {
+                Thread.sleep(500);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+            }
+            spyAgent.commandsInProgress.set(0);
+        });
+        decrementThread.start();
+
+        spyAgent.waitForCommandsAndReconnect(mockLink);
+
+        verify(spyAgent).reconnect(mockLink);
+        verify(logger).info(eq("All in-progress commands completed. Proceeding with reconnect."));
+    }
 }

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java

@@ -59,6 +59,7 @@
 
 import javax.naming.ConfigurationException;
 import java.io.File;
+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.lang.reflect.Method;
 import java.nio.file.Files;
@@ -68,6 +69,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.locks.ReentrantLock;
 
 @RunWith(MockitoJUnitRunner.class)
 public class KVMStorageProcessorTest {
@@ -765,4 +767,223 @@ public void ensureLibvirtSupportsEncryptedRbdHotplugIgnoresNonRbd() {
         // version is not even consulted for them.
         storageProcessor.ensureLibvirtSupportsEncryptedRbdHotplug(StoragePoolType.NetworkFilesystem);
     }
+
+    private File tempMarkerDir;
+
+    private void setUpMarkerDir() throws Exception {
+        tempMarkerDir = Files.createTempDirectory("snapshot_markers_test").toFile();
+        java.lang.reflect.Field field = KVMStorageProcessor.class.getDeclaredField("SNAPSHOT_MARKER_DIR");
+        field.setAccessible(true);
+
+        java.lang.reflect.Field modifiers = java.lang.reflect.Field.class.getDeclaredField("modifiers");
+        modifiers.setAccessible(true);
+        modifiers.setInt(field, field.getModifiers() & ~java.lang.reflect.Modifier.FINAL);
+        field.set(null, tempMarkerDir.getAbsolutePath());
+    }
+
+    private void cleanUpMarkerDir() {
+        if (tempMarkerDir != null && tempMarkerDir.exists()) {
+            File[] files = tempMarkerDir.listFiles();
+            if (files != null) {
+                for (File f : files) {
+                    f.delete();
+                }
+            }
+            tempMarkerDir.delete();
+        }
+    }
+
+    @Test
+    public void testWriteSnapshotMarkerCreatesFile() throws Exception {
+        File markerDir = Files.createTempDirectory("marker_test").toFile();
+        try {
+            Mockito.when(kvmStoragePoolMock.getUuid()).thenReturn("pool-uuid-1");
+            storageProcessorSpy.writeSnapshotMarker(kvmStoragePoolMock, "/mnt/pool/vol1", "snap1");
+
+            File[] files = new File("/var/cache/cloudstack/snapshot_markers").listFiles(
+                    (dir, name) -> name.contains("pool-uuid-1") && name.contains("snap1"));
+            if (files != null && files.length > 0) {
+                String content = new String(Files.readAllBytes(files[0].toPath()));
+                Assert.assertTrue(content.contains("pool-uuid-1"));
+                Assert.assertTrue(content.contains("snap1"));
+                files[0].delete();
+            }
+        } finally {
+            markerDir.delete();
+        }
+    }
+
+    @Test
+    public void testWriteAndRemoveSnapshotMarker() throws Exception {
+        File markerDir = new File("/tmp/test_snapshot_markers_" + System.currentTimeMillis());
+        markerDir.mkdirs();
+        try {
+            java.lang.reflect.Field field = KVMStorageProcessor.class.getDeclaredField("SNAPSHOT_MARKER_DIR");
+            field.setAccessible(true);
+            String originalDir = (String) field.get(null);
+
+            Mockito.when(kvmStoragePoolMock.getUuid()).thenReturn("test-pool");
+
+            storageProcessorSpy.writeSnapshotMarker(kvmStoragePoolMock, "/mnt/pool/vol", "snap");
+
+            File[] written = markerDir.listFiles();
+
+            storageProcessorSpy.removeSnapshotMarker(kvmStoragePoolMock, "/mnt/pool/vol", "snap");
+        } finally {
+            File[] remaining = markerDir.listFiles();
+            if (remaining != null) {
+                for (File f : remaining) f.delete();
+            }
+            markerDir.delete();
+        }
+    }
+
+    @Test
+    public void testReconcileSnapshotMarkersNoMarkerDir() {
+        storageProcessorSpy.reconcileSnapshotMarkers();
+    }
+
+    @Test
+    public void testReconcileSingleMarkerOverlayGone() throws Exception {
+        File tempDir = Files.createTempDirectory("reconcile_test").toFile();
+        try {
+            File marker = new File(tempDir, ".snapshot_marker_test");
+            String content = "pool-uuid|/mnt/vol|snap|||| " + System.currentTimeMillis();
+            try (FileOutputStream fos = new FileOutputStream(marker)) {
+                fos.write(content.getBytes());
+            }
+
+            Mockito.when(resource.getSnapshotTemporaryPath(Mockito.anyString(), Mockito.anyString()))
+                    .thenReturn("/nonexistent/overlay/path");
+
+            storageProcessorSpy.reconcileSingleMarker(marker);
+
+            Assert.assertFalse("Marker should be removed when overlay is gone", marker.exists());
+        } finally {
+            File[] files = tempDir.listFiles();
+            if (files != null) for (File f : files) f.delete();
+            tempDir.delete();
+        }
+    }
+
+    @Test
+    public void testReconcileSingleMarkerInvalidFormat() throws Exception {
+        File tempDir = Files.createTempDirectory("reconcile_invalid_test").toFile();
+        try {
+            File marker = new File(tempDir, ".snapshot_marker_invalid");
+            try (FileOutputStream fos = new FileOutputStream(marker)) {
+                fos.write("invalid".getBytes());
+            }
+
+            storageProcessorSpy.reconcileSingleMarker(marker);
+
+            Assert.assertTrue("Marker with invalid format should not be deleted", marker.exists());
+        } finally {
+            File[] files = tempDir.listFiles();
+            if (files != null) for (File f : files) f.delete();
+            tempDir.delete();
+        }
+    }
+
+    @Test
+    public void testGetVmBlockJobLockReturnsSameLockForSameVm() throws Exception {
+        java.lang.reflect.Method method = KVMStorageProcessor.class.getDeclaredMethod("getVmBlockJobLock", String.class);
+        method.setAccessible(true);
+
+        ReentrantLock lock1 = (ReentrantLock) method.invoke(storageProcessorSpy, "vm-1");
+        ReentrantLock lock2 = (ReentrantLock) method.invoke(storageProcessorSpy, "vm-1");
+        ReentrantLock lock3 = (ReentrantLock) method.invoke(storageProcessorSpy, "vm-2");
+
+        Assert.assertSame("Same VM should return same lock", lock1, lock2);
+        Assert.assertNotSame("Different VMs should return different locks", lock1, lock3);
+    }
+
+    @Test
+    public void testReconcileOfflineBaseModifiedSinceMarker() throws Exception {
+        File tempDir = Files.createTempDirectory("reconcile_offline_test").toFile();
+        try {
+            File baseFile = new File(tempDir, "base.qcow2");
+            File overlayFile = new File(tempDir, "overlay.qcow2");
+
+            try (FileOutputStream fos = new FileOutputStream(baseFile)) { fos.write("base".getBytes()); }
+            try (FileOutputStream fos = new FileOutputStream(overlayFile)) { fos.write("overlay".getBytes()); }
+
+            long markerTimestamp = System.currentTimeMillis() - 60000;
+
+            baseFile.setLastModified(System.currentTimeMillis());
+
+            File marker = new File(tempDir, ".snapshot_marker_test");
+            String content = String.join("|",
+                    "pool-uuid", baseFile.getAbsolutePath(), "snap", "vm1", "vda",
+                    overlayFile.getAbsolutePath(), String.valueOf(markerTimestamp));
+            try (FileOutputStream fos = new FileOutputStream(marker)) {
+                fos.write(content.getBytes());
+            }
+
+            java.lang.reflect.Method method = KVMStorageProcessor.class.getDeclaredMethod(
+                    "reconcileOffline", String.class, String.class, File.class);
+            method.setAccessible(true);
+            method.invoke(storageProcessorSpy, baseFile.getAbsolutePath(), overlayFile.getAbsolutePath(), marker);
+
+            Assert.assertTrue("Marker should be kept when base was modified after marker", marker.exists());
+        } finally {
+            File[] files = tempDir.listFiles();
+            if (files != null) for (File f : files) f.delete();
+            tempDir.delete();
+        }
+    }
+
+    @Test
+    public void testReconcileOfflineBaseDoesNotExist() throws Exception {
+        File tempDir = Files.createTempDirectory("reconcile_nobase_test").toFile();
+        try {
+            File overlayFile = new File(tempDir, "overlay.qcow2");
+            try (FileOutputStream fos = new FileOutputStream(overlayFile)) { fos.write("overlay".getBytes()); }
+
+            File marker = new File(tempDir, ".snapshot_marker_test");
+            try (FileOutputStream fos = new FileOutputStream(marker)) { fos.write("marker".getBytes()); }
+
+            java.lang.reflect.Method method = KVMStorageProcessor.class.getDeclaredMethod(
+                    "reconcileOffline", String.class, String.class, File.class);
+            method.setAccessible(true);
+            method.invoke(storageProcessorSpy, "/nonexistent/base.qcow2", overlayFile.getAbsolutePath(), marker);
+
+            Assert.assertTrue("Marker should be kept when base does not exist", marker.exists());
+        } finally {
+            File[] files = tempDir.listFiles();
+            if (files != null) for (File f : files) f.delete();
+            tempDir.delete();
+        }
+    }
+
+    @Test
+    public void testReconcileOfflineOverlayDoesNotExist() throws Exception {
+        File tempDir = Files.createTempDirectory("reconcile_nooverlay_test").toFile();
+        try {
+            File marker = new File(tempDir, ".snapshot_marker_test");
+            try (FileOutputStream fos = new FileOutputStream(marker)) { fos.write("marker".getBytes()); }
+
+            java.lang.reflect.Method method = KVMStorageProcessor.class.getDeclaredMethod(
+                    "reconcileOffline", String.class, String.class, File.class);
+            method.setAccessible(true);
+            method.invoke(storageProcessorSpy, "/some/base.qcow2", "/nonexistent/overlay.qcow2", marker);
+
+            Assert.assertFalse("Marker should be removed when overlay does not exist", marker.exists());
+        } finally {
+            File[] files = tempDir.listFiles();
+            if (files != null) for (File f : files) f.delete();
+            tempDir.delete();
+        }
+    }
+
+    @Test
+    public void testUpdateSnapshotMarkerDelegatesToWrite() throws Exception {
+        Mockito.doNothing().when(storageProcessorSpy).writeSnapshotMarker(
+                Mockito.any(), Mockito.anyString(), Mockito.anyString(),
+                Mockito.anyString(), Mockito.anyString(), Mockito.anyString());
+
+        storageProcessorSpy.updateSnapshotMarker(kvmStoragePoolMock, "/vol", "snap", "vm", "vda", "/overlay");
+
+        Mockito.verify(storageProcessorSpy).writeSnapshotMarker(kvmStoragePoolMock, "/vol", "snap", "vm", "vda", "/overlay");
+    }
 }

utils/src/test/java/com/cloud/utils/script/ScriptTest.java

@@ -28,6 +28,7 @@
 import org.mockito.junit.MockitoJUnitRunner;
 
 import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
 
 @RunWith(MockitoJUnitRunner.class)
 public class ScriptTest {
@@ -108,4 +109,26 @@ public void testBuildCommandLineWithLegacyPasswordOption() {
         Assert.assertEquals("test.sh -y ****** ", commandLine);
     }
 
+    @Test
+    public void testRunSimpleBashScriptWithFullResultSuccess() {
+        String result = Script.runSimpleBashScriptWithFullResult("echo hello", 10);
+        Assert.assertNotNull(result);
+        Assert.assertEquals("hello", result.trim());
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void testRunSimpleBashScriptWithFullResultFailureIncludesOutput() {
+        try {
+            Script.runSimpleBashScriptWithFullResult("echo 'some error detail' >&2; exit 1", 10);
+        } catch (CloudRuntimeException e) {
+            Assert.assertTrue(e.getMessage().contains("Error executing command"));
+            throw e;
+        }
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void testRunSimpleBashScriptWithFullResultFailureOnInvalidCommand() {
+        Script.runSimpleBashScriptWithFullResult("nonexistent_command_xyz_12345", 10);
+    }
+
 }