diff --git a/jenkins_jobs/builder.py b/jenkins_jobs/builder.py index a0cf57487..7f56d313d 100644 --- a/jenkins_jobs/builder.py +++ b/jenkins_jobs/builder.py @@ -35,7 +35,7 @@ from jenkins_jobs.parallel import concurrent from jenkins_jobs import utils __all__ = [ - "Jenkins" + "JenkinsManager" ] logger = logging.getLogger(__name__) @@ -133,14 +133,26 @@ class CacheStorage(object): "exit: %s" % (self.cachefilename, e)) -class Jenkins(object): - def __init__(self, url, user, password, timeout=_DEFAULT_TIMEOUT): +class JenkinsManager(object): + + def __init__(self, jjb_config): + url = jjb_config.jenkins['url'] + user = jjb_config.jenkins['user'] + password = jjb_config.jenkins['password'] + timeout = jjb_config.jenkins['timeout'] + if timeout != _DEFAULT_TIMEOUT: self.jenkins = jenkins.Jenkins(url, user, password, timeout) else: self.jenkins = jenkins.Jenkins(url, user, password) + + self.cache = CacheStorage(jjb_config.jenkins['url'], + flush=jjb_config.builder['flush_cache']) + + self._plugins_list = jjb_config.builder['plugins_info'] self._jobs = None self._job_list = None + self._jjb_config = jjb_config @property def jobs(self): @@ -181,13 +193,6 @@ class Jenkins(object): logger.info("Deleting jenkins job {0}".format(job_name)) self.jenkins.delete_job(job_name) - def delete_all_jobs(self): - # execute a groovy script to delete all jobs is much faster than - # using the doDelete REST endpoint to delete one job at a time. - script = ('for(job in jenkins.model.Jenkins.theInstance.getAllItems())' - ' { job.delete(); }') - self.jenkins.run_script(script) - def get_plugins_info(self): """ Return a list of plugin_info dicts, one for each plugin on the Jenkins instance. @@ -225,34 +230,21 @@ class Jenkins(object): pass return False - -class Builder(object): - def __init__(self, jjb_config): - self.jenkins = Jenkins(jjb_config.jenkins['url'], - jjb_config.jenkins['user'], - jjb_config.jenkins['password'], - jjb_config.jenkins['timeout']) - self.cache = CacheStorage(jjb_config.jenkins['url'], - flush=jjb_config.builder['flush_cache']) - self._plugins_list = jjb_config.builder['plugins_info'] - - self.jjb_config = jjb_config - @property def plugins_list(self): if self._plugins_list is None: - self._plugins_list = self.jenkins.get_plugins_info() + self._plugins_list = self.get_plugins_info() return self._plugins_list def delete_old_managed(self, keep=None): - jobs = self.jenkins.get_jobs() + jobs = self.get_jobs() deleted_jobs = 0 for job in jobs: if job['name'] not in keep: - if self.jenkins.is_managed(job['name']): + if self.is_managed(job['name']): logger.info("Removing obsolete jenkins job {0}" .format(job['name'])) - self.jenkins.delete_job(job['name']) + self.delete_job(job['name']) deleted_jobs += 1 else: logger.info("Not deleting unmanaged jenkins job %s", @@ -265,22 +257,24 @@ class Builder(object): if jobs is not None: logger.info("Removing jenkins job(s): %s" % ", ".join(jobs)) for job in jobs: - self.jenkins.delete_job(job) + self.delete_job(job) if(self.cache.is_cached(job)): self.cache.set(job, '') self.cache.save() def delete_all_jobs(self): - jobs = self.jenkins.get_jobs() + jobs = self.get_jobs() logger.info("Number of jobs to delete: %d", len(jobs)) - self.jenkins.delete_all_jobs() + script = ('for(job in jenkins.model.Jenkins.theInstance.getAllItems())' + ' { job.delete(); }') + self.jenkins.run_script(script) # Need to clear the JJB cache after deletion self.cache.clear() def changed(self, job): md5 = job.md5() - changed = (self.jjb_config.builder['ignore_cache'] or + changed = (self._jjb_config.builder['ignore_cache'] or self.cache.has_changed(job.name, md5)) if not changed: logger.debug("'{0}' has not changed".format(job.name)) @@ -369,5 +363,5 @@ class Builder(object): @concurrent def parallel_update_job(self, job): - self.jenkins.update_job(job.name, job.output().decode('utf-8')) + self.update_job(job.name, job.output().decode('utf-8')) return (job.name, job.md5()) diff --git a/jenkins_jobs/cli/subcommand/delete.py b/jenkins_jobs/cli/subcommand/delete.py index 32f082089..57ca306e6 100644 --- a/jenkins_jobs/cli/subcommand/delete.py +++ b/jenkins_jobs/cli/subcommand/delete.py @@ -14,7 +14,7 @@ # under the License. -from jenkins_jobs.builder import Builder +from jenkins_jobs.builder import JenkinsManager from jenkins_jobs.parser import YamlParser from jenkins_jobs.registry import ModuleRegistry import jenkins_jobs.cli.subcommand.base as base @@ -38,7 +38,7 @@ class DeleteSubCommand(base.BaseSubCommand): directories''') def execute(self, options, jjb_config): - builder = Builder(jjb_config) + builder = JenkinsManager(jjb_config) fn = options.path diff --git a/jenkins_jobs/cli/subcommand/delete_all.py b/jenkins_jobs/cli/subcommand/delete_all.py index a9a8b2b14..77677c6c9 100644 --- a/jenkins_jobs/cli/subcommand/delete_all.py +++ b/jenkins_jobs/cli/subcommand/delete_all.py @@ -18,7 +18,7 @@ import logging import sys from jenkins_jobs import utils -from jenkins_jobs.builder import Builder +from jenkins_jobs.builder import JenkinsManager import jenkins_jobs.cli.subcommand.base as base @@ -36,7 +36,7 @@ class DeleteAllSubCommand(base.BaseSubCommand): self.parse_option_recursive_exclude(delete_all) def execute(self, options, jjb_config): - builder = Builder(jjb_config) + builder = JenkinsManager(jjb_config) if not utils.confirm( 'Sure you want to delete *ALL* jobs from Jenkins ' diff --git a/jenkins_jobs/cli/subcommand/update.py b/jenkins_jobs/cli/subcommand/update.py index bf3e76362..7f5a78b57 100644 --- a/jenkins_jobs/cli/subcommand/update.py +++ b/jenkins_jobs/cli/subcommand/update.py @@ -17,7 +17,7 @@ import logging import sys import time -from jenkins_jobs.builder import Builder +from jenkins_jobs.builder import JenkinsManager from jenkins_jobs.parser import YamlParser from jenkins_jobs.registry import ModuleRegistry from jenkins_jobs.xml_config import XmlJobGenerator @@ -66,7 +66,7 @@ class UpdateSubCommand(base.BaseSubCommand): just one worker.''') def _generate_xmljobs(self, options, jjb_config=None): - builder = Builder(jjb_config) + builder = JenkinsManager(jjb_config) logger.info("Updating jobs in {0} ({1})".format( options.path, options.names)) @@ -100,8 +100,7 @@ class UpdateSubCommand(base.BaseSubCommand): builder, xml_jobs = self._generate_xmljobs(options, jjb_config) jobs, num_updated_jobs = builder.update_jobs( - xml_jobs, - n_workers=options.n_workers) + xml_jobs, n_workers=options.n_workers) logger.info("Number of jobs updated: %d", num_updated_jobs) keep_jobs = [job.name for job in xml_jobs] diff --git a/tests/builder/test_builder.py b/tests/builder/test_builder.py index 7a2d7a437..3565ffcba 100644 --- a/tests/builder/test_builder.py +++ b/tests/builder/test_builder.py @@ -29,7 +29,7 @@ class TestCaseTestBuilder(LoggingFixture, TestCase): jjb_config = JJBConfig() jjb_config.builder['plugins_info'] = ['plugin1', 'plugin2'] jjb_config.validate() - self.builder = jenkins_jobs.builder.Builder(jjb_config) + self.builder = jenkins_jobs.builder.JenkinsManager(jjb_config) def test_plugins_list(self): self.assertEqual(self.builder.plugins_list, ['plugin1', 'plugin2']) diff --git a/tests/cmd/subcommands/test_delete.py b/tests/cmd/subcommands/test_delete.py index 82d3265cd..13b60927d 100644 --- a/tests/cmd/subcommands/test_delete.py +++ b/tests/cmd/subcommands/test_delete.py @@ -24,10 +24,12 @@ from tests.base import mock from tests.cmd.test_cmd import CmdTestsBase -@mock.patch('jenkins_jobs.builder.Jenkins.get_plugins_info', mock.MagicMock) +@mock.patch('jenkins_jobs.builder.JenkinsManager.get_plugins_info', + mock.MagicMock) class DeleteTests(CmdTestsBase): - @mock.patch('jenkins_jobs.cli.subcommand.delete.Builder.delete_jobs') + @mock.patch('jenkins_jobs.cli.subcommand.update.' + 'JenkinsManager.delete_jobs') def test_delete_single_job(self, delete_job_mock): """ Test handling the deletion of a single Jenkins job. @@ -36,7 +38,8 @@ class DeleteTests(CmdTestsBase): args = ['--conf', self.default_config_file, 'delete', 'test_job'] self.execute_jenkins_jobs_with_args(args) - @mock.patch('jenkins_jobs.cli.subcommand.delete.Builder.delete_jobs') + @mock.patch('jenkins_jobs.cli.subcommand.update.' + 'JenkinsManager.delete_jobs') def test_delete_multiple_jobs(self, delete_job_mock): """ Test handling the deletion of multiple Jenkins jobs. @@ -46,7 +49,7 @@ class DeleteTests(CmdTestsBase): 'delete', 'test_job1', 'test_job2'] self.execute_jenkins_jobs_with_args(args) - @mock.patch('jenkins_jobs.builder.Jenkins.delete_job') + @mock.patch('jenkins_jobs.builder.JenkinsManager.delete_job') def test_delete_using_glob_params(self, delete_job_mock): """ Test handling the deletion of multiple Jenkins jobs using the glob diff --git a/tests/cmd/subcommands/test_test.py b/tests/cmd/subcommands/test_test.py index 4122a90dd..a1625ca38 100644 --- a/tests/cmd/subcommands/test_test.py +++ b/tests/cmd/subcommands/test_test.py @@ -35,7 +35,8 @@ from tests.base import mock from tests.cmd.test_cmd import CmdTestsBase -@mock.patch('jenkins_jobs.builder.Jenkins.get_plugins_info', mock.MagicMock) +@mock.patch('jenkins_jobs.builder.JenkinsManager.get_plugins_info', + mock.MagicMock) class TestTests(CmdTestsBase): def test_non_existing_job(self): @@ -183,7 +184,7 @@ class TestTests(CmdTestsBase): class TestJenkinsGetPluginInfoError(CmdTestsBase): """ This test class is used for testing the 'test' subcommand when we want to validate its behavior without mocking - jenkins_jobs.builder.Jenkins.get_plugins_info + jenkins_jobs.builder.JenkinsManager.get_plugins_info """ @mock.patch('jenkins.Jenkins.get_plugins_info') @@ -304,7 +305,8 @@ class MatchesDir(object): return None -@mock.patch('jenkins_jobs.builder.Jenkins.get_plugins_info', mock.MagicMock) +@mock.patch('jenkins_jobs.builder.JenkinsManager.get_plugins_info', + mock.MagicMock) class TestTestsMultiPath(CmdTestsBase): def setUp(self): diff --git a/tests/cmd/subcommands/test_update.py b/tests/cmd/subcommands/test_update.py index 77db454e4..5fbd4aa3c 100644 --- a/tests/cmd/subcommands/test_update.py +++ b/tests/cmd/subcommands/test_update.py @@ -25,7 +25,8 @@ from tests.base import mock from tests.cmd.test_cmd import CmdTestsBase -@mock.patch('jenkins_jobs.builder.Jenkins.get_plugins_info', mock.MagicMock) +@mock.patch('jenkins_jobs.builder.JenkinsManager.get_plugins_info', + mock.MagicMock) class UpdateTests(CmdTestsBase): @mock.patch('jenkins_jobs.builder.jenkins.Jenkins.job_exists') @@ -49,10 +50,11 @@ class UpdateTests(CmdTestsBase): any_order=True ) - @mock.patch('jenkins_jobs.builder.Jenkins.is_job', return_value=True) - @mock.patch('jenkins_jobs.builder.Jenkins.get_jobs') - @mock.patch('jenkins_jobs.builder.Jenkins.get_job_md5') - @mock.patch('jenkins_jobs.builder.Jenkins.update_job') + @mock.patch('jenkins_jobs.builder.JenkinsManager.is_job', + return_value=True) + @mock.patch('jenkins_jobs.builder.JenkinsManager.get_jobs') + @mock.patch('jenkins_jobs.builder.JenkinsManager.get_job_md5') + @mock.patch('jenkins_jobs.builder.JenkinsManager.update_job') def test_update_jobs_decode_job_output(self, update_job_mock, get_job_md5_mock, get_jobs_mock, is_job_mock): @@ -99,7 +101,7 @@ class UpdateTests(CmdTestsBase): jenkins_get_jobs.return_value = extra_jobs - with mock.patch('jenkins_jobs.builder.Jenkins.is_managed', + with mock.patch('jenkins_jobs.builder.JenkinsManager.is_managed', side_effect=(lambda name: name != 'unmanaged')): self.execute_jenkins_jobs_with_args(args) @@ -111,37 +113,12 @@ class UpdateTests(CmdTestsBase): jenkins_delete_job.assert_has_calls( [mock.call(name) for name in jobs if name != 'unmanaged']) - @mock.patch('jenkins_jobs.builder.jenkins.Jenkins') - def test_update_timeout_not_set(self, jenkins_mock): - """Check that timeout is left unset - - Test that the Jenkins object has the timeout set on it only when - provided via the config option. + def test_update_timeout_not_set(self): + """Validate update timeout behavior when timeout not explicitly configured. """ + self.skipTest("TODO: Develop actual update timeout test approach.") - path = os.path.join(self.fixtures_path, 'cmd-002.yaml') - args = ['--conf', self.default_config_file, 'update', path] - - self.execute_jenkins_jobs_with_args(args) - - @mock.patch('jenkins_jobs.builder.jenkins.Jenkins') - def test_update_timeout_set(self, jenkins_mock): - """Check that timeout is set correctly - - Test that the Jenkins object has the timeout set on it only when - provided via the config option. + def test_update_timeout_set(self): + """Validate update timeout behavior when timeout is explicitly configured. """ - - path = os.path.join(self.fixtures_path, 'cmd-002.yaml') - config_file = os.path.join(self.fixtures_path, - 'non-default-timeout.ini') - args = ['--conf', config_file, 'update', path] - - self.execute_jenkins_jobs_with_args(args) - - # when timeout is set, the fourth argument to builder.Jenkins should be - # the value specified from the config - jenkins_mock.assert_called_with(mock.ANY, - mock.ANY, - mock.ANY, - 0.2) + self.skipTest("TODO: Develop actual update timeout test approach.") diff --git a/tests/cmd/test_config.py b/tests/cmd/test_config.py index d32af6674..5236cdd47 100644 --- a/tests/cmd/test_config.py +++ b/tests/cmd/test_config.py @@ -1,13 +1,16 @@ import io import os -from jenkins_jobs.cli import entry from mock import patch from tests.base import mock from tests.cmd.test_cmd import CmdTestsBase +from jenkins_jobs.cli import entry +from jenkins_jobs import builder -@mock.patch('jenkins_jobs.builder.Jenkins.get_plugins_info', mock.MagicMock) + +@mock.patch('jenkins_jobs.builder.JenkinsManager.get_plugins_info', + mock.MagicMock) class TestConfigs(CmdTestsBase): global_conf = '/etc/jenkins_jobs/jenkins_jobs.ini' @@ -107,3 +110,46 @@ class TestConfigs(CmdTestsBase): self.assertEqual(jjb_config.builder['flush_cache'], True) self.assertEqual( jjb_config.yamlparser['allow_empty_variables'], True) + + @mock.patch('jenkins_jobs.cli.subcommand.update.JenkinsManager') + def test_update_timeout_not_set(self, jenkins_mock): + """Check that timeout is left unset + + Test that the Jenkins object has the timeout set on it only when + provided via the config option. + """ + + path = os.path.join(self.fixtures_path, 'cmd-002.yaml') + args = ['--conf', self.default_config_file, 'update', path] + + jenkins_mock.return_value.update_jobs.return_value = ([], 0) + self.execute_jenkins_jobs_with_args(args) + + # validate that the JJBConfig used to initialize builder.Jenkins + # contains the expected timeout value. + + jjb_config = jenkins_mock.call_args[0][0] + self.assertEquals(jjb_config.jenkins['timeout'], + builder._DEFAULT_TIMEOUT) + + @mock.patch('jenkins_jobs.cli.subcommand.update.JenkinsManager') + def test_update_timeout_set(self, jenkins_mock): + """Check that timeout is set correctly + + Test that the Jenkins object has the timeout set on it only when + provided via the config option. + """ + + path = os.path.join(self.fixtures_path, 'cmd-002.yaml') + config_file = os.path.join(self.fixtures_path, + 'non-default-timeout.ini') + args = ['--conf', config_file, 'update', path] + + jenkins_mock.return_value.update_jobs.return_value = ([], 0) + self.execute_jenkins_jobs_with_args(args) + + # validate that the JJBConfig used to initialize builder.Jenkins + # contains the expected timeout value. + + jjb_config = jenkins_mock.call_args[0][0] + self.assertEquals(jjb_config.jenkins['timeout'], 0.2)