Merge lp:~mrazik/cupstream2distro-config/jenkins-per-project into lp:cupstream2distro-config

Proposed by Martin Mrazik
Status: Rejected
Rejected by: Loïc Minier
Proposed branch: lp:~mrazik/cupstream2distro-config/jenkins-per-project
Merge into: lp:cupstream2distro-config
Diff against target: 207 lines (+94/-21)
5 files modified
c2dconfigutils/cu2dUpdateCi.py (+39/-20)
ci/config/defaults.conf (+1/-0)
credentials (+4/-0)
stacks/phablet/ubuntu-touch-coreapps.cfg (+1/-0)
tests/test_cu2dUpdateCi.py (+49/-1)
To merge this branch: bzr merge lp:~mrazik/cupstream2distro-config/jenkins-per-project
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Francis Ginther Approve
Review via email: mp+156784@code.launchpad.net

Commit message

This implements a per-job jenkins ci server (with default to ci-jenkins from the current configuration) and moves stacks/phablet/ubuntu-touch-coreapps.cfg to "touch-coreapps-jenkins" jenkins.

Description of the change

This implements a per-job jenkins ci server (with default to ci-jenkins from the current configuration) and moves stacks/phablet/ubuntu-touch-coreapps.cfg to "touch-coreapps-jenkins" jenkins.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Francis Ginther (fginther) wrote :

I would prefer to see
63 + credentials = load_jenkins_credentials(
64 + os.path.expanduser(self.credentialsfile),
65 + jenkins_name)
66 + if not credentials:
67 + logging.error(
68 + 'Credentials for "{}" not found.'.format(jenkins_name))
69 + return None
kept inside of __call__ and store the parsed credentials as 'self.credentials'.

This will cause a bad credentials file to fail the script before any stack processing is done. Should also make testing easier as you can use a pre-made credentials dictionary instead of patching load_jenkins_credentials.

Otherwise I like this change.

review: Needs Fixing
Revision history for this message
Martin Mrazik (mrazik) wrote :

> I would prefer to see
> 63 + credentials = load_jenkins_credentials(
> 64 + os.path.expanduser(self.credentialsfile),
> 65 + jenkins_name)
> 66 + if not credentials:
> 67 + logging.error(
> 68 + 'Credentials for "{}" not
> found.'.format(jenkins_name))
> 69 + return None
> kept inside of __call__ and store the parsed credentials as
> 'self.credentials'.
>
> This will cause a bad credentials file to fail the script before any stack
> processing is done. Should also make testing easier as you can use a pre-made
> credentials dictionary instead of patching load_jenkins_credentials.

I would need to modify load_jenkins_credentials to load _all_ jenkins credentials and not just the specified. The thing is that you also don't know the credentials name _before_ you process the stack.

Revision history for this message
Francis Ginther (fginther) wrote :

My mistake. I was thinking that 'credentials' was simply the entire contents of the credentials file, but now I see that 'credentials' is just the portion specific to the jenkins_name. So, my suggestion would require more refactoring which isn't really necessary.

Approving as is.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Francis Ginther (fginther) wrote :

Dropping to a work in progress. This just needs to be re-done from scratch.

Revision history for this message
Loïc Minier (lool) wrote :

(Rejecting to drop out of active reviews)

Unmerged revisions

139. By Martin Mrazik

minor change to improve logging

138. By Martin Mrazik

added tests for _get_jenkins_handle

137. By Martin Mrazik

fixed a test

136. By Martin Mrazik

Some adjustments

135. By Martin Mrazik

first attempt to make jenkins server configurable per project

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'c2dconfigutils/cu2dUpdateCi.py'
--- c2dconfigutils/cu2dUpdateCi.py 2013-03-21 12:51:16 +0000
+++ c2dconfigutils/cu2dUpdateCi.py 2013-04-03 09:23:22 +0000
@@ -53,6 +53,11 @@
53class UpdateCi(object):53class UpdateCi(object):
54 DEFAULT_CREDENTIALS = os.path.expanduser('~/.cu2d.cred')54 DEFAULT_CREDENTIALS = os.path.expanduser('~/.cu2d.cred')
55 JENKINS_CI_CONFIG_NAME = 'ci-jenkins'55 JENKINS_CI_CONFIG_NAME = 'ci-jenkins'
56 # directory of handles for different jenkins servers
57 # this is in the form of jenkins_name: jenkins_handle
58 # the directory is lazy-initialized. When a stack/job needs
59 # a specific jenkins for the first time the handle is created.
60 JENKINS_HANDLES = {}
5661
57 # Please keep list sorted62 # Please keep list sorted
58 TEMPLATE_CONTEXT_KEYS = [63 TEMPLATE_CONTEXT_KEYS = [
@@ -75,6 +80,7 @@
75 'team',80 'team',
76 'use_description_for_commit',81 'use_description_for_commit',
77 'log_rotator',82 'log_rotator',
83 'jenkins_ci_server',
78 'days_to_keep_builds',84 'days_to_keep_builds',
79 'num_to_keep_builds',85 'num_to_keep_builds',
80 ]86 ]
@@ -284,23 +290,49 @@
284 autolanding_dict, autolanding_template,290 autolanding_dict, autolanding_template,
285 build_template)291 build_template)
286292
287 def update_jenkins(self, jenkins_handle, jjenv, stack, update=False):293 def update_jenkins(self, jjenv, stack, update=False):
288 """ Add/update jenkins jobs294 """ Add/update jenkins jobs
289295
290 :param jenkins_handle: jenkins access handle
291 :param jjenv: jinja2 template environment handle296 :param jjenv: jinja2 template environment handle
292 :param stack: dictionary with configuration of the stack297 :param stack: dictionary with configuration of the stack
293 :param update: Update existing jobs if true298 :param update: Update existing jobs if true
294299
295 :return: True on success300 :return: True on success
296 """301 """
302
303 result = True
297 if stack['projects']:304 if stack['projects']:
298 job_list = []305 job_list = []
299 self.process_stack(job_list, stack)306 self.process_stack(job_list, stack)
300 for job in job_list:307 for job in job_list:
301 setup_job(jenkins_handle, jjenv, job['name'], job['template'],308 jenkins_handle = self._get_jenkins_handle(job)
302 job['ctx'], update)309 if jenkins_handle:
303 return True310 setup_job(jenkins_handle, jjenv, job['name'],
311 job['template'], job['ctx'], update)
312 else:
313 result = False
314 logging.error(
315 'Unable to acquire jenkins. Skipping this job.')
316 return result
317
318 def _get_jenkins_handle(self, job_config):
319 jenkins_name = job_config['ctx']['jenkins_ci_server']
320 if jenkins_name in self.JENKINS_HANDLES:
321 return self.JENKINS_HANDLES[jenkins_name]
322
323 credentials = load_jenkins_credentials(
324 os.path.expanduser(self.credentialsfile),
325 jenkins_name)
326 if not credentials:
327 logging.error(
328 'Credentials for "{}" not found.'.format(jenkins_name))
329 return None
330 jenkins_handle = get_jenkins_handle(credentials)
331 if not jenkins_handle:
332 logging.error('Could not acquire connection to jenkins.')
333 return None
334 self.JENKINS_HANDLES[jenkins_name] = jenkins_handle
335 return jenkins_handle
304336
305 def __call__(self, default_config_path):337 def __call__(self, default_config_path):
306 """Entry point for cu2d-update-ci"""338 """Entry point for cu2d-update-ci"""
@@ -315,23 +347,10 @@
315 logging.error('Stack configuration failed to load. Aborting!')347 logging.error('Stack configuration failed to load. Aborting!')
316 return 1348 return 1
317349
318 credentials = None
319 if args.credentials:350 if args.credentials:
320 credentialsfile = args.credentials351 self.credentialsfile = args.credentials
321 credentials = load_jenkins_credentials(
322 os.path.expanduser(credentialsfile),
323 self.JENKINS_CI_CONFIG_NAME)
324 if not credentials:
325 logging.error('Credentials not found. Aborting!')
326 return 1
327 jenkins_handle = get_jenkins_handle(credentials)
328 if not jenkins_handle:
329 logging.error('Could not acquire connection to jenkins. ' +
330 'Aborting!')
331 return 1
332 jjenv = get_jinja_environment(default_config_path, stackcfg)352 jjenv = get_jinja_environment(default_config_path, stackcfg)
333 if not self.update_jenkins(jenkins_handle, jjenv, stackcfg,353 if not self.update_jenkins(jjenv, stackcfg, args.update_jobs):
334 args.update_jobs):
335 logging.error('Failed to configure jenkins jobs. Aborting!')354 logging.error('Failed to configure jenkins jobs. Aborting!')
336 return 2355 return 2
337 return 0356 return 0
338357
=== modified file 'ci/config/defaults.conf'
--- ci/config/defaults.conf 2013-03-21 12:51:16 +0000
+++ ci/config/defaults.conf 2013-04-03 09:23:22 +0000
@@ -1,5 +1,6 @@
1stack:1stack:
2 ci_default:2 ci_default:
3 jenkins_ci_server: 'ci-jenkins'
3 ci_template: ci-config.xml.tmpl4 ci_template: ci-config.xml.tmpl
4 autolanding_template: autolanding-config.xml.tmpl5 autolanding_template: autolanding-config.xml.tmpl
5 build_template: pbuilder-config.xml.tmpl6 build_template: pbuilder-config.xml.tmpl
67
=== modified file 'credentials'
--- credentials 2013-02-28 16:31:58 +0000
+++ credentials 2013-04-03 09:23:22 +0000
@@ -8,3 +8,7 @@
8 password: JENKINS_PASSWORD8 password: JENKINS_PASSWORD
9 url: JENKINS_URL9 url: JENKINS_URL
10 token: JENKINS_TOKEN10 token: JENKINS_TOKEN
11touch-coreapps-jenkins:
12 username: JENKINS_USERNAME
13 password: JENKINS_PASSWORD
14 url: JENKINS_URL
1115
=== modified file 'stacks/phablet/ubuntu-touch-coreapps.cfg'
--- stacks/phablet/ubuntu-touch-coreapps.cfg 2013-03-26 13:37:50 +0000
+++ stacks/phablet/ubuntu-touch-coreapps.cfg 2013-04-03 09:23:22 +0000
@@ -9,6 +9,7 @@
9 daily_release_default:9 daily_release_default:
10 daily_release: False # Disable all for daily release process10 daily_release: False # Disable all for daily release process
11 ci_default:11 ci_default:
12 jenkins_ci_server: 'touch-coreapps-jenkins'
12 autolanding:13 autolanding:
13 ppa_target: ppa:ubuntu-touch-coreapps-drivers/daily14 ppa_target: ppa:ubuntu-touch-coreapps-drivers/daily
14 distributions: quantal,raring15 distributions: quantal,raring
1516
=== modified file 'tests/test_cu2dUpdateCi.py'
--- tests/test_cu2dUpdateCi.py 2013-03-15 15:27:32 +0000
+++ tests/test_cu2dUpdateCi.py 2013-04-03 09:23:22 +0000
@@ -220,6 +220,54 @@
220 self.assertEqual(expected, actual)220 self.assertEqual(expected, actual)
221221
222222
223class TestGetJenkinsHandle(TestCase):
224 def setUp(self):
225 self.update_ci = UpdateCi()
226 self.update_ci.credentialsfile = '~/.cu2d.cred'
227
228 def test_handle_exists(self):
229 jenkins_name = 'test_handle_exists'
230 self.update_ci.JENKINS_HANDLES[jenkins_name] = MagicMock()
231 jenkins_handle = self.update_ci._get_jenkins_handle(
232 {'ctx': {'jenkins_ci_server': jenkins_name}})
233 self.assertEqual(jenkins_handle,
234 self.update_ci.JENKINS_HANDLES[jenkins_name])
235
236 def test_no_credentials(self):
237 jenkins_name = 'test_no_credentials'
238 load_jenkins_credentials = lambda x, y: None
239 with patch('c2dconfigutils.cu2dUpdateCi.load_jenkins_credentials',
240 load_jenkins_credentials):
241 jenkins_handle = self.update_ci._get_jenkins_handle(
242 {'ctx': {'jenkins_ci_server': jenkins_name}})
243 self.assertIsNone(jenkins_handle)
244
245 def test_no_jenkins_handle(self):
246 jenkins_name = 'test_no_jenkins_handle'
247 load_jenkins_credentials = lambda x, y: object()
248 get_jenkins_handle = lambda x: None
249 with patch('c2dconfigutils.cu2dUpdateCi.load_jenkins_credentials',
250 load_jenkins_credentials), \
251 patch('c2dconfigutils.cu2dUpdateCi.get_jenkins_handle',
252 get_jenkins_handle):
253 jenkins_handle = self.update_ci._get_jenkins_handle(
254 {'ctx': {'jenkins_ci_server': jenkins_name}})
255 self.assertIsNone(jenkins_handle)
256
257 def test_get_jenkins_handle(self):
258 jenkins_name = 'test_get_jenkins_handle'
259 jenkins_handle = object()
260 load_jenkins_credentials = lambda x, y: object()
261 get_jenkins_handle = lambda x: jenkins_handle
262 with patch('c2dconfigutils.cu2dUpdateCi.load_jenkins_credentials',
263 load_jenkins_credentials), \
264 patch('c2dconfigutils.cu2dUpdateCi.get_jenkins_handle',
265 get_jenkins_handle):
266 result = self.update_ci._get_jenkins_handle(
267 {'ctx': {'jenkins_ci_server': jenkins_name}})
268 self.assertEqual(jenkins_handle, result)
269
270
223class TestUpdateJenkins(TestCase):271class TestUpdateJenkins(TestCase):
224 def setUp(self):272 def setUp(self):
225 self.jenkins = MagicMock()273 self.jenkins = MagicMock()
@@ -233,7 +281,7 @@
233281
234 def test_empty_stack(self):282 def test_empty_stack(self):
235 stack = {'projects': None}283 stack = {'projects': None}
236 self.update_ci.update_jenkins(None, None, stack)284 self.update_ci.update_jenkins(None, stack)
237285
238 def test_stack(self):286 def test_stack(self):
239 stack = {287 stack = {

Subscribers

People subscribed via source and target branches

to all changes: