Merge ~jk0ne/charm-k8s-discourse/+git/charm-k8s-discourse:master into charm-k8s-discourse:master

Proposed by Jay Kuri
Status: Merged
Approved by: Tom Haddon
Approved revision: 6b9dc1f889f4c715860a575d9a9b6c822997c52c
Merged at revision: 90579e591e119795abc93ea7b6c3fe6b2d1582f0
Proposed branch: ~jk0ne/charm-k8s-discourse/+git/charm-k8s-discourse:master
Merge into: charm-k8s-discourse:master
Diff against target: 293 lines (+148/-63)
5 files modified
config.yaml (+20/-4)
metadata.yaml (+1/-9)
mod/operator (+1/-1)
mod/resource-oci-image (+1/-1)
src/charm.py (+125/-48)
Reviewer Review Type Date Requested Status
Tom Haddon Approve
Canonical IS Reviewers Pending
Joel Sing Pending
Canonical IS Reviewers Pending
Review via email: mp+387441@code.launchpad.net

This proposal supersedes a proposal from 2020-07-08.

Commit message

Add readinessProbe. Return charm to stateless, update enable_local_redis config option to be false by default.

Description of the change

Fully updated, ready for review.

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

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

Revision history for this message
Tom Haddon (mthaddon) wrote : Posted in a previous version of this proposal

Some comments and questions inline.

review: Needs Fixing
Revision history for this message
Jay Kuri (jk0ne) wrote : Posted in a previous version of this proposal

Additional details inline.

Revision history for this message
Tom Haddon (mthaddon) wrote : Posted in a previous version of this proposal

Some more comments inline, and I think this is still WIP, or maybe you missed the comment about the check_config_valid function?

Revision history for this message
Jay Kuri (jk0ne) wrote : Posted in a previous version of this proposal

> Some more comments inline, and I think this is still WIP, or maybe you missed
> the comment about the check_config_valid function?

Yes, still WIP. I wanted to get the rest of the adjustments made before I jumped into more substantive changes.

Revision history for this message
Jay Kuri (jk0ne) wrote : Posted in a previous version of this proposal

comments inline.

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
Tom Haddon (mthaddon) wrote :

LGTM, thx

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

