Merge ~hloeung/jenkins-agent-charm:master into jenkins-agent-charm:master

[Haw Loeung]

Update status to show which jenkins master is being used

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Stuart Bishop (stub)]

Looks OK, but the implementation will fail outside of relation hooks per inline comment.

review: Needs Fixing

[Haw Loeung (hloeung)]

review: Needs Resubmitting

[Stuart Bishop (stub)]

Looks good.

You might have a few unneeded mocks per inline comments, which can be removed if that is the case.

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 625fa31b6ed259ed3fc0e84afa091fa2aa05c16a

diff --git a/reactive/jenkins_slave.py b/reactive/jenkins_slave.py
index 68421be..7bbb1a2 100755
--- a/reactive/jenkins_slave.py
+++ b/reactive/jenkins_slave.py
@@ -125,6 +125,9 @@ def configure_nagios(nagios):
 @reactive.when('jenkins-slave.configured')
 @reactive.when_not('jenkins-slave.active')
 def set_active():
+    config = hookenv.config()
+    kv = unitdata.kv()
+
     # Startup the jenkins-slave service. This is called in the
     # install, config-changed and upgrade-charm paths. It needs to be
     # tolerant of a running or stopped jenkins-slave.
@@ -135,8 +138,17 @@ def set_active():
         status.maintenance('Starting jenkins-slave...')
         host.service_start('jenkins-slave')
 
+    master_url = config.get('master_url')
+    if master_url:
+        status.active('ready - using jenkins {} (master_url)'.format(master_url))
+    elif kv.get('url'):
+        url = kv.get('url')
+        status.active('ready - using jenkins {} (relation)'.format(url))
+    else:
+        status.blocked("set_active() but no jenkins master")
+        return
+
     reactive.set_flag('jenkins-slave.active')
-    status.active('ready')
 
 
 # We can't use interface:jenkins-slave yet as it's not implemented.
@@ -158,6 +170,7 @@ def slave_relation_removed():
 @reactive.when_not('slave-relation.configured')
 def slave_relation():
     status.maintenance('setting up jenkins via slave relation')
+
     config = hookenv.config()
     kv = unitdata.kv()
 
diff --git a/tests/unit/test_jenkins_slave.py b/tests/unit/test_jenkins_slave.py
index efcdb7f..3ef3484 100644
--- a/tests/unit/test_jenkins_slave.py
+++ b/tests/unit/test_jenkins_slave.py
@@ -231,6 +231,23 @@ class TestSetDefaultConf(unittest.TestCase):
         self.assertFalse(service_start.assert_called_once_with('jenkins-slave'))
         self.assertFalse(service_restart.assert_not_called())
 
+    @mock.patch('charms.reactive.set_flag')
+    @mock.patch('charmhelpers.core.hookenv.config')
+    @mock.patch('charmhelpers.core.unitdata.kv')
+    @mock.patch('charmhelpers.core.host.service_running')
+    @mock.patch('charmhelpers.core.host.service_start')
+    def test_set_active_master_url_or_relation(self, service_start, service_running, unitdata_kv, config, set_flag):
+        service_running.return_value = False
+        config.return_value = {'master_url': 'http://ext-jenkins-be.internal:8080'}
+        jenkins_slave.set_active()
+        status.active.assert_called_once_with('ready - using jenkins http://ext-jenkins-be.internal:8080 (master_url)')
+
+        config.return_value = {}
+        unitdata_kv.return_value = {'url': 'http://my-jenkins-be.internal:8080'}
+        status.active.reset_mock()
+        jenkins_slave.set_active()
+        status.active.assert_called_once_with('ready - using jenkins http://my-jenkins-be.internal:8080 (relation)')
+
     @mock.patch('charms.reactive.clear_flag')
     @mock.patch('charms.reactive.set_flag')
     def test_hook_slave_relation_changed_flags(self, set_flag, clear_flag):