[OSMorphing] Check installed packages before installing packages #337

Cristi1324 · 2024-08-29T15:30:31Z

This PR adds check_installed_package for Redhat, Debian and SUSE.

Dany9966 · 2024-08-30T12:06:07Z

coriolis/osmorphing/debian.py

@@ -122,6 +122,14 @@ def set_net_config(self, nics_info, dhcp):
            cfg_name = "%s/coriolis_netplan.yaml" % netplan_base
            self._write_file_sudo(cfg_name, new_cfg)

+    def check_installed_package(self, package_name):
+        cmd = 'dpkg-query -W %s' % ("".join(package_name))


I think it would be better to just send a single command that lists all the packages installed on the migrated system. Parse that list and save it on the tools object. Then when you need to check a package, you just do a if package in self.installed_packages: return or do a set difference like below to get the list of packages that need to be installed.

The point of this is to send a minimum amount of SSH commands, instead of sending N+1commands for each package we want to install. For Debian-based, you can use apt list --installed, please check a command for the others too.

You also don't need to "".join(package_name) here, you can simply pass the string value to the string format.

Dany9966 · 2024-09-04T09:59:18Z

coriolis/osmorphing/debian.py

@@ -122,6 +122,13 @@ def set_net_config(self, nics_info, dhcp):
            cfg_name = "%s/coriolis_netplan.yaml" % netplan_base
            self._write_file_sudo(cfg_name, new_cfg)

+    def get_installed_packages(self):
+        cmd = 'apt list --installed'


This method has incomplete parsing in my opinion. I expected a list of package names, you returned a list of package names plus some other information. Please properly parse that list before returning it. Therefore please try the following:
All apt list --installed lists packages in this format:

package_name/repo,version,etc

You could split this and only get the package name:

apt_list_output = self._exec_cmd_chroot(cmd).splitlines() # and maybe also a .decode(), not 100% sure for line in apt_list_output: package_name, _ = line.split('/', 1) self.installed_packages.append(package_name) # you don't need to return anything, I'd much rather do a set difference between installed_packages and # packages_add inside (or before) install_packages. The upside to this is that you can switfly check packages # in other places too (ex. if 'grubby' not in self.installed_packages: self._install(['grubby']).

Dany9966 · 2024-09-04T10:00:34Z

coriolis/osmorphing/manager.py

