Merge charm-k8s-jenkins-agent:manual-agent into charm-k8s-jenkins-agent:master

Proposed by Alexandre Gomes
Status: Rejected
Rejected by: Alexandre Gomes
Proposed branch: charm-k8s-jenkins-agent:manual-agent
Merge into: charm-k8s-jenkins-agent:master
Diff against target: 437 lines (+258/-39)
6 files modified
Makefile (+7/-0)
dockerfile/Dockerfile (+2/-1)
dockerfile/files/entrypoint.sh (+25/-3)
metadata.yaml (+3/-0)
src/charm.py (+155/-35)
src/interface_jenkins_slave.py (+66/-0)
Reviewer Review Type Date Requested Status
Alexandre Gomes Needs Resubmitting
Tom Haddon Needs Fixing
Canonical IS Reviewers Pending
Review via email: mp+389041@code.launchpad.net
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 :

Unable to determine commit message from repository - please click "Set commit message" and enter the commit message manually.

Revision history for this message
Tom Haddon (mthaddon) wrote :

We're missing a commit message for this MP, so it wouldn't let us merge yet.

I've included some comments inline, but also I had to make the following changes to get lint and tests to run (`make test`) https://paste.ubuntu.com/p/j3tSHmbmcB/

review: Needs Fixing
Revision history for this message
Alexandre Gomes (alejdg) wrote :

> We're missing a commit message for this MP, so it wouldn't let us merge yet.
>
> I've included some comments inline, but also I had to make the following
> changes to get lint and tests to run (`make test`)
> https://paste.ubuntu.com/p/j3tSHmbmcB/

I addressed all the comments but two:

- The term "master" isn't going away as per their blog post[1], only "slave". So I think we should follow the same terms Jenkins devs are using.
- The interface is still called jenkins-slave, so we should keep that until we update the interface.

review: Needs Resubmitting
Revision history for this message
Alexandre Gomes (alejdg) wrote :
Revision history for this message
Alexandre Gomes (alejdg) wrote :

Unmerged commits

9ba35a0... by Alexandre Gomes

Improve the charm so it can work properly with juju config and add unit

79d2ac2... by Alexandre Gomes

Fix redeployment issues and make the charm not scalable

0d8faa5... by Alexandre Gomes

Save agent's secret in a dictionary instead

1ca7e4b... by Alexandre Gomes

Address comments and make use of the secret provided by jenkins through relation

8e2d166... by Alexandre Gomes

Fix logging

4f4c6cb... by Alexandre Gomes

Add slave_relation_joined

0eea73b... by Alexandre Gomes

Create on_jenkins_relation_joined event

dcfe2f6... by Alexandre Gomes

