Merge lp:~gandelman-a/charms/precise/jenkins-slave/extension_executors into lp:charms/jenkins-slave

[Adam Gandelman]

* Allows the # of executors to be configurable, defaulting to # of cores

* Adds the jenkins extenions interface, much like the master charm. This allows custom subordinates to be associated with the slaves and reduces the need to fork the main principle charm.

[lp:~gandelman-a/charms/precise/jenkins-slave/extension_executors]

12. By Adam Gandelman on 2013-10-18

Fix inspection of config.

[Adam Israel (aisrael)]

Hey Adam, thanks for the charm contribution and apologies for the delay in review. I took a look at the charm and ran into a few issues.

The initial charm proof didn't run cleanly, so I fixed those issues and continued on.

I deployed jenkins and jenkins-slave per the README, and added the relation between the news.

Once the relation is joined, changes to to the charm's executors config setting do not trigger jenkins master-relation-changed, where jenkins gets the value from the relation, like I would expect.

I then noticed that the executor count determined in slave-relation-joined isn't reflected back in the charm's config, so if a user is working with the juju gui, they don't see that value.

But -- if I destroyed the relation and manually set the executors to 3 (through juju GUI) and then added the relation, jenkins node configuration UI showed 6 executors.

Some of this may be expected behavior to someone more familiar with Jenkins. If so, it would be good to add these kind of caveats to the README.

Here's my branch, for reference:
lp:~aisrael/charms/precise/jenkins-slave/fix-extensions-executors

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.

review: Needs Fixing

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-964-results

review: Needs Fixing (cbt)

Unmerged revisions

12. By Adam Gandelman on 2013-10-18

Fix inspection of config.

11. By Adam Gandelman on 2013-10-18

Add jenkins-extension interface and make # executors configurable.

=== modified file 'config.yaml'
--- config.yaml	2011-09-22 08:51:30 +0000
+++ config.yaml	2013-10-18 02:50:45 +0000
@@ -6,3 +6,6 @@
   labels:
     type: string
     description: Jenkins labels to associate with jenkins slave node
+  executors:
+    type: int
+    description: Number of job executors.  Defaults to number of CPUs.

=== modified file 'hooks/slave-relation-joined'
--- hooks/slave-relation-joined	2012-07-27 09:52:29 +0000
+++ hooks/slave-relation-joined	2013-10-18 02:50:45 +0000
@@ -5,7 +5,12 @@
 # Set the slave hostname to match the juju unit
 # in the jenkins master instance
 slavehost=`echo ${JUJU_UNIT_NAME} | sed s,/,-,`
-noexecutors=`cat /proc/cpuinfo | grep processor | wc -l`
+
+noexecutors=`config-get executors`
+if [[ -z "$noexecutors" ]] ; then
+  noexecutors=`cat /proc/cpuinfo | grep processor | wc -l`
+fi
+
 config_labels=`config-get labels`
 labels=`uname -p`
 

=== modified file 'metadata.yaml'
--- metadata.yaml	2012-05-22 11:06:20 +0000
+++ metadata.yaml	2013-10-18 02:50:45 +0000
@@ -11,3 +11,8 @@
 provides:
   slave:
     interface: jenkins-slave
+  extension:
+    interface: jenkins-extension
+    scope: container
+    optional: true
+

=== modified file 'revision'
--- revision	2012-07-31 13:47:53 +0000
+++ revision	2013-10-18 02:50:45 +0000
@@ -1,1 +1,1 @@
-8
+11