Merge "Only delete 'owned' packages for --exists-action update"

muranoclient/tests/test_shell.py

@@ -27,6 +27,7 @@ import requests_mock
 import six
 from testtools import matchers
 
+from muranoclient.common import exceptions as common_exceptions
 from muranoclient.common import utils
 from muranoclient.openstack.common.apiclient import exceptions
 import muranoclient.shell
@@ -335,6 +336,136 @@ class ShellPackagesOperations(ShellTest):
                 'is_public': True
             }, {RESULT_PACKAGE: mock.ANY},)
 
+    def _test_conflict(self,
+                       packages, from_file, raw_input_mock,
+                       input_action, exists_action=''):
+        packages.create = mock.MagicMock(
+            side_effect=[common_exceptions.HTTPConflict("Conflict"), None])
+
+        packages.filter.return_value = [mock.Mock(id='test_id')]
+
+        raw_input_mock.return_value = input_action
+        args = TestArgs()
+        args.exists_action = exists_action
+        with tempfile.NamedTemporaryFile() as f:
+            args.filename = f.name
+
+            pkg = make_pkg({'FullName': f.name})
+            from_file.return_value = utils.Package(utils.File(pkg))
+
+            v1_shell.do_package_import(self.client, args)
+            return f.name
+
+    @mock.patch('__builtin__.raw_input')
+    @mock.patch('muranoclient.common.utils.Package.from_file')
+    def test_package_import_conflict_skip(self, from_file, raw_input_mock):
+
+        name = self._test_conflict(
+            self.client.packages,
+            from_file,
+            raw_input_mock,
+            's',
+        )
+
+        self.client.packages.create.assert_called_once_with({
+            'is_public': False,
+        }, {name: mock.ANY},)
+
+    @mock.patch('__builtin__.raw_input')
+    @mock.patch('muranoclient.common.utils.Package.from_file')
+    def test_package_import_conflict_skip_ea(self, from_file, raw_input_mock):
+
+        name = self._test_conflict(
+            self.client.packages,
+            from_file,
+            raw_input_mock,
+            '',
+            exists_action='s',
+        )
+
+        self.client.packages.create.assert_called_once_with({
+            'is_public': False,
+        }, {name: mock.ANY},)
+        self.assertFalse(raw_input_mock.called)
+
+    @mock.patch('__builtin__.raw_input')
+    @mock.patch('muranoclient.common.utils.Package.from_file')
+    def test_package_import_conflict_abort(self, from_file, raw_input_mock):
+
+        self.assertRaises(SystemExit, self._test_conflict,
+                          self.client.packages,
+                          from_file,
+                          raw_input_mock,
+                          'a',
+                          )
+
+        self.client.packages.create.assert_called_once_with({
+            'is_public': False,
+        }, mock.ANY,)
+
+    @mock.patch('__builtin__.raw_input')
+    @mock.patch('muranoclient.common.utils.Package.from_file')
+    def test_package_import_conflict_abort_ea(self,
+                                              from_file, raw_input_mock):
+
+        self.assertRaises(SystemExit, self._test_conflict,
+                          self.client.packages,
+                          from_file,
+                          raw_input_mock,
+                          '',
+                          exists_action='a',
+                          )
+
+        self.client.packages.create.assert_called_once_with({
+            'is_public': False,
+        }, mock.ANY,)
+        self.assertFalse(raw_input_mock.called)
+
+    @mock.patch('__builtin__.raw_input')
+    @mock.patch('muranoclient.common.utils.Package.from_file')
+    def test_package_import_conflict_update(self, from_file, raw_input_mock):
+
+        name = self._test_conflict(
+            self.client.packages,
+            from_file,
+            raw_input_mock,
+            'u',
+        )
+
+        self.client.packages.delete.assert_called_once_with('test_id')
+
+        self.client.packages.create.assert_has_calls(
+            [
+                mock.call({'is_public': False}, {name: mock.ANY},),
+                mock.call({'is_public': False}, {name: mock.ANY},)
+            ], any_order=True,
+        )
+        self.assertEqual(self.client.packages.create.call_count, 2)
+
+    @mock.patch('__builtin__.raw_input')
+    @mock.patch('muranoclient.common.utils.Package.from_file')
+    def test_package_import_conflict_update_ea(self,
+                                               from_file, raw_input_mock):
+
+        name = self._test_conflict(
+            self.client.packages,
+            from_file,
+            raw_input_mock,
+            '',
+            exists_action='u',
+        )
+
+        self.client.packages.delete.assert_called_once_with('test_id')
+
+        self.client.packages.create.assert_has_calls(
+            [
+                mock.call({'is_public': False}, {name: mock.ANY},),
+                mock.call({'is_public': False}, {name: mock.ANY},)
+            ], any_order=True,
+        )
+        self.assertEqual(self.client.packages.create.call_count, 2)
+        self.assertFalse(raw_input_mock.called)
+
     @mock.patch('muranoclient.common.utils.Package.from_file')
     def test_package_import_no_categories(self, from_file):
         args = TestArgs()

muranoclient/v1/shell.py

@@ -280,7 +280,8 @@ def _handle_package_exists(mc, data, package, exists_action):
         try:
             return mc.packages.create(data, {name: package.file()})
         except common_exceptions.HTTPConflict:
-            print("Package with name {0} is already registered.".format(name))
+            print("Importing package {0} failed. Package with the same"
+                  " name/classes is already registered.".format(name))
             allowed_results = ['s', 'u', 'a']
             res = exists_action
             if not res:
@@ -296,8 +297,23 @@ def _handle_package_exists(mc, data, package, exists_action):
                 print("Exiting.")
                 sys.exit()
             elif res == 'u':
-                print("Deleting package {0}".format(name))
+                pkgs = list(mc.packages.filter(fqn=name, owned=True))
-                mc.packages.delete(name)
+                if not pkgs:
+                    msg = (
+                        "Got Conflict response, but couldn't find package "
+                        "'{0}' in the current tenant.\nThis probably means "
+                        "conflicting package is in another tenant.\n"
+                        "Please delete it manually."
+                    ).format(name)
+                    raise exceptions.CommandError(msg)
+                elif len(pkgs) > 1:
+                    msg = (
+                        "Got {0} packages with name '{1}'.\nI'm not trusting "
+                        "myself, please delete the package manually"
+                    ).format(len(pkgs), name)
+                    raise exceptions.CommandError(msg)
+                print("Deleting package {0}({1})".format(name, pkgs[0].id))
+                mc.packages.delete(pkgs[0].id)
                 continue
 
 
@@ -358,6 +374,8 @@ def do_package_import(mc, args):
                       "images for {1}".format(e, name))
         try:
             _handle_package_exists(mc, data, package, args.exists_action)
+        except exceptions.CommandError:
+            raise
         except Exception as e:
             print("Error {0} occurred while installing package {1}".format(
                 e, name))
@@ -426,6 +444,8 @@ def do_bundle_import(mc, args):
             try:
                 _handle_package_exists(
                     mc, data, dep_package, args.exists_action)
+            except exceptions.CommandError:
+                raise
             except Exception as e:
                 print("Error {0} occurred while "
                       "installing package {1}".format(e, name))