Add initial jenkins-slave relation

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/Makefile b/Makefile
index 86bd8d3..8c8056a 100644
--- a/Makefile
+++ b/Makefile
@@ -18,6 +18,13 @@ build-image: | $(dockerfile_dir)
18 --build-arg REVISION="$(revision)" \18 --build-arg REVISION="$(revision)" \
19 -t $(JENKINS_IMAGE) $(dockerfile_dir)19 -t $(JENKINS_IMAGE) $(dockerfile_dir)
2020
21build-image-no-cache: | $(dockerfile_dir)
22 docker build \
23 --build-arg AUTHOR=$(author) \
24 --build-arg REVISION="$(revision)" \
25 --no-cache \
26 -t $(JENKINS_IMAGE) $(dockerfile_dir)
27
21build-release-image: | $(dockerfile_dir)28build-release-image: | $(dockerfile_dir)
22 docker build \29 docker build \
23 --build-arg DATE_CREATED="$(date_created)" \30 --build-arg DATE_CREATED="$(date_created)" \
diff --git a/dockerfile/Dockerfile b/dockerfile/Dockerfile
index 1abd7e6..e03339f 100644
--- a/dockerfile/Dockerfile
+++ b/dockerfile/Dockerfile
@@ -47,7 +47,8 @@ RUN apt-get update -y \
4747
48WORKDIR /var/lib/jenkins48WORKDIR /var/lib/jenkins
4949
50USER ${USER}50#USER ${USER}
51USER root
5152
52COPY files/entrypoint.sh /53COPY files/entrypoint.sh /
5354
diff --git a/dockerfile/files/entrypoint.sh b/dockerfile/files/entrypoint.sh
index c4e879c..c8ece01 100755
--- a/dockerfile/files/entrypoint.sh
+++ b/dockerfile/files/entrypoint.sh
@@ -3,6 +3,7 @@
3set -eu -o pipefail3set -eu -o pipefail
44
5export LC_ALL=C5export LC_ALL=C
6export TERM=xterm
67
7# defaults for jenkins-agent component of the jenkins continuous integration8# defaults for jenkins-agent component of the jenkins continuous integration
8# system9# system
@@ -25,11 +26,15 @@ typeset JENKINS_URL="${JENKINS_URL:?"URL of a jenkins server must be provided"}"
25# hostname of the server the agent is running on.26# hostname of the server the agent is running on.
26typeset JENKINS_HOSTNAME="${JENKINS_HOSTNAME:-$(hostname)}"27typeset JENKINS_HOSTNAME="${JENKINS_HOSTNAME:-$(hostname)}"
2728
29
30typeset JENKINS_WORKDIR="/var/lib/jenkins"
31
28# Arguments to pass to jenkins agent on startup32# Arguments to pass to jenkins agent on startup
29typeset -a JENKINS_ARGS33typeset -a JENKINS_ARGS
3034
31JENKINS_ARGS+=(-jnlpUrl "${JENKINS_URL}"/computer/"${JENKINS_HOSTNAME}"/slave-agent.jnlp)35# JENKINS_ARGS+=(-jnlpUrl "${JENKINS_URL}"/computer/"${JENKINS_HOSTNAME}"/slave-agent.jnlp)
32JENKINS_ARGS+=(-jnlpCredentials "${JENKINS_API_USER:?Please specify JENKINS_API_USER}:${JENKINS_API_TOKEN:?Please specify JENKINS_API_TOKEN}")36# JENKINS_ARGS+=(-jnlpCredentials "${JENKINS_API_USER:?Please specify JENKINS_API_USER}:${JENKINS_API_TOKEN:?Please specify JENKINS_API_TOKEN}")
37# JENKINS_ARGS+=(-noReconect)
3338
34# Path of the agent.jar39# Path of the agent.jar
35typeset AGENT_JAR=/var/lib/jenkins/agent.jar40typeset AGENT_JAR=/var/lib/jenkins/agent.jar
@@ -56,4 +61,21 @@ download_agent
56touch /var/lib/jenkins/agents/.ready61touch /var/lib/jenkins/agents/.ready
5762
58#shellcheck disable=SC208663#shellcheck disable=SC2086
59"${JAVA}" ${JAVA_ARGS} -jar "${AGENT_JAR}" "${JENKINS_ARGS[@]}"64# "${JAVA}" ${JAVA_ARGS} -jar "${AGENT_JAR}" "${JENKINS_ARGS[@]}"
65
66# Transform the env variables in arrays to iterate through it
67IFS=':' read -r -a AGENTS <<< ${JENKINS_AGENTS}
68IFS=':' read -r -a TOKENS <<< ${JENKINS_TOKENS}
69
70echo ${!AGENTS[@]}
71
72for index in ${!AGENTS[@]}; do
73 echo "agent : ${AGENTS[$index]}"
74 echo "value: ${TOKENS[$index]}"
75 echo "${JAVA}" "${JAVA_ARGS}" -jar "${AGENT_JAR}" -jnlpUrl "${JENKINS_URL}"/computer/"${AGENTS[$index]}"/slave-agent.jnlp -workDir "${JENKINS_WORKDIR}" -noReconnect -secret "${TOKENS[$index]}"
76 ${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
77 # ${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
78done
79echo "Tail End"
80tail -f /dev/null
81echo "Tail After End"
60\ No newline at end of file82\ No newline at end of file
diff --git a/metadata.yaml b/metadata.yaml
index b53a613..dd92ccb 100644
--- a/metadata.yaml
+++ b/metadata.yaml
@@ -9,3 +9,6 @@ series:
9 - kubernetes9 - kubernetes
10deployment:10deployment:
11 service: omit11 service: omit
12provides:
13 slave:
14 interface: jenkins-slave
diff --git a/src/charm.py b/src/charm.py
index 4ba8782..ea6d681 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -5,10 +5,14 @@
55
6import io6import io
7import pprint7import pprint
8import os
9import sys
8import logging10import logging
911
12sys.path.append('lib') # noqa: E402
13
10from ops.charm import CharmBase14from ops.charm import CharmBase
11from ops.framework import StoredState15from ops.framework import StoredState, EventSource, EventBase
12from ops.main import main16from ops.main import main
13from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus17from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus
1418
@@ -16,32 +20,10 @@ from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus
16logger = logging.getLogger()20logger = logging.getLogger()
1721
1822
19def generate_pod_config(config, secured=True):
20 """Kubernetes pod config generator.
21
22 generate_pod_config generates Kubernetes deployment config.
23 If the secured keyword is set then it will return a sanitised copy
24 without exposing secrets.
25 """
26 pod_config = {}
27
28 pod_config["JENKINS_API_USER"] = config["jenkins_user"]
29 if config.get("jenkins_master_url", None):
30 pod_config["JENKINS_URL"] = config["jenkins_master_url"]
31 if config.get("jenkins_agent_name", None):
32 pod_config["JENKINS_HOSTNAME"] = config["jenkins_agent_name"]
33
34 if secured:
35 return pod_config
36
37 # Add secrets from charm config
38 pod_config["JENKINS_API_TOKEN"] = config["jenkins_api_token"]
39
40 return pod_config
41
42
43class JenkinsAgentCharm(CharmBase):23class JenkinsAgentCharm(CharmBase):
44 state = StoredState()24 _stored = StoredState()
25
26 # on_slave_relation_configured = EventSource(SlaveRelationConfigureEvent)
4527
46 def __init__(self, framework, parent):28 def __init__(self, framework, parent):
47 super().__init__(framework, parent)29 super().__init__(framework, parent)
@@ -49,8 +31,10 @@ class JenkinsAgentCharm(CharmBase):
49 framework.observe(self.on.start, self.configure_pod)31 framework.observe(self.on.start, self.configure_pod)
50 framework.observe(self.on.config_changed, self.configure_pod)32 framework.observe(self.on.config_changed, self.configure_pod)
51 framework.observe(self.on.upgrade_charm, self.configure_pod)33 framework.observe(self.on.upgrade_charm, self.configure_pod)
34 framework.observe(self.on.slave_relation_joined, self.on_slave_relation_joined)
35 framework.observe(self.on.slave_relation_changed, self.on_slave_relation_changed)
5236
53 self.state.set_default(_spec=None)37 self._stored.set_default(_spec=None, jenkins_url=None, agent_tokens=None, agents=None)
5438
55 def on_upgrade_charm(self, event):39 def on_upgrade_charm(self, event):
56 pass40 pass
@@ -58,14 +42,52 @@ class JenkinsAgentCharm(CharmBase):
58 def on_config_changed(self, event):42 def on_config_changed(self, event):
59 pass43 pass
6044
45 def generate_pod_config(self, config, secured=True):
46 """Kubernetes pod config generator.
47
48 generate_pod_config generates Kubernetes deployment config.
49 If the secured keyword is set then it will return a sanitised copy
50 without exposing secrets.
51 """
52 pod_config = {}
53
54 pod_config["JENKINS_API_USER"] = config["jenkins_user"]
55
56 if self._stored.jenkins_url:
57 pod_config["JENKINS_URL"] = self._stored.jenkins_url
58 elif config.get("jenkins_master_url", None):
59 pod_config["JENKINS_URL"] = config["jenkins_master_url"]
60 if config.get("jenkins_agent_name", None):
61 pod_config["JENKINS_HOSTNAME"] = config["jenkins_agent_name"]
62
63 if secured:
64 return pod_config
65
66 # pod_config["JENKINS_API_TOKEN"] = self._stored.agent_tokens or config["jenkins_api_token"]
67 if self._stored.agent_tokens and self._stored.agents:
68 # for agent in self._stored.agents:
69 # logger.info("ALEJDG - generate_pod_config - self._stored.agent: %s", agent)
70 logger.info("ALEJDG - generate_pod_config - self._stored.agent: %s", self._stored.agents)
71 # pod_config["JENKINS_AGENTS"] = ":".join(self._stored.agents)
72 pod_config["JENKINS_AGENTS"] = ":".join(self._stored.agents)
73 pod_config["JENKINS_TOKENS"] = ":".join(self._stored.agent_tokens)
74 else:
75 pod_config["JENKINS_TOKENS"] = config["jenkins_api_token"]
76
77 return pod_config
78
61 def configure_pod(self, event):79 def configure_pod(self, event):
62 is_valid = self.is_valid_config()80 is_valid = self.is_valid_config()
63 if not is_valid:81 if not is_valid:
64 return82 return
6583
84 if not self.unit.is_leader():
85 self.unit.status = ActiveStatus()
86 return
87
66 spec = self.make_pod_spec()88 spec = self.make_pod_spec()
67 if spec != self.state._spec:89 if spec != self._stored._spec:
68 self.state._spec = spec90 self._stored._spec = spec
69 # only the leader can set_spec()91 # only the leader can set_spec()
70 if self.model.unit.is_leader():92 if self.model.unit.is_leader():
71 spec = self.make_pod_spec()93 spec = self.make_pod_spec()
@@ -81,13 +103,15 @@ class JenkinsAgentCharm(CharmBase):
81 else:103 else:
82 logger.info("Pod spec unchanged")104 logger.info("Pod spec unchanged")
83105
84 self.state.is_started = True106 self._stored.is_started = True
85 self.model.unit.status = ActiveStatus()107 self.model.unit.status = ActiveStatus()
86108
87 def make_pod_spec(self):109 def make_pod_spec(self):
88 config = self.model.config110 config = self.model.config
89 full_pod_config = generate_pod_config(config, secured=False)111 logger.info("ALEJDG - config type: %s - config data: %s", type(config), config)
90 secure_pod_config = generate_pod_config(config, secured=True)112
113 full_pod_config = self.generate_pod_config(config, secured=False)
114 secure_pod_config = self.generate_pod_config(config, secured=True)
91115
92 spec = {116 spec = {
93 "containers": [117 "containers": [
@@ -101,18 +125,23 @@ class JenkinsAgentCharm(CharmBase):
101 }125 }
102126
103 out = io.StringIO()127 out = io.StringIO()
104 pprint.pprint(spec, out)
105 logger.info("This is the Kubernetes Pod spec config (sans secrets) <<EOM\n{}\nEOM".format(out.getvalue()))128 logger.info("This is the Kubernetes Pod spec config (sans secrets) <<EOM\n{}\nEOM".format(out.getvalue()))
106129
107 secure_pod_config.update(full_pod_config)130 secure_pod_config.update(full_pod_config)
131 pprint.pprint(spec, out)
132 logger.info("This is the Kubernetes Pod spec config (with secrets) <<EOM\n{}\nEOM".format(out.getvalue()))
108133
109 return spec134 return spec
110135
111 def is_valid_config(self):136 def is_valid_config(self):
112 is_valid = True137 is_valid = True
113 config = self.model.config
114138
115 want = ("image", "jenkins_user", "jenkins_api_token")139 config = self.model.config
140 logger.info("ALEJDG SPEC: %s", self._stored._spec)
141 if self._stored.agent_tokens:
142 want = ("image", "jenkins_user")
143 else:
144 want = ("image", "jenkins_user", "jenkins_api_token")
116 missing = [k for k in want if config[k].rstrip() == ""]145 missing = [k for k in want if config[k].rstrip() == ""]
117 if missing:146 if missing:
118 message = "Missing required config: {}".format(" ".join(missing))147 message = "Missing required config: {}".format(" ".join(missing))
@@ -122,6 +151,97 @@ class JenkinsAgentCharm(CharmBase):
122151
123 return is_valid152 return is_valid
124153
154 def on_slave_relation_joined(self, event):
155 logger.info("Jenkins relation joined")
156 noexecutors = os.cpu_count()
157 config_labels = self.model.config.get('labels')
158 agent_name = ""
159 if self._stored.agents:
160 self._stored.agents[-1]
161 name, number = self._stored.agents[-1].rsplit('-', 1)
162 agent_name = "{}-{}".format(name, int(number) + 1)
163 else:
164 self._stored.agents = []
165 agent_name = self.unit.name.replace('/', '-')
166
167 if config_labels:
168 labels = config_labels
169 else:
170 labels = os.uname()[4]
171
172 # slave_address = hookenv.unit_private_ip()
173
174 logger.info("noexecutors: %s - type: %s", noexecutors, type(noexecutors))
175 logger.info("labels: %s - type: %s",labels, type(labels))
176 event.relation.data[self.model.unit]["executors"] = str(noexecutors)
177 event.relation.data[self.model.unit]["labels"] = labels
178 event.relation.data[self.model.unit]["slavehost"] = agent_name
179
180 remote_data = event.relation.data[event.app]
181 logger.info("ALEJDG - remote_data_app: %s", remote_data)
182 for i in remote_data:
183 logger.info("ALEJDG - remote_data_app['%s']: %s", i, remote_data[i])
184
185 if event.unit is not None:
186 remote_data = event.relation.data[event.unit]
187 logger.info("ALEJDG - os.environ: %s", os.environ)
188
189 logger.info("ALEJDG - remote_data_post_app: %s", remote_data)
190 for i in remote_data:
191 logger.info("ALEJDG - remote_data_post_app['%s']: %s", i, remote_data[i])
192
193 def on_slave_relation_changed(self, event):
194 logger.info("Jenkins relation changed")
195 try:
196 logger.info("ALEJDG - event.relation.data[event.unit]['url']: %s", event.relation.data[event.unit]['url'])
197 self._stored.jenkins_url = event.relation.data[event.unit]['url']
198 except KeyError:
199 pass
200
201 try:
202 logger.info("ALEJDG - event.relation.data[event.unit]['secret']: %s", event.relation.data[event.unit]['secret'])
203 logger.info("ALEJDG - event.unit.name: %s", event.unit.name)
204 logger.info("ALEJDG - self.unit.name.: %s", self.unit.name)
205 self._stored.agent_tokens = self._stored.agent_tokens or []
206 self._stored.agent_tokens.append(event.relation.data[event.unit]['secret'])
207 agent_name = ""
208 if self._stored.agents:
209 logger.info("ALEJDG - self._stored.agents[-1]: %s", self._stored.agents[-1])
210 self._stored.agents[-1]
211 name, number = self._stored.agents[-1].rsplit('-', 1)
212 agent_name = "{}-{}".format(name, int(number) + 1)
213 self._stored.agents.append(agent_name)
214 else:
215 self._stored.agents = []
216 agent_name = self.unit.name.replace('/', '-')
217 self._stored.agents.append(agent_name)
218 except KeyError:
219 pass
220
221 self.configure_slave_through_relation(event)
222
223 def configure_slave_through_relation(self, event):
224 logger.info("Setting up jenkins via slave relation")
225 logger.info("ALEJDG - on_slave_relation_joined - self._stored.agents_setup: %s", self._stored.agents)
226 self.model.unit.status = MaintenanceStatus("Configuring jenkins agent")
227
228 if self.model.config.get("url"):
229 logger.info("Config option 'url' is set. Can't use agent relation.")
230 self.model.unit.status = ActiveStatus()
231 return
232
233 if self._stored.jenkins_url is None:
234 logger.info("Jenkins hasn't exported its url yet. Skipping setup for now.")
235 self.model.unit.status = ActiveStatus()
236 return
237
238 if self._stored.agent_tokens is None:
239 logger.info("Jenkins hasn't exported the agent secret yet. Skipping setup for now.")
240 self.model.unit.status = ActiveStatus()
241 return
242
243 self.configure_pod(event)
244
125245
126if __name__ == '__main__':246if __name__ == '__main__':
127 main(JenkinsAgentCharm)247 main(JenkinsAgentCharm)
diff --git a/src/interface_jenkins_slave.py b/src/interface_jenkins_slave.py
128new file mode 100644248new file mode 100644
index 0000000..dec278d
--- /dev/null
+++ b/src/interface_jenkins_slave.py
@@ -0,0 +1,66 @@
1import json
2
3from ops.framework import EventBase, EventsBase, EventSource, Object, StoredState
4
5
6class NewClient(EventBase):
7 def __init__(self, handle, client):
8 super().__init__(handle)
9 self.client = client
10
11 def snapshot(self):
12 return {
13 'relation_name': self.client._relation.name,
14 'relation_id': self.client._relation.id,
15 }
16
17 def restore(self, snapshot):
18 relation = self.model.get_relation(snapshot['relation_name'], snapshot['relation_id'])
19 self.client = HTTPInterfaceClient(relation, self.model.unit)
20
21
22class HTTPServerEvents(EventsBase):
23 new_client = EventSource(NewClient)
24
25
26class HTTPServer(Object):
27 on = HTTPServerEvents()
28 state = StoredState()
29
30 def __init__(self, charm, relation_name):
31 super().__init__(charm, relation_name)
32 self.relation_name = relation_name
33 self.framework.observe(charm.on.start, self.init_state)
34 self.framework.observe(charm.on[relation_name].relation_joined, self.on_joined)
35 self.framework.observe(charm.on[relation_name].relation_departed, self.on_departed)
36
37 def init_state(self, event):
38 self.state.apps = []
39
40 @property
41 def _relations(self):
42 return self.model.relations[self.relation_name]
43
44 def on_joined(self, event):
45 if event.app not in self.state.apps:
46 self.state.apps.append(event.app)
47 self.on.new_client.emit(HTTPInterfaceClient(event.relation, self.model.unit))
48
49 def on_departed(self, event):
50 self.state.apps = [app for app in self._relations]
51
52 def clients(self):
53 return [HTTPInterfaceClient(relation, self.model.unit) for relation in self._relations]
54
55
56class HTTPInterfaceClient:
57 def __init__(self, relation, local_unit):
58 self._relation = relation
59 self._local_unit = local_unit
60 self.ingress_address = relation.data[local_unit]['ingress-address']
61
62 def serve(self, hosts, port):
63 self._relation.data[self._local_unit]['extended_data'] = json.dumps([{
64 'hostname': host,
65 'port': port,
66 } for host in hosts])
0\ No newline at end of file67\ No newline at end of file

Subscribers

People subscribed via source and target branches

to all changes: