Merge jenkins-agent-charm:fix_master_relation_setup into jenkins-agent-charm:master

[Alexandre Gomes]

Fix the relation setup by making use of the slave secret provided by the master.

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

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

[Tom Haddon (mthaddon)]

One very minor change, otherwise looks good.

review: Approve

[Haw Loeung (hloeung)]

write_default_conf() is also called outside of relation context though, specifically in configure_jenkins_slave(). This is why 'url' is saved using unitdata.kv.set().

So I think we'll want to also save the secret, then in configure_jenkins_slave():

| elif kv.get('url'):
| status.maintenance("Using url from relation as 'master_url'")
| write_default_conf(kv.get('url'))

This should retrieve the saved secret and use it for write_default_conf().

review: Needs Fixing

diff --git a/files/jenkins-slave-systemd-config b/files/jenkins-slave-systemd-config
index 516b703..21a4809 100644
--- a/files/jenkins-slave-systemd-config
+++ b/files/jenkins-slave-systemd-config
@@ -11,7 +11,7 @@ Type=simple
 # Always be Jenkinsing!
 Restart=always
 RestartSec=3
-ExecStartPre=/bin/bash -c '[ -r /etc/default/jenkins-slave ] && . /etc/default/jenkins-slave ; [ -n "$JENKINS_URL" ] || { exit 1; };  mkdir $JENKINS_RUN  > /dev/null 2>&1  || true ; chown -R $JENKINS_USER $JENKINS_RUN || true ; /usr/local/sbin/download-slave.sh $JENKINS_URL'
+ExecStartPre=/bin/bash -c '[ -r /etc/default/jenkins-slave ] && . /etc/default/jenkins-slave ; [ -n "$JENKINS_URL" ] || { exit 1; };  mkdir $JENKINS_RUN  > /dev/null 2>&1  || true ; chown -R $JENKINS_USER $JENKINS_RUN || true ; /usr/local/sbin/download-slave.sh $JENKINS_URL ; chown -R $JENKINS_USER $JENKINS_HOME'
 ExecStart=/bin/bash -c '[ -r /etc/default/jenkins-slave ] && . /etc/default/jenkins-slave ; exec start-stop-daemon --start -c $JENKINS_USER --exec $JAVA --name jenkins-slave -- $JAVA_ARGS -jar $JENKINS_RUN/slave.jar $JENKINS_ARGS'
 
 [Install]
diff --git a/files/jenkins-slave-upstart-config b/files/jenkins-slave-upstart-config
index 0456e1d..c02ec9d 100644
--- a/files/jenkins-slave-upstart-config
+++ b/files/jenkins-slave-upstart-config
@@ -18,6 +18,7 @@ pre-start script
     mkdir $JENKINS_RUN  > /dev/null 2>&1  || true
     chown -R $JENKINS_USER $JENKINS_RUN || true
     /usr/local/sbin/download-slave.sh $JENKINS_URL
+    chown -R $JENKINS_USER $JENKINS_HOME || true
 end script
 
 script
diff --git a/reactive/jenkins_slave.py b/reactive/jenkins_slave.py
index 8d54436..615084e 100644
--- a/reactive/jenkins_slave.py
+++ b/reactive/jenkins_slave.py
@@ -187,7 +187,8 @@ def slave_relation():
     url = hookenv.relation_get('url')
     if url:
         kv.set('url', url)
-        write_default_conf(url)
+        secret = hookenv.relation_get('secret')
+        write_default_conf(url, secret=secret)
     else:
         hookenv.log("Master hasn't exported its url yet, exiting...")
         return
@@ -228,13 +229,13 @@ def file_to_units(local_path, unit_path, perms=None, owner='root', group='root')
         host.write_file(path=unit_path, content=fh.read().encode(), owner=owner, group=group, perms=file_perms)
 
 
-def write_default_conf(master_url=None, owner='root', group='root', conf_path='/etc/default/jenkins-slave'):
+def write_default_conf(master_url=None, owner='root', group='root', conf_path='/etc/default/jenkins-slave', secret=None):
     templates_dir = os.path.join(hookenv.charm_dir(), 'templates')
     slave_host = hookenv.local_unit().replace('/', '-')
     templating.render(
         'jenkins-slave-default',
         conf_path,
-        {'master_url': master_url, 'slave_host': slave_host},
+        {'master_url': master_url, 'slave_host': slave_host, 'secret': secret},
         owner=owner,
         group=group,
         perms=0o444,
diff --git a/templates/jenkins-slave-default b/templates/jenkins-slave-default
index 2e32e7d..b0db0d3 100644
--- a/templates/jenkins-slave-default
+++ b/templates/jenkins-slave-default
@@ -24,6 +24,7 @@ JENKINS_USER=jenkins
 JENKINS_ROOT=/usr/share/jenkins
 
 # jenkins home location
+
 JENKINS_HOME=/var/lib/jenkins
 
 # jenkins /run location
@@ -60,4 +61,9 @@ JENKINS_SLAVE_LOG=/var/log/jenkins/$NAME.log
 MAXOPENFILES=8192
 
 # Arguments to pass to jenkins slave on startup
+{% if secret %}
+JENKINS_SLAVE_SECRET="{{secret}}"
+JENKINS_ARGS="-jnlpUrl $JENKINS_URL/computer/$JENKINS_HOSTNAME/slave-agent.jnlp -secret ${JENKINS_SLAVE_SECRET}"
+{% else %}
 JENKINS_ARGS="-jnlpUrl $JENKINS_URL/computer/$JENKINS_HOSTNAME/slave-agent.jnlp"
+{% endif %}