Merge ~barryprice/charm-k8s-bind/+git/charm-k8s-bind:sidecar into charm-k8s-bind:master
- Git
- lp:~barryprice/charm-k8s-bind/+git/charm-k8s-bind
- sidecar
- Merge into master
Status: | Work in progress |
---|---|
Proposed branch: | ~barryprice/charm-k8s-bind/+git/charm-k8s-bind:sidecar |
Merge into: | charm-k8s-bind:master |
Diff against target: |
749 lines (+206/-370) 9 files modified
.jujuignore (+10/-5) Makefile (+10/-3) README.md (+1/-1) config.yaml (+0/-29) metadata.yaml (+7/-3) requirements.txt (+1/-1) src/charm.py (+89/-99) tests/unit/test_charm.py (+87/-228) tox.ini (+1/-1) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Bind Charmers | Pending | ||
Review via email: mp+402876@code.launchpad.net |
Commit message
WIP sidecar conversion for bind (test coverage not yet complete), with a few tweaks to base files
Description of the change
- 492731a... by Barry Price
-
Rename charm for consistency with others
- 66bae80... by Barry Price
-
Bugfixes
- 0511d12... by Barry Price
-
Bind, not Mattermost!
- 0ae5151... by Barry Price
-
Remove duplicate hook
- 270c6f0... by Barry Price
-
Use the new image spec with a single wrapper to apply config and run the service
- 3c06f70... by Barry Price
-
Remove obsolete unit tests (i.e. most of them)
- 0056497... by Barry Price
-
Address conflicts
- a353662... by Barry Price
-
Merge from master, address conflicts
- 1646220... by Barry Price
-
Add tests, bring us up to 55% coverage
- 1594dbe... by Barry Price
-
Add HTML test coverage report
- eb36238... by Barry Price
-
Add a few more tests
- 059eb95... by Barry Price
-
More test fixes, giving us 68% coverage
- 6a7adf0... by Barry Price
-
Pull from upstream, enable RFC1918 net recursion by default, add tests to 69% coverage
- 7f3608e... by Barry Price
-
Pull from master, fix conflicts
- 3334949... by Barry Price
-
Pull from master, fix conflicts
- e9fb7e4... by Barry Price
-
Charmcraft should pack, not build
- 44c1096... by Barry Price
-
Add logging/verbosity throughout
- 74d5bf1... by Barry Price
-
Move wrapper logging inside the script, fix the exec bug
- 6c1203c... by Barry Price
-
Move wrapper logging into the pebble command
- 6baed5e... by Barry Price
-
Restore semicolons to wrapper script
- 9e4f75e... by Barry Price
-
Restore default config if recursion is toggled off post-install
- 3e2c7cb... by Barry Price
-
Merge from master, address conflicts
- 3f0d665... by Barry Price
-
Merge from master, address conflicts
- f524d71... by Barry Price
-
Merge branch 'master' into sidecar
Unmerged commits
- f524d71... by Barry Price
-
Merge branch 'master' into sidecar
- 3f0d665... by Barry Price
-
Merge from master, address conflicts
- 3e2c7cb... by Barry Price
-
Merge from master, address conflicts
- 9e4f75e... by Barry Price
-
Restore default config if recursion is toggled off post-install
- 6baed5e... by Barry Price
-
Restore semicolons to wrapper script
- 6c1203c... by Barry Price
-
Move wrapper logging into the pebble command
- 74d5bf1... by Barry Price
-
Move wrapper logging inside the script, fix the exec bug
- 44c1096... by Barry Price
-
Add logging/verbosity throughout
- e9fb7e4... by Barry Price
-
Charmcraft should pack, not build
- 3334949... by Barry Price
-
Pull from master, fix conflicts
Preview Diff
1 | diff --git a/.jujuignore b/.jujuignore | |||
2 | index 128851d..2a8f147 100644 | |||
3 | --- a/.jujuignore | |||
4 | +++ b/.jujuignore | |||
5 | @@ -1,9 +1,14 @@ | |||
6 | 1 | *~ | ||
7 | 2 | *.charm | 1 | *.charm |
8 | 2 | *.py[cod] | ||
9 | 3 | *~ | ||
10 | 3 | .coverage | 4 | .coverage |
11 | 4 | .gitignore | 5 | .gitignore |
16 | 5 | __pycache__ | 6 | /Dockerfile |
13 | 6 | /dockerfile | ||
14 | 7 | /image-scripts/ | ||
15 | 8 | /tests/ | ||
17 | 9 | /Makefile | 7 | /Makefile |
18 | 8 | /env | ||
19 | 9 | /htmlcov/ | ||
20 | 10 | /image-scripts/ | ||
21 | 11 | __pycache__ | ||
22 | 12 | requirements.txt | ||
23 | 13 | tests/ | ||
24 | 14 | tox.ini | ||
25 | diff --git a/Makefile b/Makefile | |||
26 | index 564aeb9..9869bc8 100644 | |||
27 | --- a/Makefile | |||
28 | +++ b/Makefile | |||
29 | @@ -5,11 +5,16 @@ blacken: | |||
30 | 5 | @echo "Normalising python layout with black." | 5 | @echo "Normalising python layout with black." |
31 | 6 | @tox -e black | 6 | @tox -e black |
32 | 7 | 7 | ||
33 | 8 | |||
34 | 9 | lint: blacken | 8 | lint: blacken |
35 | 10 | @echo "Running flake8" | 9 | @echo "Running flake8" |
36 | 11 | @tox -e lint | 10 | @tox -e lint |
37 | 12 | 11 | ||
38 | 12 | deps: | ||
39 | 13 | @echo "Checking charmcraft is present." | ||
40 | 14 | @command -v charmcraft >/dev/null || { echo "Please install charmcraft to continue ('sudo snap install charmcraft')" && false; } | ||
41 | 15 | @echo "Checking tox is present." | ||
42 | 16 | @command -v tox >/dev/null || { echo "Please install tox to continue ('sudo apt-get install tox')" && false; } | ||
43 | 17 | |||
44 | 13 | # We actually use the build directory created by charmcraft, | 18 | # We actually use the build directory created by charmcraft, |
45 | 14 | # but the .charm file makes a much more convenient sentinel. | 19 | # but the .charm file makes a much more convenient sentinel. |
46 | 15 | unittest: bind-k8s.charm | 20 | unittest: bind-k8s.charm |
47 | @@ -22,11 +27,13 @@ clean: | |||
48 | 22 | @git clean -fXd | 27 | @git clean -fXd |
49 | 23 | 28 | ||
50 | 24 | bind-k8s.charm: src/*.py requirements.txt | 29 | bind-k8s.charm: src/*.py requirements.txt |
52 | 25 | charmcraft build | 30 | charmcraft pack |
53 | 26 | 31 | ||
54 | 27 | image-deps: | 32 | image-deps: |
55 | 28 | @echo "Checking shellcheck is present." | 33 | @echo "Checking shellcheck is present." |
56 | 29 | @command -v shellcheck >/dev/null || { echo "Please install shellcheck to continue ('sudo snap install shellcheck')" && false; } | 34 | @command -v shellcheck >/dev/null || { echo "Please install shellcheck to continue ('sudo snap install shellcheck')" && false; } |
57 | 35 | @echo "Checking docker is present." | ||
58 | 36 | @command -v docker >/dev/null || { echo "Please install docker to continue ('sudo apt-get install docker.io')" && false; } | ||
59 | 30 | 37 | ||
60 | 31 | image-lint: image-deps | 38 | image-lint: image-deps |
61 | 32 | @echo "Running shellcheck." | 39 | @echo "Running shellcheck." |
62 | @@ -43,4 +50,4 @@ image-build: image-lint | |||
63 | 43 | -t bind:$(DIST_RELEASE)-latest \ | 50 | -t bind:$(DIST_RELEASE)-latest \ |
64 | 44 | . | 51 | . |
65 | 45 | 52 | ||
67 | 46 | .PHONY: blacken lint unittest test clean image-deps image-lint image-build | 53 | .PHONY: blacken lint deps unittest test clean image-deps image-lint image-build |
68 | diff --git a/README.md b/README.md | |||
69 | index 8a2b7e6..c4f750f 100644 | |||
70 | --- a/README.md | |||
71 | +++ b/README.md | |||
72 | @@ -17,7 +17,7 @@ details on using Juju with MicroK8s for easy local testing [see here](https://ju | |||
73 | 17 | 17 | ||
74 | 18 | To deploy this charm in a juju k8s model: | 18 | To deploy this charm in a juju k8s model: |
75 | 19 | ``` | 19 | ``` |
77 | 20 | juju deploy bind-k8s | 20 | juju deploy bind-k8s --channel edge |
78 | 21 | ``` | 21 | ``` |
79 | 22 | The charm will deploy bind with its stock Ubuntu package configuration, which | 22 | The charm will deploy bind with its stock Ubuntu package configuration, which |
80 | 23 | will forward all queries to root name servers. DNSSEC is enabled by default. | 23 | will forward all queries to root name servers. DNSSEC is enabled by default. |
81 | diff --git a/config.yaml b/config.yaml | |||
82 | index 9b1de1d..e65e986 100644 | |||
83 | --- a/config.yaml | |||
84 | +++ b/config.yaml | |||
85 | @@ -1,33 +1,4 @@ | |||
86 | 1 | options: | 1 | options: |
87 | 2 | bind_image_path: | ||
88 | 3 | type: string | ||
89 | 4 | description: | | ||
90 | 5 | The location of the image to use, e.g. "registry.example.com/bind:v1". | ||
91 | 6 | |||
92 | 7 | This setting is required. | ||
93 | 8 | default: "bindcharmers/bind:v1.0-20.04_edge" | ||
94 | 9 | bind_image_username: | ||
95 | 10 | type: string | ||
96 | 11 | description: "Username to use for the configured image registry, if required" | ||
97 | 12 | default: "" | ||
98 | 13 | bind_image_password: | ||
99 | 14 | type: string | ||
100 | 15 | description: "Password to use for the configured image registry, if required" | ||
101 | 16 | default: "" | ||
102 | 17 | container_config: | ||
103 | 18 | type: string | ||
104 | 19 | description: > | ||
105 | 20 | YAML formatted map of container config keys & values. These are | ||
106 | 21 | generally accessed from inside the image as environment variables. | ||
107 | 22 | Use to configure customized Wordpress images. This configuration | ||
108 | 23 | gets logged; use container_secrets for secrets. | ||
109 | 24 | default: "" | ||
110 | 25 | container_secrets: | ||
111 | 26 | type: string | ||
112 | 27 | description: > | ||
113 | 28 | YAML formatted map of secrets. Works just like container_config, | ||
114 | 29 | except that values should not be logged. | ||
115 | 30 | default: "" | ||
116 | 31 | custom_config_repo: | 2 | custom_config_repo: |
117 | 32 | type: string | 3 | type: string |
118 | 33 | description: | | 4 | description: | |
119 | diff --git a/metadata.yaml b/metadata.yaml | |||
120 | index 6c09e29..87d17fc 100644 | |||
121 | --- a/metadata.yaml | |||
122 | +++ b/metadata.yaml | |||
123 | @@ -5,11 +5,15 @@ docs: https://discourse.charmhub.io/t/bind-documentation-overview/3973 | |||
124 | 5 | description: | | 5 | description: | |
125 | 6 | The original, complete open source DNS implementation. | 6 | The original, complete open source DNS implementation. |
126 | 7 | https://www.isc.org/bind/ | 7 | https://www.isc.org/bind/ |
127 | 8 | min-juju-version: 2.8.0 | ||
128 | 9 | maintainers: | 8 | maintainers: |
129 | 10 | - https://launchpad.net/~bind-charmers <bind-charmers@lists.launchpad.net> | 9 | - https://launchpad.net/~bind-charmers <bind-charmers@lists.launchpad.net> |
130 | 11 | tags: | 10 | tags: |
131 | 12 | - network | 11 | - network |
132 | 13 | - ops | 12 | - ops |
135 | 14 | series: | 13 | containers: |
136 | 15 | - kubernetes | 14 | bind: |
137 | 15 | resource: bind-image | ||
138 | 16 | resources: | ||
139 | 17 | bind-image: | ||
140 | 18 | type: oci-image | ||
141 | 19 | description: Docker image for Bind to run | ||
142 | diff --git a/requirements.txt b/requirements.txt | |||
143 | index 2d81d3b..fcf2df8 100644 | |||
144 | --- a/requirements.txt | |||
145 | +++ b/requirements.txt | |||
146 | @@ -1 +1 @@ | |||
148 | 1 | ops | 1 | https://github.com/canonical/operator/archive/refs/heads/master.zip |
149 | diff --git a/src/charm.py b/src/charm.py | |||
150 | index 964922c..c9383e0 100755 | |||
151 | --- a/src/charm.py | |||
152 | +++ b/src/charm.py | |||
153 | @@ -5,29 +5,96 @@ | |||
154 | 5 | 5 | ||
155 | 6 | import logging | 6 | import logging |
156 | 7 | from ops.charm import CharmBase | 7 | from ops.charm import CharmBase |
157 | 8 | from ops.framework import StoredState | ||
158 | 8 | from ops.main import main | 9 | from ops.main import main |
162 | 9 | from ops.model import ActiveStatus, MaintenanceStatus | 10 | from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus |
160 | 10 | from pprint import pformat | ||
161 | 11 | from yaml import safe_load | ||
163 | 12 | 11 | ||
165 | 13 | logger = logging.getLogger() | 12 | logger = logging.getLogger(__name__) |
166 | 14 | 13 | ||
168 | 15 | REQUIRED_SETTINGS = ['bind_image_path'] | 14 | CONTAINER_NAME = 'bind' # per metadata.yaml |
169 | 15 | REQUIRED_SETTINGS = [] | ||
170 | 16 | 16 | ||
171 | 17 | 17 | ||
172 | 18 | class BindK8sCharm(CharmBase): | 18 | class BindK8sCharm(CharmBase): |
173 | 19 | |||
174 | 20 | state = StoredState() | ||
175 | 21 | |||
176 | 19 | def __init__(self, *args): | 22 | def __init__(self, *args): |
177 | 20 | """Initialise our class, we only care about 'start' and 'config-changed' hooks.""" | 23 | """Initialise our class, we only care about 'start' and 'config-changed' hooks.""" |
178 | 21 | super().__init__(*args) | 24 | super().__init__(*args) |
181 | 22 | self.framework.observe(self.on.start, self.on_config_changed) | 25 | |
182 | 23 | self.framework.observe(self.on.config_changed, self.on_config_changed) | 26 | self.framework.observe(self.on.config_changed, self._on_config_changed) |
183 | 27 | self.framework.observe(self.on.bind_pebble_ready, self._on_bind_pebble_ready) | ||
184 | 28 | |||
185 | 29 | # state | ||
186 | 30 | self.state.set_default(bind_pebble_ready=False) | ||
187 | 31 | |||
188 | 32 | def _get_pebble_config(self): | ||
189 | 33 | """Generate our pebble config.""" | ||
190 | 34 | logger.debug('Generating pebble config') | ||
191 | 35 | env_config = self._generate_env_config() | ||
192 | 36 | pebble_config = { | ||
193 | 37 | "summary": "Bind layer", | ||
194 | 38 | "description": "Bind layer", | ||
195 | 39 | "services": { | ||
196 | 40 | "bind": { | ||
197 | 41 | "override": "replace", | ||
198 | 42 | "summary": "Bind service", | ||
199 | 43 | "command": 'bash -c "/usr/local/bin/docker-wrapper.sh &> /proc/1/fd/1"', | ||
200 | 44 | "startup": "enabled", | ||
201 | 45 | "environment": env_config, | ||
202 | 46 | } | ||
203 | 47 | }, | ||
204 | 48 | } | ||
205 | 49 | return pebble_config | ||
206 | 50 | |||
207 | 51 | def _on_bind_pebble_ready(self, event): | ||
208 | 52 | """Handle the on pebble ready event.""" | ||
209 | 53 | logger.debug('Setting self.state.bind_pebble_ready to True') | ||
210 | 54 | self.state.bind_pebble_ready = True | ||
211 | 55 | |||
212 | 56 | def _on_config_changed(self, event): | ||
213 | 57 | """Handle config-changed event.""" | ||
214 | 58 | problems = self._check_for_config_problems() | ||
215 | 59 | if problems: | ||
216 | 60 | self.unit.status = BlockedStatus(problems) | ||
217 | 61 | return | ||
218 | 62 | |||
219 | 63 | if not self.state.bind_pebble_ready: | ||
220 | 64 | logger.debug('Waiting for pebble') | ||
221 | 65 | self.unit.status = WaitingStatus('Waiting for pebble') | ||
222 | 66 | event.defer() | ||
223 | 67 | return | ||
224 | 68 | |||
225 | 69 | pebble_config = self._get_pebble_config() | ||
226 | 70 | if not pebble_config: | ||
227 | 71 | # Charm will be in blocked status. | ||
228 | 72 | event.defer() | ||
229 | 73 | return | ||
230 | 74 | |||
231 | 75 | container = self.unit.get_container(CONTAINER_NAME) | ||
232 | 76 | services = container.get_plan().to_dict().get("services", {}) | ||
233 | 77 | if services != pebble_config["services"]: | ||
234 | 78 | logger.debug('Adding layer to pebble') | ||
235 | 79 | self.unit.status = MaintenanceStatus('Adding layer to pebble') | ||
236 | 80 | container.add_layer("bind", pebble_config, combine=True) | ||
237 | 81 | |||
238 | 82 | self.unit.status = MaintenanceStatus('Restarting bind') | ||
239 | 83 | service = container.get_service("bind") | ||
240 | 84 | if service.is_running(): | ||
241 | 85 | logger.debug('Stopping bind') | ||
242 | 86 | container.stop("bind") | ||
243 | 87 | logger.debug('Starting bind') | ||
244 | 88 | container.start("bind") | ||
245 | 89 | |||
246 | 90 | self.unit.status = ActiveStatus() | ||
247 | 24 | 91 | ||
248 | 25 | def _check_for_config_problems(self): | 92 | def _check_for_config_problems(self): |
249 | 26 | """Return a string describing any configuration problems (or an empty string if none).""" | 93 | """Return a string describing any configuration problems (or an empty string if none).""" |
250 | 27 | problems = '' | 94 | problems = '' |
251 | 28 | 95 | ||
252 | 29 | missing = self._missing_charm_settings() | 96 | missing = self._missing_charm_settings() |
254 | 30 | if missing: | 97 | if missing: # pragma: no cover |
255 | 31 | problems = 'required setting(s) empty: {}'.format(', '.join(sorted(missing))) | 98 | problems = 'required setting(s) empty: {}'.format(', '.join(sorted(missing))) |
256 | 32 | 99 | ||
257 | 33 | return problems | 100 | return problems |
258 | @@ -38,104 +105,27 @@ class BindK8sCharm(CharmBase): | |||
259 | 38 | 105 | ||
260 | 39 | missing = {setting for setting in REQUIRED_SETTINGS if not config[setting]} | 106 | missing = {setting for setting in REQUIRED_SETTINGS if not config[setting]} |
261 | 40 | 107 | ||
262 | 41 | if config['bind_image_username'] and not config['bind_image_password']: | ||
263 | 42 | missing.add('bind_image_password') | ||
264 | 43 | |||
265 | 44 | return missing | 108 | return missing |
266 | 45 | 109 | ||
301 | 46 | def on_config_changed(self, event): | 110 | def _generate_env_config(self): |
302 | 47 | """Check that we're leader, and if so, set up the pod.""" | 111 | """Kubernetes environment config generator.""" |
269 | 48 | if self.model.unit.is_leader(): | ||
270 | 49 | # Only the leader can set_spec(). | ||
271 | 50 | resources = self.make_pod_resources() | ||
272 | 51 | spec = self.make_pod_spec() | ||
273 | 52 | spec.update(resources) | ||
274 | 53 | |||
275 | 54 | msg = "Configuring pod" | ||
276 | 55 | logger.info(msg) | ||
277 | 56 | self.model.unit.status = MaintenanceStatus(msg) | ||
278 | 57 | self.model.pod.set_spec(spec) | ||
279 | 58 | |||
280 | 59 | msg = "Pod configured" | ||
281 | 60 | logger.info(msg) | ||
282 | 61 | self.model.unit.status = ActiveStatus(msg) | ||
283 | 62 | else: | ||
284 | 63 | logger.info("Spec changes ignored by non-leader") | ||
285 | 64 | self.model.unit.status = ActiveStatus() | ||
286 | 65 | |||
287 | 66 | def make_pod_resources(self): | ||
288 | 67 | """Compile and return our pod resources (e.g. ingresses).""" | ||
289 | 68 | # LP#1889746: We need to define a manual ingress here to work around LP#1889703. | ||
290 | 69 | resources = {} # TODO | ||
291 | 70 | logger.info("This is the Kubernetes Pod resources <<EOM\n{}\nEOM".format(pformat(resources))) | ||
292 | 71 | return resources | ||
293 | 72 | |||
294 | 73 | def generate_pod_config(self, secured=True): | ||
295 | 74 | """Kubernetes pod config generator. | ||
296 | 75 | |||
297 | 76 | generate_pod_config generates Kubernetes deployment config. | ||
298 | 77 | If the secured keyword is set then it will return a sanitised copy | ||
299 | 78 | without exposing secrets. | ||
300 | 79 | """ | ||
303 | 80 | config = self.model.config | 112 | config = self.model.config |
308 | 81 | pod_config = {} | 113 | env_config = {} |
305 | 82 | if config["container_config"].strip(): | ||
306 | 83 | pod_config = safe_load(config["container_config"]) | ||
307 | 84 | |||
309 | 85 | if config["custom_config_repo"].strip(): | 114 | if config["custom_config_repo"].strip(): |
324 | 86 | pod_config["CUSTOM_CONFIG_REPO"] = config["custom_config_repo"] | 115 | logger.debug('Custom config repo set') |
325 | 87 | 116 | env_config["CUSTOM_CONFIG_REPO"] = config["custom_config_repo"] | |
326 | 88 | if config["enable_rfc1918_recursion"]: | 117 | elif config["enable_rfc1918_recursion"]: |
327 | 89 | pod_config["ENABLE_RFC1918_RECURSION"] = 1 | 118 | logger.debug('No custom config repo set, recursion will be enabled') |
328 | 90 | 119 | env_config["ENABLE_RFC1918_RECURSION"] = 1 | |
315 | 91 | if config["https_proxy"].strip(): | ||
316 | 92 | pod_config["http_proxy"] = config["https_proxy"] | ||
317 | 93 | pod_config["https_proxy"] = config["https_proxy"] | ||
318 | 94 | |||
319 | 95 | if secured: | ||
320 | 96 | return pod_config | ||
321 | 97 | |||
322 | 98 | if config["container_secrets"].strip(): | ||
323 | 99 | container_secrets = safe_load(config["container_secrets"]) | ||
329 | 100 | else: | 120 | else: |
360 | 101 | container_secrets = {} | 121 | logger.debug('No custom config repo set, recursion disabled too, deploying with stock config') |
361 | 102 | 122 | pass | |
332 | 103 | pod_config.update(container_secrets) | ||
333 | 104 | return pod_config | ||
334 | 105 | |||
335 | 106 | def make_pod_spec(self): | ||
336 | 107 | """Set up and return our full pod spec here.""" | ||
337 | 108 | config = self.model.config | ||
338 | 109 | full_pod_config = self.generate_pod_config(secured=False) | ||
339 | 110 | secure_pod_config = self.generate_pod_config(secured=True) | ||
340 | 111 | |||
341 | 112 | ports = [ | ||
342 | 113 | {"name": "domain-tcp", "containerPort": 53, "protocol": "TCP"}, | ||
343 | 114 | {"name": "domain-udp", "containerPort": 53, "protocol": "UDP"}, | ||
344 | 115 | ] | ||
345 | 116 | |||
346 | 117 | spec = { | ||
347 | 118 | "version": 2, | ||
348 | 119 | "containers": [ | ||
349 | 120 | { | ||
350 | 121 | "name": self.app.name, | ||
351 | 122 | "imageDetails": {"imagePath": config["bind_image_path"]}, | ||
352 | 123 | "ports": ports, | ||
353 | 124 | "config": secure_pod_config, | ||
354 | 125 | "kubernetes": {"readinessProbe": {"exec": {"command": ["/usr/local/bin/dns-check.sh"]}}}, | ||
355 | 126 | } | ||
356 | 127 | ], | ||
357 | 128 | } | ||
358 | 129 | |||
359 | 130 | logger.info("This is the Kubernetes Pod spec config (sans secrets) <<EOM\n{}\nEOM".format(pformat(spec))) | ||
362 | 131 | 123 | ||
368 | 132 | if config.get("bind_image_username") and config.get("bind_image_password"): | 124 | if config["https_proxy"].strip(): |
369 | 133 | spec.get("containers")[0].get("imageDetails")["username"] = config["bind_image_username"] | 125 | env_config["http_proxy"] = config["https_proxy"] |
370 | 134 | spec.get("containers")[0].get("imageDetails")["password"] = config["bind_image_password"] | 126 | env_config["https_proxy"] = config["https_proxy"] |
366 | 135 | |||
367 | 136 | secure_pod_config.update(full_pod_config) | ||
371 | 137 | 127 | ||
373 | 138 | return spec | 128 | return env_config |
374 | 139 | 129 | ||
375 | 140 | 130 | ||
376 | 141 | if __name__ == "__main__": # pragma: no cover | 131 | if __name__ == "__main__": # pragma: no cover |
377 | diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py | |||
378 | index c9834d1..1736527 100644 | |||
379 | --- a/tests/unit/test_charm.py | |||
380 | +++ b/tests/unit/test_charm.py | |||
381 | @@ -3,268 +3,127 @@ | |||
382 | 3 | 3 | ||
383 | 4 | import unittest | 4 | import unittest |
384 | 5 | 5 | ||
385 | 6 | from unittest.mock import MagicMock | ||
386 | 7 | |||
387 | 6 | from charm import BindK8sCharm | 8 | from charm import BindK8sCharm |
388 | 7 | 9 | ||
389 | 8 | from ops import testing | 10 | from ops import testing |
391 | 9 | from ops.model import ActiveStatus | 11 | from ops.model import WaitingStatus |
392 | 10 | 12 | ||
399 | 11 | CONFIG_EMPTY = { | 13 | CONFIG_VALID = { |
394 | 12 | 'bind_image_path': '', | ||
395 | 13 | 'bind_image_username': '', | ||
396 | 14 | 'bind_image_password': '', | ||
397 | 15 | 'container_config': '', | ||
398 | 16 | 'container_secrets': '', | ||
400 | 17 | 'custom_config_repo': '', | 14 | 'custom_config_repo': '', |
401 | 15 | 'enable_rfc1918_recursion': True, | ||
402 | 18 | 'https_proxy': '', | 16 | 'https_proxy': '', |
403 | 19 | } | 17 | } |
404 | 20 | 18 | ||
405 | 21 | CONFIG_IMAGE_PASSWORD_MISSING = { | ||
406 | 22 | 'bind_image_path': 'example.com/bind:v1', | ||
407 | 23 | 'bind_image_username': 'username', | ||
408 | 24 | 'bind_image_password': '', | ||
409 | 25 | 'container_config': '', | ||
410 | 26 | 'container_secrets': '', | ||
411 | 27 | 'custom_config_repo': '', | ||
412 | 28 | 'https_proxy': '', | ||
413 | 29 | } | ||
414 | 30 | 19 | ||
424 | 31 | CONFIG_VALID = { | 20 | def _pebble_services_bind(pebble_config): |
425 | 32 | 'bind_image_path': 'example.com/bind:v1', | 21 | """Extract bind's service from the given pebble_config.""" |
426 | 33 | 'bind_image_username': '', | 22 | return pebble_config['services']['bind'] |
418 | 34 | 'bind_image_password': '', | ||
419 | 35 | 'container_config': '', | ||
420 | 36 | 'container_secrets': '', | ||
421 | 37 | 'custom_config_repo': '', | ||
422 | 38 | 'https_proxy': '', | ||
423 | 39 | } | ||
427 | 40 | 23 | ||
428 | 41 | CONFIG_VALID_WITHOUT_RECURSION = { | ||
429 | 42 | 'bind_image_path': 'example.com/bind:v1', | ||
430 | 43 | 'bind_image_username': '', | ||
431 | 44 | 'bind_image_password': '', | ||
432 | 45 | 'enable_rfc1918_recursion': False, | ||
433 | 46 | 'container_config': '', | ||
434 | 47 | 'container_secrets': '', | ||
435 | 48 | 'custom_config_repo': '', | ||
436 | 49 | 'https_proxy': '', | ||
437 | 50 | } | ||
438 | 51 | 24 | ||
448 | 52 | CONFIG_VALID_WITH_CREDS = { | 25 | def _pebble_services_bind_env(pebble_config): |
449 | 53 | 'bind_image_path': 'secure.example.com/bind:v1', | 26 | """Extract bind's service environment from the given pebble_config.""" |
450 | 54 | 'bind_image_username': 'test-user', | 27 | return pebble_config['services']['bind']['environment'] |
442 | 55 | 'bind_image_password': 'test-password', | ||
443 | 56 | 'container_config': '', | ||
444 | 57 | 'container_secrets': '', | ||
445 | 58 | 'custom_config_repo': '', | ||
446 | 59 | 'https_proxy': '', | ||
447 | 60 | } | ||
451 | 61 | 28 | ||
452 | 62 | CONFIG_VALID_WITH_CONTAINER_CONFIG = { | ||
453 | 63 | 'bind_image_path': 'example.com/bind:v1', | ||
454 | 64 | 'bind_image_username': '', | ||
455 | 65 | 'bind_image_password': '', | ||
456 | 66 | 'container_config': '"magic_number": 123', | ||
457 | 67 | 'container_secrets': '', | ||
458 | 68 | 'custom_config_repo': '', | ||
459 | 69 | 'https_proxy': '', | ||
460 | 70 | } | ||
461 | 71 | 29 | ||
471 | 72 | CONFIG_VALID_WITH_CONTAINER_CONFIG_AND_SECRETS = { | 30 | def _pebble_summary(pebble_config): |
472 | 73 | 'bind_image_path': 'example.com/bind:v1', | 31 | """Extract bind's summary from the given pebble_config.""" |
473 | 74 | 'bind_image_username': '', | 32 | return pebble_config['summary'] |
465 | 75 | 'bind_image_password': '', | ||
466 | 76 | 'container_config': '"magic_number": 123', | ||
467 | 77 | 'container_secrets': '"secret_password": "xyzzy"', | ||
468 | 78 | 'custom_config_repo': '', | ||
469 | 79 | 'https_proxy': '', | ||
470 | 80 | } | ||
474 | 81 | 33 | ||
484 | 82 | CONFIG_VALID_WITH_CUSTOM_CONFIG_REPO_AND_PROXY = { | 34 | |
485 | 83 | 'bind_image_path': 'example.com/bind:v1', | 35 | def _pebble_description(pebble_config): |
486 | 84 | 'bind_image_username': '', | 36 | """Extract bind's description from the given pebble_config.""" |
487 | 85 | 'bind_image_password': '', | 37 | return pebble_config['description'] |
479 | 86 | 'container_config': '', | ||
480 | 87 | 'container_secrets': '', | ||
481 | 88 | 'custom_config_repo': 'https://git.example.com/example-bind-config.git', | ||
482 | 89 | 'https_proxy': 'http://webproxy.example.com:3128/', | ||
483 | 90 | } | ||
488 | 91 | 38 | ||
489 | 92 | 39 | ||
490 | 93 | class TestBindK8s(unittest.TestCase): | 40 | class TestBindK8s(unittest.TestCase): |
491 | 94 | maxDiff = None | 41 | maxDiff = None |
492 | 95 | 42 | ||
493 | 96 | def setUp(self): | 43 | def setUp(self): |
494 | 44 | """Setup the harness object.""" | ||
495 | 97 | self.harness = testing.Harness(BindK8sCharm) | 45 | self.harness = testing.Harness(BindK8sCharm) |
496 | 98 | self.harness.begin() | 46 | self.harness.begin() |
498 | 99 | self.harness.disable_hooks() | 47 | self.harness.add_oci_resource('bind-image') |
499 | 100 | 48 | ||
505 | 101 | def test_check_for_config_problems_empty_image_path(self): | 49 | self._expected = dict() |
501 | 102 | """Confirm that we generate an error if we're not told what image to use.""" | ||
502 | 103 | self.harness.update_config(CONFIG_EMPTY) | ||
503 | 104 | expected = 'required setting(s) empty: bind_image_path' | ||
504 | 105 | self.assertEqual(self.harness.charm._check_for_config_problems(), expected) | ||
506 | 106 | 50 | ||
512 | 107 | def test_check_for_config_problems_empty_image_password(self): | 51 | def tearDown(self): |
513 | 108 | """Confirm that we generate an error if we're not given valid registry creds.""" | 52 | """Cleanup the harness.""" |
514 | 109 | self.harness.update_config(CONFIG_IMAGE_PASSWORD_MISSING) | 53 | self.harness.cleanup() |
510 | 110 | expected = 'required setting(s) empty: bind_image_password' | ||
511 | 111 | self.assertEqual(self.harness.charm._check_for_config_problems(), expected) | ||
515 | 112 | 54 | ||
516 | 113 | def test_check_for_config_problems_none(self): | 55 | def test_check_for_config_problems_none(self): |
517 | 114 | """Confirm that we accept valid config.""" | 56 | """Confirm that we accept valid config.""" |
518 | 115 | self.harness.update_config(CONFIG_VALID) | 57 | self.harness.update_config(CONFIG_VALID) |
521 | 116 | expected = '' | 58 | self._expected = '' |
522 | 117 | self.assertEqual(self.harness.charm._check_for_config_problems(), expected) | 59 | self.assertEqual(self.harness.charm._check_for_config_problems(), self._expected) |
523 | 118 | 60 | ||
531 | 119 | def test_make_pod_resources(self): | 61 | def test_on_bind_pebble_ready_false(self): |
525 | 120 | """Confirm that we generate the expected pod resources (see LP#1889746).""" | ||
526 | 121 | expected = {} | ||
527 | 122 | self.assertEqual(self.harness.charm.make_pod_resources(), expected) | ||
528 | 123 | |||
529 | 124 | def test_make_pod_spec_basic(self): | ||
530 | 125 | """Confirm that we generate the expected pod spec from valid config.""" | ||
532 | 126 | self.harness.update_config(CONFIG_VALID) | 62 | self.harness.update_config(CONFIG_VALID) |
569 | 127 | expected = { | 63 | # the event hasn't run yet, so we expect this to be False |
570 | 128 | 'version': 2, | 64 | self._expected = False |
571 | 129 | 'containers': [ | 65 | self.assertEqual(self.harness.charm.state.bind_pebble_ready, self._expected) |
536 | 130 | { | ||
537 | 131 | 'name': 'bind-k8s', | ||
538 | 132 | 'imageDetails': {'imagePath': 'example.com/bind:v1'}, | ||
539 | 133 | 'ports': [ | ||
540 | 134 | {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'}, | ||
541 | 135 | {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'}, | ||
542 | 136 | ], | ||
543 | 137 | 'config': {'ENABLE_RFC1918_RECURSION': 1}, | ||
544 | 138 | 'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}}, | ||
545 | 139 | } | ||
546 | 140 | ], | ||
547 | 141 | } | ||
548 | 142 | self.assertEqual(self.harness.charm.make_pod_spec(), expected) | ||
549 | 143 | |||
550 | 144 | def test_make_pod_spec_without_recursion(self): | ||
551 | 145 | """Confirm that we generate the expected pod spec from config disabling recursion.""" | ||
552 | 146 | self.harness.update_config(CONFIG_VALID_WITHOUT_RECURSION) | ||
553 | 147 | expected = { | ||
554 | 148 | 'version': 2, | ||
555 | 149 | 'containers': [ | ||
556 | 150 | { | ||
557 | 151 | 'name': 'bind-k8s', | ||
558 | 152 | 'imageDetails': {'imagePath': 'example.com/bind:v1'}, | ||
559 | 153 | 'ports': [ | ||
560 | 154 | {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'}, | ||
561 | 155 | {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'}, | ||
562 | 156 | ], | ||
563 | 157 | 'config': {}, | ||
564 | 158 | 'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}}, | ||
565 | 159 | } | ||
566 | 160 | ], | ||
567 | 161 | } | ||
568 | 162 | self.assertEqual(self.harness.charm.make_pod_spec(), expected) | ||
572 | 163 | 66 | ||
619 | 164 | def test_make_pod_spec_with_creds(self): | 67 | def test_on_config_changed(self): |
620 | 165 | """Confirm that we generate the expected pod spec from config containing credentials.""" | 68 | self.harness.update_config(CONFIG_VALID) |
621 | 166 | self.harness.update_config(CONFIG_VALID_WITH_CREDS) | 69 | self._expected = WaitingStatus('Waiting for pebble') |
622 | 167 | expected = { | 70 | self.assertEqual(self.harness.model.unit.status, self._expected) |
577 | 168 | 'version': 2, | ||
578 | 169 | 'containers': [ | ||
579 | 170 | { | ||
580 | 171 | 'name': 'bind-k8s', | ||
581 | 172 | 'imageDetails': { | ||
582 | 173 | 'imagePath': 'secure.example.com/bind:v1', | ||
583 | 174 | 'username': 'test-user', | ||
584 | 175 | 'password': 'test-password', | ||
585 | 176 | }, | ||
586 | 177 | 'ports': [ | ||
587 | 178 | {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'}, | ||
588 | 179 | {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'}, | ||
589 | 180 | ], | ||
590 | 181 | 'config': {'ENABLE_RFC1918_RECURSION': 1}, | ||
591 | 182 | 'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}}, | ||
592 | 183 | } | ||
593 | 184 | ], | ||
594 | 185 | } | ||
595 | 186 | self.assertEqual(self.harness.charm.make_pod_spec(), expected) | ||
596 | 187 | |||
597 | 188 | def test_make_pod_spec_with_extra_config(self): | ||
598 | 189 | """Confirm that we generate the expected pod spec from a more involved valid config.""" | ||
599 | 190 | self.harness.update_config(CONFIG_VALID_WITH_CONTAINER_CONFIG) | ||
600 | 191 | expected = { | ||
601 | 192 | 'version': 2, | ||
602 | 193 | 'containers': [ | ||
603 | 194 | { | ||
604 | 195 | 'name': 'bind-k8s', | ||
605 | 196 | 'imageDetails': {'imagePath': 'example.com/bind:v1'}, | ||
606 | 197 | 'ports': [ | ||
607 | 198 | {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'}, | ||
608 | 199 | {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'}, | ||
609 | 200 | ], | ||
610 | 201 | 'config': { | ||
611 | 202 | 'ENABLE_RFC1918_RECURSION': 1, | ||
612 | 203 | 'magic_number': 123, | ||
613 | 204 | }, | ||
614 | 205 | 'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}}, | ||
615 | 206 | } | ||
616 | 207 | ], | ||
617 | 208 | } | ||
618 | 209 | self.assertEqual(self.harness.charm.make_pod_spec(), expected) | ||
623 | 210 | 71 | ||
643 | 211 | def test_make_pod_spec_with_extra_config_and_secrets(self): | 72 | def test_missing_charm_settings(self): |
644 | 212 | """Confirm that we generate the expected pod spec from a more involved valid config that includes secrets.""" | 73 | """Confirm that we're missing no settings.""" |
645 | 213 | self.harness.update_config(CONFIG_VALID_WITH_CONTAINER_CONFIG_AND_SECRETS) | 74 | self.harness.update_config(CONFIG_VALID) |
646 | 214 | expected = { | 75 | self._expected = set() |
647 | 215 | 'version': 2, | 76 | self.assertEqual(self.harness.charm._missing_charm_settings(), self._expected) |
629 | 216 | 'containers': [ | ||
630 | 217 | { | ||
631 | 218 | 'name': 'bind-k8s', | ||
632 | 219 | 'imageDetails': {'imagePath': 'example.com/bind:v1'}, | ||
633 | 220 | 'ports': [ | ||
634 | 221 | {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'}, | ||
635 | 222 | {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'}, | ||
636 | 223 | ], | ||
637 | 224 | 'config': {'ENABLE_RFC1918_RECURSION': 1, 'magic_number': 123, 'secret_password': 'xyzzy'}, | ||
638 | 225 | 'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}}, | ||
639 | 226 | } | ||
640 | 227 | ], | ||
641 | 228 | } | ||
642 | 229 | self.assertEqual(self.harness.charm.make_pod_spec(), expected) | ||
648 | 230 | 77 | ||
673 | 231 | def test_make_pod_spec_with_custom_config_repo(self): | 78 | def test_get_pebble_config_description(self): |
674 | 232 | """Confirm that we generate the expected pod spec from a config that includes a custom repo.""" | 79 | self.harness.update_config(CONFIG_VALID) |
675 | 233 | self.harness.update_config(CONFIG_VALID_WITH_CUSTOM_CONFIG_REPO_AND_PROXY) | 80 | self._expected = "Bind layer" |
676 | 234 | expected = { | 81 | self.assertEqual(_pebble_description(self.harness.charm._get_pebble_config()), self._expected) |
653 | 235 | 'version': 2, | ||
654 | 236 | 'containers': [ | ||
655 | 237 | { | ||
656 | 238 | 'name': 'bind-k8s', | ||
657 | 239 | 'imageDetails': {'imagePath': 'example.com/bind:v1'}, | ||
658 | 240 | 'ports': [ | ||
659 | 241 | {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'}, | ||
660 | 242 | {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'}, | ||
661 | 243 | ], | ||
662 | 244 | 'config': { | ||
663 | 245 | 'CUSTOM_CONFIG_REPO': 'https://git.example.com/example-bind-config.git', | ||
664 | 246 | 'ENABLE_RFC1918_RECURSION': 1, | ||
665 | 247 | 'http_proxy': 'http://webproxy.example.com:3128/', | ||
666 | 248 | 'https_proxy': 'http://webproxy.example.com:3128/', | ||
667 | 249 | }, | ||
668 | 250 | 'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}}, | ||
669 | 251 | } | ||
670 | 252 | ], | ||
671 | 253 | } | ||
672 | 254 | self.assertEqual(self.harness.charm.make_pod_spec(), expected) | ||
677 | 255 | 82 | ||
682 | 256 | def test_configure_pod_as_leader(self): | 83 | def test_get_pebble_config_summary(self): |
679 | 257 | """Confirm that our status is set correctly when we're the leader.""" | ||
680 | 258 | self.harness.enable_hooks() | ||
681 | 259 | self.harness.set_leader(True) | ||
683 | 260 | self.harness.update_config(CONFIG_VALID) | 84 | self.harness.update_config(CONFIG_VALID) |
686 | 261 | expected = ActiveStatus('Pod configured') | 85 | self._expected = "Bind layer" |
687 | 262 | self.assertEqual(self.harness.model.unit.status, expected) | 86 | self.assertEqual(_pebble_summary(self.harness.charm._get_pebble_config()), self._expected) |
688 | 263 | 87 | ||
693 | 264 | def test_configure_pod_as_non_leader(self): | 88 | def test_get_pebble_config_services_bind(self): |
690 | 265 | """Confirm that our status is set correctly when we're not the leader.""" | ||
691 | 266 | self.harness.enable_hooks() | ||
692 | 267 | self.harness.set_leader(False) | ||
694 | 268 | self.harness.update_config(CONFIG_VALID) | 89 | self.harness.update_config(CONFIG_VALID) |
697 | 269 | expected = ActiveStatus() | 90 | self._expected = { |
698 | 270 | self.assertEqual(self.harness.model.unit.status, expected) | 91 | "override": "replace", |
699 | 92 | "summary": "Bind service", | ||
700 | 93 | "command": 'bash -c "/usr/local/bin/docker-wrapper.sh &> /proc/1/fd/1"', | ||
701 | 94 | "startup": "enabled", | ||
702 | 95 | "environment": {'ENABLE_RFC1918_RECURSION': 1}, | ||
703 | 96 | } | ||
704 | 97 | self.assertEqual(_pebble_services_bind(self.harness.charm._get_pebble_config()), self._expected) | ||
705 | 98 | |||
706 | 99 | def test_generate_env_config_custom_repo(self): | ||
707 | 100 | self.harness.update_config({"custom_config_repo": "https://git.example.com/example-bind-config.git"}) | ||
708 | 101 | self._expected['CUSTOM_CONFIG_REPO'] = "https://git.example.com/example-bind-config.git" | ||
709 | 102 | self.assertEqual(_pebble_services_bind_env(self.harness.charm._get_pebble_config()), self._expected) | ||
710 | 103 | |||
711 | 104 | def test_generage_env_config_custom_repo_and_proxy(self): | ||
712 | 105 | self.harness.update_config( | ||
713 | 106 | { | ||
714 | 107 | "custom_config_repo": "https://git.example.com/example-bind-config.git", | ||
715 | 108 | "https_proxy": "http://webproxy.example.com:3128/", | ||
716 | 109 | } | ||
717 | 110 | ) | ||
718 | 111 | self._expected['CUSTOM_CONFIG_REPO'] = "https://git.example.com/example-bind-config.git" | ||
719 | 112 | self._expected['http_proxy'] = "http://webproxy.example.com:3128/" | ||
720 | 113 | self._expected['https_proxy'] = "http://webproxy.example.com:3128/" | ||
721 | 114 | self.assertEqual(_pebble_services_bind_env(self.harness.charm._get_pebble_config()), self._expected) | ||
722 | 115 | |||
723 | 116 | def test_generate_env_config_just_proxy(self): | ||
724 | 117 | self.harness.update_config({}) | ||
725 | 118 | self._expected['ENABLE_RFC1918_RECURSION'] = 1 | ||
726 | 119 | self.assertEqual(_pebble_services_bind_env(self.harness.charm._get_pebble_config()), self._expected) | ||
727 | 120 | |||
728 | 121 | def test_on_bind_pebble_ready(self): | ||
729 | 122 | """Test the _on_bind_pebble_ready function.""" | ||
730 | 123 | |||
731 | 124 | # No problem | ||
732 | 125 | mock_event = MagicMock() | ||
733 | 126 | expected_ret = None | ||
734 | 127 | |||
735 | 128 | r = self.harness.charm._on_bind_pebble_ready(mock_event) | ||
736 | 129 | self.assertEqual(r, expected_ret) | ||
737 | diff --git a/tox.ini b/tox.ini | |||
738 | index 0de5149..fb74730 100644 | |||
739 | --- a/tox.ini | |||
740 | +++ b/tox.ini | |||
741 | @@ -10,7 +10,7 @@ setenv = | |||
742 | 10 | [testenv:unit] | 10 | [testenv:unit] |
743 | 11 | commands = | 11 | commands = |
744 | 12 | pytest --ignore mod --ignore {toxinidir}/tests/functional \ | 12 | pytest --ignore mod --ignore {toxinidir}/tests/functional \ |
746 | 13 | {posargs:-v --cov=src --cov-report=term-missing --cov-branch} | 13 | {posargs:-v --cov=src --cov-report=term-missing --cov-branch --cov-report=html} |
747 | 14 | deps = -r{toxinidir}/tests/unit/requirements.txt | 14 | deps = -r{toxinidir}/tests/unit/requirements.txt |
748 | 15 | -r{toxinidir}/requirements.txt | 15 | -r{toxinidir}/requirements.txt |
749 | 16 | setenv = | 16 | setenv = |