Merge ~stub/layer-snap/+git/layer-snap-stub:no-snap-proxy into layer-snap:master

Proposed by Stuart Bishop on 2020-12-01
Status: Merged
Merged at revision: 2d16a6810b124648ff53b1fdd138a490fa4efdef
Proposed branch: ~stub/layer-snap/+git/layer-snap-stub:no-snap-proxy
Merge into: layer-snap:master
Diff against target: 63 lines (+27/-13)
2 files modified
config.yaml (+15/-12)
reactive/snap.py (+12/-1)
Reviewer Review Type Date Requested Status
Michał Ajduk (community) 2020-12-01 Approve on 2020-12-10
Stuart Bishop Pending
Review via email: mp+394665@code.launchpad.net

Commit message

Remove snap proxy config, preserve Juju snap proxy config

Removes the snap proxy config items, deprecated in favour of using
Juju model configuration. Charmers can manually add them back if
necessary, such as maintaining old charms in old Juju deployments.

Avoid erasing snap proxy information set by Juju, unless we are
sure the charm snap proxy config has actually been changed and
the operator is trying to clear the settings.

To post a comment you must log in.
Michał Ajduk (majduk) :
review: Approve
Simon Fels (morphis) wrote :

This makes layer-snap break on our end with

[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install Traceback (most recent call last):
[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install File "/var/lib/juju/agents/unit-lxd-0/charm/hooks/install", line 22, in <module>
[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install main()
[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install File "/var/lib/juju/agents/unit-lxd-0/.venv/lib/python3.6/site-packages/charms/reactive/__init__.py", line 73, in main
[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install hookenv._run_atstart()
[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install File "/var/lib/juju/agents/unit-lxd-0/.venv/lib/python3.6/site-packages/charmhelpers/core/hookenv.py", line 1334, in _run_atstart
[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install callback(*args, **kwargs)
[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install File "/var/lib/juju/agents/unit-lxd-0/charm/reactive/snap.py", line 192, in update_snap_proxy
[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install proxy = proxy_settings()
[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install File "/var/lib/juju/agents/unit-lxd-0/charm/reactive/snap.py", line 174, in proxy_settings
[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install snap_proxy = hookenv.config()['snap_proxy']
[2020-12-14T11:53:09.929Z] unit-lxd-0: 10:25:38 WARNING unit.lxd/0.install KeyError: 'snap_proxy'

I see why the config item is dropped but then we should also drop it from https://git.launchpad.net/layer-snap/tree/reactive/snap.py#n174 to not cause the exception above.

Stuart Bishop (stub) wrote :

(a fix for the above bug was pushed out)

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 b5e16ff..543b480 100644
3--- a/config.yaml
4+++ b/config.yaml
5@@ -14,18 +14,21 @@
6 # See the License for the specific language governing permissions and
7 # limitations under the License.
8 options:
9- snap_proxy:
10- description: >
11- DEPRECATED. Use snap-http-proxy and snap-https-proxy model configuration settings.
12- HTTP/HTTPS web proxy for Snappy to use when accessing the snap store.
13- type: string
14- default: ""
15- snap_proxy_url:
16- default: ""
17- type: string
18- description: >
19- DEPRECATED. Use snap-store-proxy model configuration setting.
20- The address of a Snap Store Proxy to use for snaps e.g. http://snap-proxy.example.com
21+ # snap_proxy and snap_proxy_url have been deprecated for some time.
22+ # If your charm still needs them, add these config items manually
23+ # to your charm's config.yaml.
24+ # snap_proxy:
25+ # description: >
26+ # DEPRECATED. Use snap-http-proxy and snap-https-proxy model configuration settings.
27+ # HTTP/HTTPS web proxy for Snappy to use when accessing the snap store.
28+ # type: string
29+ # default: ""
30+ # snap_proxy_url:
31+ # default: ""
32+ # type: string
33+ # description: >
34+ # DEPRECATED. Use snap-store-proxy model configuration setting.
35+ # The address of a Snap Store Proxy to use for snaps e.g. http://snap-proxy.example.com
36 snapd_refresh:
37 default: ""
38 type: string
39diff --git a/reactive/snap.py b/reactive/snap.py
40index 669a56d..11d8329 100644
41--- a/reactive/snap.py
42+++ b/reactive/snap.py
43@@ -291,8 +291,19 @@ def configure_snap_store_proxy():
44
45 if not reactive.is_flag_set('config.changed.snap_proxy_url'):
46 return
47+ config = hookenv.config()
48+ if 'snap_proxy_url' not in config:
49+ # The deprecated snap_proxy_url config items have been removed
50+ # from config.yaml. If the charm author hasn't added them back
51+ # explicitly, there is nothing to do. Juju is maintaining these
52+ # settings as model configuration.
53+ return
54+ snap_store_proxy_url = config['snap_proxy_url']
55+ if not snap_store_proxy_url and not config.previous('snap_proxy_url'):
56+ # Proxy url is not set, and was not set previous hook. Do nothing,
57+ # to avoid overwriting the Juju maintained setting.
58+ return
59 ensure_snapd_min_version('2.30')
60- snap_store_proxy_url = hookenv.config()['snap_proxy_url']
61 if snap_store_proxy_url:
62 bundle, store_id = download_assertion_bundle(snap_store_proxy_url)
63 try:

Subscribers

People subscribed via source and target branches

to all changes: