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
diff --git a/config.yaml b/config.yaml
index 6d656db..bd8bebe 100644
--- a/config.yaml
+++ b/config.yaml
@@ -1,4 +1,16 @@
1options:1options:
2 discourse_image:
3 type: string
4 description: "Discourse image to use"
5 default: ""
6 image_user:
7 type: string
8 description: "Private registry username"
9 default: ""
10 image_pass:
11 type: string
12 description: "Private registry password"
13 default: ""
2 db_host:14 db_host:
3 type: string15 type: string
4 description: "PostgreSQL database host"16 description: "PostgreSQL database host"
@@ -15,10 +27,6 @@ options:
15 type: string27 type: string
16 description: "PostgreSQL database user's password"28 description: "PostgreSQL database user's password"
17 default: ""29 default: ""
18 service_port:
19 type: int
20 description: "TCP port discourse should listen on"
21 default: 80
22 external_hostname:30 external_hostname:
23 type: string31 type: string
24 description: "External hostname this discourse instance should respond to"32 description: "External hostname this discourse instance should respond to"
@@ -47,6 +55,14 @@ options:
47 type: int55 type: int
48 description: "Port to use when connecting to SMTP server"56 description: "Port to use when connecting to SMTP server"
49 default: 58757 default: 587
58 smtp_authentication:
59 type: string
60 description: "Type of smtp authentication to use"
61 default: "none"
62 smtp_openssl_verify_mode:
63 type: string
64 description: "Should discourse verify SSL certs"
65 default: "none"
50 smtp_username:66 smtp_username:
51 type: string67 type: string
52 description: "Username to use when sending mail via SMTP"68 description: "Username to use when sending mail via SMTP"
diff --git a/metadata.yaml b/metadata.yaml
index 97d952c..01334ec 100644
--- a/metadata.yaml
+++ b/metadata.yaml
@@ -9,14 +9,6 @@ tags:
9 - forum9 - forum
10series:10series:
11 - kubernetes11 - kubernetes
12provide:12provides:
13 website:13 website:
14 interface: http14 interface: http
15resources:
16 discourse_image:
17 type: oci-image
18 description: Image used for discourse pod.
19storage:
20 discourse-storage:
21 type: filesystem
22 location: /shared
diff --git a/mod/operator b/mod/operator
index 60c43f8..59dd098 160000
--- a/mod/operator
+++ b/mod/operator
@@ -1 +1 @@
1Subproject commit 60c43f81e36139ab4044c185247eb27fe389bce61Subproject commit 59dd09875421668366ffcaff123bec34a0054ec3
diff --git a/mod/resource-oci-image b/mod/resource-oci-image
index e583429..3141de0 160000
--- a/mod/resource-oci-image
+++ b/mod/resource-oci-image
@@ -1 +1 @@
1Subproject commit e583429139d874bbcab8330c13f3fde30912abdf1Subproject commit 3141de04152375d2f8df7863d8d78aa9345024b1
diff --git a/src/charm.py b/src/charm.py
index 86f8ab6..29db945 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -8,12 +8,114 @@ from ops.framework import StoredState
8from ops.main import main8from ops.main import main
9from ops.model import (9from ops.model import (
10 ActiveStatus,10 ActiveStatus,
11 BlockedStatus,
11 MaintenanceStatus,12 MaintenanceStatus,
12 ModelError,13 ModelError,
13 WaitingStatus,14 WaitingStatus,
14)15)
1516
16from oci_image import OCIImageResource, ResourceError17
18def create_discourse_pod_config(config):
19 pod_config = {
20 'DISCOURSE_POSTGRES_USERNAME': config['db_user'],
21 'DISCOURSE_POSTGRES_PASSWORD': config['db_password'],
22 'DISCOURSE_POSTGRES_HOST': config['db_host'],
23 'DISCOURSE_POSTGRES_NAME': config['db_name'],
24 'DISCOURSE_DEVELOPER_EMAILS': config['developer_emails'],
25 'DISCOURSE_HOSTNAME': config['external_hostname'],
26 'DISCOURSE_SMTP_DOMAIN': config['smtp_domain'],
27 'DISCOURSE_SMTP_ADDRESS': config['smtp_address'],
28 'DISCOURSE_SMTP_PORT': config['smtp_port'],
29 'DISCOURSE_SMTP_AUTHENTICATION': config['smtp_authentication'],
30 'DISCOURSE_SMTP_OPENSSL_VERIFY_MODE': config['smtp_openssl_verify_mode'],
31 'DISCOURSE_SMTP_USER_NAME': config['smtp_username'],
32 'DISCOURSE_SMTP_PASSWORD': config['smtp_password'],
33 'DISCOURSE_REDIS_HOST': config['redis_host'],
34 }
35 return pod_config
36
37
38def create_ingress_config(app_name, config):
39 ingressResource = {
40 "name": app_name + "-ingress",
41 "spec": {
42 "rules": [
43 {
44 "host": config['external_hostname'],
45 "http": {
46 "paths": [
47 {
48 "path": "/",
49 "backend": {
50 "serviceName": app_name,
51 "servicePort": 3000
52 }
53 }
54 ]
55 }
56 }
57 ]
58 }
59 }
60 return ingressResource
61
62
63def get_pod_spec(app_name, config):
64 pod_spec = {
65 "version": 3,
66 "containers": [{
67 "name": app_name,
68 "imageDetails": {"imagePath": config['discourse_image']},
69 "imagePullPolicy": "IfNotPresent",
70 "ports": [{
71 "containerPort": 3000,
72 "protocol": "TCP",
73 }],
74 "envConfig": create_discourse_pod_config(config),
75 "kubernetes": {
76 "readinessProbe": {
77 "httpGet": {
78 "path": "/srv/status",
79 "port": 3000,
80 }
81 }
82 },
83 }],
84 "kubernetesResources": {
85 "ingressResources": [
86 create_ingress_config(app_name, config)
87 ]
88 }
89 }
90 # This handles when we are trying to get an image from a private
91 # registry.
92 if config['image_user'] and config['image_pass']:
93 pod_spec['containers'][0]['imageDetails'].set("username", config['image_user'])
94 pod_spec['containers'][0]['imageDetails'].set("password", config['image_pass'])
95
96 return pod_spec
97
98
99def check_for_config_problems(config):
100 errors = []
101 missing_fields = check_for_missing_config_fields(config)
102
103 if len(missing_fields):
104 errors.append('Required configuration missing: {}'.format(" ".join(missing_fields)))
105
106 return errors
107
108
109def check_for_missing_config_fields(config):
110 missing_fields = []
111
112 needed_fields = ['db_user', 'db_password', 'db_host', 'db_name', 'smtp_address',
113 'redis_host']
114 for key in needed_fields:
115 if len(config[key]) == 0:
116 missing_fields.append(key)
117
118 return sorted(missing_fields)
17119
18120
19class DiscourseCharm(CharmBase):121class DiscourseCharm(CharmBase):
@@ -23,57 +125,37 @@ class DiscourseCharm(CharmBase):
23 super().__init__(framework, key)125 super().__init__(framework, key)
24126
25 self.state.set_default(is_started=False)127 self.state.set_default(is_started=False)
26 # get our discourse_image from juju
27 # ie: juju deploy . --resource discourse_image=discourse-canonical:1.0.0 )
28 self.discourse_image = OCIImageResource(self, 'discourse_image')
29 self.framework.observe(self.on.leader_elected, self.configure_pod)128 self.framework.observe(self.on.leader_elected, self.configure_pod)
30 self.framework.observe(self.on.config_changed, self.configure_pod)129 self.framework.observe(self.on.config_changed, self.configure_pod)
31 self.framework.observe(self.on.upgrade_charm, self.configure_pod)130 self.framework.observe(self.on.upgrade_charm, self.configure_pod)
32131
33 def verify_leadership(self):132 def check_config_is_valid(self, config):
34 if not self.model.unit.is_leader():133 valid_config = True
35 raise LeadershipError()134 errors = check_for_config_problems(config)
36135
37 def configure_pod(self, event):136 # set status if we have a bad config
38 # Verify we are leader - throws exception if we are not137 if errors:
39 self.verify_leadership()138 self.model.unit.status = BlockedStatus(", ".join(errors))
139 valid_config = False
140 else:
141 self.model.unit.status = MaintenanceStatus("Configuration passed validation")
142
143 return valid_config
40144
41 # Get the image details for our discourse image - this will145 def configure_pod(self, event):
42 # obtain all the details needed to access our docker registry / etc.
43 discourse_image_details = self.discourse_image.fetch()
44146
45 # set our status while we get configured147 # Set our status while we get configured.
46 self.model.unit.status = MaintenanceStatus('Configuring pod')148 self.model.unit.status = MaintenanceStatus('Configuring pod')
47149
48 # get our config150 # Leader must set the pod spec.
49 config = self.framework.model.config151 if self.model.unit.is_leader():
50152 # Get our spec definition.
51 # set our spec per our config153 if self.check_config_is_valid(self.framework.model.config):
52 self.model.pod.set_spec({154 # Get pod spec using our app name and config
53 'containers': [{155 pod_spec = get_pod_spec(self.framework.model.app.name, self.framework.model.config)
54 'name': self.framework.model.app.name,156 # Set our pod spec.
55 'imageDetails': discourse_image_details,157 self.model.pod.set_spec(pod_spec)
56 'imagePullPolicy': 'Never',158
57 'ports': [{
58 'containerPort': int(config['service_port']),
59 'protocol': 'TCP',
60 }],
61 'config': {
62 'DISCOURSE_DB_USERNAME': config['db_user'],
63 'DISCOURSE_DB_PASSWORD': config['db_password'],
64 'DISCOURSE_DB_HOST': config['db_host'],
65 'DISCOURSE_DB_NAME': config['db_name'],
66 'DISCOURSE_DEVELOPER_EMAILS': config['developer_emails'],
67 'DISCOURSE_HOSTNAME': config['external_hostname'],
68 'DISCOURSE_SMTP_DOMAIN': config['smtp_domain'],
69 'DISCOURSE_SMTP_ADDRESS': config['smtp_address'],
70 'DISCOURSE_SMTP_PORT': config['smtp_port'],
71 'DISCOURSE_SMTP_USER_NAME': config['smtp_username'],
72 'DISCOURSE_SMTP_PASSWORD': config['smtp_password'],
73 'DISCOURSE_REDIS_HOST': config['redis_host'],
74 },
75 }]
76 })
77 self.state.is_started = True159 self.state.is_started = True
78 self.model.unit.status = ActiveStatus()160 self.model.unit.status = ActiveStatus()
79161
@@ -84,11 +166,6 @@ class DiscourseCharm(CharmBase):
84 event.client.serve(hosts=[event.client.ingress_address],166 event.client.serve(hosts=[event.client.ingress_address],
85 port=self.model.config['http_port'])167 port=self.model.config['http_port'])
86168
87class LeadershipError(ModelError):
88 def __init__(self):
89 super().__init__('not leader')
90 self.status = WaitingStatus('Deferring to leader unit to configure pod')
91
92169
93if __name__ == '__main__':170if __name__ == '__main__':
94 main(DiscourseCharm)171 main(DiscourseCharm)

Subscribers

People subscribed via source and target branches