Merge ~woutervb/various/+git/snapstore-proxy-charm:resource_installation into ~woutervb/various/+git/snapstore-proxy-charm:main
- Git
- lp:~woutervb/various/+git/snapstore-proxy-charm
- resource_installation
- Merge into main
Status: | Merged |
---|---|
Merged at revision: | 1b556f74b3732e5549e232d70529796754c32515 |
Proposed branch: | ~woutervb/various/+git/snapstore-proxy-charm:resource_installation |
Merge into: | ~woutervb/various/+git/snapstore-proxy-charm:main |
Prerequisite: | ~woutervb/various/+git/snapstore-proxy-charm:github_actions |
Diff against target: |
700 lines (+324/-33) 13 files modified
.gitignore (+2/-0) Makefile (+17/-2) README.md (+64/-5) charmcraft.yaml (+2/-3) juju/resource-overlay.yaml (+9/-0) juju/test-bundle.yaml (+1/-1) metadata.yaml (+9/-0) src/charm.py (+62/-15) src/helpers.py (+1/-1) src/resource_helpers.py (+17/-0) tests/test_charm.py (+115/-4) tests/test_helpers.py (+3/-1) tests/test_resource_helpers.py (+22/-1) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Przemysław Suliga (community) | Approve | ||
Wouter van Bommel | Pending | ||
Review via email:
|
Commit message
Implement the option to install from a resource
* Allow the installation from a resource, requires both a core and the
snap-store-proxy snap to be added.
* Updated the Makefile, to include a deploy-resources target, for simple
testing. This target will download snaps if needed.
* Ensured complete coverage of the added code.
Description of the change
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Przemysław Suliga (suligap) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Wouter van Bommel (woutervb) wrote : | # |
Thanks for the thorough review. Fixed most of the issues found.
Also ensured proper rebase etc
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Przemysław Suliga (suligap) wrote : | # |
Thanks, +1, but please also consider/test if snapd snap should also be required here as a resource.
The snap-store-proxy itself does bundle snapd in the offline bundle, because on fresh bionic containers it's not present if I remember correctly. Or let me know if that's wrong.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Wouter van Bommel (woutervb) wrote : | # |
> Thanks, +1, but please also consider/test if snapd snap should also be
> required here as a resource.
>
> The snap-store-proxy itself does bundle snapd in the offline bundle, because
> on fresh bionic containers it's not present if I remember correctly. Or let me
> know if that's wrong.
You are right that it is needed on Bionic, but not on Focal. Tested by using lxd containers and juju deployments of ubuntu for both series.
For now I will drop Bionic support to overcome this problem. It can always be added if needed.
Preview Diff
1 | diff --git a/.gitignore b/.gitignore |
2 | index 785f70f..c30d7df 100644 |
3 | --- a/.gitignore |
4 | +++ b/.gitignore |
5 | @@ -9,3 +9,5 @@ __pycache__/ |
6 | .pytest_cache/ |
7 | |
8 | juju/overlay.yaml |
9 | + |
10 | +*.snap |
11 | diff --git a/Makefile b/Makefile |
12 | index bcd2827..c79e049 100644 |
13 | --- a/Makefile |
14 | +++ b/Makefile |
15 | @@ -6,7 +6,7 @@ VENV ?= $(VENV_TMP) |
16 | CHARM_NAME = snapstore-proxy |
17 | CHARMCRAFT = $(shell which charmcraft) |
18 | # Releases should match the run-on section in charmcraft.yaml in the same order |
19 | -RELEASES = 20.04 18.04 |
20 | +RELEASES = 20.04 |
21 | CHARM_FILENAME = $(CHARM_NAME)$(subst $(noop) $(noop),,$(addsuffix -amd64, $(addprefix _ubuntu-, $(RELEASES)))).charm |
22 | DEPS = $(shell find src -name '*.py') requirements.txt actions.yaml config.yaml |
23 | |
24 | @@ -51,16 +51,22 @@ lint: $(VENV) |
25 | .PHONY: clean |
26 | clean: $(CHARMCRAFT) |
27 | @echo "Cleaning the project" |
28 | - @# Delete the helper reference, so we don't kill shared virtualenvs |
29 | @$(CHARMCRAFT) clean |
30 | + @# Delete the helper reference, so we don't kill shared virtualenvs |
31 | @rm -rf $(VENV_TMP) .pytest_cache build |
32 | @rm -f $(CHARM_FILENAME) .coverage |
33 | + @rm -f *.snap |
34 | |
35 | .PHONY: deploy |
36 | deploy: $(CHARM_FILENAME) ./juju/test-bundle.yaml ./juju/overlay.yaml |
37 | @echo deploying the test bundle |
38 | @juju deploy ./juju/test-bundle.yaml --overlay ./juju/overlay.yaml |
39 | |
40 | +.PHONY: deploy-resource |
41 | +deploy-resource: $(CHARM_FILENAME) ./juju/test-bundle.yaml ./juju/resource-overlay.yaml ./juju/overlay.yaml core20.snap snap-store-proxy.snap |
42 | + @echo deploying the resouce bundle |
43 | + @juju deploy ./juju/test-bundle.yaml --overlay ./juju/resource-overlay.yaml --overlay ./juju/overlay.yaml |
44 | + |
45 | .PHONY: upgrade_charm |
46 | upgrade_charm: $(CHARM_FILENAME) |
47 | @echo Upgrading the charm with the latest version |
48 | @@ -84,5 +90,14 @@ $(CHARMCRAFT): |
49 | exit 1;\ |
50 | fi |
51 | |
52 | + |
53 | +%.snap: |
54 | + @echo Downloading $(@:.snap=) |
55 | + @snap download $(@:.snap=) |
56 | + @echo Removing the assert |
57 | + @rm *.assert |
58 | + @echo Renaming the snap |
59 | + @mv $(@:.snap=)*snap $(@) |
60 | + |
61 | # The noop below is a helper variable, don't change it to something |
62 | noop := |
63 | diff --git a/README.md b/README.md |
64 | index 7881848..cd1ef35 100644 |
65 | --- a/README.md |
66 | +++ b/README.md |
67 | @@ -4,17 +4,76 @@ |
68 | |
69 | This charm will allow the installation and management of the snapstore proxy charm via juju. This means that all needed settings, etc. can be done via a juju installation bundle, allowing for reproducable installations. |
70 | |
71 | -## Usage |
72 | +# Usage |
73 | |
74 | Minimal items that have to be set are the domain this proxy runs on, and a username / password, on ubuntu one that correspond to an account that does not have 2fa enabled, and has accepted the developer agreement on snapstore.io. |
75 | |
76 | +## Example bundle |
77 | |
78 | -## Relations |
79 | +Below is a minimal bundle example that can be used to deploy the charm using juju. |
80 | |
81 | -This charm depends on a postgresql database, so this charm should relate to a porgresql database, before anything else can be done. |
82 | + applications: |
83 | + postgresql: |
84 | + charm: cs:postgresql |
85 | + channel: stable |
86 | + snapstore-proxy: |
87 | + charm: snapstore-proxy |
88 | + options: |
89 | + domain: http://something.not.local |
90 | + username: some.ubuntu@one.account |
91 | + password: UserPassword |
92 | |
93 | -## Actions |
94 | + relations: |
95 | + - - postgresql:db-admin |
96 | + - snapstore-proxy:db-admin |
97 | + |
98 | +# Relations |
99 | + |
100 | +This charm depends on a postgresql database, so this charm should relate to a postgresql database, before anything else can be done. |
101 | + |
102 | +# Actions |
103 | |
104 | The charm supports the following actions: |
105 | |
106 | - * status - Get the status result of the proxy, use like; `juju run-action snapstore-proxy/leader status --wait` |
107 | +* **status** - Get the status result of the proxy, use like; |
108 | + |
109 | + `juju run-action snapstore-proxy/leader status --wait` |
110 | + |
111 | +# Advanced options |
112 | + |
113 | +## Options |
114 | + |
115 | +There are some mandatory config options that have to be configured for the charm to work, these are: |
116 | + |
117 | +* **domain** - The domain on which the proxy will listen. This is needed for the registration, and the proxy itself needs to be able to resolve it. Ip addresses don't work. It has to be prepended with the protocol (http). |
118 | + |
119 | +* **username** - This is the username of the ubuntu one account that will be used to register the proxy |
120 | +* **password** - The password of the above username |
121 | + |
122 | +## Juju resource usage |
123 | + |
124 | +The charm supports 2 modes to install the snapstore proxy code itself. This code is distributed as a snap from the actual snapstore, which means that the unit to which this charm is deployed, will need internet access, or at least access to the actual snapstore. If this is undesired, it is possible to deploy this charm in complete offline mode, which means that the snaps(\*) will need to be added as a resource. |
125 | + |
126 | +(\*) One will need both the `core20.snap` and the `snap-store-proxy.snap` to be added, as the snap-store-proxy.snap depends on the core. |
127 | + |
128 | +An example bundle to do such a deployment will look like the following. |
129 | + |
130 | + applications: |
131 | + postgresql: |
132 | + charm: cs:postgresql |
133 | + channel: stable |
134 | + snapstore-proxy: |
135 | + charm: snapstore-proxy |
136 | + options: |
137 | + domain: http://something.not.local |
138 | + username: some.ubuntu@one.account |
139 | + password: UserPassword |
140 | + resources: |
141 | + snap-store-proxy: ./snap-store-proxy.snap |
142 | + core: ./core20.snap |
143 | + |
144 | + relations: |
145 | + - - postgresql:db-admin |
146 | + - snapstore-proxy:db-admin |
147 | + |
148 | +With the above example it is assumed that both the `core20.snap` and the `snap-store-proxy.snap` are available in the current directory. The snaps can be downloaded using the `snap download core20` and `snap download snap-store-proxy` resp. |
149 | diff --git a/charmcraft.yaml b/charmcraft.yaml |
150 | index b295f15..77b5206 100644 |
151 | --- a/charmcraft.yaml |
152 | +++ b/charmcraft.yaml |
153 | @@ -9,9 +9,8 @@ type: "charm" |
154 | bases: |
155 | - build-on: |
156 | - name: "ubuntu" |
157 | - channel: "18.04" |
158 | + channel: "20.04" |
159 | run-on: |
160 | - name: "ubuntu" |
161 | channel: "20.04" |
162 | - - name: "ubuntu" |
163 | - channel: "18.04" |
164 | + |
165 | diff --git a/juju/resource-overlay.yaml b/juju/resource-overlay.yaml |
166 | new file mode 100644 |
167 | index 0000000..be135a9 |
168 | --- /dev/null |
169 | +++ b/juju/resource-overlay.yaml |
170 | @@ -0,0 +1,9 @@ |
171 | +# Copyright 2021 Canonical |
172 | +# See LICENSE file for licensing details. |
173 | +# |
174 | + |
175 | +applications: |
176 | + snapstore-proxy: |
177 | + resources: |
178 | + snap-store-proxy: ../snap-store-proxy.snap |
179 | + core: ../core20.snap |
180 | diff --git a/juju/test-bundle.yaml b/juju/test-bundle.yaml |
181 | index e41aea5..39c1f08 100644 |
182 | --- a/juju/test-bundle.yaml |
183 | +++ b/juju/test-bundle.yaml |
184 | @@ -11,7 +11,7 @@ applications: |
185 | wal-e: 0 |
186 | num_units: 1 |
187 | snapstore-proxy: |
188 | - charm: ../snapstore-proxy_ubuntu-20.04-amd64_ubuntu-18.04-amd64.charm |
189 | + charm: ../snapstore-proxy_ubuntu-20.04-amd64.charm |
190 | num_units: 1 |
191 | options: |
192 | domain: http://something.not.local |
193 | diff --git a/metadata.yaml b/metadata.yaml |
194 | index 5d27a04..6550676 100644 |
195 | --- a/metadata.yaml |
196 | +++ b/metadata.yaml |
197 | @@ -17,4 +17,13 @@ requires: |
198 | limit: 1 |
199 | desciprtion: Postgresql database used for charm administration, needs to connect to the db-admin interface |
200 | |
201 | +resources: |
202 | + snap-store-proxy: |
203 | + type: file |
204 | + filename: snap-store-proxy.snap |
205 | + description: Snapstore Proxy Snap, used for offline / airgapped installation |
206 | |
207 | + core: |
208 | + type: file |
209 | + filename: core20.snap |
210 | + description: Core snap needed for the snap-store-proxy.snap |
211 | diff --git a/src/charm.py b/src/charm.py |
212 | index 419643a..591965f 100755 |
213 | --- a/src/charm.py |
214 | +++ b/src/charm.py |
215 | @@ -13,7 +13,7 @@ from ops.charm import CharmBase |
216 | from ops.framework import StoredState |
217 | from ops.lib import use |
218 | from ops.main import main |
219 | -from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus |
220 | +from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, ModelError |
221 | |
222 | from exceptions import ConfigException |
223 | from helpers import all_config_options, configure_proxy, default_values |
224 | @@ -21,6 +21,8 @@ from optionvalidation import OptionValidation |
225 | from resource_helpers import ( |
226 | create_database, |
227 | get_status, |
228 | + hash_from_resource, |
229 | + install_from_resource, |
230 | install_from_the_charmstore, |
231 | register_store, |
232 | snap_details, |
233 | @@ -41,6 +43,9 @@ class SnapstoreProxyCharm(CharmBase): |
234 | |
235 | def __init__(self, *args): |
236 | super().__init__(*args) |
237 | + |
238 | + self.errors = [] |
239 | + |
240 | self.framework.observe(self.on.config_changed, self._on_config_changed) |
241 | self.framework.observe(self.on.update_status, self._on_update_status) |
242 | |
243 | @@ -80,6 +85,15 @@ class SnapstoreProxyCharm(CharmBase): |
244 | event.defer() |
245 | return |
246 | |
247 | + if getattr(self._stored, "snap_install_source", None) is None: |
248 | + self.errors.append("Snap has not been installed") |
249 | + logger.warning("Store is not installed") |
250 | + self.evaluate_status() |
251 | + event.defer() |
252 | + return |
253 | + |
254 | + self.handle_pgsql_dsn_change() |
255 | + |
256 | self.unit.status = MaintenanceStatus("Configuring application snap") |
257 | |
258 | # Handle the validation and interal storage of the configuration options |
259 | @@ -87,10 +101,9 @@ class SnapstoreProxyCharm(CharmBase): |
260 | try: |
261 | self.validate_config_item() |
262 | except ConfigException as exc: |
263 | - self.unit.status = BlockedStatus( |
264 | - f"Could not parse the config option for {exc.message}" |
265 | - ) |
266 | - raise |
267 | + self.errors.append(f"Could not parse the config option for {exc.message}") |
268 | + self.evaluate_status() |
269 | + return |
270 | else: |
271 | # With config options stored, we can try to configure the snap |
272 | for option in all_config_options: |
273 | @@ -105,7 +118,6 @@ class SnapstoreProxyCharm(CharmBase): |
274 | logger.debug(f"Storing {value} in {option}") |
275 | self._stored.__setattr__(option, value) |
276 | |
277 | - # Order is important, we first need to install the snap |
278 | # Then configure it, and only then we can attempt to configure it |
279 | self.handle_config() |
280 | |
281 | @@ -128,9 +140,35 @@ class SnapstoreProxyCharm(CharmBase): |
282 | |
283 | We can install updates if provided via resources, but we cannot switch |
284 | between resource and store installation""" |
285 | + |
286 | + # If we have the attribute snap_install_source with the value set to resource, |
287 | + # or it is absent, we can try to install from a resource |
288 | + if getattr(self._stored, "snap_install_source", None) in [None, "resource"]: |
289 | + try: |
290 | + snap = self.model.resources.fetch("snap-store-proxy") |
291 | + core = self.model.resources.fetch("core") |
292 | + snap_md5 = hash_from_resource(snap) |
293 | + core_md5 = hash_from_resource(core) |
294 | + if (getattr(self._stored, "snap_md5", None) != snap_md5) or ( |
295 | + getattr(self._stored, "core_md5", None) != core_md5 |
296 | + ): |
297 | + install_from_resource(core, snap) |
298 | + self._stored.core_md5 = core_md5 |
299 | + self._stored.snap_md5 = snap_md5 |
300 | + self._stored.snap_install_source = "resource" |
301 | + logger.info("Installed the proxy from the resource") |
302 | + except ModelError: |
303 | + logger.info("Could not install snap from resource, attempting online") |
304 | + |
305 | if not hasattr(self._stored, "snap_install_source"): |
306 | - install_from_the_charmstore("snap-store-proxy") |
307 | - self._stored.snap_install_source = "store" |
308 | + try: |
309 | + install_from_the_charmstore("snap-store-proxy") |
310 | + self._stored.snap_install_source = "store" |
311 | + except Exception: |
312 | + logger.info( |
313 | + "Could not install online, retry again if resources are updated" |
314 | + ) |
315 | + self.errors.append("Failed to install the snap-store-proxy") |
316 | |
317 | def handle_config(self): |
318 | |
319 | @@ -139,12 +177,9 @@ class SnapstoreProxyCharm(CharmBase): |
320 | |
321 | for item in all_config_options.keys(): |
322 | logger.debug(f"Testing config item {item} for a new value") |
323 | + |
324 | # try to get the value from the charm config, if set |
325 | - new_value = getattr(self._stored, item, None) |
326 | - if new_value is None: |
327 | - logger.debug(f"Value for {item} not set, trying to get default") |
328 | - # The value is not set, see what the default is supposed to be |
329 | - new_value = default_values.get(item, None) |
330 | + new_value = getattr(self._stored, item, default_values.get(item, None)) |
331 | if new_value is None: |
332 | # No value set, or default for this item, skip |
333 | logger.debug(f"No default for {item}, skipping") |
334 | @@ -221,11 +256,23 @@ class SnapstoreProxyCharm(CharmBase): |
335 | self._stored.registered = registered |
336 | |
337 | def handle_pgsql_dsn_change(self): |
338 | - logger.info("Handling the database configuration of the proxy") |
339 | - create_database(self._stored.db_uri) |
340 | + """If the database is configured, then we try to configure the snap.""" |
341 | + installation_source = getattr(self._stored, "snap_install_source", None) |
342 | + database_url = getattr(self._stored, "db_uri", None) |
343 | + database_created = getattr(self._stored, "database_created", False) |
344 | + if installation_source and database_url and not database_created: |
345 | + logger.info("Handling the database configuration of the proxy") |
346 | + if not hasattr(self._stored, "database_created"): |
347 | + create_database(self._stored.db_uri) |
348 | + self._stored.database_created = True |
349 | |
350 | def evaluate_status(self): |
351 | """This is the only place where we can set the status to Active""" |
352 | + |
353 | + if self.errors: |
354 | + self.unit.status = BlockedStatus("\n".join(self.errors).strip()) |
355 | + return |
356 | + |
357 | if self._stored.db_uri is None: |
358 | self.unit.status = BlockedStatus("Missing database relation") |
359 | return |
360 | diff --git a/src/helpers.py b/src/helpers.py |
361 | index 6b622a7..0575279 100644 |
362 | --- a/src/helpers.py |
363 | +++ b/src/helpers.py |
364 | @@ -13,7 +13,7 @@ logger = logging.getLogger(__name__) |
365 | def configure_proxy(option, value): |
366 | """Simple wrapper around calling snap settings""" |
367 | logger.debug(f"Running config for option {option} with value {value}") |
368 | - run(["snap-proxy", "config", f"{option}={value}"]) |
369 | + run(["snap-store-proxy", "config", f"{option}={value}"]) |
370 | |
371 | |
372 | def config_options(): |
373 | diff --git a/src/resource_helpers.py b/src/resource_helpers.py |
374 | index e5e9e76..007683b 100644 |
375 | --- a/src/resource_helpers.py |
376 | +++ b/src/resource_helpers.py |
377 | @@ -2,6 +2,7 @@ |
378 | # See LICENSE file for licensing details. |
379 | # |
380 | |
381 | +import hashlib |
382 | import logging |
383 | from subprocess import PIPE, CalledProcessError, check_output, run |
384 | |
385 | @@ -12,6 +13,22 @@ def install_from_the_charmstore(snap, channel="stable"): |
386 | return run(["snap", "install", f"--channel={channel}", snap], check=True) |
387 | |
388 | |
389 | +def install_from_resource(core, proxy): |
390 | + run(["snap", "install", "--dangerous", core], check=True) |
391 | + run(["snap", "install", "--dangerous", proxy], check=True) |
392 | + |
393 | + |
394 | +def hash_from_resource(resource): |
395 | + with open(resource, mode="rb") as fh: |
396 | + hash = hashlib.md5() |
397 | + while True: |
398 | + buf = fh.read(4096) |
399 | + if not buf: |
400 | + break |
401 | + hash.update(buf) |
402 | + return hash.hexdigest() |
403 | + |
404 | + |
405 | def snap_details(snap): |
406 | return ( |
407 | run(["snap", "list", snap], stdout=PIPE) |
408 | diff --git a/tests/test_charm.py b/tests/test_charm.py |
409 | index f89e6a0..a224ba7 100644 |
410 | --- a/tests/test_charm.py |
411 | +++ b/tests/test_charm.py |
412 | @@ -14,7 +14,6 @@ import yaml |
413 | from ops.model import ActiveStatus, BlockedStatus |
414 | from ops.testing import Harness |
415 | |
416 | -import exceptions |
417 | from charm import DATABASE_NAME, SnapstoreProxyCharm |
418 | |
419 | |
420 | @@ -37,6 +36,8 @@ class TestConfig(CharmTest): |
421 | def test_default_config_values(self): |
422 | # Our implicit requirement is that the database uri has been set. |
423 | self.harness.charm._stored.db_uri = "postgresql://fake/database" |
424 | + self.harness.charm._stored.snap_install_source = "store" |
425 | + self.harness.charm._stored.database_created = True |
426 | |
427 | # Since valid values will be pushed on to the proxy, we need to stub that here |
428 | with patch("charm.configure_proxy"): |
429 | @@ -44,11 +45,14 @@ class TestConfig(CharmTest): |
430 | self.harness.update_config() |
431 | |
432 | for key, value in self.config_defaults.items(): |
433 | + # Accept the default value of None, for items that don't have a default value |
434 | assert getattr(self.harness.charm._stored, key, None) == value |
435 | |
436 | def test_config_changed_option_update(self): |
437 | # Our implicit requirement is that the database uri has been set. |
438 | self.harness.charm._stored.db_uri = "postgresql://fake/database" |
439 | + self.harness.charm._stored.snap_install_source = "store" |
440 | + self.harness.charm._stored.database_created = True |
441 | |
442 | # Since valid values will be pushed on to the proxy, we need to stub that here |
443 | with patch("charm.configure_proxy"): |
444 | @@ -60,6 +64,8 @@ class TestConfig(CharmTest): |
445 | # When there is an upodate, but no database set, we should end up with an |
446 | # Missing database relation blocked state |
447 | event = MagicMock() |
448 | + self.harness.charm._stored.snap_install_source = "store" |
449 | + |
450 | self.harness.charm._on_config_changed(event) |
451 | |
452 | event.defer.assert_called_once() |
453 | @@ -67,6 +73,23 @@ class TestConfig(CharmTest): |
454 | "Missing database relation" |
455 | ) |
456 | |
457 | + def test_on_config_changed_missing_snap(self): |
458 | + # Here we test what happens if the snap installation fails for some reason |
459 | + event = MagicMock() |
460 | + self.harness.charm._stored.db_uri = "postgresql://fake/database" |
461 | + |
462 | + with patch.object( |
463 | + self.harness.charm, "handle_installation" |
464 | + ) as patched_handle_installation: |
465 | + self.harness.charm._on_config_changed(event) |
466 | + |
467 | + patched_handle_installation.assert_called_once() |
468 | + event.defer.assert_called_once() |
469 | + assert not getattr(self.harness.charm._stored, "snap_install_source", None) |
470 | + assert self.harness.charm.unit.status == BlockedStatus( |
471 | + "Snap has not been installed" |
472 | + ) |
473 | + |
474 | def test_field_is_updated(self): |
475 | # Check that we have an update is the md5 matches |
476 | |
477 | @@ -94,6 +117,7 @@ class TestConfig(CharmTest): |
478 | caplog.clear() |
479 | caplog.set_level(logging.INFO) |
480 | |
481 | + self.harness.charm._stored.snap_install_source = "store" |
482 | self.harness.charm._stored.domain = "My new domain" |
483 | # We know the md5 |
484 | self.harness.charm._stored.domain_md5 = "79911e6e27b92c112ea0034734bf9f14" |
485 | @@ -125,8 +149,17 @@ class TestConfig(CharmTest): |
486 | """ |
487 | # Our implicit requirement is that the database uri has been set. |
488 | self.harness.charm._stored.db_uri = "postgresql://fake/database" |
489 | - with pytest.raises(exceptions.InvalidTypeException): |
490 | - self.harness.update_config({config: value}) |
491 | + self.harness.charm.errors = [] |
492 | + self.harness.charm._stored.snap_install_source = "store" |
493 | + self.harness.charm._stored.database_created = True |
494 | + |
495 | + self.harness.update_config({config: value}) |
496 | + |
497 | + assert len(self.harness.charm.errors) == 1 |
498 | + assert ( |
499 | + f"Could not parse the config option for Charm option '{config}'" |
500 | + in self.harness.charm.errors[0] |
501 | + ) |
502 | |
503 | @pytest.mark.parametrize( |
504 | "config,value", |
505 | @@ -145,6 +178,8 @@ class TestConfig(CharmTest): |
506 | """ |
507 | # Our implicit requirement is that the database uri has been set. |
508 | self.harness.charm._stored.db_uri = "postgresql://fake/database" |
509 | + self.harness.charm._stored.snap_install_source = "store" |
510 | + self.harness.charm._stored.database_created = True |
511 | |
512 | # Since valid values will be pushed on to the proxy, we need to stub that here |
513 | with patch("charm.configure_proxy"): |
514 | @@ -152,11 +187,14 @@ class TestConfig(CharmTest): |
515 | assert getattr(self.harness.charm._stored, config) == value |
516 | |
517 | def test_handle_pgsql_dsn_change(self): |
518 | + self.harness.charm._stored.db_uri = "postgresql://fake/database" |
519 | + self.harness.charm._stored.snap_install_source = "store" |
520 | + |
521 | with patch("charm.create_database") as patched_proxy: |
522 | self.harness.charm.handle_pgsql_dsn_change() |
523 | |
524 | assert patched_proxy.called |
525 | - patched_proxy.assert_called_once_with(None) |
526 | + patched_proxy.assert_called_once_with("postgresql://fake/database") |
527 | |
528 | |
529 | class TestStatus(CharmTest): |
530 | @@ -292,6 +330,8 @@ class TestEvents(CharmTest): |
531 | mocked_event.database = DATABASE_NAME |
532 | mocked_event.master.uri = dummy_uri |
533 | |
534 | + self.harness.charm._stored.snap_install_source = "store" |
535 | + |
536 | assert self.harness.charm._stored.db_uri is None |
537 | |
538 | with patch("charm.create_database") as mocked_call: |
539 | @@ -428,12 +468,15 @@ class TestRegistration(CharmTest): |
540 | |
541 | class TestSanity(CharmTest): |
542 | def test_empty_username_after_registration_and_active_status(self): |
543 | + self.harness.charm._stored.snap_install_source = "store" |
544 | self.harness.charm._stored.registered = True |
545 | + self.harness.charm._stored.database_created = True |
546 | self.harness.charm._stored.username = "Some value" |
547 | self.harness.charm._stored.db_uri = "postgresql://user@host/database" |
548 | |
549 | # Since valid values will be pushed on to the proxy, we need to stub that here |
550 | with patch("charm.configure_proxy"): |
551 | + # This should not result in a change, as none is not possible to be passed on |
552 | self.harness.update_config({"username": None}) |
553 | |
554 | assert self.harness.charm._stored.username == "Some value" |
555 | @@ -441,6 +484,7 @@ class TestSanity(CharmTest): |
556 | |
557 | def test_empty_password_after_registration_and_active_status(self): |
558 | self.harness.charm._stored.registered = True |
559 | + self.harness.charm._stored.database_created = True |
560 | self.harness.charm._stored.password = "Some value" |
561 | self.harness.charm._stored.db_uri = "postgresql://user@host/database" |
562 | |
563 | @@ -452,7 +496,9 @@ class TestSanity(CharmTest): |
564 | assert self.harness.charm.unit.status == ActiveStatus("") |
565 | |
566 | def test_update_domain_after_registration(self): |
567 | + self.harness.charm._stored.snap_install_source = "store" |
568 | self.harness.charm._stored.registered = True |
569 | + self.harness.charm._stored.database_created = True |
570 | self.harness.charm._stored.domain = "Some value" |
571 | self.harness.charm._stored.db_uri = "postgresql://user@host/database" |
572 | |
573 | @@ -462,3 +508,68 @@ class TestSanity(CharmTest): |
574 | |
575 | assert self.harness.charm._stored.domain == "Some value" |
576 | assert self.harness.charm.unit.status == ActiveStatus("") |
577 | + |
578 | + |
579 | +class TestInstallation(CharmTest): |
580 | + def test_install_from_resource(self): |
581 | + self.harness.add_resource("core", "Dummy core") |
582 | + self.harness.add_resource("snap-store-proxy", "Dummy proxy") |
583 | + |
584 | + with patch("charm.install_from_resource") as patched_install: |
585 | + self.harness.charm.handle_installation() |
586 | + |
587 | + # We cannot determine the parameters, as the resources are |
588 | + # placed in a randomly named directory |
589 | + patched_install.assert_called_once() |
590 | + assert self.harness.charm._stored.snap_install_source == "resource" |
591 | + assert self.harness.charm._stored.core_md5 == "73eeccd64008a01814825637a6593bb2" |
592 | + assert self.harness.charm._stored.snap_md5 == "97bc2632325e456ad67ca1626baa573c" |
593 | + |
594 | + def test_upgrade_from_resource(self): |
595 | + self.harness.charm._stored.snap_install_source = "resource" |
596 | + self.harness.charm._stored.core_md5 = "73eeccd64008a01814825637a6593bb2" |
597 | + self.harness.charm._stored.snap_md5 = "97bc2632325e456ad67ca1626baa573c" |
598 | + |
599 | + self.harness.add_resource("core", "My new core") |
600 | + self.harness.add_resource("snap-store-proxy", "My new proxy") |
601 | + |
602 | + with patch("charm.install_from_resource") as patched_install: |
603 | + self.harness.charm.handle_installation() |
604 | + |
605 | + # We cannot determine the parameters, as the resources are |
606 | + # placed in a randomly named directory |
607 | + patched_install.assert_called_once() |
608 | + assert self.harness.charm._stored.snap_install_source == "resource" |
609 | + assert self.harness.charm._stored.core_md5 == "59f15fd274b73cb00369c905eeab5e70" |
610 | + assert self.harness.charm._stored.snap_md5 == "d7486ac5da29427a54fe1d6e731bd334" |
611 | + |
612 | + def test_install_from_store(self): |
613 | + with patch("charm.install_from_the_charmstore") as patched_install: |
614 | + self.harness.charm.handle_installation() |
615 | + |
616 | + patched_install.assert_called_once_with("snap-store-proxy") |
617 | + assert self.harness.charm._stored.snap_install_source == "store" |
618 | + assert not hasattr(self.harness.charm._stored, "core_md5") |
619 | + assert not hasattr(self.harness.charm._stored, "snap_md5") |
620 | + |
621 | + def test_install_from_store_update(self): |
622 | + # In case of an update (repeated call) nothing should happen if installed from store |
623 | + self.harness.charm._stored.snap_install_source = "store" |
624 | + |
625 | + with patch("charm.install_from_the_charmstore") as patched_install: |
626 | + self.harness.charm.handle_installation() |
627 | + |
628 | + patched_install.assert_not_called() |
629 | + assert not hasattr(self.harness.charm._stored, "core_md5") |
630 | + assert not hasattr(self.harness.charm._stored, "snap_md5") |
631 | + |
632 | + def test_install_from_store_fails(self): |
633 | + with patch("charm.install_from_the_charmstore") as patched_install: |
634 | + patched_install.side_effect = Exception("dummy") |
635 | + self.harness.charm.handle_installation() |
636 | + |
637 | + patched_install.assert_called_once() |
638 | + assert not hasattr(self.harness.charm._stored, "core_md5") |
639 | + assert not hasattr(self.harness.charm._stored, "snap_md5") |
640 | + assert len(self.harness.charm.errors) == 1 |
641 | + assert self.harness.charm.errors[0] == "Failed to install the snap-store-proxy" |
642 | diff --git a/tests/test_helpers.py b/tests/test_helpers.py |
643 | index 3c4eaf4..730f683 100644 |
644 | --- a/tests/test_helpers.py |
645 | +++ b/tests/test_helpers.py |
646 | @@ -8,7 +8,9 @@ def test_configure_proxy(): |
647 | helpers.configure_proxy("myvalue", "myoption") |
648 | |
649 | assert mocked_run.called |
650 | - mocked_run.assert_called_once_with(["snap-proxy", "config", "myvalue=myoption"]) |
651 | + mocked_run.assert_called_once_with( |
652 | + ["snap-store-proxy", "config", "myvalue=myoption"] |
653 | + ) |
654 | |
655 | |
656 | def test_config_options(): |
657 | diff --git a/tests/test_resource_helpers.py b/tests/test_resource_helpers.py |
658 | index 1db2005..2827148 100644 |
659 | --- a/tests/test_resource_helpers.py |
660 | +++ b/tests/test_resource_helpers.py |
661 | @@ -1,6 +1,6 @@ |
662 | import logging |
663 | from subprocess import PIPE, CalledProcessError |
664 | -from unittest.mock import MagicMock, patch |
665 | +from unittest.mock import MagicMock, call, mock_open, patch |
666 | |
667 | import resource_helpers |
668 | |
669 | @@ -15,6 +15,20 @@ def test_install_from_the_charmstore(): |
670 | ) |
671 | |
672 | |
673 | +def test_install_from_resource(): |
674 | + with patch("resource_helpers.run") as patched_run: |
675 | + resource_helpers.install_from_resource("core", "proxy") |
676 | + |
677 | + assert patched_run.called |
678 | + assert patched_run.call_count == 2 |
679 | + patched_run.assert_has_calls( |
680 | + [ |
681 | + call(["snap", "install", "--dangerous", "core"], check=True), |
682 | + call(["snap", "install", "--dangerous", "proxy"], check=True), |
683 | + ] |
684 | + ) |
685 | + |
686 | + |
687 | def test_snap_details(): |
688 | with patch("resource_helpers.run") as patched_run: |
689 | resource_helpers.snap_details("mysnap") |
690 | @@ -99,3 +113,10 @@ def test_get_status(): |
691 | ) |
692 | assert exitcode == 10 |
693 | assert output == "My magic result" |
694 | + |
695 | + |
696 | +def test_hash_from_resource(): |
697 | + with patch("builtins.open", mock_open(read_data="My Dummy Resource".encode())): |
698 | + hash = resource_helpers.hash_from_resource("dummy") |
699 | + |
700 | + assert hash == "edfe387dbf4a72c067943c79dffa51b8" |
Added some comments and getting these lint errors:
Running flake8 test_charm. py:557: 1: W293 blank line contains whitespace test_charm. py:562: 88: W292 no newline at end of file
./tests/
./tests/
make[1]: *** [Makefile:46: lint] Error 1