Merge charm-k8s-jenkins-agent:relations-config into charm-k8s-jenkins-agent:master

[Alexandre Gomes]

Add feature to configure agents through juju relations

[🤖 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)]

Some comments inline. Tests will need updating a little based on what changes you make.

review: Needs Fixing

a868510... by Alexandre Gomes on 2020-08-22

Address comments

Unmerged commits

a868510... by Alexandre Gomes on 2020-08-22

Address comments

7661412... by Alexandre Gomes on 2020-08-15

Add unit tests

f82f78e... by Alexandre Gomes on 2020-08-15

Add feature to configure agent through juju relations

80ab0b9... by Alexandre Gomes on 2020-08-15

Fix shellcheck command

c3716be... by Alexandre Gomes on 2020-08-12

Add comment on why root is used in Dockerfile

2d4df32... by Alexandre Gomes on 2020-08-12

Add unit tests

74abd55... by Alexandre Gomes on 2020-08-12

Add unit/ to .gitignore

e5a08a7... by Alexandre Gomes on 2020-08-12

Remove some unecessary bits

1394cc7... by Alexandre Gomes on 2020-08-12

Update init()

2807a77... by Alexandre Gomes on 2020-08-11

Remove some unused imports

diff --git a/.gitignore b/.gitignore
index bf7fffd..eea14ad 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,3 +6,4 @@ __pycache__
 build
 lib/ops
 lib/ops-*.dist-info
+unit/
diff --git a/config.yaml b/config.yaml
index 7ad8e02..c419381 100644
--- a/config.yaml
+++ b/config.yaml
@@ -3,32 +3,30 @@ options:
     type: string
     default: "jenkins-agent-operator"
     description: "The docker image to install. Required. Defaults to jenkins-agent-operator from rocks.canonical.com"
-  tools:
-    type: string
-    default: git gcc make bzr
-    description: Tooling to deploy on jenkins agent node
-  labels:
-    type: string
-    description: Jenkins labels to associate with jenkins agent node
-    default: ""
   jenkins_master_url:
     type: string
     default: ""
     description: |
-      Configure the agent to use an explicit jenkins master instead of using
-      the jenkins-agent relation. This allows for the agent and master to
-      be deployed in different environments.
+      Configure the agent to use an explicit Jenkins master instead of using
+      the jenkins-agent relation. This allows the agent to connect to a Jenkins instance not managed 
+      by Juju.
   jenkins_agent_name:
     type: string
     default: ""
     description: |
-      Jenkins agent name that will be seen on the jenkins master.
-      Will default to the hostname of the container.
-  jenkins_api_token:
+      Agent name as configured in Jenkins. Multiple names can be input by using `:` as a separator. 
+      Example: "agent-one:agent-two:agent-three"
+  jenkins_agent_token:
     type: string
     default: ""
-    description: "Jenkins API token used to join the master"
-  jenkins_user:
+    description: |
+      Agent token provided by Jenkins. Can be found in your Jenkins instance at 
+      ${JENKINS_URL}/computer/${AGENT_NAME}/. Multiple tokens can be input by 
+      using `:` as a separator matching the order of the agents in `jenkins_agent_name`.
+      Example: "token-one:token-two:token-three"
+  jenkins_agent_labels:
     type: string
-    default: "admin"
-    description: "Jenkins user to download agent.jar from master"
+    default: ""
+    description: |
+      Comma-separated list of labels to be assigned to the agent in Jenkins. If not set it will default to 
+      the agents hardware identifier, e.g.: 'x86_64'
diff --git a/dockerfile/Dockerfile b/dockerfile/Dockerfile
index e03339f..700e70f 100644
--- a/dockerfile/Dockerfile
+++ b/dockerfile/Dockerfile
@@ -47,6 +47,7 @@ RUN apt-get update -y \
 
 WORKDIR /var/lib/jenkins
 
+# Using root due to https://bugs.launchpad.net/juju/+bug/1879598
 #USER ${USER}
 USER root
 
diff --git a/dockerfile/files/entrypoint.sh b/dockerfile/files/entrypoint.sh
index c8ece01..a2f820e 100755
--- a/dockerfile/files/entrypoint.sh
+++ b/dockerfile/files/entrypoint.sh
@@ -21,21 +21,11 @@ typeset JAVA_ARGS=${JAVA_ARGS:-""}
 # job from running.
 typeset JENKINS_URL="${JENKINS_URL:?"URL of a jenkins server must be provided"}"
 
