From 5531e8bbbc57310ab855aec66ff9aecde5cc9ac0 Mon Sep 17 00:00:00 2001 From: Bogdan Dobrelya Date: Tue, 19 Jun 2018 13:00:25 +0300 Subject: [PATCH] Leverage log_file option to capture more UC logs Undercloud deployment starts logging into its log file too late, omitting log records from undercloud_config, preflight checks. This is a problem as we want to log at least the commands used for undercloud install/upgrade. Ideally, messages in stdout/stderr should not miss the log file. Fix this via the added custom undercloud_log_file config (use --log-file for standalone CLI). Use those to configure logging to a file. Fix missing formatting, like timestamps and source, for undercloud config and preflight checks. Write preflight/install/upgrade logs into the same logfile. Move the load oslo config method into shared utilities. Add the configure logging shared utility for the classless modules (w/o oslo_log supported). Such modules can mimic oslo_log behavior defined for the main deployment modules derived from openstack client classes, which support logging to files natively and are not affected by the subject issue. With an exception made for those classless modules allowing them to log INFO+ be default. So operators will have the pre-flight check notes logged, for example. Change-Id: I340be11bc9471df22f038629679634c3542d34d6 Signed-off-by: Bogdan Dobrelya --- .gitignore | 1 + .../tripleo_logfile-237209469088b8c5.yaml | 11 +++++ tripleoclient/config/undercloud.py | 9 +++++ tripleoclient/constants.py | 2 + .../tests/config/test_config_undercloud.py | 2 + .../tests/v1/undercloud/test_undercloud.py | 40 +++++++++---------- tripleoclient/utils.py | 34 ++++++++++++++++ tripleoclient/v1/undercloud.py | 18 +++++++-- tripleoclient/v1/undercloud_config.py | 27 +++---------- tripleoclient/v1/undercloud_preflight.py | 6 ++- 10 files changed, 101 insertions(+), 49 deletions(-) create mode 100644 releasenotes/notes/tripleo_logfile-237209469088b8c5.yaml diff --git a/.gitignore b/.gitignore index 1649f470a..65171c9af 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,7 @@ lib64 # Installer logs pip-log.txt +install-undercloud.log # Unit test / coverage reports .coverage diff --git a/releasenotes/notes/tripleo_logfile-237209469088b8c5.yaml b/releasenotes/notes/tripleo_logfile-237209469088b8c5.yaml new file mode 100644 index 000000000..a99f03d98 --- /dev/null +++ b/releasenotes/notes/tripleo_logfile-237209469088b8c5.yaml @@ -0,0 +1,11 @@ +--- +security: + - | + Undercloud and tripleo standalone deployments support logging + into a log file. In ``undercloud.conf`` the log file path may be + defined via `undercloud_log_file`. For the standalone + deployments, use the ``--log-file`` commmand line argument. + + By default, undercloud pre-flight/installation/upgrade logs + will be written into ``install-undercloud.log`` in the current dir + (wherefrom the client command is executed). diff --git a/tripleoclient/config/undercloud.py b/tripleoclient/config/undercloud.py index b584c53cb..890338b7d 100644 --- a/tripleoclient/config/undercloud.py +++ b/tripleoclient/config/undercloud.py @@ -17,6 +17,9 @@ import copy from osc_lib.i18n import _ from oslo_config import cfg + +from tripleoclient import constants + from tripleoclient.config.standalone import StandaloneConfig CONF = cfg.CONF @@ -53,6 +56,12 @@ class UndercloudConfig(StandaloneConfig): def get_base_opts(self): _base_opts = super(UndercloudConfig, self).get_base_opts() _opts = [ + cfg.StrOpt('undercloud_log_file', + default=constants.UNDERCLOUD_LOG_FILE, + help=_( + 'The path to a log file to store the ' + 'undercloud install/upgrade logs.'), + ), cfg.StrOpt('undercloud_hostname', help=_( 'Fully qualified hostname (including domain) to ' diff --git a/tripleoclient/constants.py b/tripleoclient/constants.py index 0312d1807..1107c4473 100644 --- a/tripleoclient/constants.py +++ b/tripleoclient/constants.py @@ -20,6 +20,8 @@ OVERCLOUD_YAML_NAME = "overcloud.yaml" OVERCLOUD_ROLES_FILE = "roles_data.yaml" UNDERCLOUD_ROLES_FILE = "roles_data_undercloud.yaml" UNDERCLOUD_OUTPUT_DIR = os.path.join(os.environ.get('HOME')) +UNDERCLOUD_LOG_FILE = "install-undercloud.log" +UNDERCLOUD_CONF_PATH = os.path.join(UNDERCLOUD_OUTPUT_DIR, "undercloud.conf") OVERCLOUD_NETWORKS_FILE = "network_data.yaml" RHEL_REGISTRATION_EXTRACONFIG_NAME = ( "extraconfig/pre_deploy/rhel-registration/") diff --git a/tripleoclient/tests/config/test_config_undercloud.py b/tripleoclient/tests/config/test_config_undercloud.py index 18dff0595..4a2e93ed8 100644 --- a/tripleoclient/tests/config/test_config_undercloud.py +++ b/tripleoclient/tests/config/test_config_undercloud.py @@ -62,6 +62,7 @@ class TestUndercloudConfig(base.TestCase): 'undercloud_admin_host', 'undercloud_debug', 'undercloud_hostname', + 'undercloud_log_file', 'undercloud_nameservers', 'undercloud_ntp_servers', 'undercloud_public_host', @@ -119,6 +120,7 @@ class TestUndercloudConfig(base.TestCase): 'undercloud_admin_host', 'undercloud_debug', 'undercloud_hostname', + 'undercloud_log_file', 'undercloud_nameservers', 'undercloud_ntp_servers', 'undercloud_public_host', diff --git a/tripleoclient/tests/v1/undercloud/test_undercloud.py b/tripleoclient/tests/v1/undercloud/test_undercloud.py index 196ce7760..8bae39837 100644 --- a/tripleoclient/tests/v1/undercloud/test_undercloud.py +++ b/tripleoclient/tests/v1/undercloud/test_undercloud.py @@ -61,10 +61,9 @@ class TestUndercloudInstall(TestPluginV1): @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) - @mock.patch('os.getcwd', return_value='/tmp') @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_install_with_heat_custom_output(self, mock_subprocess, - mock_cwd, mock_wr, + mock_wr, mock_os, mock_copy): self.conf.config(output_dir='/foo') self.conf.config(roles_file='foo/roles.yaml') @@ -120,7 +119,7 @@ class TestUndercloudInstall(TestPluginV1): '--cleanup', '-e', '/foo/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', - '--log-file=/tmp/install-undercloud.log']) + '--log-file=install-undercloud.log']) @mock.patch('shutil.copy') @mock.patch('os.mkdir') @@ -135,10 +134,9 @@ class TestUndercloudInstall(TestPluginV1): '_get_unknown_instack_tags', return_value=None, autospec=True) @mock.patch('jinja2.meta.find_undeclared_variables', return_value={}, autospec=True) - @mock.patch('os.getcwd', return_value='/tmp') @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_install_with_heat_net_conf_over(self, mock_subprocess, - mock_cwd, mock_j2_meta, + mock_j2_meta, mock_get_unknown_tags, mock_get_j2, mock_sroutes, @@ -282,16 +280,18 @@ class TestUndercloudInstall(TestPluginV1): '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', - '--log-file=/tmp/install-undercloud.log']) + '--log-file=install-undercloud.log']) + @mock.patch('six.moves.builtins.open') @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) - @mock.patch('os.getcwd', return_value='/tmp') @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_install_with_heat_and_debug(self, mock_subprocess, - mock_cwd, mock_wr, - mock_os, mock_copy): + mock_wr, + mock_os, mock_copy, + mock_open): + self.conf.config(undercloud_log_file='/foo/bar') arglist = ['--use-heat', '--no-validations'] verifylist = [] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -341,15 +341,14 @@ class TestUndercloudInstall(TestPluginV1): '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', - '--debug', '--log-file=/tmp/install-undercloud.log']) + '--debug', '--log-file=/foo/bar']) @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) - @mock.patch('os.getcwd', return_value='/tmp') @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_install_with_swift_encryption(self, mock_subprocess, - mock_cwd, mock_wr, + mock_wr, mock_os, mock_copy): arglist = ['--use-heat', '--no-validations'] verifylist = [] @@ -402,7 +401,7 @@ class TestUndercloudInstall(TestPluginV1): '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', - '--log-file=/tmp/install-undercloud.log']) + '--log-file=install-undercloud.log']) class TestUndercloudUpgrade(TestPluginV1): @@ -442,10 +441,9 @@ class TestUndercloudUpgrade(TestPluginV1): @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) - @mock.patch('os.getcwd', return_value='/tmp') @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_upgrade_with_heat(self, mock_subprocess, - mock_cwd, mock_wr, + mock_wr, mock_os, mock_copy): arglist = ['--use-heat', '--no-validations'] verifylist = [] @@ -496,15 +494,14 @@ class TestUndercloudUpgrade(TestPluginV1): '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', - '--log-file=/tmp/install-undercloud.log']) + '--log-file=install-undercloud.log']) @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) - @mock.patch('os.getcwd', return_value='/tmp') @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_upgrade_with_heat_and_yes(self, mock_subprocess, - mock_cwd, mock_wr, + mock_wr, mock_os, mock_copy): arglist = ['--use-heat', '--no-validations', '-y'] verifylist = [] @@ -555,15 +552,14 @@ class TestUndercloudUpgrade(TestPluginV1): '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', - '--log-file=/tmp/install-undercloud.log']) + '--log-file=install-undercloud.log']) @mock.patch('shutil.copy') @mock.patch('os.mkdir') @mock.patch('tripleoclient.utils.write_env_file', autospec=True) - @mock.patch('os.getcwd', return_value='/tmp') @mock.patch('subprocess.check_call', autospec=True) def test_undercloud_upgrade_with_heat_and_debug(self, mock_subprocess, - mock_cwd, mock_wr, + mock_wr, mock_os, mock_copy): arglist = ['--use-heat', '--no-validations'] verifylist = [] @@ -617,4 +613,4 @@ class TestUndercloudUpgrade(TestPluginV1): '--output-dir=/home/stack', '--cleanup', '-e', '/home/stack/tripleo-config-generated-env-files/' 'undercloud_parameters.yaml', - '--debug', '--log-file=/tmp/install-undercloud.log']) + '--debug', '--log-file=install-undercloud.log']) diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py index 3040bb2c4..b27fda1e7 100644 --- a/tripleoclient/utils.py +++ b/tripleoclient/utils.py @@ -1150,3 +1150,37 @@ def rel_or_abs_path(tht_root, file_path): raise exceptions.DeploymentError( "Can't find path %s %s" % (file_path, path)) return path + + +def load_config(osloconf, path): + '''Load oslo config from a file path. ''' + log = logging.getLogger(__name__ + ".load_config") + conf_params = [] + if os.path.isfile(path): + conf_params += ['--config-file', path] + else: + log.warning(_('%s does not exist. Using defaults.') % path) + osloconf(conf_params) + + +def configure_logging(log, level, log_file): + '''Mimic oslo_log default levels and formatting for the logger. ''' + fhandler = logging.FileHandler(log_file) + formatter = logging.Formatter( + '%(asctime)s.%(msecs)03d %(process)d %(levelname)s ' + '%(name)s [ ] %(message)s', + '%Y-%m-%d %H:%M:%S') + + if level > 1: + log.setLevel(logging.DEBUG) + fhandler.setLevel(logging.DEBUG) + else: + # NOTE(bogdando): we are making an exception to the oslo_log'ish + # default WARN level to have INFO logs as well. Some modules + # produce INFO msgs we want to see and keep by default, like + # pre-flight valiation notes. + log.setLevel(logging.INFO) + fhandler.setLevel(logging.INFO) + + fhandler.setFormatter(formatter) + log.addHandler(fhandler) diff --git a/tripleoclient/v1/undercloud.py b/tripleoclient/v1/undercloud.py index 8b9217cf8..bb7079ebf 100644 --- a/tripleoclient/v1/undercloud.py +++ b/tripleoclient/v1/undercloud.py @@ -21,7 +21,10 @@ import subprocess from openstackclient.i18n import _ +from oslo_config import cfg + from tripleoclient import command +from tripleoclient import constants from tripleoclient import utils from tripleoclient.v1 import undercloud_config @@ -31,6 +34,7 @@ class InstallUndercloud(command.Command): auth_required = False log = logging.getLogger(__name__ + ".InstallUndercloud") + osloconfig = cfg.CONF def get_parser(self, prog_name): parser = argparse.ArgumentParser( @@ -65,6 +69,10 @@ class InstallUndercloud(command.Command): return parser def take_action(self, parsed_args): + # Fetch configuration used to add logging to a file + utils.load_config(self.osloconfig, constants.UNDERCLOUD_CONF_PATH) + utils.configure_logging(self.log, self.app_args.verbose_level, + self.osloconfig['undercloud_log_file']) self.log.debug("take_action(%s)" % parsed_args) utils.ensure_run_as_normal_user() @@ -80,9 +88,7 @@ class InstallUndercloud(command.Command): cmd = ["instack-install-undercloud"] self.log.warning("Running: %s" % ' '.join(cmd)) - if parsed_args.dry_run: - print(' '.join(cmd)) - else: + if not parsed_args.dry_run: subprocess.check_call(cmd) @@ -91,8 +97,13 @@ class UpgradeUndercloud(InstallUndercloud): auth_required = False log = logging.getLogger(__name__ + ".UpgradeUndercloud") + osloconfig = cfg.CONF def take_action(self, parsed_args): + # Fetch configuration used to add logging to a file + utils.load_config(self.osloconfig, constants.UNDERCLOUD_CONF_PATH) + utils.configure_logging(self.log, self.app_args.verbose_level, + self.osloconfig['undercloud_log_file']) self.log.debug("take action(%s)" % parsed_args) utils.ensure_run_as_normal_user() @@ -104,7 +115,6 @@ class UpgradeUndercloud(InstallUndercloud): no_validations=parsed_args. no_validations, verbose_level=self.app_args.verbose_level) - print("Running: %s" % ' '.join(cmd)) self.log.warning("Running: %s" % ' '.join(cmd)) subprocess.check_call(cmd) else: diff --git a/tripleoclient/v1/undercloud_config.py b/tripleoclient/v1/undercloud_config.py index 295564e2b..7de503ae6 100644 --- a/tripleoclient/v1/undercloud_config.py +++ b/tripleoclient/v1/undercloud_config.py @@ -86,16 +86,7 @@ TELEMETRY_DOCKER_ENV_YAML = [ 'environments/services/undercloud-panko.yaml', 'environments/services/undercloud-ceilometer.yaml'] - -class Paths(object): - @property - def CONF_PATH(self): - return os.path.expanduser('~/undercloud.conf') - - CONF = cfg.CONF -PATHS = Paths() - # When adding new options to the lists below, make sure to regenerate the # sample config by running "tox -e genconfig" in the project root. @@ -116,15 +107,6 @@ def _load_subnets_config_groups(): LOG = logging.getLogger(__name__ + ".undercloud_config") -def _load_config(): - conf_params = [] - if os.path.isfile(PATHS.CONF_PATH): - conf_params += ['--config-file', PATHS.CONF_PATH] - else: - LOG.warning(_('%s does not exist. Using defaults.') % PATHS.CONF_PATH) - CONF(conf_params) - - def _get_jinja_env_source(f): path, filename = os.path.split(f) env = jinja2.Environment(loader=jinja2.FileSystemLoader(path)) @@ -264,7 +246,9 @@ def prepare_undercloud_deploy(upgrade=False, no_validations=False, env_data = {} registry_overwrites = {} deploy_args = [] - _load_config() + # Fetch configuration and use its log file param to add logging to a file + utils.load_config(CONF, constants.UNDERCLOUD_CONF_PATH) + utils.configure_logging(LOG, verbose_level, CONF['undercloud_log_file']) _load_subnets_config_groups() # NOTE(bogdando): the generated env files are stored another path then @@ -555,15 +539,14 @@ def prepare_undercloud_deploy(upgrade=False, no_validations=False, deploy_args += ['--hieradata-override=%s' % data_file] if CONF.get('enable_validations') and not no_validations: - undercloud_preflight.check() + undercloud_preflight.check(verbose_level) deploy_args += ['-e', os.path.join( tht_templates, "environments/tripleo-validations.yaml")] if verbose_level > 1: deploy_args.append('--debug') - LOG_FILE = os.path.join(os.getcwd() + '/install-undercloud.log') - deploy_args.append('--log-file=' + LOG_FILE) + deploy_args.append('--log-file=%s' % CONF['undercloud_log_file']) cmd = ["sudo", "openstack", "tripleo", "deploy", "--standalone", "--standalone-role", "Undercloud"] diff --git a/tripleoclient/v1/undercloud_preflight.py b/tripleoclient/v1/undercloud_preflight.py index 69f980ced..77033ec40 100644 --- a/tripleoclient/v1/undercloud_preflight.py +++ b/tripleoclient/v1/undercloud_preflight.py @@ -28,6 +28,7 @@ import psutil from oslo_config import cfg from tripleoclient import constants +from tripleoclient import utils class FailedValidation(Exception): @@ -428,7 +429,10 @@ def _run_yum_update(instack_env): LOG.info(_('yum-update completed successfully')) -def check(): +def check(verbose_level): + # Fetch configuration and use its log file param to add logging to a file + utils.load_config(CONF, constants.UNDERCLOUD_CONF_PATH) + utils.configure_logging(LOG, verbose_level, CONF['undercloud_log_file']) # data = {opt.name: CONF[opt.name] for opt in _opts} try: