Move the keep_old flag out of the packager and let the component check it.

devstack/component.py

@@ -398,6 +398,9 @@ class PkgUninstallComponent(ComponentBase):
         pass
 
     def _uninstall_pkgs(self):
+        if self.keep_old:
+            LOG.info('Keep-old flag set, not removing packages')
+            return
         pkgs = self.tracereader.packages_installed()
         if pkgs:
             pkg_names = set([p['name'] for p in pkgs])

devstack/packager.py

@@ -14,6 +14,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import abc
+
 from devstack import decorators
 from devstack import log as logging
 from devstack import utils
@@ -22,19 +24,24 @@ LOG = logging.getLogger("devstack.packager")
 
 
 class Packager(object):
+    __meta__ = abc.ABCMeta
 
     @decorators.log_debug
-    def __init__(self, distro, keep_packages):
+    def __init__(self, distro):
         self.distro = distro
-        self.keep_packages = keep_packages
 
+    @abc.abstractmethod
     def install(self, pkg):
-        raise NotImplementedError()
+        pass
+
+    @abc.abstractmethod
+    def _remove(self, pkg):
+        pass
 
     def remove(self, pkg):
-        if self.keep_packages:
+        removable = pkg.get('removable', True)
+        if not removable:
             return False
-        else:
         return self._remove(pkg)
 
     def pre_install(self, pkgs, params=None):
@@ -52,6 +59,3 @@ class Packager(object):
                 LOG.info("Running post-install commands for package %s.",
                          info['name'])
                 utils.execute_template(*cmds, params=params)
-
-    def _remove(self, pkg):
-        raise NotImplementedError()

devstack/packaging/apt.py

@@ -40,8 +40,9 @@ VERSION_TEMPL = "%s=%s"
 
 
 class AptPackager(pack.Packager):
-    def __init__(self, distro, keep_packages):
+    def __init__(self, distro):
-        pack.Packager.__init__(self, distro, keep_packages)
+        pack.Packager.__init__(self, distro)
+        # FIXME: Should this be coming from a setting somewhere?
         self.auto_remove = True
 
     def _format_pkg_name(self, name, version):
@@ -58,9 +59,6 @@ class AptPackager(pack.Packager):
             **kargs)
 
     def _remove(self, pkg):
-        removable = pkg.get('removable', True)
-        if not removable:
-            return False
         name = pkg['name']
         if self._remove_special(name, pkg):
             return True

devstack/packaging/yum.py

@@ -32,8 +32,8 @@ VERSION_TEMPL = "%s-%s"
 
 
 class YumPackager(pack.Packager):
-    def __init__(self, distro, keep_packages):
+    def __init__(self, distro):
-        pack.Packager.__init__(self, distro, keep_packages)
+        pack.Packager.__init__(self, distro)
 
     def _format_pkg_name(self, name, version):
         if version:
@@ -63,13 +63,9 @@ class YumPackager(pack.Packager):
             self._execute_yum(cmd)
 
     def _remove(self, pkg):
-        removable = pkg.get('removable', True)
-        if not removable:
-            return False
         name = pkg['name']
         if self._remove_special(name, pkg):
             return True
-        else:
         pkg_full = self._format_pkg_name(name, pkg.get("version"))
         cmd = YUM_REMOVE + [pkg_full]
         self._execute_yum(cmd)

stack

@@ -151,7 +151,7 @@ def run(args):
     config = cfg.get_config()
     pw_gen = passwords.PasswordGenerator(config, args.get('prompt_for_passwords', True))
     pkg_cls = dist.get_packager_factory()
-    pkg_manager = pkg_cls(dist, args.get('keep_old', False))
+    pkg_manager = pkg_cls(dist)
 
     runner_factory = actions.get_runner_factory(action)
     runner = runner_factory(dist,