Jenkins CI Agent Charm

Merge ~hloeung/jenkins-agent-charm:switch-to-layer-status into jenkins-agent-charm:master

  1. Git
  2. lp:~hloeung/jenkins-agent-charm
  3. switch-to-layer-status
  4. Merge into master
Proposed by Haw Loeung
Status: Merged
Approved by: Alexandre Gomes
Approved revision: 901a909d302fb4e3eeaa00be8cb2401a25240495
Merged at revision: e010d61685b05c5a20614b23fd97a94fb9bac2e2
Proposed branch: ~hloeung/jenkins-agent-charm:switch-to-layer-status
Merge into: jenkins-agent-charm:master
Diff against target: 175 lines (+26/-22)
3 files modified
layer.yaml (+1/-0)
reactive/jenkins_slave.py (+24/-22)
tests/unit/test_jenkins_slave.py (+1/-0)
Reviewer Review Type Date Requested Status
Joel Sing (community) +1 Approve
Canonical IS Reviewers Pending
Review via email: mp+364411@code.launchpad.net

Commit message

Switch from hookenv.status_set() to layer-status

Description of the change

As advised by Stuart, using layer-status has many benefits. One major one being that we don't have to keep track of 'blocked' status and that it is set to be the final state (avoids bouncing between non-blocked and blocked).

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
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision e010d61685b05c5a20614b23fd97a94fb9bac2e2

Revision history for this message
Joel Sing (jsing) wrote :

LGTM

