Jenkins CI Agent Charm

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

  1. Git
  2. lp:~hloeung/jenkins-agent-charm
  3. cleanup
  4. Merge into master
Proposed by Haw Loeung
Status: Merged
Approved by: Haw Loeung
Approved revision: 63b61019bea5db95177e6c58bdea4395b635bafe
Merged at revision: 7a20fa8564669d72c9350fcfef8b0a6b92c8b40c
Proposed branch: ~hloeung/jenkins-agent-charm:cleanup
Merge into: jenkins-agent-charm:master
Prerequisite: ~hloeung/jenkins-agent-charm:master
Diff against target: 107 lines (+4/-19)
2 files modified
reactive/jenkins_slave.py (+1/-10)
tests/unit/test_jenkins_slave.py (+3/-9)
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Approve
Canonical IS Reviewers Pending
Review via email: mp+370979@code.launchpad.net

This proposal supersedes a proposal from 2019-08-06.

Commit message

Fixed stuck on blocked state discovered from functional testing

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

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

Revision history for this message
Stuart Bishop (stub) wrote :

Looks good. Yay testing.

review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 7a20fa8564669d72c9350fcfef8b0a6b92c8b40c

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/reactive/jenkins_slave.py b/reactive/jenkins_slave.py
2index ac888a4..68421be 100755
3--- a/reactive/jenkins_slave.py
4+++ b/reactive/jenkins_slave.py
5@@ -65,7 +65,6 @@ def install():
6 host.service('enable', 'jenkins-slave')
7
8 status.maintenance('Installing jenkins-slave... done.')
9- reactive.clear_flag('jenkins-slave.blocked')
10 reactive.clear_flag('jenkins-slave.configured')
11 reactive.set_flag('jenkins-slave.installed')
12
13@@ -73,14 +72,12 @@ def install():
14 @reactive.when('config.changed')
15 def config_changed():
16 status.maintenance('Configuration changed')
17- reactive.clear_flag('jenkins-slave.blocked')
18 reactive.clear_flag('jenkins-slave.configured')
19 reactive.clear_flag('nagios-nrpe.configured')
20
21
22 @reactive.when('jenkins-slave.installed')
23 @reactive.when_not('jenkins-slave.configured')
24-@reactive.when_not('jenkins-slave.blocked')
25 def configure_jenkins_slave():
26 status.maintenance('configuring jenkins-slave')
27 reactive.clear_flag('jenkins-slave.active')
28@@ -97,7 +94,7 @@ def configure_jenkins_slave():
29 else:
30 status.maintenance("No 'master_url' set; not configuring slave at this time.")
31 status.blocked("requires either slave relation or 'master_url'")
32- reactive.set_flag('jenkins-slave.blocked')
33+ reactive.clear_flag('jenkins-slave.active')
34 return
35
36 file_to_units('files/jenkins-slave-sudoers', '/etc/sudoers.d/jenkins', perms=0o440)
37@@ -106,11 +103,6 @@ def configure_jenkins_slave():
38 reactive.set_flag('jenkins-slave.configured')
39
40
41-@reactive.when('jenkins-slave.blocked')
42-def blocked_on_jenkins_url():
43- reactive.clear_flag('jenkins-slave.active')
44-
45-
46 @reactive.when('jenkins-slave.configured')
47 @reactive.when('jenkins-slave.active')
48 @reactive.when('nrpe-external-master.available')
49@@ -151,7 +143,6 @@ def set_active():
50 @reactive.hook('slave-relation-joined', 'slave-relation-changed')
51 def slave_relation_changed():
52 reactive.set_flag('slave-relation.available')
53- reactive.clear_flag('jenkins-slave.blocked')
54 reactive.clear_flag('jenkins-slave.configured')
55 reactive.clear_flag('slave-relation.configured')
56
57diff --git a/tests/unit/test_jenkins_slave.py b/tests/unit/test_jenkins_slave.py
58index a09c457..efcdb7f 100644
59--- a/tests/unit/test_jenkins_slave.py
60+++ b/tests/unit/test_jenkins_slave.py
61@@ -81,6 +81,7 @@ class TestSetDefaultConf(unittest.TestCase):
62
63 apt.reset_mock()
64 status.active.reset_mock()
65+ status.blocked.reset_mock()
66 status.maintenance.reset_mock()
67
68 @mock.patch('charms.reactive.clear_flag')
69@@ -98,8 +99,7 @@ class TestSetDefaultConf(unittest.TestCase):
70 '''Test correct flags are set via config-changed charm hook'''
71 jenkins_slave.config_changed()
72 self.assertFalse(status.maintenance.assert_called())
73- expected = [mock.call('jenkins-slave.blocked'),
74- mock.call('jenkins-slave.configured'),
75+ expected = [mock.call('jenkins-slave.configured'),
76 mock.call('nagios-nrpe.configured')]
77 self.assertFalse(clear_flag.assert_has_calls(expected, any_order=True))
78
79@@ -174,7 +174,7 @@ class TestSetDefaultConf(unittest.TestCase):
80 unitdata_kv.return_value = {}
81 jenkins_slave.configure_jenkins_slave()
82 self.assertFalse(write_default_conf.assert_not_called())
83- self.assertFalse(set_flag.assert_has_calls([mock.call('jenkins-slave.blocked')], any_order=True))
84+ self.assertFalse(status.blocked.assert_called())
85 self.assertFalse(clear_flag.assert_has_calls([mock.call('jenkins-slave.active')], any_order=True))
86
87 @mock.patch('charms.reactive.clear_flag')
88@@ -209,11 +209,6 @@ class TestSetDefaultConf(unittest.TestCase):
89 expected = [mock.call('jenkins-slave.active'), mock.call('nagios-nrpe.configured')]
90 self.assertFalse(clear_flag.assert_has_calls(expected, any_order=True))
91
92- @mock.patch('charms.reactive.clear_flag')
93- def test_blocked_on_jenkins_url(self, clear_flag):
94- jenkins_slave.blocked_on_jenkins_url()
95- self.assertFalse(clear_flag.assert_has_calls([mock.call('jenkins-slave.active')], any_order=True))
96-
97 @mock.patch('charmhelpers.core.host.service_running')
98 @mock.patch('charmhelpers.core.host.service_restart')
99 @mock.patch('charmhelpers.core.host.service_start')
100@@ -241,7 +236,6 @@ class TestSetDefaultConf(unittest.TestCase):
101 def test_hook_slave_relation_changed_flags(self, set_flag, clear_flag):
102 jenkins_slave.slave_relation_changed()
103 expected = [
104- mock.call('jenkins-slave.blocked'),
105 mock.call('jenkins-slave.configured'),
106 mock.call('slave-relation.configured'),
107 ]

Subscribers

People subscribed via source and target branches