Stop trying to converge during Ceph upgrade command

This had no effect as we only updated the plan and not the
stack. We'll do a normal converge after Ceph upgrade command, which
will also take care of putting back the normal deploy
CephAnsiblePlaybook.

Furthermore it was prepending the converge env file to a list of envs
which already had the *prepare* env file at a latter position (we
reused the env list from the prepare call :) ), so it kept doing
prepare operation instead of converge.

And perhaps most importantly, the messages printed to user are
misleading since no converge was really happening.

Also adds support for --container-registry-file to ceph-upgrade command,
while not necessary, some of the functions called by ceph-upgrade
command expect to receive the container-registry parameters.

Co-Authored-By: Giulio Fidente <gfidente@redhat.com>
Change-Id: I025eac40f8bda5f23c789e7fef1a9e9b49947f66
Partial-Bug: #1768586

tripleoclient/constants.py

@@ -63,4 +63,3 @@ UPGRADE_CONVERGE_ENV = "environments/lifecycle/upgrade-converge.yaml"
 FFWD_UPGRADE_PREPARE_ENV = "environments/lifecycle/ffwd-upgrade-prepare.yaml"
 FFWD_UPGRADE_CONVERGE_ENV = "environments/lifecycle/ffwd-upgrade-converge.yaml"
 CEPH_UPGRADE_PREPARE_ENV = "environments/lifecycle/ceph-upgrade-prepare.yaml"
-CEPH_UPGRADE_CONVERGE_ENV = "environments/lifecycle/ceph-upgrade-converge.yaml"

tripleoclient/tests/v1/overcloud_ceph_upgrade/test_overcloud_ceph_upgrade.py

@@ -28,80 +28,81 @@ class TestCephUpgrade(fakes.TestCephUpgrade):
         super(TestCephUpgrade, self).setUp()
 
         # Get the command object to test
-        app_args = mock.Mock()
-        app_args.verbose_level = 1
-        self.cmd = overcloud_ceph_upgrade.CephUpgrade(self.app, app_args)
+        self.app_args = mock.Mock()
+        self.app_args.verbose_level = 1
+        self.cmd = overcloud_ceph_upgrade.CephUpgrade(self.app, self.app_args)
         uuid4_patcher = mock.patch('uuid.uuid4', return_value="UUID4")
         self.mock_uuid4 = uuid4_patcher.start()
         self.addCleanup(self.mock_uuid4.stop)
 
-    @mock.patch('tripleoclient.workflows.package_update.ffwd_converge_nodes',
-                autospec=True)
-    @mock.patch('tripleoclient.utils.prepend_environment', autospec=True)
-    @mock.patch('tripleoclient.utils.get_stack',
-                autospec=True)
+    @mock.patch('tripleoclient.utils.load_container_registry')
+    @mock.patch('tripleoclient.utils.get_stack')
+    @mock.patch('tripleoclient.workflows.package_update.update', autospec=True)
     @mock.patch(
-        'tripleoclient.v1.overcloud_ceph_upgrade.CephUpgrade.log',
-        autospec=True)
-    @mock.patch('tripleoclient.workflows.package_update.update',
-                autospec=True)
-    @mock.patch('yaml.load')
-    @mock.patch('shutil.copytree', autospec=True)
-    @mock.patch('six.moves.builtins.open')
-    @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
-                '_deploy_tripleo_heat_templates', autospec=True)
-    def test_ceph_upgrade(self, mock_deploy, mock_open, mock_copy, mock_yaml,
-                          mock_ceph_upgrade, mock_logger,
-                          mock_get_stack, mock_prepend_env,
-                          ffwd_converge_nodes):
+        'tripleoclient.v1.overcloud_ceph_upgrade.DeployOvercloud.take_action')
+    def test_ceph_upgrade(self, mock_deploy, mock_ceph_upgrade, mock_get_stack,
+                          mock_load_registry):
+        # get a fresh cmd so that the superclass mock takes effect
+        cmd = overcloud_ceph_upgrade.CephUpgrade(self.app, self.app_args)
+
         mock_stack = mock.Mock()
         mock_stack.stack_name = 'mystack'
         mock_get_stack.return_value = mock_stack
-        mock_yaml.return_value = {'fake_container': 'fake_value'}
+        mock_load_registry.return_value = {'fake_container': 'fake_value'}
 
-        argslist = ['--stack', 'mystack', '--templates']
+        argslist = ['--stack', 'mystack', '--templates',
+                    '--container-registry-file', 'my-fake-registry.yaml']
         verifylist = [
             ('stack', 'mystack'),
-            ('templates', constants.TRIPLEO_HEAT_TEMPLATES)
+            ('templates', constants.TRIPLEO_HEAT_TEMPLATES),
+            ('container_registry_file', 'my-fake-registry.yaml')
         ]
-
         parsed_args = self.check_parser(self.cmd, argslist, verifylist)