-# Name of agent configuration to use at JENKINS_URL
-# Override if it need to be something other than the
-# hostname of the server the agent is running on.
-typeset JENKINS_HOSTNAME="${JENKINS_HOSTNAME:-$(hostname)}"
-
-
 typeset JENKINS_WORKDIR="/var/lib/jenkins"
 
 # Arguments to pass to jenkins agent on startup
 typeset -a JENKINS_ARGS
 
-# JENKINS_ARGS+=(-jnlpUrl "${JENKINS_URL}"/computer/"${JENKINS_HOSTNAME}"/slave-agent.jnlp)
-# JENKINS_ARGS+=(-jnlpCredentials "${JENKINS_API_USER:?Please specify JENKINS_API_USER}:${JENKINS_API_TOKEN:?Please specify JENKINS_API_TOKEN}")
-# JENKINS_ARGS+=(-noReconect)
-
 # Path of the agent.jar
 typeset AGENT_JAR=/var/lib/jenkins/agent.jar
 
@@ -60,9 +50,6 @@ download_agent
 # Specify the pod as ready
 touch /var/lib/jenkins/agents/.ready
 
-#shellcheck disable=SC2086
-# "${JAVA}" ${JAVA_ARGS} -jar "${AGENT_JAR}"  "${JENKINS_ARGS[@]}"
-
 # Transform the env variables in arrays to iterate through it
 IFS=':' read -r -a AGENTS <<< ${JENKINS_AGENTS}
 IFS=':' read -r -a TOKENS <<< ${JENKINS_TOKENS}
@@ -70,12 +57,6 @@ IFS=':' read -r -a TOKENS <<< ${JENKINS_TOKENS}
 echo ${!AGENTS[@]}
 
 for index in ${!AGENTS[@]}; do
-    echo "agent  : ${AGENTS[$index]}"
-    echo "value: ${TOKENS[$index]}"
-    echo "${JAVA}" "${JAVA_ARGS}" -jar "${AGENT_JAR}" -jnlpUrl "${JENKINS_URL}"/computer/"${AGENTS[$index]}"/slave-agent.jnlp -workDir "${JENKINS_WORKDIR}" -noReconnect -secret "${TOKENS[$index]}"
-    ${JAVA} ${JAVA_ARGS} -jar ${AGENT_JAR} -jnlpUrl ${JENKINS_URL}/computer/${AGENTS[$index]}/slave-agent.jnlp -workDir ${JENKINS_WORKDIR} -noReconnect -secret ${TOKENS[$index]} || echo "Invalid or already used credentials." || True
-    # ${JAVA} ${JAVA_ARGS} -jar ${AGENT_JAR} -jnlpUrl ${JENKINS_URL}/computer/${AGENTS[$index]}/slave-agent.jnlp -workDir ${JENKINS_WORKDIR} -noReconnect -secret ${TOKENS[$index]} || tail -f /dev/null
+    echo "About to run ${JAVA}" "${JAVA_ARGS}" -jar "${AGENT_JAR}" -jnlpUrl "${JENKINS_URL}"/computer/"${AGENTS[$index]}"/slave-agent.jnlp -workDir "${JENKINS_WORKDIR}" -noReconnect -secret "${TOKENS[$index]}"
+    ${JAVA} ${JAVA_ARGS} -jar ${AGENT_JAR} -jnlpUrl ${JENKINS_URL}/computer/${AGENTS[$index]}/slave-agent.jnlp -workDir ${JENKINS_WORKDIR} -noReconnect -secret ${TOKENS[$index]} || echo "Invalid or already used credentials."
 done
-echo "Tail End"
-tail -f /dev/null
-echo "Tail After End"
\ No newline at end of file
diff --git a/src/charm.py b/src/charm.py
index ea6d681..212e5b3 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -4,15 +4,12 @@
 # Licensed under the GPLv3, see LICENCE file for details.
 
 import io
-import pprint
-import os
-import sys
 import logging
-
-sys.path.append('lib')  # noqa: E402
+import os
+import pprint
 
 from ops.charm import CharmBase
-from ops.framework import StoredState, EventSource, EventBase
+from ops.framework import StoredState
 from ops.main import main
 from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus
 
@@ -23,25 +20,17 @@ logger = logging.getLogger()
 class JenkinsAgentCharm(CharmBase):
     _stored = StoredState()
 
-    # on_slave_relation_configured = EventSource(SlaveRelationConfigureEvent)
+    def __init__(self, *args):
+        super().__init__(*args)
 