@@ -239,6 +239,10 @@ def morph_image(origin_provider, destination_provider, connection_info,
        LOG.info("Post packages uninstall")
        export_os_morphing_tools.post_packages_uninstall(packages_remove)

+    LOG.info("Checking for packages already installed")
+    packages_add = import_os_morphing_tools.check_installed_packages(


You can replace this with calling get_installed_packages, and then only install the set difference

Dany9966 · 2024-09-04T10:01:08Z

coriolis/osmorphing/redhat.py

-    def _has_package_installed(self, package_name):
-        cmd = 'rpm -q %s' % ("".join(package_name))
+    def get_installed_packages(self):
+        cmd = 'rpm -qa'


You can finetune this command also to only get the list of package names:

rpm -qa --qf '%{NAME}\n'

Dany9966 · 2024-09-04T10:07:57Z

coriolis/osmorphing/redhat.py

@@ -295,7 +294,7 @@ def pre_packages_install(self, package_names):
        super(BaseRedHatMorphingTools, self).pre_packages_install(
            package_names)
        self._yum_clean_all()
-        if not self._has_package_installed('grubby'):
+        if not any('grubby' in package for package in self.installed_packages):


As mentioned above, you can just check if 'grubbyis inself.installed_packages`. Do please check the package name string as a whole, as this could have buggy results. Let's say your list has a package called 'grubby-somethhing', but you need 'grubby' package instead, this code would skip that install.

Dany9966 · 2024-09-04T10:09:07Z

coriolis/osmorphing/suse.py

@@ -65,6 +65,15 @@ def set_net_config(self, nics_info, dhcp):
        # TODO(alexpilotti): add networking support
        pass

+    def get_installed_packages(self):
+        cmd = 'rpm -qa'


Make this the same as redhat please.

Dany9966 · 2024-09-04T11:51:03Z

coriolis/osmorphing/redhat.py

-    def _has_package_installed(self, package_name):
-        cmd = 'rpm -q %s' % ("".join(package_name))
+    def get_installed_packages(self):
+        cmd = 'rpm -qa --qf \'%{NAME}\n\''


This is most probably going to fail. If it does, please try:

cmd = 'rpm -qa --qf "%{NAME}\\n"'

Dany9966 · 2024-09-05T09:53:39Z

coriolis/osmorphing/debian.py

@@ -122,6 +122,17 @@ def set_net_config(self, nics_info, dhcp):
            cfg_name = "%s/coriolis_netplan.yaml" % netplan_base
            self._write_file_sudo(cfg_name, new_cfg)

+    def get_installed_packages(self):
+        cmd = "apt list --installed 2>/dev/null | awk -F/ '{print $1}'"


You can remove the 2>/dev/null as stderr is returned elsewhere, and will not be included in apt_list_output

Dany9966 · 2024-09-06T12:24:43Z

coriolis/osmorphing/debian.py

+    def get_installed_packages(self):
+        cmd = "dpkg-query -f '${binary:Package}\n' -W"
+        try:
+            apt_list_output = self._exec_cmd_chroot(


Also, could you please check whether you could do the following:

apt_list_output = self._exec_cmd_chroot(cmd).decode().splitlines()

This way, you only call for decode() once.

Dany9966 · 2024-09-06T12:25:13Z

coriolis/osmorphing/manager.py

@@ -239,6 +239,13 @@ def morph_image(origin_provider, destination_provider, connection_info,
        LOG.info("Post packages uninstall")
        export_os_morphing_tools.post_packages_uninstall(packages_remove)

+    LOG.info("Checking for packages already installed")
+    import_os_morphing_tools.get_installed_packages()
+    LOG.info("Packages already installed: %s" %


This could be a big list, please remove it.

Dany9966 · 2024-09-06T12:25:41Z

coriolis/osmorphing/redhat.py

-            self._exec_cmd_chroot(cmd)
-            return True
+            apt_list_output = self._exec_cmd_chroot(
+                cmd).splitlines()


Same as above about the decode()

Dany9966 · 2024-09-06T12:26:05Z

coriolis/osmorphing/suse.py

+        cmd = 'rpm -qa --qf \'%{NAME}\n\''
+        try:
+            apt_list_output = self._exec_cmd_chroot(
+                cmd).splitlines()


Same as above about the decode()

Dany9966

LGTM. If your testing also looks good, please go ahead and update the unit tests also.

Dany9966

LGTM! Thank you!

Dany9966 requested changes Aug 30, 2024

View reviewed changes

Cristi1324 force-pushed the check_installed_packages branch 8 times, most recently from fc8e18b to b234a6f Compare September 2, 2024 12:46

Dany9966 requested changes Sep 4, 2024

View reviewed changes

Cristi1324 force-pushed the check_installed_packages branch 4 times, most recently from 91b2075 to 2ca7cb4 Compare September 4, 2024 11:47

Dany9966 requested changes Sep 4, 2024

View reviewed changes

Cristi1324 force-pushed the check_installed_packages branch 14 times, most recently from e290ed8 to 96f506b Compare September 5, 2024 11:30

Dany9966 requested changes Sep 6, 2024

View reviewed changes

Cristi1324 force-pushed the check_installed_packages branch 2 times, most recently from 1619902 to 4bea23c Compare September 9, 2024 09:41

Dany9966 reviewed Sep 11, 2024

View reviewed changes

Cristi1324 force-pushed the check_installed_packages branch from 4bea23c to c264810 Compare September 12, 2024 08:56

Cristi1324 added 2 commits September 12, 2024 12:10

osmorphing: Check installed packages before installing packages

3bcc84b

osmorphing: Add unit tests for get_installed_packages

e96a52c

Cristi1324 force-pushed the check_installed_packages branch from c264810 to e96a52c Compare September 12, 2024 09:12

Dany9966 approved these changes Sep 16, 2024

View reviewed changes

Dany9966 merged commit 87d3541 into cloudbase:master Sep 16, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[OSMorphing] Check installed packages before installing packages #337

[OSMorphing] Check installed packages before installing packages #337

Cristi1324 commented Aug 29, 2024

Dany9966 Aug 30, 2024

Dany9966 Aug 30, 2024

Dany9966 Sep 4, 2024

Dany9966 Sep 4, 2024

Dany9966 Sep 4, 2024

Dany9966 Sep 4, 2024

Dany9966 Sep 4, 2024

Dany9966 Sep 4, 2024

Dany9966 Sep 5, 2024

Dany9966 Sep 6, 2024

Dany9966 Sep 6, 2024

Dany9966 Sep 6, 2024

Dany9966 Sep 6, 2024

Dany9966 left a comment

Dany9966 left a comment

[OSMorphing] Check installed packages before installing packages #337

[OSMorphing] Check installed packages before installing packages #337

Conversation

Cristi1324 commented Aug 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dany9966 left a comment

Choose a reason for hiding this comment

Dany9966 left a comment

Choose a reason for hiding this comment