From 91601d788e414be3766d574c0dc67d4592de80fb Mon Sep 17 00:00:00 2001 From: Tristan Cacqueray Date: Thu, 15 Jun 2017 06:00:12 +0000 Subject: [PATCH] config: refactor config get default This change adds a new get_default library procedure to simplify getting default value of config object. Change-Id: I0546b1175b259472a10690273af611ef4bad5a99 --- zuul/cmd/client.py | 21 +++--------- zuul/cmd/executor.py | 26 ++++++--------- zuul/cmd/merger.py | 14 ++++---- zuul/cmd/scheduler.py | 51 ++++++++--------------------- zuul/executor/client.py | 22 +++---------- zuul/executor/server.py | 71 +++++++++++------------------------------ zuul/lib/config.py | 23 +++++++++++++ zuul/merger/client.py | 21 +++--------- zuul/merger/server.py | 39 ++++++---------------- zuul/rpclistener.py | 21 +++--------- zuul/scheduler.py | 22 ++++--------- 11 files changed, 106 insertions(+), 225 deletions(-) mode change 100644 => 100755 zuul/cmd/client.py create mode 100644 zuul/lib/config.py diff --git a/zuul/cmd/client.py b/zuul/cmd/client.py old mode 100644 new mode 100755 index 94414f2c5f..dec15e756f --- a/zuul/cmd/client.py +++ b/zuul/cmd/client.py @@ -25,6 +25,7 @@ import time import zuul.rpcclient import zuul.cmd +from zuul.lib.config import get_default class Client(zuul.cmd.ZuulApp): @@ -122,22 +123,10 @@ class Client(zuul.cmd.ZuulApp): self.setup_logging() self.server = self.config.get('gearman', 'server') - if self.config.has_option('gearman', 'port'): - self.port = self.config.get('gearman', 'port') - else: - self.port = 4730 - if self.config.has_option('gearman', 'ssl_key'): - self.ssl_key = self.config.get('gearman', 'ssl_key') - else: - self.ssl_key = None - if self.config.has_option('gearman', 'ssl_cert'): - self.ssl_cert = self.config.get('gearman', 'ssl_cert') - else: - self.ssl_cert = None - if self.config.has_option('gearman', 'ssl_ca'): - self.ssl_ca = self.config.get('gearman', 'ssl_ca') - else: - self.ssl_ca = None + self.port = get_default(self.config, 'gearman', 'port', 4730) + self.ssl_key = get_default(self.config, 'gearman', 'ssl_key') + self.ssl_cert = get_default(self.config, 'gearman', 'ssl_cert') + self.ssl_ca = get_default(self.config, 'gearman', 'ssl_ca') if self.args.func(): sys.exit(0) diff --git a/zuul/cmd/executor.py b/zuul/cmd/executor.py index 7cc8dd8ffd..57ecfa3bfd 100755 --- a/zuul/cmd/executor.py +++ b/zuul/cmd/executor.py @@ -32,6 +32,7 @@ import tempfile import zuul.cmd import zuul.executor.server +from zuul.lib.config import get_default # No zuul imports that pull in paramiko here; it must not be # imported until after the daemonization. @@ -63,11 +64,8 @@ class Executor(zuul.cmd.ZuulApp): self.args = parser.parse_args() def send_command(self, cmd): - if self.config.has_option('zuul', 'state_dir'): - state_dir = os.path.expanduser( - self.config.get('zuul', 'state_dir')) - else: - state_dir = '/var/lib/zuul' + state_dir = get_default(self.config, 'zuul', 'state_dir', + '/var/lib/zuul', expand_user=True) path = os.path.join(state_dir, 'executor.socket') s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) s.connect(path) @@ -114,10 +112,7 @@ class Executor(zuul.cmd.ZuulApp): def main(self, daemon=True): # See comment at top of file about zuul imports - if self.config.has_option('executor', 'user'): - self.user = self.config.get('executor', 'user') - else: - self.user = 'zuul' + self.user = get_default(self.config, 'executor', 'user', 'zuul') if self.config.has_option('zuul', 'jobroot_dir'): self.jobroot_dir = os.path.expanduser( @@ -132,10 +127,8 @@ class Executor(zuul.cmd.ZuulApp): self.setup_logging('executor', 'log_config') self.log = logging.getLogger("zuul.Executor") - if self.config.has_option('executor', 'finger_port'): - self.finger_port = int(self.config.get('executor', 'finger_port')) - else: - self.finger_port = DEFAULT_FINGER_PORT + self.finger_port = int(get_default(self.config, 'executor', + 'finger_port', DEFAULT_FINGER_PORT)) self.start_log_streamer() self.change_privs() @@ -170,10 +163,9 @@ def main(): server.configure_connections(source_only=True) - if server.config.has_option('executor', 'pidfile'): - pid_fn = os.path.expanduser(server.config.get('executor', 'pidfile')) - else: - pid_fn = '/var/run/zuul-executor/zuul-executor.pid' + pid_fn = get_default(server.config, 'executor', 'pidfile', + '/var/run/zuul-executor/zuul-executor.pid', + expand_user=True) pid = pid_file_module.TimeoutPIDLockFile(pid_fn, 10) if server.args.nodaemon: diff --git a/zuul/cmd/merger.py b/zuul/cmd/merger.py index 686f34a12c..97f208c47e 100755 --- a/zuul/cmd/merger.py +++ b/zuul/cmd/merger.py @@ -27,6 +27,7 @@ import sys import signal import zuul.cmd +from zuul.lib.config import get_default # No zuul imports here because they pull in paramiko which must not be # imported until after the daemonization. @@ -79,10 +80,8 @@ def main(): server.read_config() server.configure_connections(source_only=True) - if server.config.has_option('zuul', 'state_dir'): - state_dir = os.path.expanduser(server.config.get('zuul', 'state_dir')) - else: - state_dir = '/var/lib/zuul' + state_dir = get_default(server.config, 'zuul', 'state_dir', + '/var/lib/zuul', expand_user=True) test_fn = os.path.join(state_dir, 'test') try: f = open(test_fn, 'w') @@ -92,10 +91,9 @@ def main(): print("\nUnable to write to state directory: %s\n" % state_dir) raise - if server.config.has_option('merger', 'pidfile'): - pid_fn = os.path.expanduser(server.config.get('merger', 'pidfile')) - else: - pid_fn = '/var/run/zuul-merger/zuul-merger.pid' + pid_fn = get_default(server.config, 'merger', 'pidfile', + '/var/run/zuul-merger/zuul-merger.pid', + expand_user=True) pid = pid_file_module.TimeoutPIDLockFile(pid_fn, 10) if server.args.nodaemon: diff --git a/zuul/cmd/scheduler.py b/zuul/cmd/scheduler.py index d16eb17dd1..b32deaf042 100755 --- a/zuul/cmd/scheduler.py +++ b/zuul/cmd/scheduler.py @@ -28,6 +28,7 @@ import sys import signal import zuul.cmd +from zuul.lib.config import get_default # No zuul imports here because they pull in paramiko which must not be # imported until after the daemonization. @@ -98,22 +99,10 @@ class Scheduler(zuul.cmd.ZuulApp): import zuul.lib.gearserver statsd_host = os.environ.get('STATSD_HOST') statsd_port = int(os.environ.get('STATSD_PORT', 8125)) - if self.config.has_option('gearman_server', 'listen_address'): - host = self.config.get('gearman_server', 'listen_address') - else: - host = None - if self.config.has_option('gearman_server', 'ssl_key'): - ssl_key = self.config.get('gearman_server', 'ssl_key') - else: - ssl_key = None - if self.config.has_option('gearman_server', 'ssl_cert'): - ssl_cert = self.config.get('gearman_server', 'ssl_cert') - else: - ssl_cert = None - if self.config.has_option('gearman_server', 'ssl_ca'): - ssl_ca = self.config.get('gearman_server', 'ssl_ca') - else: - ssl_ca = None + host = get_default(self.config, 'gearman_server', 'listen_address') + ssl_key = get_default(self.config, 'gearman_server', 'ssl_key') + ssl_cert = get_default(self.config, 'gearman_server', 'ssl_cert') + ssl_ca = get_default(self.config, 'gearman_server', 'ssl_ca') zuul.lib.gearserver.GearServer(4730, ssl_key=ssl_key, ssl_cert=ssl_cert, @@ -161,27 +150,16 @@ class Scheduler(zuul.cmd.ZuulApp): nodepool = zuul.nodepool.Nodepool(self.sched) zookeeper = zuul.zk.ZooKeeper() - if self.config.has_option('zuul', 'zookeeper_hosts'): - zookeeper_hosts = self.config.get('zuul', 'zookeeper_hosts') - else: - zookeeper_hosts = '127.0.0.1:2181' + zookeeper_hosts = get_default(self.config, 'zuul', 'zookeeper_hosts', + '127.0.0.1:2181') zookeeper.connect(zookeeper_hosts) - if self.config.has_option('zuul', 'status_expiry'): - cache_expiry = self.config.getint('zuul', 'status_expiry') - else: - cache_expiry = 1 + cache_expiry = get_default(self.config, 'zuul', 'status_expiry', 1) - if self.config.has_option('webapp', 'listen_address'): - listen_address = self.config.get('webapp', 'listen_address') - else: - listen_address = '0.0.0.0' - - if self.config.has_option('webapp', 'port'): - port = self.config.getint('webapp', 'port') - else: - port = 8001 + listen_address = get_default(self.config, 'webapp', 'listen_address', + '0.0.0.0') + port = get_default(self.config, 'webapp', 'port', 8001) webapp = zuul.webapp.WebApp( self.sched, port=port, cache_expiry=cache_expiry, @@ -230,10 +208,9 @@ def main(): if scheduler.args.validate: sys.exit(scheduler.test_config()) - if scheduler.config.has_option('zuul', 'pidfile'): - pid_fn = os.path.expanduser(scheduler.config.get('zuul', 'pidfile')) - else: - pid_fn = '/var/run/zuul-scheduler/zuul-scheduler.pid' + pid_fn = get_default(scheduler.config, 'zuul', 'pidfile', + '/var/run/zuul-scheduler/zuul-scheduler.pid', + expand_user=True) pid = pid_file_module.TimeoutPIDLockFile(pid_fn, 10) if scheduler.args.nodaemon: diff --git a/zuul/executor/client.py b/zuul/executor/client.py index 6ecb27cc91..2e17b3eec3 100644 --- a/zuul/executor/client.py +++ b/zuul/executor/client.py @@ -21,6 +21,7 @@ import threading from uuid import uuid4 import zuul.model +from zuul.lib.config import get_default from zuul.model import Build @@ -115,23 +116,10 @@ class ExecutorClient(object): self.meta_jobs = {} # A list of meta-jobs like stop or describe server = config.get('gearman', 'server') - if config.has_option('gearman', 'port'): - port = config.get('gearman', 'port') - else: - port = 4730 - if self.config.has_option('gearman', 'ssl_key'): - ssl_key = self.config.get('gearman', 'ssl_key') - else: - ssl_key = None - if self.config.has_option('gearman', 'ssl_cert'): - ssl_cert = self.config.get('gearman', 'ssl_cert') - else: - ssl_cert = None - if self.config.has_option('gearman', 'ssl_ca'): - ssl_ca = self.config.get('gearman', 'ssl_ca') - else: - ssl_ca = None - + port = get_default(self.config, 'gearman', 'port', 4730) + ssl_key = get_default(self.config, 'gearman', 'ssl_key') + ssl_cert = get_default(self.config, 'gearman', 'ssl_cert') + ssl_ca = get_default(self.config, 'gearman', 'ssl_ca') self.gearman = ZuulGearmanClient(self) self.gearman.addServer(server, port, ssl_key, ssl_cert, ssl_ca) diff --git a/zuul/executor/server.py b/zuul/executor/server.py index f2aedbabb1..7d7299e9e1 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -25,6 +25,7 @@ import threading import time import traceback from zuul.lib.yamlutil import yaml +from zuul.lib.config import get_default import gear from six.moves import shlex_quote @@ -375,32 +376,14 @@ class ExecutorServer(object): unverbose=self.verboseOff, ) - if self.config.has_option('executor', 'git_dir'): - self.merge_root = self.config.get('executor', 'git_dir') - else: - self.merge_root = '/var/lib/zuul/executor-git' - - if self.config.has_option('executor', 'default_username'): - self.default_username = self.config.get('executor', - 'default_username') - else: - self.default_username = 'zuul' - - if self.config.has_option('merger', 'git_user_email'): - self.merge_email = self.config.get('merger', 'git_user_email') - else: - self.merge_email = None - - if self.config.has_option('merger', 'git_user_name'): - self.merge_name = self.config.get('merger', 'git_user_name') - else: - self.merge_name = None - - if self.config.has_option('executor', 'untrusted_wrapper'): - untrusted_wrapper_name = self.config.get( - 'executor', 'untrusted_wrapper').strip() - else: - untrusted_wrapper_name = 'bubblewrap' + self.merge_root = get_default(self.config, 'executor', 'git_dir', + '/var/lib/zuul/executor-git') + self.default_username = get_default(self.config, 'executor', + 'default_username', 'zuul') + self.merge_email = get_default(self.config, 'merger', 'git_user_email') + self.merge_name = get_default(self.config, 'merger', 'git_user_name') + untrusted_wrapper_name = get_default(self.config, 'executor', + 'untrusted_wrapper', 'bubblewrap') self.untrusted_wrapper = connections.drivers[untrusted_wrapper_name] self.connections = connections @@ -411,11 +394,8 @@ class ExecutorServer(object): self.merger = self._getMerger(self.merge_root) self.update_queue = DeduplicateQueue() - if self.config.has_option('zuul', 'state_dir'): - state_dir = os.path.expanduser( - self.config.get('zuul', 'state_dir')) - else: - state_dir = '/var/lib/zuul' + state_dir = get_default(self.config, 'zuul', 'state_dir', + '/var/lib/zuul', expand_user=True) path = os.path.join(state_dir, 'executor.socket') self.command_socket = commandsocket.CommandSocket(path) ansible_dir = os.path.join(state_dir, 'ansible') @@ -457,22 +437,10 @@ class ExecutorServer(object): self._running = True self._command_running = True server = self.config.get('gearman', 'server') - if self.config.has_option('gearman', 'port'): - port = self.config.get('gearman', 'port') - else: - port = 4730 - if self.config.has_option('gearman', 'ssl_key'): - ssl_key = self.config.get('gearman', 'ssl_key') - else: - ssl_key = None - if self.config.has_option('gearman', 'ssl_cert'): - ssl_cert = self.config.get('gearman', 'ssl_cert') - else: - ssl_cert = None - if self.config.has_option('gearman', 'ssl_ca'): - ssl_ca = self.config.get('gearman', 'ssl_ca') - else: - ssl_ca = None + port = get_default(self.config, 'gearman', 'port', 4730) + ssl_key = get_default(self.config, 'gearman', 'ssl_key') + ssl_cert = get_default(self.config, 'gearman', 'ssl_cert') + ssl_ca = get_default(self.config, 'gearman', 'ssl_ca') self.merger_worker = ExecutorMergeWorker(self, 'Zuul Executor Merger') self.merger_worker.addServer(server, port, ssl_key, ssl_cert, ssl_ca) self.executor_worker = gear.TextWorker('Zuul Executor Server') @@ -715,12 +683,9 @@ class AnsibleJob(object): self.thread = None self.ssh_agent = None - if self.executor_server.config.has_option( - 'executor', 'private_key_file'): - self.private_key_file = self.executor_server.config.get( - 'executor', 'private_key_file') - else: - self.private_key_file = '~/.ssh/id_rsa' + self.private_key_file = get_default(self.executor_server.config, + 'executor', 'private_key_file', + '~/.ssh/id_rsa') self.ssh_agent = SshAgent() def run(self): diff --git a/zuul/lib/config.py b/zuul/lib/config.py new file mode 100644 index 0000000000..9cdf66e5fb --- /dev/null +++ b/zuul/lib/config.py @@ -0,0 +1,23 @@ +# 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 os + + +def get_default(config, section, option, default=None, expand_user=False): + if config.has_option(section, option): + value = config.get(section, option) + else: + value = default + if expand_user and value: + return os.path.expanduser(value) + return value diff --git a/zuul/merger/client.py b/zuul/merger/client.py index 4054df6f3a..e92d9fde82 100644 --- a/zuul/merger/client.py +++ b/zuul/merger/client.py @@ -20,6 +20,7 @@ from uuid import uuid4 import gear import zuul.model +from zuul.lib.config import get_default def getJobData(job): @@ -75,22 +76,10 @@ class MergeClient(object): self.config = config self.sched = sched server = self.config.get('gearman', 'server') - if self.config.has_option('gearman', 'port'): - port = self.config.get('gearman', 'port') - else: - port = 4730 - if self.config.has_option('gearman', 'ssl_key'): - ssl_key = self.config.get('gearman', 'ssl_key') - else: - ssl_key = None - if self.config.has_option('gearman', 'ssl_cert'): - ssl_cert = self.config.get('gearman', 'ssl_cert') - else: - ssl_cert = None - if self.config.has_option('gearman', 'ssl_ca'): - ssl_ca = self.config.get('gearman', 'ssl_ca') - else: - ssl_ca = None + port = get_default(self.config, 'gearman', 'port', 4730) + ssl_key = get_default(self.config, 'gearman', 'ssl_key') + ssl_cert = get_default(self.config, 'gearman', 'ssl_cert') + ssl_ca = get_default(self.config, 'gearman', 'ssl_ca') self.log.debug("Connecting to gearman at %s:%s" % (server, port)) self.gearman = MergeGearmanClient(self) self.gearman.addServer(server, port, ssl_key, ssl_cert, ssl_ca) diff --git a/zuul/merger/server.py b/zuul/merger/server.py index 7d7e771f2e..cbc4cb8bc0 100644 --- a/zuul/merger/server.py +++ b/zuul/merger/server.py @@ -19,6 +19,7 @@ import traceback import gear +from zuul.lib.config import get_default from zuul.merger import merger @@ -29,20 +30,10 @@ class MergeServer(object): self.config = config self.zuul_url = config.get('merger', 'zuul_url') - if self.config.has_option('merger', 'git_dir'): - merge_root = self.config.get('merger', 'git_dir') - else: - merge_root = '/var/lib/zuul/merger-git' - - if self.config.has_option('merger', 'git_user_email'): - merge_email = self.config.get('merger', 'git_user_email') - else: - merge_email = None - - if self.config.has_option('merger', 'git_user_name'): - merge_name = self.config.get('merger', 'git_user_name') - else: - merge_name = None + merge_root = get_default(self.config, 'merger', 'git_dir', + '/var/lib/zuul/merger-git') + merge_email = get_default(self.config, 'merger', 'git_user_email') + merge_name = get_default(self.config, 'merger', 'git_user_name') self.merger = merger.Merger(merge_root, connections, merge_email, merge_name) @@ -50,22 +41,10 @@ class MergeServer(object): def start(self): self._running = True server = self.config.get('gearman', 'server') - if self.config.has_option('gearman', 'port'): - port = self.config.get('gearman', 'port') - else: - port = 4730 - if self.config.has_option('gearman', 'ssl_key'): - ssl_key = self.config.get('gearman', 'ssl_key') - else: - ssl_key = None - if self.config.has_option('gearman', 'ssl_cert'): - ssl_cert = self.config.get('gearman', 'ssl_cert') - else: - ssl_cert = None - if self.config.has_option('gearman', 'ssl_ca'): - ssl_ca = self.config.get('gearman', 'ssl_ca') - else: - ssl_ca = None + port = get_default(self.config, 'gearman', 'port', 4730) + ssl_key = get_default(self.config, 'gearman', 'ssl_key') + ssl_cert = get_default(self.config, 'gearman', 'ssl_cert') + ssl_ca = get_default(self.config, 'gearman', 'ssl_ca') self.worker = gear.TextWorker('Zuul Merger') self.worker.addServer(server, port, ssl_key, ssl_cert, ssl_ca) self.log.debug("Waiting for server") diff --git a/zuul/rpclistener.py b/zuul/rpclistener.py index 0079ab8c4a..5474c8a0ef 100644 --- a/zuul/rpclistener.py +++ b/zuul/rpclistener.py @@ -22,6 +22,7 @@ import gear import six from zuul import model +from zuul.lib.config import get_default class RPCListener(object): @@ -34,22 +35,10 @@ class RPCListener(object): def start(self): self._running = True server = self.config.get('gearman', 'server') - if self.config.has_option('gearman', 'port'): - port = self.config.get('gearman', 'port') - else: - port = 4730 - if self.config.has_option('gearman', 'ssl_key'): - ssl_key = self.config.get('gearman', 'ssl_key') - else: - ssl_key = None - if self.config.has_option('gearman', 'ssl_cert'): - ssl_cert = self.config.get('gearman', 'ssl_cert') - else: - ssl_cert = None - if self.config.has_option('gearman', 'ssl_ca'): - ssl_ca = self.config.get('gearman', 'ssl_ca') - else: - ssl_ca = None + port = get_default(self.config, 'gearman', 'port', 4730) + ssl_key = get_default(self.config, 'gearman', 'ssl_key') + ssl_cert = get_default(self.config, 'gearman', 'ssl_cert') + ssl_ca = get_default(self.config, 'gearman', 'ssl_ca') self.worker = gear.TextWorker('Zuul RPC Listener') self.worker.addServer(server, port, ssl_key, ssl_cert, ssl_ca) self.log.debug("Waiting for server") diff --git a/zuul/scheduler.py b/zuul/scheduler.py index a63d270427..098cf87fd5 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -31,6 +31,7 @@ from zuul import configloader from zuul import model from zuul import exceptions from zuul import version as zuul_version +from zuul.lib.config import get_default class ManagementEvent(object): @@ -370,30 +371,21 @@ class Scheduler(threading.Thread): self.log.debug("Waiting for exit") def _get_queue_pickle_file(self): - if self.config.has_option('zuul', 'state_dir'): - state_dir = os.path.expanduser(self.config.get('zuul', - 'state_dir')) - else: - state_dir = '/var/lib/zuul' + state_dir = get_default(self.config, 'zuul', 'state_dir', + '/var/lib/zuul', expand_user=True) return os.path.join(state_dir, 'queue.pickle') def _get_time_database_dir(self): - if self.config.has_option('zuul', 'state_dir'): - state_dir = os.path.expanduser(self.config.get('zuul', - 'state_dir')) - else: - state_dir = '/var/lib/zuul' + state_dir = get_default(self.config, 'zuul', 'state_dir', + '/var/lib/zuul', expand_user=True) d = os.path.join(state_dir, 'times') if not os.path.exists(d): os.mkdir(d) return d def _get_project_key_dir(self): - if self.config.has_option('zuul', 'state_dir'): - state_dir = os.path.expanduser(self.config.get('zuul', - 'state_dir')) - else: - state_dir = '/var/lib/zuul' + state_dir = get_default(self.config, 'zuul', 'state_dir', + '/var/lib/zuul', expand_user=True) key_dir = os.path.join(state_dir, 'keys') if not os.path.exists(key_dir): os.mkdir(key_dir, 0o700)