From 095142c40d00746c3d45e9cceac90b85022bfbbb Mon Sep 17 00:00:00 2001 From: Saad Zaher Date: Wed, 2 Dec 2015 19:37:36 +0000 Subject: [PATCH] Switch freezer-scheduler to oslo.config and oslo.log switch freezer-scheduler to use oslo.config and switch from native python logging module to oslo.log This commit includes: - using oslo.config for parsing cli and config files options - using oslo.log instead of native python logging module - this applied only on freezer-scheduler Implements: blueprint using-oslo-libs Change-Id: I92e99c087cb2c2f836770644621f711af597dffc --- HACKING.rst | 22 +++ freezer/__init__.py | 20 +++ freezer/apiclient/client.py | 159 +++++++++++----------- freezer/scheduler/arguments.py | 180 ++++++++++++++----------- freezer/scheduler/freezer_scheduler.py | 73 +++++----- requirements.txt | 2 + tests/test_apiclient_client.py | 11 -- tests/test_scheduler_arguments.py | 40 ------ 8 files changed, 267 insertions(+), 240 deletions(-) create mode 100644 HACKING.rst delete mode 100644 tests/test_scheduler_arguments.py diff --git a/HACKING.rst b/HACKING.rst new file mode 100644 index 00000000..b8ccb568 --- /dev/null +++ b/HACKING.rst @@ -0,0 +1,22 @@ +Freezer Style Commandments +=========================== + +- Step 1: Read the OpenStack Style Commandments + http://docs.openstack.org/developer/hacking/ +- Step 2: Read on + +Freezer Specific Commandments +------------------------------ + + +Logging +------- + +Use the common logging module, and ensure you ``getLogger``:: + + from oslo_log import log + + LOG = log.getLogger(__name__) + + LOG.debug('Foobar') + diff --git a/freezer/__init__.py b/freezer/__init__.py index e69de29b..2a6bbd8e 100644 --- a/freezer/__init__.py +++ b/freezer/__init__.py @@ -0,0 +1,20 @@ +# (c) Copyright 2014,2015 Hewlett-Packard Development Company, L.P. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Freezer Versions + +import pbr.version + + +__version__ = pbr.version.VersionInfo('freezer').version_string() diff --git a/freezer/apiclient/client.py b/freezer/apiclient/client.py index 3d707212..a05c3b62 100644 --- a/freezer/apiclient/client.py +++ b/freezer/apiclient/client.py @@ -29,6 +29,9 @@ from registration import RegistrationManager from jobs import JobManager from actions import ActionManager from sessions import SessionManager +from oslo_config import cfg + +CONF = cfg.CONF FREEZER_SERVICE_TYPE = 'backup' @@ -55,84 +58,82 @@ class cached_property(object): return value -def build_os_option_parser(parser): - parser.add_argument( - '--os-username', action='store', - help=('Name used for authentication with the OpenStack ' - 'Identity service. Defaults to env[OS_USERNAME].'), - dest='os_username', default=env('OS_USERNAME')) - parser.add_argument( - '--os-password', action='store', - help=('Password used for authentication with the OpenStack ' - 'Identity service. Defaults to env[OS_PASSWORD].'), - dest='os_password', default=env('OS_PASSWORD')) - parser.add_argument( - '--os-project-name', action='store', - help=('Project name to scope to. Defaults to ' - 'env[OS_PROJECT_NAME].'), - dest='os_project_name', - default=env('OS_PROJECT_NAME', default='default')) - parser.add_argument( - '--os-project-domain-name', action='store', - help=('Domain name containing project. Defaults to ' - 'env[OS_PROJECT_DOMAIN_NAME].'), - dest='os_project_domain_name', default=env('OS_PROJECT_DOMAIN_NAME', - default='default')) - parser.add_argument( - '--os-user-domain-name', action='store', - help=('User\'s domain name. Defaults to ' - 'env[OS_USER_DOMAIN_NAME].'), - dest='os_user_domain_name', default=env('OS_USER_DOMAIN_NAME', - default='default')) - parser.add_argument( - '--os-tenant-name', action='store', - help=('Tenant to request authorization on. Defaults to ' - 'env[OS_TENANT_NAME].'), - dest='os_tenant_name', default=env('OS_TENANT_NAME')) - parser.add_argument( - '--os-tenant-id', action='store', - help=('Tenant to request authorization on. Defaults to ' - 'env[OS_TENANT_ID].'), - dest='os_tenant_id', default=env('OS_TENANT_ID')) - parser.add_argument( - '--os-auth-url', action='store', - help=('Specify the Identity endpoint to use for ' - 'authentication. Defaults to env[OS_AUTH_URL].'), - dest='os_auth_url', default=env('OS_AUTH_URL')) - parser.add_argument( - '--os-backup-url', action='store', - help=('Specify the Freezer backup service endpoint to use. ' - 'Defaults to env[OS_BACKUP_URL].'), - dest='os_backup_url', default=env('OS_BACKUP_URL')) - parser.add_argument( - '--os-region-name', action='store', - help=('Specify the region to use. Defaults to ' - 'env[OS_REGION_NAME].'), - dest='os_region_name', default=env('OS_REGION_NAME')) - parser.add_argument( - '--os-token', action='store', - help=('Specify an existing token to use instead of retrieving' - ' one via authentication (e.g. with username & password). ' - 'Defaults to env[OS_TOKEN].'), - dest='os_token', default=env('OS_TOKEN')) - parser.add_argument( - '--os-identity-api-version', action='store', - help=('Identity API version: 2.0 or 3. ' - 'Defaults to env[OS_IDENTITY_API_VERSION]'), - dest='os_identity_api_version', - default=env('OS_IDENTITY_API_VERSION')) - parser.add_argument( - '--os-endpoint-type', action='store', - choices=['public', 'publicURL', 'internal', 'internalURL', - 'admin', 'adminURL'], - help=('Endpoint type to select. ' - 'Valid endpoint types: "public" or "publicURL", ' - '"internal" or "internalURL", "admin" or "adminURL". ' - 'Defaults to env[OS_ENDPOINT_TYPE] or "public"'), - dest='os_endpoint_type', - default=env('OS_ENDPOINT_TYPE', default='public')) +def build_os_options(): + osclient_opts = [ + cfg.StrOpt('os-username', + default=env('OS_USERNAME'), + help='Name used for authentication with the OpenStack ' + 'Identity service. Defaults to env[OS_USERNAME].', + dest='os_username'), + cfg.StrOpt('os-password', + default=env('OS_PASSWORD'), + help='Password used for authentication with the OpenStack ' + 'Identity service. Defaults to env[OS_PASSWORD].', + dest='os_password'), + cfg.StrOpt('os-project-name', + default=env('OS_PROJECT_NAME'), + help='Project name to scope to. Defaults to ' + 'env[OS_PROJECT_NAME].', + dest='os_project_name'), + cfg.StrOpt('os-project-domain-name', + default=env('OS_PROJECT_DOMAIN_NAME'), + help='Domain name containing project. Defaults to ' + 'env[OS_PROJECT_DOMAIN_NAME].', + dest='os_project_domain_name'), + cfg.StrOpt('os-user-domain-name', + default=env('OS_USER_DOMAIN_NAME'), + help='User\'s domain name. Defaults to ' + 'env[OS_USER_DOMAIN_NAME].', + dest='os_user_domain_name'), + cfg.StrOpt('os-tenant-name', + default=env('OS_TENANT_NAME'), + help='Tenant to request authorization on. Defaults to ' + 'env[OS_TENANT_NAME].', + dest='os_tenant_name'), + cfg.StrOpt('os-tenant-id', + default=env('OS_TENANT_ID'), + help='Tenant to request authorization on. Defaults to ' + 'env[OS_TENANT_ID].', + dest='os_tenant_id'), + cfg.StrOpt('os-auth-url', + default=env('OS_AUTH_URL'), + help='Specify the Identity endpoint to use for ' + 'authentication. Defaults to env[OS_AUTH_URL].', + dest='os_auth_url'), + cfg.StrOpt('os-backup-url', + default=env('OS_BACKUP_URL'), + help='Specify the Freezer backup service endpoint to use. ' + 'Defaults to env[OS_BACKUP_URL].', + dest='os_backup_url'), + cfg.StrOpt('os-region-name', + default=env('OS_REGION_NAME'), + help='Specify the region to use. Defaults to ' + 'env[OS_REGION_NAME].', + dest='os_region_name'), + cfg.StrOpt('os-token', + default=env('OS_TOKEN'), + help='Specify an existing token to use instead of retrieving' + ' one via authentication (e.g. with username & ' + 'password). Defaults to env[OS_TOKEN].', + dest='os_token'), + cfg.StrOpt('os-identity-api-version', + default=env('OS_IDENTITY_API_VERSION'), + help='Identity API version: 2.0 or 3. ' + 'Defaults to env[OS_IDENTITY_API_VERSION]', + dest='os_identity_api_version'), + cfg.StrOpt('os-endpoint-type', + choices=['public', 'publicURL', 'internal', 'internalURL', + 'admin', 'adminURL'], + default=env('OS_ENDPOINT_TYPE') or 'public', + help='Endpoint type to select. Valid endpoint types: ' + '"public" or "publicURL", "internal" or "internalURL",' + ' "admin" or "adminURL". Defaults to ' + 'env[OS_ENDPOINT_TYPE] or "public"', + dest='os_endpoint_type'), - return parser + ] + + return osclient_opts def guess_auth_version(opts): @@ -193,9 +194,7 @@ class Client(object): project_domain_name=None, verify=True): - self.opts = opts or build_os_option_parser( - argparse.ArgumentParser(description='Freezer Client') - ).parse_known_args()[0] + self.opts = opts if token: self.opts.os_token = token if username: diff --git a/freezer/scheduler/arguments.py b/freezer/scheduler/arguments.py index 3e16bfa3..f633a6a8 100644 --- a/freezer/scheduler/arguments.py +++ b/freezer/scheduler/arguments.py @@ -17,6 +17,14 @@ limitations under the License. import argparse import os +from oslo_config import cfg +from oslo_log import log +from freezer import __version__ as FREEZER_VERSION +import sys +CONF = cfg.CONF +_LOG = log.getLogger(__name__) + + from freezer.apiclient import client as api_client from freezer import winutils @@ -27,88 +35,102 @@ else: DEFAULT_FREEZER_SCHEDULER_CONF_D = '/etc/freezer/scheduler/conf.d' -def base_parser(parser): +def getCommonOpts(): scheduler_conf_d = os.environ.get('FREEZER_SCHEDULER_CONF_D', DEFAULT_FREEZER_SCHEDULER_CONF_D) - parser.add_argument( - '-j', '--job', action='store', - help=('name or ID of the job'), - dest='job_id', default=None) - parser.add_argument( - '-s', '--session', action='store', - help=('name or ID of the session'), - dest='session_id', default=None) - parser.add_argument( - '--file', action='store', - help=('Local file that contains the resource ' - 'to be uploaded/downloaded'), - dest='fname', default=None) - parser.add_argument( - '-c', '--client-id', action='store', - help=('Specifies the client_id used when contacting the service.' - 'If not specified it will be automatically created' - 'using the tenant-id and the machine hostname.'), - dest='client_id', default=None) - parser.add_argument( - '-n', '--no-api', action='store_true', - help='Prevents the scheduler from using the api service', - dest='no_api', default=False) - parser.add_argument( - '-a', '--active-only', action='store_true', - help='Filter only active jobs/session', - dest='active_only', default=False) - parser.add_argument( - '-f', '--conf', action='store', - help=('Used to store/retrieve files on local storage, including ' - 'those exchanged with the api service. ' - 'Default value is {0} ' - '(Env: FREEZER_SCHEDULER_CONF_D)'.format(scheduler_conf_d)), - dest='jobs_dir', default=scheduler_conf_d) - parser.add_argument( - '-i', '--interval', action='store', - help=('Specifies the api-polling interval in seconds.' - 'Defaults to 60 seconds'), - dest='interval', default=60) - parser.add_argument( - '-v', '--verbose', - action='count', - dest='verbose_level', - default=1, - help='Increase verbosity of output. Can be repeated.', - ) - parser.add_argument( - '--debug', - default=False, - action='store_true', - help='show tracebacks on errors', - ) - parser.add_argument( - '--no-daemon', - action='store_true', - help='Prevents the scheduler from running in daemon mode', - dest='no_daemon', default=False - ) - parser.add_argument( - '-l', '--log-file', action='store', - help=('location of log file'), - dest='log_file', default=None) + common_opts = [ + cfg.StrOpt('job', + default=None, + dest='job_id', + short='j', + help='Name or ID of the job'), + cfg.StrOpt('session', + default=None, + dest='session_id', + short='s', + help='Name or ID of the session'), + cfg.StrOpt('file', + default=None, + dest='fname', + help='Local file that contains the resource to be ' + 'uploaded/downloaded'), + cfg.StrOpt('client-id', + default=None, + dest='client_id', + short='c', + help='Specifies the client_id used when contacting the service.' + '\n If not specified it will be automatically created \n' + 'using the tenant-id and the machine hostname.'), + cfg.BoolOpt('no-api', + default=False, + dest='no_api', + short='n', + help='Prevents the scheduler from using the api service'), + cfg.BoolOpt('active-only', + default=False, + dest='active_only', + short='a', + help='Filter only active jobs/session'), + cfg.StrOpt('conf', + default=scheduler_conf_d, + dest='jobs_dir', + short='f', + help='Used to store/retrieve files on local storage, including ' + 'those exchanged with the api service.Default value is {0} ' + '(Env: FREEZER_SCHEDULER_CONF_D)'.format(scheduler_conf_d)), + cfg.IntOpt('interval', + default=60, + dest='interval', + short='i', + help='Specifies the api-polling interval in seconds. ' + 'Defaults to 60 seconds'), + cfg.BoolOpt('no-daemon', + default=False, + dest='no_daemon', + help='Prevents the scheduler from running in daemon mode'), + cfg.BoolOpt('insecure', + default=False, + dest='insecure', + help='Initialize freezer scheduler with insecure mode'), + ] - parser.add_argument( - '--insecure', - action='store_true', - help='Initialize freezer scheduler with insecure mode', - dest='insecure', default=False - ) - - return parser + return common_opts -def get_args(choices): - parser = base_parser( - api_client.build_os_option_parser( - argparse.ArgumentParser(description='Freezer Scheduler') - )) - parser.add_argument( - 'action', action='store', default=None, choices=choices, help='') - return parser.parse_args() +def parse_args(choices): + default_conf = cfg.find_config_files('freezer', 'freezer-scheduler', + '.conf') + CONF.register_cli_opts(api_client.build_os_options()) + CONF.register_cli_opts(getCommonOpts()) + log.register_options(CONF) + + positional = [ + cfg.StrOpt('action', + choices=choices, + default=None, + help='{0}'.format(choices), positional=True), + + ] + CONF.register_cli_opts(positional) + CONF(args=sys.argv[1:], + project='freezer-scheduler', + default_config_files=default_conf, + version=FREEZER_VERSION + ) + + +def setup_logging(): + _DEFAULT_LOG_LEVELS = ['amqp=WARN', 'amqplib=WARN', 'boto=WARN', + 'qpid=WARN', 'stevedore=WARN', + 'oslo_log=INFO', 'iso8601=WARN', + 'requests.packages.urllib3.connectionpool=WARN', + 'urllib3.connectionpool=WARN', 'websocket=WARN', + 'keystonemiddleware=WARN', 'freezer=INFO'] + + _DEFAULT_LOGGING_CONTEXT_FORMAT = ('%(asctime)s.%(msecs)03d %(process)d ' + '%(levelname)s %(name)s [%(request_id)s ' + '%(user_identity)s] %(instance)s' + '%(message)s') + log.set_defaults(_DEFAULT_LOGGING_CONTEXT_FORMAT, _DEFAULT_LOG_LEVELS) + log.setup(CONF, 'freezer-scheduler', version=FREEZER_VERSION) diff --git a/freezer/scheduler/freezer_scheduler.py b/freezer/scheduler/freezer_scheduler.py index da616235..86655162 100755 --- a/freezer/scheduler/freezer_scheduler.py +++ b/freezer/scheduler/freezer_scheduler.py @@ -15,8 +15,6 @@ See the License for the specific language governing permissions and limitations under the License. """ - -import logging import os import sys import threading @@ -36,17 +34,24 @@ if winutils.is_windows(): else: from daemon import Daemon, NoDaemon from scheduler_job import Job +from oslo_config import cfg +from oslo_log import log + + +CONF = cfg.CONF +LOG = log.getLogger(__name__) class FreezerScheduler(object): def __init__(self, apiclient, interval, job_path): # config_manager self.client = apiclient - self.freezerc_executable = spawn.find_executable('freezerc') + self.freezerc_executable = spawn.find_executable('freezer-agent') if self.freezerc_executable is None: # Needed in the case of a non-activated virtualenv self.freezerc_executable = spawn.find_executable( - 'freezerc', path=':'.join(sys.path)) + 'freezer-agent', path=':'.join(sys.path)) + LOG.debug('Freezer-agent found at {0}'.format(self.freezerc_executable)) self.job_path = job_path self._client = None self.lock = threading.Lock() @@ -70,7 +75,7 @@ class FreezerScheduler(object): try: utils.save_jobs_to_disk(job_doc_list, self.job_path) except Exception as e: - logging.error('Unable to save jobs to {0}. ' + LOG.error('Unable to save jobs to {0}. ' '{1}'.format(self.job_path, e)) return job_doc_list else: @@ -107,7 +112,7 @@ class FreezerScheduler(object): try: return self.client.jobs.update(job_id, job_doc) except Exception as e: - logging.error("[*] Job update error: {0}".format(e)) + LOG.error("[*] Job update error: {0}".format(e)) def update_job_status(self, job_id, status): doc = {'job_schedule': {'status': status}} @@ -120,14 +125,14 @@ class FreezerScheduler(object): job = Job.create(self, self.freezerc_executable, job_doc) if job: self.jobs[job.id] = job - logging.info("Created job {0}".format(job.id)) + LOG.info("Created job {0}".format(job.id)) return job def poll(self): try: work_job_doc_list = self.get_jobs() except Exception as e: - logging.error("[*] Unable to get jobs: {0}".format(e)) + LOG.error("[*] Unable to get jobs: {0}".format(e)) return work_job_id_list = [] @@ -158,7 +163,7 @@ class FreezerScheduler(object): pass def reload(self): - logging.warning("reload not supported") + LOG.warning("reload not supported") def _get_doers(module): @@ -176,56 +181,64 @@ def main(): possible_actions = doers.keys() + ['start', 'stop', 'status'] - args = arguments.get_args(possible_actions) + arguments.parse_args(possible_actions) + arguments.setup_logging() - if args.action is None: - print ('No action') + if CONF.action is None: + CONF.print_help() return 65 # os.EX_DATAERR apiclient = None verify = True - if args.insecure: + if CONF.insecure: verify = False - if args.no_api is False: - apiclient = client.Client(opts=args, verify=verify) - if args.client_id: - apiclient.client_id = args.client_id + if CONF.no_api is False: + try: + apiclient = client.Client(opts=CONF, verify=verify) + if CONF.client_id: + apiclient.client_id = CONF.client_id + except Exception as e: + LOG.error(e) + print e + sys.exit(1) else: if winutils.is_windows(): print("--no-api mode is not available on windows") return 69 # os.EX_UNAVAILABLE - if args.action in doers: + if CONF.action in doers: try: - return doers[args.action](apiclient, args) + return doers[CONF.action](apiclient, CONF) except Exception as e: + LOG.error(e) print ('ERROR {0}'.format(e)) return 70 # os.EX_SOFTWARE freezer_scheduler = FreezerScheduler(apiclient=apiclient, - interval=int(args.interval), - job_path=args.jobs_dir) + interval=int(CONF.interval), + job_path=CONF.jobs_dir) - if args.no_daemon: + if CONF.no_daemon: print ('Freezer Scheduler running in no-daemon mode') + LOG.debug('Freezer Scheduler running in no-daemon mode') daemon = NoDaemon(daemonizable=freezer_scheduler) else: if winutils.is_windows(): daemon = Daemon(daemonizable=freezer_scheduler, - interval=int(args.interval), - job_path=args.jobs_dir, - insecure=args.insecure) + interval=int(CONF.interval), + job_path=CONF.jobs_dir, + insecure=CONF.insecure) else: daemon = Daemon(daemonizable=freezer_scheduler) - if args.action == 'start': - daemon.start(log_file=args.log_file) - elif args.action == 'stop': + if CONF.action == 'start': + daemon.start(log_file=CONF.log_file) + elif CONF.action == 'stop': daemon.stop() - elif args.action == 'reload': + elif CONF.action == 'reload': daemon.reload() - elif args.action == 'status': + elif CONF.action == 'status': daemon.status() # os.RETURN_CODES are only available to posix like systems, on windows diff --git a/requirements.txt b/requirements.txt index 8943f282..8846bb86 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,6 +9,8 @@ python-novaclient>=2.28.1,!=2.33.0 python-openstackclient>=1.5.0 oslo.utils>=2.0.0,!=2.6.0 # Apache-2.0 oslo.i18n>=1.5.0 # Apache-2.0 +oslo.log>=1.8.0 # Apache-2.0 +oslo.config>=2.3.0 # Apache-2.0 PyMySQL>=0.6.2 # MIT License pymongo>=3.0.2 diff --git a/tests/test_apiclient_client.py b/tests/test_apiclient_client.py index 9c5470c1..f5bf6d69 100644 --- a/tests/test_apiclient_client.py +++ b/tests/test_apiclient_client.py @@ -35,17 +35,6 @@ class TestSupportFunctions(unittest.TestCase): var = client.env('TEST_ENV_VAR') self.assertEquals(var, '') - @patch('freezer.apiclient.client.env') - def test_build_os_option_parser(self, mock_env): - mock_env.return_value = '' - mock_parser = Mock() - mock_parser._me = 'test12345' - retval = client.build_os_option_parser(mock_parser) - self.assertEquals(retval._me, 'test12345') - - call_count = mock_parser.add_argument.call_count - self.assertGreater(call_count, 10) - def test_guess_auth_version_returns_none(self): mock_opts = Mock() mock_opts.os_identity_api_version = '' diff --git a/tests/test_scheduler_arguments.py b/tests/test_scheduler_arguments.py deleted file mode 100644 index da76c7d0..00000000 --- a/tests/test_scheduler_arguments.py +++ /dev/null @@ -1,40 +0,0 @@ -# (c) Copyright 2014,2015 Hewlett-Packard Development Company, L.P. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - - -import unittest -from mock import Mock, patch - -from freezer.scheduler import arguments - - -class TestBaseParser(unittest.TestCase): - - def test_returns_parser(self): - mock_parser = Mock() - mock_parser._me = 'test12345' - retval = arguments.base_parser(mock_parser) - self.assertEquals(retval._me, 'test12345') - - call_count = mock_parser.add_argument.call_count - self.assertGreater(call_count, 10) - - @patch('freezer.scheduler.arguments.base_parser') - def test_get_args_return_parsed_args(self, mock_base_parser): - mock_parser = Mock() - mock_parser.parse_args.return_value = 'pluto' - mock_base_parser.return_value = mock_parser - retval = arguments.get_args(['alpha', 'bravo']) - call_count = mock_parser.add_argument.call_count - self.assertGreater(call_count, 0)