review: Approve (+1)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/layer.yaml b/layer.yaml
2index 70c79a6..52ccf61 100644
3--- a/layer.yaml
4+++ b/layer.yaml
5@@ -2,4 +2,5 @@ includes:
6 - layer:basic
7 - layer:apt
8 - layer:nagios
9+ - layer:status
10 repo: lp:jenkins-slave-charm
11diff --git a/reactive/jenkins_slave.py b/reactive/jenkins_slave.py
12index beecf87..1d5c545 100755
13--- a/reactive/jenkins_slave.py
14+++ b/reactive/jenkins_slave.py
15@@ -4,29 +4,30 @@ from charmhelpers.core import hookenv, host, templating, unitdata
16 from charmhelpers.contrib.charmsupport import nrpe
17 from charmhelpers.fetch import apt_purge
18 from charms import apt, reactive
19+from charms.layer import status
20
21
22 @reactive.hook('upgrade-charm')
23 def upgrade_charm():
24- hookenv.status_set('maintenance', 'forcing reconfiguration on upgrade-charm')
25+ status.maintenance('forcing reconfiguration on upgrade-charm')
26 reactive.clear_flag('jenkins-slave.active')
27 reactive.clear_flag('jenkins-slave.installed')
28
29
30 @reactive.when_not('jenkins-slave.installed')
31 def install():
32- hookenv.status_set('maintenance', 'installing jenkins-slave')
33+ status.maintenance('installing jenkins-slave')
34 reactive.clear_flag('jenkins-slave.active')
35
36 config = hookenv.config()
37
38- hookenv.log('Adding jenkins-slave dependencies to be installed')
39+ status.maintenance('Adding jenkins-slave dependencies to be installed')
40 packages = ['wget', 'default-jre-headless']
41
42 # Install extra packages needed by the slave.
43 tools = config.get('tools')
44 if tools:
45- hookenv.log('Adding jenkins-slave additional tools to be installed: {}'.format(tools))
46+ status.maintenance('Adding jenkins-slave additional tools to be installed: {}'.format(tools))
47 for package in tools.split():
48 packages.append(package)
49 apt.queue_install(packages)
50@@ -34,35 +35,35 @@ def install():
51 return # apt layer already set blocked state.
52
53 # Get rid of the legacy jenkins-slave package, including config files.
54- hookenv.log('Removing the old jenkins-slave package... (obsoleted by this charm)')
55+ status.maintenance('Removing the old jenkins-slave package... (obsoleted by this charm)')
56 apt_purge(['jenkins-slave'])
57
58 # Create jenkins user if it doesn't exist.
59 if host.user_exists('jenkins'):
60- hookenv.log('Installing jenkins-slave (user account already exists)...')
61+ status.maintenance('Installing jenkins-slave (user account already exists)...')
62 else:
63- hookenv.log('Installing jenkins-slave (user account)...')
64+ status.maintenance('Installing jenkins-slave (user account)...')
65 host.adduser(username='jenkins', system_user=True, home_dir='/var/lib/jenkins')
66
67 # And ensure required directories exist and are set up.
68- hookenv.log('Installing jenkins-slave (directories)...')
69+ status.maintenance('Installing jenkins-slave (directories)...')
70 host.mkdir('/var/lib/jenkins', owner='jenkins', group='jenkins')
71 host.mkdir('/var/log/jenkins', owner='jenkins', group='jenkins')
72
73- hookenv.log('Installing jenkins-slave (common files)...')
74+ status.maintenance('Installing jenkins-slave (common files)...')
75 write_default_conf()
76 file_to_units('files/download-slave.sh', '/usr/local/sbin/download-slave.sh')
77 file_to_units('files/jenkins-slave-logrotate-config', '/etc/logrotate.d/jenkins-slave')
78
79 if host.lsb_release()['DISTRIB_CODENAME'] == 'trusty':
80- hookenv.log('Installing jenkins-slave (upstart job)...')
81+ status.maintenance('Installing jenkins-slave (upstart job)...')
82 file_to_units('files/jenkins-slave-upstart-config', '/etc/init/jenkins-slave.conf')
83 else:
84- hookenv.log('Installing jenkins-slave (system unit)...')
85+ status.maintenance('Installing jenkins-slave (system unit)...')
86 file_to_units('files/jenkins-slave-systemd-config', '/lib/systemd/system/jenkins-slave.service')
87 host.service('enable', 'jenkins-slave')
88
89- hookenv.log('Installing jenkins-slave... done.')
90+ status.maintenance('Installing jenkins-slave... done.')
91 reactive.clear_flag('jenkins-slave.blocked')
92 reactive.clear_flag('jenkins-slave.configured')
93 reactive.set_flag('jenkins-slave.installed')
94@@ -70,6 +71,7 @@ def install():
95
96 @reactive.when('config.changed')
97 def config_changed():
98+ status.maintenance('Configuration changed')
99 reactive.clear_flag('jenkins-slave.blocked')
100 reactive.clear_flag('jenkins-slave.configured')
101 reactive.clear_flag('nagios-nrpe.configured')
102@@ -79,21 +81,21 @@ def config_changed():
103 @reactive.when_not('jenkins-slave.configured')
104 @reactive.when_not('jenkins-slave.blocked')
105 def configure_jenkins_slave():
106- hookenv.status_set('maintenance', 'configuring jenkins-slave')
107+ status.maintenance('configuring jenkins-slave')
108 reactive.clear_flag('jenkins-slave.active')
109
110 config = hookenv.config()
111 kv = unitdata.kv()
112
113 if config.get('master_url'):
114- hookenv.log("Using 'master_url' to configure the slave.")
115+ status.maintenance("Using 'master_url' to configure the slave.")
116 write_default_conf(config.get('master_url'))
117 elif kv.get('url'):
118- hookenv.log("Using url from relation as 'master_url'")
119+ status.maintenance("Using url from relation as 'master_url'")
120 write_default_conf(kv.get('url'))
121 else:
122- hookenv.log("No 'master_url' set; not configuring slave at this time.")
123- hookenv.status_set('blocked', "requires either slave relation or 'master_url'")
124+ status.maintenance("No 'master_url' set; not configuring slave at this time.")
125+ status.blocked("requires either slave relation or 'master_url'")
126 reactive.set_flag('jenkins-slave.blocked')
127 return
128
129@@ -113,7 +115,7 @@ def blocked_on_jenkins_url():
130 @reactive.when('jenkins-slave.active')
131 @reactive.when_not('nagios-nrpe.configured')
132 def configure_nagios(nagios):
133- hookenv.status_set('maintenance', 'setting up NRPE checks')
134+ status.maintenance('setting up NRPE checks')
135
136 # Use charmhelpers.contrib.charmsupport's nrpe to determine hostname
137 hostname = nrpe.get_nagios_hostname()
138@@ -133,13 +135,13 @@ def set_active():
139 # install, config-changed and upgrade-charm paths. It needs to be
140 # tolerant of a running or stopped jenkins-slave.
141 if host.service_running('jenkins-slave'):
142- hookenv.log('Restarting jenkins-slave...')
143+ status.maintenance('Restarting jenkins-slave...')
144 host.service_restart('jenkins-slave')
145 else:
146- hookenv.log('Starting jenkins-slave...')
147+ status.maintenance('Starting jenkins-slave...')
148 host.service_start('jenkins-slave')
149
150- hookenv.status_set('active', 'ready')
151+ status.active('ready')
152 reactive.set_flag('jenkins-slave.active')
153
154
155@@ -162,7 +164,7 @@ def slave_relation_removed():
156 @reactive.when('slave-relation.available')
157 @reactive.when_not('slave-relation.configured')
158 def slave_relation():
159- hookenv.status_set('maintenance', 'setting up jenkins via slave relation')
160+ status.maintenance('setting up jenkins via slave relation')
161 config = hookenv.config()
162 kv = unitdata.kv()
163
164diff --git a/tests/unit/test_jenkins_slave.py b/tests/unit/test_jenkins_slave.py
165index 06879e5..7bd3ead 100644
166--- a/tests/unit/test_jenkins_slave.py
167+++ b/tests/unit/test_jenkins_slave.py
168@@ -9,6 +9,7 @@ from unittest import mock
169
170 sys.modules['charms.apt'] = mock.MagicMock()
171 from charms import apt # NOQA: E402
172+sys.modules['charms.layer'] = mock.MagicMock()
173
174 # Add path to where our reactive layer lives and import.
175 sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))))

Subscribers

People subscribed via source and target branches