From 6ca0ee146da5ac6feb8c50374139151047092c95 Mon Sep 17 00:00:00 2001 From: Mathieu Bultel Date: Mon, 5 Feb 2018 10:54:12 +0100 Subject: [PATCH] Inherit UpdateCommand class from overcloud DeployCommand class Instead of dealing with some decidated functions in Update/upgrade classes for manipulating the plan and the env files, inherit the update class from the DeployCommand class will allow the update && upgrade to take benefit from the plan management code and all the tips and tricks that the deploy class does for handling the corner cases of the plan (access rights, path of user files) Also it avoid to rewrite/duplicate some code to solve the issue where the --templates option is ignored by the update workflow (and upgrade for Queens btw). Related tripleo-common review: https://review.openstack.org/540816 Depends-On: Ica4bb404cd3a38a97300b8af36af41c35f09825c Change-Id: Ib96f4f5078ec1c45981d051e546bf931d2885ae2 --- tripleoclient/constants.py | 2 + .../overcloud_deploy/test_overcloud_deploy.py | 20 ++++++++- .../tests/v1/overcloud_update/fakes.py | 1 + .../overcloud_update/test_overcloud_update.py | 27 ++++++++---- tripleoclient/v1/overcloud_deploy.py | 17 +++++--- tripleoclient/v1/overcloud_update.py | 42 ++++--------------- tripleoclient/workflows/plan_management.py | 36 +++++++++++++--- 7 files changed, 92 insertions(+), 53 deletions(-) diff --git a/tripleoclient/constants.py b/tripleoclient/constants.py index 6a9717ea0..ee1123f02 100644 --- a/tripleoclient/constants.py +++ b/tripleoclient/constants.py @@ -23,6 +23,8 @@ RHEL_REGISTRATION_EXTRACONFIG_NAME = ( # The name of the file which holds the plan environment contents PLAN_ENVIRONMENT = 'plan-environment.yaml' +USER_ENVIRONMENT = 'user-environment.yaml' +USER_PARAMETERS = 'user-environments/tripleoclient-parameters.yaml' # This directory may contain additional environments to use during deploy DEFAULT_ENV_DIRECTORY = "~/.tripleo/environments" diff --git a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py index 5ef699aff..eaed97985 100644 --- a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py +++ b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py @@ -695,13 +695,18 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): env, update_plan_only, run_validations, skip_deploy_identifier, plan_env_file): assertEqual( - {'parameter_defaults': {}, + {'parameter_defaults': {'NovaComputeLibvirtType': 'qemu'}, 'resource_registry': { 'Test': 'OS::Heat::None', 'resources': {'*': {'*': { 'UpdateDeployment': {'hooks': []}}}}}}, env) mock_deploy_heat.side_effect = _fake_heat_deploy + object_client = clients.tripleoclient.object_store + object_client.get_object = mock.Mock() + mock_env = yaml.safe_dump({'parameter_defaults': + {'NovaComputeLibvirtType': 'qemu'}}) + object_client.get_object.return_value = ({}, mock_env) parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) @@ -816,6 +821,12 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): fixtures.EnvironmentVariable('TRIPLEO_ENVIRONMENT_DIRECTORY', self.tmp_dir.join('env'))) + object_client = clients.tripleoclient.object_store + object_client.get_object = mock.Mock() + mock_env = yaml.safe_dump({'parameter_defaults': + {'NovaComputeLibvirtType': 'qemu'}}) + object_client.get_object.return_value = ({}, mock_env) + parsed_args = self.check_parser(self.cmd, arglist, verifylist) error = self.assertRaises(hc_exc.CommandError, self.cmd.take_action, parsed_args) @@ -993,6 +1004,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): self.cmd.take_action(parsed_args) user_env = { + 'environments': [], 'resource_registry': {'Test': 'OS::Heat::None', 'Cleanup': 'OS::Heat::None'}} parameters_env = { @@ -1204,6 +1216,12 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): ('disable_password_generation', True)] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + object_client = clients.tripleoclient.object_store + object_client.get_object = mock.Mock() + mock_env = yaml.safe_dump({'parameter_defaults': + {'NovaComputeLibvirtType': 'qemu'}}) + object_client.get_object.return_value = ({}, mock_env) + self.cmd.take_action(parsed_args) self.assertTrue(mock_heat_deploy.called) diff --git a/tripleoclient/tests/v1/overcloud_update/fakes.py b/tripleoclient/tests/v1/overcloud_update/fakes.py index 5a5555a65..a4253386b 100644 --- a/tripleoclient/tests/v1/overcloud_update/fakes.py +++ b/tripleoclient/tests/v1/overcloud_update/fakes.py @@ -45,6 +45,7 @@ class TestOvercloudUpdate(utils.TestCommand): super(TestOvercloudUpdate, self).setUp() self.app.client_manager.auth_ref = mock.Mock(auth_token="TOKEN") + self.app.client_manager.baremetal = mock.Mock() self.app.client_manager.orchestration = mock.Mock() self.app.client_manager.tripleoclient = FakeClientWrapper() self.app.client_manager.workflow_engine = mock.Mock() diff --git a/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py b/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py index c7419d1f7..543a4fbaf 100644 --- a/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py +++ b/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py @@ -41,20 +41,26 @@ class TestOvercloudUpdate(fakes.TestOvercloudUpdate): 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') - def test_update_out(self, mock_yaml, mock_abspath, mock_open, mock_update, - mock_logger, mock_get_stack): + @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_update_out(self, mock_deploy, mock_open, mock_copy, mock_yaml, + mock_abspath, mock_update, mock_logger, + mock_get_stack): mock_stack = mock.Mock() mock_stack.stack_name = 'mystack' mock_get_stack.return_value = mock_stack mock_abspath.return_value = '/home/fake/my-fake-registry.yaml' mock_yaml.return_value = {'fake_container': 'fake_value'} - argslist = ['--stack', 'overcloud', '--init-update', + + argslist = ['--stack', 'overcloud', '--init-update', '--templates', '--container-registry-file', 'my-fake-registry.yaml'] verifylist = [ ('stack', 'overcloud'), + ('templates', constants.TRIPLEO_HEAT_TEMPLATES), ('init_update', True), ('container_registry_file', 'my-fake-registry.yaml') ] @@ -66,8 +72,7 @@ class TestOvercloudUpdate(fakes.TestOvercloudUpdate): container='mystack', container_registry={'fake_container': 'fake_value'}, ceph_ansible_playbook='/usr/share/ceph-ansible' - '/site-docker.yml.sample', - environments={} + '/site-docker.yml.sample' ) @mock.patch('tripleoclient.workflows.package_update.update', @@ -75,15 +80,19 @@ class TestOvercloudUpdate(fakes.TestOvercloudUpdate): @mock.patch('six.moves.builtins.open') @mock.patch('os.path.abspath') @mock.patch('yaml.load') - def test_update_failed(self, mock_yaml, mock_abspath, mock_open, - mock_update): + @mock.patch('shutil.copytree', autospec=True) + @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' + '_deploy_tripleo_heat_templates', autospec=True) + def test_update_failed(self, mock_deploy, mock_copy, mock_yaml, + mock_abspath, mock_open, mock_update): mock_update.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', '--init-update', + argslist = ['--stack', 'overcloud', '--init-update', '--templates', '--container-registry-file', 'my-fake-registry.yaml'] verifylist = [ ('stack', 'overcloud'), + ('templates', constants.TRIPLEO_HEAT_TEMPLATES), ('init_update', True), ('container_registry_file', 'my-fake-registry.yaml') ] diff --git a/tripleoclient/v1/overcloud_deploy.py b/tripleoclient/v1/overcloud_deploy.py index 59ab9e14e..5d1c1d350 100644 --- a/tripleoclient/v1/overcloud_deploy.py +++ b/tripleoclient/v1/overcloud_deploy.py @@ -423,7 +423,6 @@ class DeployOvercloud(command.Command): def _deploy_tripleo_heat_templates(self, stack, parsed_args, tht_root, user_tht_root): """Deploy the fixed templates in TripleO Heat Templates""" - parameters = self._update_parameters(parsed_args, stack) plans = plan_management.list_deployment_plans(self.workflow_client) generate_passwords = not parsed_args.disable_password_generation @@ -459,10 +458,18 @@ class DeployOvercloud(command.Command): if parsed_args.environment_directories: created_env_files.extend(utils.load_environment_directories( parsed_args.environment_directories)) - - env.update(self._create_parameters_env(parameters, - tht_root, - parsed_args.stack)) + parameters = {} + if stack: + try: + # If user environment already exist then keep it + user_env = yaml.safe_load(self.object_client.get_object( + parsed_args.stack, constants.USER_ENVIRONMENT)[1]) + template_utils.deep_update(env, user_env) + except ClientException: + pass + parameters.update(self._update_parameters(parsed_args, stack)) + template_utils.deep_update(env, self._create_parameters_env( + parameters, tht_root, parsed_args.stack)) if parsed_args.rhel_reg: reg_env_files, reg_env = self._create_registration_env( diff --git a/tripleoclient/v1/overcloud_update.py b/tripleoclient/v1/overcloud_update.py index 75ac2cb55..dc01f069f 100644 --- a/tripleoclient/v1/overcloud_update.py +++ b/tripleoclient/v1/overcloud_update.py @@ -17,37 +17,23 @@ import logging import os import yaml -from heatclient.common import template_utils from osc_lib.i18n import _ from oslo_concurrency import processutils -from tripleoclient import command from tripleoclient import constants from tripleoclient import exceptions from tripleoclient import utils as oooutils +from tripleoclient.v1.overcloud_deploy import DeployOvercloud from tripleoclient.workflows import package_update -class UpdateOvercloud(command.Command): +class UpdateOvercloud(DeployOvercloud): """Updates packages on overcloud nodes""" log = logging.getLogger(__name__ + ".UpdateOvercloud") def get_parser(self, prog_name): parser = super(UpdateOvercloud, self).get_parser(prog_name) - parser.add_argument('--stack', - nargs='?', - dest='stack', - help=_('Name or ID of heat stack to scale ' - '(default=Env: OVERCLOUD_STACK_NAME)'), - default='overcloud' - ) - parser.add_argument('--templates', - nargs='?', - default=constants.TRIPLEO_HEAT_TEMPLATES, - help=_("The directory containing the Heat" - "templates to deploy. "), - ) parser.add_argument('--init-update', dest='init_update', action='store_true', @@ -69,13 +55,6 @@ class UpdateOvercloud(command.Command): 'used for update. This value should be set ' 'during the init-minor-update step.') ) - parser.add_argument('--extra-environment', '-e', - action='append', - dest='environment_files', - default=[], - help=_("Extra environment required for the " - "major update"), - ) parser.add_argument('--nodes', action="store", default=None, @@ -123,19 +102,17 @@ class UpdateOvercloud(command.Command): "to re-run this command and provide the registry file " "with: --container-registry-file option.") registry = None - # Extra env file - environment_files = parsed_args.environment_files - env = {} - if environment_files: - env_files, env = ( - template_utils.process_multiple_environments_and_files( - env_paths=environment_files)) # Run update ceph_ansible_playbook = parsed_args.ceph_ansible_playbook + # Run Overcloud deploy (stack update) + # In case of update and upgrade we need to force the + # update_plan_only. The heat stack update is done by the + # packag_update mistral action + parsed_args.update_plan_only = True + super(UpdateOvercloud, self).take_action(parsed_args) package_update.update(clients, container=stack_name, container_registry=registry, - ceph_ansible_playbook=ceph_ansible_playbook, - environments=env) + ceph_ansible_playbook=ceph_ansible_playbook) package_update.get_config(clients, container=stack_name) print("Update init on stack {0} complete.".format( parsed_args.stack)) @@ -197,7 +174,6 @@ class UpgradeOvercloud(UpdateOvercloud): stack_name = stack.stack_name converge = parsed_args.converge - if converge: converge_file = parsed_args.upgrade_converge_file # Add the converge file to the user environment: diff --git a/tripleoclient/workflows/plan_management.py b/tripleoclient/workflows/plan_management.py index eaa2428b5..cd7bb2ac3 100644 --- a/tripleoclient/workflows/plan_management.py +++ b/tripleoclient/workflows/plan_management.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. import logging +import os import tempfile import yaml @@ -157,13 +158,24 @@ def update_plan_from_templates(clients, name, tht_root, roles_file=None, # 'passwords' if they exist as they can't be recreated from the # templates content. passwords = [] + user_env = {} + # If the user provides a plan-environment files, then used it + if plan_env_file: + with open(os.path.abspath(plan_env_file)) as content: + env = yaml.load(content.read()) + else: + try: + env = yaml.safe_load(swift_client.get_object( + name, constants.PLAN_ENVIRONMENT)[1]) + except swift_exc.ClientException: + pass try: - env = yaml.safe_load(swift_client.get_object( - name, constants.PLAN_ENVIRONMENT)[1]) - passwords = env.get('passwords', []) + # Get user environment + user_env = yaml.safe_load(swift_client.get_object( + name, constants.USER_ENVIRONMENT)[1]) except swift_exc.ClientException: pass - + passwords = env.get('passwords', []) # TODO(dmatthews): Removing the existing plan files should probably be # a Mistral action. print("Removing the current plan files") @@ -183,10 +195,24 @@ def update_plan_from_templates(clients, name, tht_root, roles_file=None, print("Uploading new plan files") _upload_templates(swift_client, name, tht_root, roles_file, plan_env_file, networks_file) + # Update password and user parameters into swift _update_passwords(swift_client, name, passwords) + _update_user_environment(swift_client, name, user_env, + constants.USER_ENVIRONMENT) update_deployment_plan(clients, container=name, generate_passwords=generate_passwords, - source_url=None) + source_url=None, plan_environment=env) + + +def _update_user_environment(swift_client, name, user_params, filename): + if user_params: + try: + swift_client.put_object(name, + filename, + yaml.safe_dump(user_params, + default_flow_style=False)) + except swift_exc.ClientException: + LOG.debug("Unable to put %s in %s", filename, name) def _update_passwords(swift_client, name, passwords):