feat: Improvements in clone_vm_hdd_image logic

mfd_kvm/hypervisor.py

@@ -891,16 +891,50 @@ def clone_vm_hdd_image(
         :param timeout: Time to clone image
         :raises KVMHypervisorException: if cloning failed or timeout occurred
         :raises FileNotFoundError: if not found path_to_source_image
-        :return: Path for cloned hdd image
+        :return: Path for cloned or already matching hdd image
         """
+        path_to_source_image = self._conn.path(str(path_to_source_image))
+        path_to_destination_image = self._conn.path(str(path_to_destination_image))
         logger.log(
             log_levels.MODULE_DEBUG,
             msg=f"Cloning disk image {path_to_source_image} into {path_to_destination_image}, "
             f"timeout - {timeout} secs.",
         )
         if path_to_source_image.is_file():
+            if path_to_destination_image.exists():
+                # if source image contains suffix that matches checksum pattern use it for comparison
+                source_checksum = path_to_source_image.name.rsplit(".", maxsplit=1)[-1]
+                if re.fullmatch(r"[a-fA-F0-9]{32}", source_checksum):
+                    logger.log(
+                        log_levels.MODULE_DEBUG,
+                        msg=f"Pre-calculated checksum found in the source filename: {source_checksum}. "
+                        "It will be used for comparison with the destination image.",
+                    )
+                else:
+                    logger.log(
+                        log_levels.MODULE_DEBUG,
+                        msg="Pre-calculated checksum not found in the source filename. "
+                        "It will be calculated directly for comparison.",
+                    )
+                    source_checksum = self._conn.execute_command(f"md5sum {path_to_source_image}").stdout.split()[0]
+                destination_checksum = self._conn.execute_command(f"md5sum {path_to_destination_image}").stdout.split()[
+                    0
+                ]
+                if source_checksum.lower() == destination_checksum.lower():
+                    logger.log(
+                        log_levels.MODULE_DEBUG,
+                        msg=f"Destination image {path_to_destination_image} already matches the source; "
+                        "skipping cloning.",
+                    )
+                    return path_to_destination_image
+                logger.log(
+                    log_levels.MODULE_DEBUG,
+                    msg=f"Destination image {path_to_destination_image} differs from the source; removing it.",
+                )
+                path_to_destination_image.unlink()
+
             timeout_counter = TimeoutCounter(timeout)
-            copy_command = "rsync -aqc" if self._rsync_available else "scp"
+            copy_command = "rsync -aq" if self._rsync_available else "scp"
             # show 5 column of ls, size of file
             target_size = self._conn.execute_command(
                 f"ls {path_to_source_image} -l | awk '{{print $5}}'", shell=True
@@ -921,10 +955,15 @@ def clone_vm_hdd_image(
                     if path_to_destination_image.exists()
                     else 0
                 )
-                logging.log(
-                    log_levels.MODULE_DEBUG,
-                    msg=f"still cloning... {int(current_size) / int(target_size) * 100:.0f} %, next check in 30secs.",
-                )
+                try:
+                    current_size = int(current_size)
+                except (TypeError, ValueError):
+                    current_size = 0
+                if current_size > 0:
+                    logging.log(
+                        log_levels.MODULE_DEBUG,
+                        msg=f"still cloning... {current_size / int(target_size) * 100:.0f} %, next check in 30secs.",
+                    )
                 sleep(30)
             else:
                 raise KVMHypervisorException(

tests/unit/test_mfd_kvm/test_hypervisor.py

@@ -871,16 +871,133 @@ def test_attach_device(self, hv, mocker, state):
     def test_clone_vm_hdd_image_check_if_source_file_exists(self, hv):
         source_path = Path("/foo/source")
         dest_path = Path("/foo/destination")
+        hv._conn.path.side_effect = Path
         escaped_source_path = re.escape(str(source_path))
         with pytest.raises(FileNotFoundError, match=f"Not found {escaped_source_path} in system."):
             hv.clone_vm_hdd_image(path_to_source_image=source_path, path_to_destination_image=dest_path)
 
+    def test_clone_vm_hdd_image_checks_destination_on_connection(self, hv, mocker, tmp_path):
+        source_path = tmp_path / "source.img"
+        source_path.touch()
+        destination_path = tmp_path / "destination.img"
+        destination_path.touch()
+        remote_source_path = mocker.MagicMock()
+        remote_source_path.__str__.return_value = "/remote/source.img"
+        remote_source_path.is_file.return_value = True
+        remote_destination_path = mocker.MagicMock()
+        remote_destination_path.__str__.return_value = "/remote/destination.img"
+        remote_destination_path.exists.return_value = False
+        hv._conn.path.side_effect = [remote_source_path, remote_destination_path]
+        hv.__dict__["_rsync_available"] = True
+        hv._conn.execute_command.return_value = ConnectionCompletedProcess(args="", stdout="6", return_code=0)
+        process = mocker.create_autospec(RPyCProcess)
+        process.running = False
+        hv._conn.start_process.return_value = process
+
+        result = hv.clone_vm_hdd_image(
+            path_to_source_image=source_path,
+            path_to_destination_image=destination_path,
+        )
+
+        assert result == remote_destination_path
+        assert hv._conn.path.call_args_list == [mocker.call(str(source_path)), mocker.call(str(destination_path))]
+        hv._conn.execute_command.assert_called_once_with(
+            "ls /remote/source.img -l | awk '{print $5}'",
+            shell=True,
+        )
+        hv._conn.start_process.assert_called_once_with("rsync -aq /remote/source.img /remote/destination.img")
+
+    @pytest.mark.parametrize(
+        "source_name",
+        [
+            pytest.param("source.img", id="no-checksum"),
+            pytest.param("source.d41d8cd98f00b204e9800998ecf8427e.img", id="checksum-not-last"),
+            pytest.param("source.img.d41d8cd98f00b204e9800998ecf8427e0", id="checksum-too-long"),
+            pytest.param("source.img.g41d8cd98f00b204e9800998ecf8427e", id="checksum-not-hex"),
+        ],
+    )
+    def test_clone_vm_hdd_image_calculated_checksums_match(self, hv, mocker, tmp_path, source_name):
+        source_path = tmp_path / source_name
+        source_path.touch()
+        destination_path = tmp_path / "destination.img"
+        destination_path.touch()
+        hv._conn.path.side_effect = Path
+        checksum = "d41d8cd98f00b204e9800998ecf8427e"
+        hv._conn.execute_command.side_effect = [
+            ConnectionCompletedProcess(args="", stdout=f"{checksum}  {source_path}", return_code=0),
+            ConnectionCompletedProcess(args="", stdout=f"{checksum}  {destination_path}", return_code=0),
+        ]
+
+        result = hv.clone_vm_hdd_image(
+            path_to_source_image=source_path,
+            path_to_destination_image=destination_path,
+        )
+
+        assert result == destination_path
+        assert hv._conn.execute_command.call_args_list == [
+            mocker.call(f"md5sum {source_path}"),
+            mocker.call(f"md5sum {destination_path}"),
+        ]
+        assert hv._conn.path.call_args_list == [mocker.call(str(source_path)), mocker.call(str(destination_path))]
+        hv._conn.start_process.assert_not_called()
+
+    def test_clone_vm_hdd_image_filename_checksum_matches(self, hv, mocker, tmp_path):
+        checksum = "d41d8cd98f00b204e9800998ecf8427e"
+        source_path = tmp_path / f"source.img.{checksum.upper()}"
+        source_path.touch()
+        destination_path = tmp_path / "destination.img"
+        destination_path.touch()
+        hv._conn.path.side_effect = Path
+        hv._conn.execute_command.return_value = ConnectionCompletedProcess(
+            args="", stdout=f"{checksum}  {destination_path}", return_code=0
+        )
+
+        result = hv.clone_vm_hdd_image(
+            path_to_source_image=source_path,
+            path_to_destination_image=destination_path,
+        )
+
+        assert result == destination_path
+        hv._conn.execute_command.assert_called_once_with(f"md5sum {destination_path}")
+        hv._conn.start_process.assert_not_called()
+
+    def test_clone_vm_hdd_image_filename_checksum_differs(self, hv, mocker, tmp_path, caplog):
+        caplog.set_level(log_levels.MODULE_DEBUG)
+        timeout_mocker = mocker.patch("mfd_kvm.hypervisor.TimeoutCounter")
+        timeout_mocker.return_value.__bool__.return_value = False
+        process = mocker.create_autospec(RPyCProcess)
+        process.running = False
+        hv._conn.start_process.return_value = process
+        source_checksum = "36cd38f49b9afa08222c0dc9ebfe35eb"
+        source_path = tmp_path / f"source.img.{source_checksum}"
+        source_path.write_text("source")
+        destination_path = tmp_path / "destination.img"
+        destination_path.write_text("destination")
+        hv._conn.path.side_effect = Path
+        destination_checksum = "cc17cbb4f2b32c03a280a68544e8516f"
+        hv._conn.execute_command.side_effect = [
+            ConnectionCompletedProcess(args="", stdout=f"{destination_checksum}  {destination_path}", return_code=0),
+            ConnectionCompletedProcess(args="", return_code=0),
+            ConnectionCompletedProcess(args="", stdout="6", return_code=0),
+        ]
+
+        result = hv.clone_vm_hdd_image(
+            path_to_source_image=source_path,
+            path_to_destination_image=destination_path,
+        )
+
+        assert result == destination_path
+        assert not destination_path.exists()
+        assert f"Destination image {destination_path} differs from the source; removing it." in caplog.text
+        hv._conn.start_process.assert_called_once_with(f"rsync -aq {source_path} {destination_path}")
+
     def test_clone_vm_hdd_image_timeout_exceeded(self, hv, mocker):
         timeout_mocker = mocker.patch("mfd_kvm.hypervisor.TimeoutCounter")
         timeout_mocker.return_value.__bool__.return_value = True
         timeout = 12345
         dest_path = Path("/foo/destination")
         mock_path = mocker.patch("mfd_kvm.hypervisor.Path")
+        hv._conn.path.side_effect = [mock_path, dest_path]
         escaped_source_path = re.escape(str(mock_path))
         mock_path.is_file.return_value = True
         with pytest.raises(
@@ -895,7 +1012,7 @@ def test_clone_vm_hdd_image_timeout_exceeded(self, hv, mocker):
 
     @pytest.mark.parametrize(
         ("rsync_return_code", "copy_command"),
-        [(0, "rsync -aqc"), (1, "scp")],
+        [(0, "rsync -aq"), (1, "scp")],
     )
     def test_clone_vm_hdd_image_source_file_exists_and_succeeded(self, hv, mocker, rsync_return_code, copy_command):
         timeout_mocker = mocker.patch("mfd_kvm.hypervisor.TimeoutCounter")
@@ -906,6 +1023,7 @@ def test_clone_vm_hdd_image_source_file_exists_and_succeeded(self, hv, mocker, r
         dest_path = Path("/foo/destination")
         path_mocker = mocker.patch("mfd_kvm.hypervisor.Path")
         path_mocker.is_file.return_value = True
+        hv._conn.path.side_effect = [path_mocker, dest_path, path_mocker, dest_path]
         hv._conn.execute_command.side_effect = [
             ConnectionCompletedProcess(args="", return_code=rsync_return_code),
             ConnectionCompletedProcess(args="", stdout="100", return_code=0),
@@ -938,7 +1056,8 @@ def test_clone_vm_hdd_image_still_cloning(self, hv, mocker, caplog):
         dest_path = mocker.patch("mfd_kvm.hypervisor.Path")
         mocker.patch("mfd_kvm.hypervisor.sleep")
         path_mocker.is_file.return_value = True
-        dest_path.exists.return_value = True
+        dest_path.exists.side_effect = [False, True]
+        hv._conn.path.side_effect = [path_mocker, dest_path]
         hv._conn.execute_command.side_effect = [
             ConnectionCompletedProcess(args="", return_code=0),
             ConnectionCompletedProcess(args="", stdout="100", return_code=0),
@@ -947,6 +1066,30 @@ def test_clone_vm_hdd_image_still_cloning(self, hv, mocker, caplog):
         hv.clone_vm_hdd_image(path_to_source_image=path_mocker, path_to_destination_image=dest_path)
         assert "still cloning... 10 %, next check in 30secs." in caplog.text
 
+    @pytest.mark.parametrize("current_size", ["", "invalid", "0", "-1"])
+    def test_clone_vm_hdd_image_invalid_current_size(self, hv, mocker, caplog, current_size):
+        caplog.set_level(log_levels.MODULE_DEBUG)
+        timeout_mocker = mocker.patch("mfd_kvm.hypervisor.TimeoutCounter")
+        timeout_mocker.return_value.__bool__.return_value = False
+        process = mocker.create_autospec(RPyCProcess)
+        type(process).running = mocker.PropertyMock(side_effect=[True, False])
+        hv._conn.start_process.return_value = process
+        path_mocker = mocker.patch("mfd_kvm.hypervisor.Path")
+        dest_path = mocker.patch("mfd_kvm.hypervisor.Path")
+        mocker.patch("mfd_kvm.hypervisor.sleep")
+        path_mocker.is_file.return_value = True
+        dest_path.exists.side_effect = [False, True]
+        hv._conn.path.side_effect = [path_mocker, dest_path]
+        hv._conn.execute_command.side_effect = [
+            ConnectionCompletedProcess(args="", return_code=0),
+            ConnectionCompletedProcess(args="", stdout="100", return_code=0),
+            ConnectionCompletedProcess(args="", stdout=current_size, return_code=0),
+        ]
+
+        hv.clone_vm_hdd_image(path_to_source_image=path_mocker, path_to_destination_image=dest_path)
+
+        assert "still cloning..." not in caplog.text
+
     def test_create_mdev(self, hv, mocker):
         command = r'echo "a1234" | tee /sys/class/mdev_bus/0000\:b9\:00.0/mdev_supported_types/ice-vdcm/create'
         hv._conn.execute_command.return_value = ConnectionCompletedProcess(args="", stdout="a1234", return_code=0)