-    def __init__(self, framework, parent):
-        super().__init__(framework, parent)
-
-        framework.observe(self.on.start, self.configure_pod)
-        framework.observe(self.on.config_changed, self.configure_pod)
-        framework.observe(self.on.upgrade_charm, self.configure_pod)
-        framework.observe(self.on.slave_relation_joined, self.on_slave_relation_joined)
-        framework.observe(self.on.slave_relation_changed, self.on_slave_relation_changed)
+        self.framework.observe(self.on.start, self.configure_pod)
+        self.framework.observe(self.on.config_changed, self.configure_pod)
+        self.framework.observe(self.on.upgrade_charm, self.configure_pod)
+        self.framework.observe(self.on.slave_relation_joined, self.on_agent_relation_joined)
+        self.framework.observe(self.on.slave_relation_changed, self.on_agent_relation_changed)
 
         self._stored.set_default(_spec=None, jenkins_url=None, agent_tokens=None, agents=None)
 
-    def on_upgrade_charm(self, event):
-        pass
-
-    def on_config_changed(self, event):
-        pass
-
     def generate_pod_config(self, config, secured=True):
         """Kubernetes pod config generator.
 
@@ -51,40 +40,29 @@ class JenkinsAgentCharm(CharmBase):
         """
         pod_config = {}
 
-        pod_config["JENKINS_API_USER"] = config["jenkins_user"]
-
         if self._stored.jenkins_url:
             pod_config["JENKINS_URL"] = self._stored.jenkins_url
-        elif config.get("jenkins_master_url", None):
+        else:
             pod_config["JENKINS_URL"] = config["jenkins_master_url"]
-        if config.get("jenkins_agent_name", None):
-            pod_config["JENKINS_HOSTNAME"] = config["jenkins_agent_name"]
 
         if secured:
             return pod_config
 
-        # pod_config["JENKINS_API_TOKEN"] = self._stored.agent_tokens or config["jenkins_api_token"]
         if self._stored.agent_tokens and self._stored.agents:
-            # for agent in self._stored.agents:
-            #     logger.info("ALEJDG - generate_pod_config - self._stored.agent: %s", agent)
-            logger.info("ALEJDG - generate_pod_config - self._stored.agent: %s", self._stored.agents)
-            # pod_config["JENKINS_AGENTS"] = ":".join(self._stored.agents)
             pod_config["JENKINS_AGENTS"] = ":".join(self._stored.agents)
             pod_config["JENKINS_TOKENS"] = ":".join(self._stored.agent_tokens)
         else:
-            pod_config["JENKINS_TOKENS"] = config["jenkins_api_token"]
+            pod_config["JENKINS_AGENTS"] = config["jenkins_agent_name"]
+            pod_config["JENKINS_TOKENS"] = config["jenkins_agent_token"]
 
         return pod_config
 
     def configure_pod(self, event):
+        """Assemble the pod spec and apply it, if possible."""
         is_valid = self.is_valid_config()
         if not is_valid:
             return
 
-        if not self.unit.is_leader():
-            self.unit.status = ActiveStatus()
-            return
-
         spec = self.make_pod_spec()
         if spec != self._stored._spec:
             self._stored._spec = spec
@@ -107,8 +85,8 @@ class JenkinsAgentCharm(CharmBase):
         self.model.unit.status = ActiveStatus()
 
     def make_pod_spec(self):
+        """Prepare and return a pod spec."""
         config = self.model.config
-        logger.info("ALEJDG - config type: %s - config data: %s", type(config), config)
 
         full_pod_config = self.generate_pod_config(config, secured=False)
         secure_pod_config = self.generate_pod_config(config, secured=True)
@@ -125,23 +103,25 @@ class JenkinsAgentCharm(CharmBase):
         }
 
         out = io.StringIO()
+        pprint.pprint(spec, out)
         logger.info("This is the Kubernetes Pod spec config (sans secrets) <<EOM\n{}\nEOM".format(out.getvalue()))
 
         secure_pod_config.update(full_pod_config)
-        pprint.pprint(spec, out)
-        logger.info("This is the Kubernetes Pod spec config (with secrets) <<EOM\n{}\nEOM".format(out.getvalue()))
-
         return spec
 
     def is_valid_config(self):
+        """Validate required configuration.
+
+        When not configuring the agent through relations
+        'jenkins_master_url', 'jenkins_agent_name' and 'jenkins_agent_token'
+        are required."""
         is_valid = True
 
         config = self.model.config
-        logger.info("ALEJDG SPEC: %s", self._stored._spec)
         if self._stored.agent_tokens:
-            want = ("image", "jenkins_user")
+            want = ("image",)
         else:
-            want = ("image", "jenkins_user", "jenkins_api_token")
+            want = ("image", "jenkins_master_url", "jenkins_agent_name", "jenkins_agent_token")
         missing = [k for k in want if config[k].rstrip() == ""]
         if missing:
             message = "Missing required config: {}".format(" ".join(missing))
@@ -151,82 +131,45 @@ class JenkinsAgentCharm(CharmBase):
 
         return is_valid
 
-    def on_slave_relation_joined(self, event):
+    def on_agent_relation_joined(self, event):
         logger.info("Jenkins relation joined")
-        noexecutors = os.cpu_count()
-        config_labels = self.model.config.get('labels')
-        agent_name = ""
-        if self._stored.agents:
-            self._stored.agents[-1]
-            name, number = self._stored.agents[-1].rsplit('-', 1)
-            agent_name = "{}-{}".format(name, int(number) + 1)
-        else:
-            self._stored.agents = []
-            agent_name = self.unit.name.replace('/', '-')
+        num_executors = os.cpu_count()
+        config_labels = self.model.config.get('jenkins_agent_labels')
+        agent_name = self._gen_agent_name()
 
         if config_labels:
             labels = config_labels
         else:
-            labels = os.uname()[4]
-
-        # slave_address = hookenv.unit_private_ip()
+            print(os.uname())
+            labels = os.uname().machine
 
-        logger.info("noexecutors: %s - type: %s", noexecutors, type(noexecutors))
-        logger.info("labels: %s - type: %s",labels, type(labels))
-        event.relation.data[self.model.unit]["executors"] = str(noexecutors)
+        event.relation.data[self.model.unit]["executors"] = str(num_executors)
         event.relation.data[self.model.unit]["labels"] = labels
         event.relation.data[self.model.unit]["slavehost"] = agent_name
 
-        remote_data = event.relation.data[event.app]
-        logger.info("ALEJDG - remote_data_app: %s", remote_data)
-        for i in remote_data:
-            logger.info("ALEJDG - remote_data_app['%s']: %s", i, remote_data[i])
-
-        if event.unit is not None:
-            remote_data = event.relation.data[event.unit]
-            logger.info("ALEJDG - os.environ: %s", os.environ)
-
-        logger.info("ALEJDG - remote_data_post_app: %s", remote_data)
-        for i in remote_data:
-            logger.info("ALEJDG - remote_data_post_app['%s']: %s", i, remote_data[i])
-
-    def on_slave_relation_changed(self, event):
+    def on_agent_relation_changed(self, event):
+        """Populate local configuration with data from relation"""
         logger.info("Jenkins relation changed")
         try:
-            logger.info("ALEJDG - event.relation.data[event.unit]['url']: %s", event.relation.data[event.unit]['url'])
             self._stored.jenkins_url = event.relation.data[event.unit]['url']
         except KeyError:
             pass
 
         try:
-            logger.info("ALEJDG - event.relation.data[event.unit]['secret']: %s", event.relation.data[event.unit]['secret'])
-            logger.info("ALEJDG - event.unit.name: %s", event.unit.name)
-            logger.info("ALEJDG - self.unit.name.: %s", self.unit.name)
             self._stored.agent_tokens = self._stored.agent_tokens or []
             self._stored.agent_tokens.append(event.relation.data[event.unit]['secret'])
-            agent_name = ""
-            if self._stored.agents:
-                logger.info("ALEJDG - self._stored.agents[-1]: %s", self._stored.agents[-1])
-                self._stored.agents[-1]
-                name, number = self._stored.agents[-1].rsplit('-', 1)
-                agent_name = "{}-{}".format(name, int(number) + 1)
-                self._stored.agents.append(agent_name)
-            else:
-                self._stored.agents = []
-                agent_name = self.unit.name.replace('/', '-')
-                self._stored.agents.append(agent_name)
+            self._gen_agent_name(store=True)
         except KeyError:
             pass
 
-        self.configure_slave_through_relation(event)
+        self.configure_through_relation(event)
 
-    def configure_slave_through_relation(self, event):
-        logger.info("Setting up jenkins via slave relation")
-        logger.info("ALEJDG - on_slave_relation_joined - self._stored.agents_setup: %s", self._stored.agents)
+    def configure_through_relation(self, event):
+        logger.info("Setting up jenkins via agent relation")
         self.model.unit.status = MaintenanceStatus("Configuring jenkins agent")
 
-        if self.model.config.get("url"):
-            logger.info("Config option 'url' is set. Can't use agent relation.")
+        if self.model.config.get("jenkins_master_url"):
+            logger.info("Config option 'jenkins_master_url' is set. Can't use agent relation.")
             self.model.unit.status = ActiveStatus()
             return
 
@@ -242,6 +185,20 @@ class JenkinsAgentCharm(CharmBase):
 
         self.configure_pod(event)
 
+    def _gen_agent_name(self, store=False):
+        agent_name = ""
+        if self._stored.agents:
+            name, number = self._stored.agents[-1].rsplit('-', 1)
+            agent_name = "{}-{}".format(name, int(number) + 1)
+            if store:
+                self._stored.agents.append(agent_name)
+        else:
+            agent_name = self.unit.name.replace('/', '-')
+            if store:
+                self._stored.agents = []
+                self._stored.agents.append(agent_name)
+        return agent_name
+
 
 if __name__ == '__main__':
     main(JenkinsAgentCharm)
diff --git a/src/interface_jenkins_slave.py b/src/interface_jenkins_slave.py
deleted file mode 100644
index dec278d..0000000
--- a/src/interface_jenkins_slave.py
+++ /dev/null
@@ -1,66 +0,0 @@
-import json
-
-from ops.framework import EventBase, EventsBase, EventSource, Object, StoredState
-
-
-class NewClient(EventBase):
-    def __init__(self, handle, client):
-        super().__init__(handle)
-        self.client = client
-
-    def snapshot(self):
-        return {
-            'relation_name': self.client._relation.name,
-            'relation_id': self.client._relation.id,
-        }
-
-    def restore(self, snapshot):
-        relation = self.model.get_relation(snapshot['relation_name'], snapshot['relation_id'])
-        self.client = HTTPInterfaceClient(relation, self.model.unit)
-
-
-class HTTPServerEvents(EventsBase):
-    new_client = EventSource(NewClient)
-
-
-class HTTPServer(Object):
-    on = HTTPServerEvents()
-    state = StoredState()
-
-    def __init__(self, charm, relation_name):
-        super().__init__(charm, relation_name)
-        self.relation_name = relation_name
-        self.framework.observe(charm.on.start, self.init_state)
-        self.framework.observe(charm.on[relation_name].relation_joined, self.on_joined)
-        self.framework.observe(charm.on[relation_name].relation_departed, self.on_departed)
-
-    def init_state(self, event):
-        self.state.apps = []
-
-    @property
-    def _relations(self):
-        return self.model.relations[self.relation_name]
-
-    def on_joined(self, event):
-        if event.app not in self.state.apps:
-            self.state.apps.append(event.app)
-            self.on.new_client.emit(HTTPInterfaceClient(event.relation, self.model.unit))
-
-    def on_departed(self, event):
-        self.state.apps = [app for app in self._relations]
-
-    def clients(self):
-        return [HTTPInterfaceClient(relation, self.model.unit) for relation in self._relations]
-
-
-class HTTPInterfaceClient:
-    def __init__(self, relation, local_unit):
-        self._relation = relation
-        self._local_unit = local_unit
-        self.ingress_address = relation.data[local_unit]['ingress-address']
-
-    def serve(self, hosts, port):
-        self._relation.data[self._local_unit]['extended_data'] = json.dumps([{
-            'hostname': host,
-            'port': port,
-        } for host in hosts])
\ No newline at end of file
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 65705e7..b447a9a 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -1,21 +1,315 @@
 # Copyright 2020 Canonical Ltd.
 # Licensed under the GPLv3, see LICENCE file for details.
 
+from mock import MagicMock, patch
 import unittest
 
+from ops import testing
+from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus
+
+
 import sys
 
 sys.path.append('src')  # noqa: E402
 
-from charm import generate_pod_config
+from charm import JenkinsAgentCharm
+
+CONFIG_DEFAULT = {
+    "image": "jenkins-agent-operator",
+    "jenkins_master_url": "",
+    "jenkins_agent_name": "",
+    "jenkins_agent_token": "",
+    "jenkins_agent_label": ""
+}
+
+CONFIG_ONE_AGENT = {
+    "jenkins_master_url": "http://test",
+    "jenkins_agent_name": "agent-one",
+    "jenkins_agent_token": "token-one"
+}
+
+CONFIG_ONE_AGENT_CUSTOM_IMAGE = {
+    "image": "image-name",
+    "jenkins_master_url": "http://test",
+    "jenkins_agent_name": "agent-one",
+    "jenkins_agent_token": "token-one"
+}
+
+SPEC_EXPECTED = {
+    'containers': [{
+        'config': {
+            'JENKINS_AGENTS': 'agent-one',
+            'JENKINS_TOKENS': 'token-one',
+            'JENKINS_URL': 'http://test'
+        },
+        'imageDetails': {
+            'imagePath': 'image-name'
+        },
+        'name': 'jenkins-agent',
+        'readinessProbe': {
+            'exec': {
+                'command': [
+                    '/bin/cat',
+                    '/var/lib/jenkins/agents/.ready'
+                ]
+            }
+        }
+    }]
+}
 
 
 class TestJenkinsAgentCharm(unittest.TestCase):
-    def test_generate_pod_config(self):
-        config = {
-            "jenkins_user": "test",
+    def setUp(self):
+        self.harness = testing.Harness(JenkinsAgentCharm)
+        self.harness.begin()
+        self.harness.disable_hooks()
+        self.harness.update_config(CONFIG_DEFAULT)
+
+    def test__generate_pod_config(self):
+        """Test generate_pod_config"""
+        expected = {
+            "JENKINS_URL": "http://test",
         }
+        self.assertEqual(self.harness.charm.generate_pod_config(CONFIG_ONE_AGENT, secured=True), expected)
+
+        expected["JENKINS_AGENTS"] = "agent-one"
+        expected["JENKINS_TOKENS"] = "token-one"
+        self.assertEqual(self.harness.charm.generate_pod_config(CONFIG_ONE_AGENT, secured=False), expected)
+
+    def test__generate_pod_config__with__relation__data(self):
+        """Test generate_pod_config with relation data"""
         expected = {
-            "JENKINS_API_USER": "test",
+            "JENKINS_URL": "http://test",
+        }
+        agent_name = "jenkins-agent-0"
+        url = "http://test"
+        token = "token"
+
+        self.harness.charm._stored.jenkins_url = url
+        self.harness.charm._stored.agent_tokens = [token]
+        self.harness.charm._stored.agents = [agent_name]
+
+        self.assertEqual(self.harness.charm.generate_pod_config(CONFIG_ONE_AGENT, secured=True), expected)
+
+        expected["JENKINS_AGENTS"] = "jenkins-agent-0"
+        expected["JENKINS_TOKENS"] = "token"
+        self.assertEqual(self.harness.charm.generate_pod_config(CONFIG_ONE_AGENT, secured=False), expected)
+
+    def test__configure_pod__invalid__config(self):
+        """Test configure_pod when the config is invalid"""
+        self.harness.charm.on.config_changed.emit()
+        message = "Missing required config: jenkins_master_url jenkins_agent_name jenkins_agent_token"
+        self.assertEqual(self.harness.model.unit.status, BlockedStatus(message))
+        self.assertEqual(self.harness.get_pod_spec(), None)
+
+    def test__configure_pod__unit__not__leader(self):
+        """Test configure_pod when the unit isn't leader"""
+        self.harness.set_leader(is_leader=False)
+        self.harness.update_config(CONFIG_ONE_AGENT)
+        with self.assertLogs(level='INFO') as logger:
+            self.harness.charm.on.config_changed.emit()
+            self.assertEqual(self.harness.model.unit.status, ActiveStatus())
+            message = "INFO:root:Spec changes ignored by non-leader"
+            self.assertEqual(logger.output[-1], message)
+            self.assertEqual(self.harness.get_pod_spec(), None)
+
+    def test__configure_pod(self):
+        """Test configure_pod"""
+        expected = (SPEC_EXPECTED, None)
+
+        self.harness.set_leader(is_leader=True)
+        self.harness.update_config(CONFIG_ONE_AGENT_CUSTOM_IMAGE)
+        self.harness.charm.on.config_changed.emit()
+        self.assertEqual(self.harness.model.unit.status, ActiveStatus())
+        self.assertEqual(self.harness.get_pod_spec(), expected)
+
+    def test__configure_pod__no__spec_change(self):
+        """Test configure_pod when there is no change in the spec"""
+        self.harness.set_leader(is_leader=True)
+        self.harness.update_config(CONFIG_ONE_AGENT_CUSTOM_IMAGE)
+        self.harness.charm._stored._spec = self.harness.charm.make_pod_spec()
+        with self.assertLogs(level='INFO') as logger:
+            self.harness.charm.on.config_changed.emit()
+            self.assertEqual(self.harness.model.unit.status, ActiveStatus())
+            message = "INFO:root:Pod spec unchanged"
+            self.assertEqual(logger.output[-1], message)
+            self.assertEqual(self.harness.get_pod_spec(), None)
+
+    def test__make_pod_spec(self):
+        """Test the construction of the spec based on juju config"""
+        self.harness.update_config(CONFIG_ONE_AGENT_CUSTOM_IMAGE)
+        self.assertEqual(self.harness.charm.make_pod_spec(), SPEC_EXPECTED)
+
+    def test__is_valid_config(self):
+        """Test config validation"""
+        config = {
+            "image": "image-name",
+            "jenkins_master_url": "http://test",
+            "jenkins_agent_name": "agent-one",
+            "jenkins_agent_token": "token-one"
         }
-        self.assertEqual(generate_pod_config(config), expected)
+        self.assertEqual(self.harness.charm.is_valid_config(), False)
+        with self.subTest("Config from relation"):
+            self.harness.charm._stored.agent_tokens = "token"
+            self.assertEqual(self.harness.charm.is_valid_config(), True)
+        with self.subTest("Config from juju config"):
+            self.harness.update_config(config)
+            self.assertEqual(self.harness.charm.is_valid_config(), True)
+
+    @patch("os.uname")
+    @patch("os.cpu_count")
+    def test__on_agent_relation_joined(self, mock_os_cpu_count, mock_os_uname):
+        """Test relation_data is set when a new relation joins"""
+        mock_os_cpu_count.return_value = 8
+        mock_os_uname.return_value.machine = "x86_64"
+        expected_relation_data = {
+            'executors': '8',
+            'labels': 'x86_64',
+            'slavehost': 'jenkins-agent-0'
+        }
+        self.harness.enable_hooks()
+        rel_id = self.harness.add_relation("slave", "jenkins")
+        self.harness.add_relation_unit(rel_id, "jenkins/0")
+
+        self.assertEqual(self.harness.get_relation_data(rel_id, "jenkins-agent/0"), expected_relation_data)
+
+    @patch("charm.JenkinsAgentCharm.configure_pod")
+    def test__configure_through_relation__no__jenkins_master_url(self, mock_configure_pod):
+        """Test configure_through_relation when no configuration has been provided"""
+        mock_event = MagicMock()
+        with self.assertLogs(level='INFO') as logger:
+            self.harness.charm.configure_through_relation(mock_event)
+            expected_output = [
+                "INFO:root:Setting up jenkins via agent relation",
+                "INFO:root:Jenkins hasn't exported its url yet. Skipping setup for now."
+            ]
+            self.assertEqual(logger.output, expected_output)
+            mock_configure_pod.assert_not_called()
+            self.assertEqual(self.harness.model.unit.status, ActiveStatus())
+
+    @patch("charm.JenkinsAgentCharm.configure_pod")
+    def test__configure_through_relation__no__jenkins_agent_tokens(self, mock_configure_pod):
+        """Test configure_through_relation when no tokens have been provided"""
+        mock_event = MagicMock()
+        self.harness.charm._stored.jenkins_url = "http://test"
+        with self.assertLogs(level='INFO') as logger:
+            self.harness.charm.configure_through_relation(mock_event)
+            expected_output = [
+                "INFO:root:Setting up jenkins via agent relation",
+                "INFO:root:Jenkins hasn't exported the agent secret yet. Skipping setup for now."
+            ]
+            self.assertEqual(logger.output, expected_output)
+            mock_configure_pod.assert_not_called()
+            self.assertEqual(self.harness.model.unit.status, ActiveStatus())
+
+    @patch("charm.JenkinsAgentCharm.configure_pod")
+    def test__configure_through_relation__manual__config(self, mock_configure_pod):
+        """Test configure_through_relation when no configuration has been provided"""
+        mock_event = MagicMock()
+        self.harness.update_config({"jenkins_master_url": "http://test"})
+        with self.assertLogs(level='INFO') as logger:
+            self.harness.charm.configure_through_relation(mock_event)
+            expected_output = [
+                "INFO:root:Setting up jenkins via agent relation",
+                "INFO:root:Config option 'jenkins_master_url' is set. Can't use agent relation."
+            ]
+            self.assertEqual(logger.output, expected_output)
+            mock_configure_pod.assert_not_called()
+            self.assertEqual(self.harness.model.unit.status, ActiveStatus())
+
+    @patch("charm.JenkinsAgentCharm.configure_pod")
+    def test__configure_through_relation(self, mock_configure_pod):
+        """Test configure_through_relation when the relation have provided all needed data"""
+        mock_event = MagicMock()
+        self.harness.charm._stored.jenkins_url = "http://test"
+        self.harness.charm._stored.agent_tokens = "token"
+        with self.assertLogs(level='INFO') as logger:
+            self.harness.charm.configure_through_relation(mock_event)
+            expected_output = [
+                "INFO:root:Setting up jenkins via agent relation"
+            ]
+            self.assertEqual(logger.output, expected_output)
+            mock_configure_pod.assert_called()
+            self.assertEqual(self.harness.model.unit.status, MaintenanceStatus("Configuring jenkins agent"))
+
+    @patch("os.uname")
+    @patch("os.cpu_count")
+    def test__on_agent_relation_joined__custom__label(self, mock_os_cpu_count, mock_os_uname):
+        """Test relation_data is set when a new relation joins
+            and custom labels are set"""
+        mock_os_cpu_count.return_value = 8
+        mock_os_uname.return_value.machine = "x86_64"
+        labels = "test, label"
+        expected_relation_data = {
+            'executors': '8',
+            'labels': labels,
+            'slavehost': 'jenkins-agent-0'
+        }
+        self.harness.update_config({"jenkins_agent_labels": labels})
+        self.harness.enable_hooks()
+        rel_id = self.harness.add_relation("slave", "jenkins")
+        self.harness.add_relation_unit(rel_id, "jenkins/0")
+        self.assertEqual(self.harness.get_relation_data(rel_id, "jenkins-agent/0"), expected_relation_data)
+
+    @patch("charm.JenkinsAgentCharm.configure_through_relation")
+    @patch("os.uname")
+    @patch("os.cpu_count")
+    def test__on_agent_relation_changed__noop(self, mock_os_cpu_count, mock_os_uname, mock_configure_through_relation):
+        """Test on_agent_relation_changed when jenkins hasn't provided information yet"""
+        mock_os_cpu_count.return_value = 8
+        mock_os_uname.return_value.machine = "x86_64"
+        remote_unit = "jenkins/0"
+        self.harness.enable_hooks()
+        rel_id = self.harness.add_relation("slave", "jenkins")
+        self.harness.add_relation_unit(rel_id, remote_unit)
+        self.harness.update_relation_data(rel_id, remote_unit, {})
+
+        mock_configure_through_relation.assert_called()
+
+    @patch("charm.JenkinsAgentCharm.configure_through_relation")
+    @patch("os.uname")
+    @patch("os.cpu_count")
+    def test__on_agent_relation_changed_old(self, mock_os_cpu_count, mock_os_uname, mock_configure_through_relation):
+        """Test relation_data is set when a new relation joins"""
+        mock_os_cpu_count.return_value = 8
+        mock_os_uname.return_value.machine = "x86_64"
+        remote_unit = "jenkins/0"
+        agent_name = "jenkins-agent-0"
+        url = "http://test"
+        secret = "token"
+        self.harness.enable_hooks()
+        rel_id = self.harness.add_relation("slave", "jenkins")
+        self.harness.add_relation_unit(rel_id, remote_unit)
+        self.harness.update_relation_data(rel_id, remote_unit, {"url": url, "secret": secret})
+        self.assertEqual(self.harness.charm._stored.jenkins_url, url)
+        self.assertEqual(self.harness.charm._stored.agent_tokens[-1], secret)
+        self.assertEqual(self.harness.charm._stored.agents[-1], agent_name)
+        mock_configure_through_relation.assert_called()
+
+    @patch("charm.JenkinsAgentCharm.configure_through_relation")
+    @patch("os.uname")
+    @patch("os.cpu_count")
+    def test__on_agent_relation_changed__multiple__agents(
+        self,
+        mock_os_cpu_count,
+        mock_os_uname,
+        mock_configure_through_relation
+    ):
+        """Test relation_data is set when a new relation joins"""
+        mock_os_cpu_count.return_value = 8
+        mock_os_uname.return_value.machine = "x86_64"
+        remote_unit = "jenkins/0"
+        agent_name = "jenkins-agent-0"
+        expected_new_agent = "jenkins-agent-1"
+        url = "http://test"
+        secret = "token"
+
+        self.harness.charm._stored.agents = [agent_name]
+        self.harness.enable_hooks()
+        rel_id = self.harness.add_relation("slave", "jenkins")
+        self.harness.add_relation_unit(rel_id, remote_unit)
+        self.harness.update_relation_data(rel_id, remote_unit, {"url": url, "secret": secret})
+        self.assertEqual(self.harness.charm._stored.jenkins_url, url)
+        self.assertEqual(self.harness.charm._stored.agent_tokens[-1], secret)
+        self.assertEqual(self.harness.charm._stored.agents[-1], expected_new_agent)
+        mock_configure_through_relation.assert_called()