Change successfully merged at revision 90579e591e119795abc93ea7b6c3fe6b2d1582f0

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/config.yaml b/config.yaml
2index 6d656db..bd8bebe 100644
3--- a/config.yaml
4+++ b/config.yaml
5@@ -1,4 +1,16 @@
6 options:
7+ discourse_image:
8+ type: string
9+ description: "Discourse image to use"
10+ default: ""
11+ image_user:
12+ type: string
13+ description: "Private registry username"
14+ default: ""
15+ image_pass:
16+ type: string
17+ description: "Private registry password"
18+ default: ""
19 db_host:
20 type: string
21 description: "PostgreSQL database host"
22@@ -15,10 +27,6 @@ options:
23 type: string
24 description: "PostgreSQL database user's password"
25 default: ""
26- service_port:
27- type: int
28- description: "TCP port discourse should listen on"
29- default: 80
30 external_hostname:
31 type: string
32 description: "External hostname this discourse instance should respond to"
33@@ -47,6 +55,14 @@ options:
34 type: int
35 description: "Port to use when connecting to SMTP server"
36 default: 587
37+ smtp_authentication:
38+ type: string
39+ description: "Type of smtp authentication to use"
40+ default: "none"
41+ smtp_openssl_verify_mode:
42+ type: string
43+ description: "Should discourse verify SSL certs"
44+ default: "none"
45 smtp_username:
46 type: string
47 description: "Username to use when sending mail via SMTP"
48diff --git a/metadata.yaml b/metadata.yaml
49index 97d952c..01334ec 100644
50--- a/metadata.yaml
51+++ b/metadata.yaml
52@@ -9,14 +9,6 @@ tags:
53 - forum
54 series:
55 - kubernetes
56-provide:
57+provides:
58 website:
59 interface: http
60-resources:
61- discourse_image:
62- type: oci-image
63- description: Image used for discourse pod.
64-storage:
65- discourse-storage:
66- type: filesystem
67- location: /shared
68diff --git a/mod/operator b/mod/operator
69index 60c43f8..59dd098 160000
70--- a/mod/operator
71+++ b/mod/operator
72@@ -1 +1 @@
73-Subproject commit 60c43f81e36139ab4044c185247eb27fe389bce6
74+Subproject commit 59dd09875421668366ffcaff123bec34a0054ec3
75diff --git a/mod/resource-oci-image b/mod/resource-oci-image
76index e583429..3141de0 160000
77--- a/mod/resource-oci-image
78+++ b/mod/resource-oci-image
79@@ -1 +1 @@
80-Subproject commit e583429139d874bbcab8330c13f3fde30912abdf
81+Subproject commit 3141de04152375d2f8df7863d8d78aa9345024b1
82diff --git a/src/charm.py b/src/charm.py
83index 86f8ab6..29db945 100755
84--- a/src/charm.py
85+++ b/src/charm.py
86@@ -8,12 +8,114 @@ from ops.framework import StoredState
87 from ops.main import main
88 from ops.model import (
89 ActiveStatus,
90+ BlockedStatus,
91 MaintenanceStatus,
92 ModelError,
93 WaitingStatus,
94 )
95
96-from oci_image import OCIImageResource, ResourceError
97+
98+def create_discourse_pod_config(config):
99+ pod_config = {
100+ 'DISCOURSE_POSTGRES_USERNAME': config['db_user'],
101+ 'DISCOURSE_POSTGRES_PASSWORD': config['db_password'],
102+ 'DISCOURSE_POSTGRES_HOST': config['db_host'],
103+ 'DISCOURSE_POSTGRES_NAME': config['db_name'],
104+ 'DISCOURSE_DEVELOPER_EMAILS': config['developer_emails'],
105+ 'DISCOURSE_HOSTNAME': config['external_hostname'],
106+ 'DISCOURSE_SMTP_DOMAIN': config['smtp_domain'],
107+ 'DISCOURSE_SMTP_ADDRESS': config['smtp_address'],
108+ 'DISCOURSE_SMTP_PORT': config['smtp_port'],
109+ 'DISCOURSE_SMTP_AUTHENTICATION': config['smtp_authentication'],
110+ 'DISCOURSE_SMTP_OPENSSL_VERIFY_MODE': config['smtp_openssl_verify_mode'],
111+ 'DISCOURSE_SMTP_USER_NAME': config['smtp_username'],
112+ 'DISCOURSE_SMTP_PASSWORD': config['smtp_password'],
113+ 'DISCOURSE_REDIS_HOST': config['redis_host'],
114+ }
115+ return pod_config
116+
117+
118+def create_ingress_config(app_name, config):
119+ ingressResource = {
120+ "name": app_name + "-ingress",
121+ "spec": {
122+ "rules": [
123+ {
124+ "host": config['external_hostname'],
125+ "http": {
126+ "paths": [
127+ {
128+ "path": "/",
129+ "backend": {
130+ "serviceName": app_name,
131+ "servicePort": 3000
132+ }
133+ }
134+ ]
135+ }
136+ }
137+ ]
138+ }
139+ }
140+ return ingressResource
141+
142+
143+def get_pod_spec(app_name, config):
144+ pod_spec = {
145+ "version": 3,
146+ "containers": [{
147+ "name": app_name,
148+ "imageDetails": {"imagePath": config['discourse_image']},
149+ "imagePullPolicy": "IfNotPresent",
150+ "ports": [{
151+ "containerPort": 3000,
152+ "protocol": "TCP",
153+ }],
154+ "envConfig": create_discourse_pod_config(config),
155+ "kubernetes": {
156+ "readinessProbe": {
157+ "httpGet": {
158+ "path": "/srv/status",
159+ "port": 3000,
160+ }
161+ }
162+ },
163+ }],
164+ "kubernetesResources": {
165+ "ingressResources": [
166+ create_ingress_config(app_name, config)
167+ ]
168+ }
169+ }
170+ # This handles when we are trying to get an image from a private
171+ # registry.
172+ if config['image_user'] and config['image_pass']:
173+ pod_spec['containers'][0]['imageDetails'].set("username", config['image_user'])
174+ pod_spec['containers'][0]['imageDetails'].set("password", config['image_pass'])
175+
176+ return pod_spec
177+
178+
179+def check_for_config_problems(config):
180+ errors = []
181+ missing_fields = check_for_missing_config_fields(config)
182+
183+ if len(missing_fields):
184+ errors.append('Required configuration missing: {}'.format(" ".join(missing_fields)))
185+
186+ return errors
187+
188+
189+def check_for_missing_config_fields(config):
190+ missing_fields = []
191+
192+ needed_fields = ['db_user', 'db_password', 'db_host', 'db_name', 'smtp_address',
193+ 'redis_host']
194+ for key in needed_fields:
195+ if len(config[key]) == 0:
196+ missing_fields.append(key)
197+
198+ return sorted(missing_fields)
199
200
201 class DiscourseCharm(CharmBase):
202@@ -23,57 +125,37 @@ class DiscourseCharm(CharmBase):
203 super().__init__(framework, key)
204
205 self.state.set_default(is_started=False)
206- # get our discourse_image from juju
207- # ie: juju deploy . --resource discourse_image=discourse-canonical:1.0.0 )
208- self.discourse_image = OCIImageResource(self, 'discourse_image')
209 self.framework.observe(self.on.leader_elected, self.configure_pod)
210 self.framework.observe(self.on.config_changed, self.configure_pod)
211 self.framework.observe(self.on.upgrade_charm, self.configure_pod)
212
213- def verify_leadership(self):
214- if not self.model.unit.is_leader():
215- raise LeadershipError()
216+ def check_config_is_valid(self, config):
217+ valid_config = True
218+ errors = check_for_config_problems(config)
219
220- def configure_pod(self, event):
221- # Verify we are leader - throws exception if we are not
222- self.verify_leadership()
223+ # set status if we have a bad config
224+ if errors:
225+ self.model.unit.status = BlockedStatus(", ".join(errors))
226+ valid_config = False
227+ else:
228+ self.model.unit.status = MaintenanceStatus("Configuration passed validation")
229+
230+ return valid_config
231
232- # Get the image details for our discourse image - this will
233- # obtain all the details needed to access our docker registry / etc.
234- discourse_image_details = self.discourse_image.fetch()
235+ def configure_pod(self, event):
236
237- # set our status while we get configured
238+ # Set our status while we get configured.
239 self.model.unit.status = MaintenanceStatus('Configuring pod')
240
241- # get our config
242- config = self.framework.model.config
243-
244- # set our spec per our config
245- self.model.pod.set_spec({
246- 'containers': [{
247- 'name': self.framework.model.app.name,
248- 'imageDetails': discourse_image_details,
249- 'imagePullPolicy': 'Never',
250- 'ports': [{
251- 'containerPort': int(config['service_port']),
252- 'protocol': 'TCP',
253- }],
254- 'config': {
255- 'DISCOURSE_DB_USERNAME': config['db_user'],
256- 'DISCOURSE_DB_PASSWORD': config['db_password'],
257- 'DISCOURSE_DB_HOST': config['db_host'],
258- 'DISCOURSE_DB_NAME': config['db_name'],
259- 'DISCOURSE_DEVELOPER_EMAILS': config['developer_emails'],
260- 'DISCOURSE_HOSTNAME': config['external_hostname'],
261- 'DISCOURSE_SMTP_DOMAIN': config['smtp_domain'],
262- 'DISCOURSE_SMTP_ADDRESS': config['smtp_address'],
263- 'DISCOURSE_SMTP_PORT': config['smtp_port'],
264- 'DISCOURSE_SMTP_USER_NAME': config['smtp_username'],
265- 'DISCOURSE_SMTP_PASSWORD': config['smtp_password'],
266- 'DISCOURSE_REDIS_HOST': config['redis_host'],
267- },
268- }]
269- })
270+ # Leader must set the pod spec.
271+ if self.model.unit.is_leader():
272+ # Get our spec definition.
273+ if self.check_config_is_valid(self.framework.model.config):
274+ # Get pod spec using our app name and config
275+ pod_spec = get_pod_spec(self.framework.model.app.name, self.framework.model.config)
276+ # Set our pod spec.
277+ self.model.pod.set_spec(pod_spec)
278+
279 self.state.is_started = True
280 self.model.unit.status = ActiveStatus()
281
282@@ -84,11 +166,6 @@ class DiscourseCharm(CharmBase):
283 event.client.serve(hosts=[event.client.ingress_address],
284 port=self.model.config['http_port'])
285
286-class LeadershipError(ModelError):
287- def __init__(self):
288- super().__init__('not leader')
289- self.status = WaitingStatus('Deferring to leader unit to configure pod')
290-
291
292 if __name__ == '__main__':
293 main(DiscourseCharm)

Subscribers

People subscribed via source and target branches