-        self.cmd.take_action(parsed_args)
-        mock_ceph_upgrade.assert_called_once_with(
-            self.app.client_manager,
-            container='mystack',
-            ceph_ansible_playbook='/usr/share/ceph-ansible'
-                                  '/infrastructure-playbooks'
-                                  '/rolling_update.yml',
-        )
-        with mock.patch('os.path.exists') as mock_exists:
+
+        with mock.patch('os.path.exists') as mock_exists, \
+                mock.patch('os.path.isfile') as mock_isfile:
             mock_exists.return_value = True
-            ffwd_converge_nodes.assert_called_once_with(
+            mock_isfile.return_value = True
+            cmd.take_action(parsed_args)
+            mock_deploy.assert_called_once_with(parsed_args)
+            mock_ceph_upgrade.assert_called_once_with(
                 self.app.client_manager,
-                container='mystack')
+                container='mystack',
+                container_registry={'fake_container': 'fake_value'},
+                ceph_ansible_playbook='/usr/share/ceph-ansible'
+                                      '/infrastructure-playbooks'
+                                      '/rolling_update.yml',
+            )
 
     @mock.patch('tripleoclient.utils.prepend_environment', autospec=True)
     @mock.patch('tripleoclient.workflows.package_update.update',
                 autospec=True)
     @mock.patch('six.moves.builtins.open')
+    @mock.patch('os.path.abspath')
     @mock.patch('yaml.load')
     @mock.patch('shutil.copytree', autospec=True)
-    @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
-                '_deploy_tripleo_heat_templates', autospec=True)
+    @mock.patch(
+        'tripleoclient.v1.overcloud_ceph_upgrade.DeployOvercloud.take_action')
     def test_ceph_upgrade_failed(
-        self, mock_deploy, mock_copy, mock_yaml, mock_open,
+        self, mock_deploy, mock_copy, mock_yaml, mock_abspath, mock_open,
             mock_ceph_upgrade, mock_prepend_env):
         mock_ceph_upgrade.side_effect = exceptions.DeploymentError()
+        mock_abspath.return_value = '/home/fake/my-fake-registry.yaml'
         mock_yaml.return_value = {'fake_container': 'fake_value'}
-        argslist = ['--stack', 'overcloud', '--templates']
+        argslist = ['--stack', 'overcloud', '--templates',
+                    '--container-registry-file', 'my-fake-registry.yaml']
         verifylist = [
             ('stack', 'overcloud'),
-            ('templates', constants.TRIPLEO_HEAT_TEMPLATES)
+            ('templates', constants.TRIPLEO_HEAT_TEMPLATES),
+            ('container_registry_file', 'my-fake-registry.yaml')
         ]
         parsed_args = self.check_parser(self.cmd, argslist, verifylist)
 
+        # get a fresh cmd so that the superclass mock takes effect
+        cmd = overcloud_ceph_upgrade.CephUpgrade(self.app, self.app_args)
+
         self.assertRaises(exceptions.DeploymentError,
-                          self.cmd.take_action, parsed_args)
+                          cmd.take_action, parsed_args)
 
     @mock.patch('tripleoclient.workflows.package_update.update_ansible',
                 autospec=True)

tripleoclient/v1/overcloud_ceph_upgrade.py

@@ -30,6 +30,12 @@ class CephUpgrade(DeployOvercloud):
 
     def get_parser(self, prog_name):
         parser = super(CephUpgrade, self).get_parser(prog_name)
+        parser.add_argument('--container-registry-file',
+                            dest='container_registry_file',
+                            default=None,
+                            help=_("Optional path to file with container "
+                                   "registry data for the update"),
+                            )
         parser.add_argument('--ceph-ansible-playbook',
                             action="store",
                             default="/usr/share/ceph-ansible"
@@ -47,6 +53,8 @@ class CephUpgrade(DeployOvercloud):
                                    parsed_args.stack)
 
         stack_name = stack.stack_name
+        registry = oooutils.load_container_registry(
+            self.log, parsed_args.container_registry_file)
 
         # Run update
         ceph_ansible_playbook = parsed_args.ceph_ansible_playbook
@@ -65,19 +73,8 @@ class CephUpgrade(DeployOvercloud):
 
         super(CephUpgrade, self).take_action(parsed_args)
         package_update.update(clients, container=stack_name,
+                              container_registry=registry,
                               ceph_ansible_playbook=ceph_ansible_playbook)
         package_update.get_config(clients, container=stack_name)
-        print("Ceph Upgrade on stack {0} complete. Cleaning up".format(
-              parsed_args.stack))
-
-        if not parsed_args.environment_files:
-            parsed_args.environment_files = []
-        oooutils.prepend_environment(
-            parsed_args.environment_files, templates_dir,
-            constants.CEPH_UPGRADE_CONVERGE_ENV)
-
-        super(CephUpgrade, self).take_action(parsed_args)
-        # Run converge steps
-        package_update.ffwd_converge_nodes(clients, container=stack_name)
-        print("Ceph Upgrade Cleanup on stack {0} complete.".format(
+        print("Ceph Upgrade on stack {0} complete.".format(
               parsed_args.stack))