Merge lp:~cjwatson/charms/xenial/storage/fix-nfs into lp:~stub/charms/xenial/storage/trunk

Proposed by Colin Watson
Status: Needs review
Proposed branch: lp:~cjwatson/charms/xenial/storage/fix-nfs
Merge into: lp:~stub/charms/xenial/storage/trunk
Diff against target: 121 lines (+11/-13)
8 files modified
hooks/common_util.py (+5/-6)
hooks/storage-provider.d/block-storage-broker/block-storage-relation-broken (+1/-1)
hooks/storage-provider.d/block-storage-broker/block-storage-relation-changed (+1/-1)
hooks/storage-provider.d/local/data-relation-changed (+1/-1)
hooks/storage-provider.d/nfs/config-changed (+0/-2)
hooks/storage-provider.d/nfs/data-relation-changed (+1/-1)
hooks/storage-provider.d/nfs/data-relation-departed (+1/-1)
metadata.yaml (+1/-0)
To merge this branch: bzr merge lp:~cjwatson/charms/xenial/storage/fix-nfs
Reviewer Review Type Date Requested Status
Stuart Bishop Approve
Review via email: mp+360965@code.launchpad.net

Commit message

Drop idmapd reconfiguration from nfs provider. Declare bionic support.

Description of the change

There hasn't been a NEED_IDMAPD entry in /etc/default/nfs-common since at least precise (it's been unconditionally started where needed), and so we don't need to change it or restart idmapd. Furthermore, restarting idmapd fails as of vivid (it's called nfs-idmapd instead), and is superseded on the client side by nfsidmap in any case. With this I was able to deploy this on bionic as a subordinate for charms requiring NFS storage.

I don't really know if this is the best place to send this MP, but it was the only multi-series version I could find. Merging all the disparate versions together, maybe moving them into a storage-charm project, and getting an updated and unified version into the store would be nice ...

To post a comment you must log in.
43. By Colin Watson

Port to Python 3.

This should be safe on >= trusty, as cloud images contain python3, and
charm-helpers will bootstrap the rest as long as the hook #! line works.

I dropped unicode() from the data persistence functions, as it's
unnecessary: on Python 2 a file opened with "w" will accept str writes, and
on Python 3 json.dumps always returns str rather than bytes.

Revision history for this message
Stuart Bishop (stub) wrote :

Erm, ok.

Any bionic deployments should be using Juju storage rather than this charm as of Juju 2.5.2, after confirming lp:1783419 is fixed.

The code should move out of the charms/xenial namespace to a top level storage-charm project. That isn't your problem though, as the charm already is multi-series and in the wrong namespace. Whoever takes up maintenance of this charm can worry about that.

review: Approve

Unmerged revisions

43. By Colin Watson

Port to Python 3.

This should be safe on >= trusty, as cloud images contain python3, and
charm-helpers will bootstrap the rest as long as the hook #! line works.

I dropped unicode() from the data persistence functions, as it's
unnecessary: on Python 2 a file opened with "w" will accept str writes, and
on Python 3 json.dumps always returns str rather than bytes.

42. By Colin Watson

Declare bionic support.

41. By Colin Watson

Drop idmapd reconfiguration from nfs provider.

There hasn't been a NEED_IDMAPD entry in /etc/default/nfs-common since
at least precise (it's been unconditionally started where needed), and
so we don't need to change it or restart idmapd. Furthermore,
restarting idmapd fails as of vivid (it's called nfs-idmapd instead),
and is superseded on the client side by nfsidmap in any case.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'hooks/common_util.py'
--- hooks/common_util.py 2014-08-27 11:01:14 +0000
+++ hooks/common_util.py 2018-12-17 14:11:37 +0000
@@ -29,7 +29,7 @@
29 data = json.load(inputfile)29 data = json.load(inputfile)
30 data[key] = value30 data[key] = value
31 with open(filepath, "w") as outfile:31 with open(filepath, "w") as outfile:
32 outfile.write(unicode(json.dumps(data, ensure_ascii=False)))32 outfile.write(json.dumps(data, ensure_ascii=False))
3333
3434
35def _get_persistent_data(key=None, remove_values=False):35def _get_persistent_data(key=None, remove_values=False):
@@ -50,8 +50,7 @@
50 data.pop(key, None)50 data.pop(key, None)
51 if data:51 if data:
52 with open(filepath, "w") as outfile:52 with open(filepath, "w") as outfile:
53 outfile.write(53 outfile.write(json.dumps(data, ensure_ascii=False))
54 unicode(json.dumps(data, ensure_ascii=False)))
55 elif os.path.exists(filepath):54 elif os.path.exists(filepath):
56 os.remove(filepath)55 os.remove(filepath)
57 else:56 else:
@@ -108,7 +107,7 @@
108 message = "WARNING: umount %s failed. Retrying." % mountpoint107 message = "WARNING: umount %s failed. Retrying." % mountpoint
109 try:108 try:
110 lsof = subprocess.check_output(["lsof", mountpoint])109 lsof = subprocess.check_output(["lsof", mountpoint])
111 except subprocess.CalledProcessError, e:110 except subprocess.CalledProcessError as e:
112 log("WARNING: %s" % str(e), hookenv.WARNING)111 log("WARNING: %s" % str(e), hookenv.WARNING)
113 else:112 else:
114 process_name = lsof.split("\n")[1].split()[0]113 process_name = lsof.split("\n")[1].split()[0]
@@ -312,7 +311,7 @@
312 metadata = subprocess.check_output(311 metadata = subprocess.check_output(
313 "curl %s/instance-id" % EC2_METADATA_URL, shell=True)312 "curl %s/instance-id" % EC2_METADATA_URL, shell=True)
314 return metadata.strip()313 return metadata.strip()
315 except subprocess.CalledProcessError, e:314 except subprocess.CalledProcessError as e:
316 log("Error: couldn't discover ec2 instance id. %s" % str(e),315 log("Error: couldn't discover ec2 instance id. %s" % str(e),
317 hookenv.ERROR)316 hookenv.ERROR)
318 sys.exit(1)317 sys.exit(1)
@@ -337,7 +336,7 @@
337 "curl %s/placement/availability-zone" % EC2_METADATA_URL,336 "curl %s/placement/availability-zone" % EC2_METADATA_URL,
338 shell=True)337 shell=True)
339 return metadata.strip()338 return metadata.strip()
340 except subprocess.CalledProcessError, e:339 except subprocess.CalledProcessError as e:
341 log("Error: couldn't discover ec2 availability zone. %s" % str(e),340 log("Error: couldn't discover ec2 availability zone. %s" % str(e),
342 hookenv.ERROR)341 hookenv.ERROR)
343 sys.exit(1)342 sys.exit(1)
344343
=== modified file 'hooks/storage-provider.d/block-storage-broker/block-storage-relation-broken'
--- hooks/storage-provider.d/block-storage-broker/block-storage-relation-broken 2014-02-11 17:34:15 +0000
+++ hooks/storage-provider.d/block-storage-broker/block-storage-relation-broken 2018-12-17 14:11:37 +0000
@@ -1,4 +1,4 @@
1#!/usr/bin/python1#!/usr/bin/python3
22
3import common_util3import common_util
44
55
=== modified file 'hooks/storage-provider.d/block-storage-broker/block-storage-relation-changed'
--- hooks/storage-provider.d/block-storage-broker/block-storage-relation-changed 2014-03-07 08:01:15 +0000
+++ hooks/storage-provider.d/block-storage-broker/block-storage-relation-changed 2018-12-17 14:11:37 +0000
@@ -1,4 +1,4 @@
1#!/usr/bin/python1#!/usr/bin/python3
22
3import common_util3import common_util
4import sys4import sys
55
=== modified file 'hooks/storage-provider.d/local/data-relation-changed'
--- hooks/storage-provider.d/local/data-relation-changed 2014-08-27 05:53:12 +0000
+++ hooks/storage-provider.d/local/data-relation-changed 2018-12-17 14:11:37 +0000
@@ -1,4 +1,4 @@
1#!/usr/bin/python1#!/usr/bin/python3
2import sys2import sys
3import os3import os
44
55
=== modified file 'hooks/storage-provider.d/nfs/config-changed'
--- hooks/storage-provider.d/nfs/config-changed 2014-01-22 19:02:54 +0000
+++ hooks/storage-provider.d/nfs/config-changed 2018-12-17 14:11:37 +0000
@@ -7,6 +7,4 @@
7 echo nfs-common already installed.7 echo nfs-common already installed.
8else8else
9 apt-get -y install -qq nfs-common9 apt-get -y install -qq nfs-common
10 sed -i -e "s/NEED_IDMAPD.*/NEED_IDMAPD=yes/" /etc/default/nfs-common
11fi10fi
12service idmapd restart || service idmapd start
1311
=== modified file 'hooks/storage-provider.d/nfs/data-relation-changed'
--- hooks/storage-provider.d/nfs/data-relation-changed 2014-03-06 16:02:32 +0000
+++ hooks/storage-provider.d/nfs/data-relation-changed 2018-12-17 14:11:37 +0000
@@ -1,4 +1,4 @@
1#!/usr/bin/python1#!/usr/bin/python3
22
3import common_util3import common_util
44
55
=== modified file 'hooks/storage-provider.d/nfs/data-relation-departed'
--- hooks/storage-provider.d/nfs/data-relation-departed 2014-02-10 18:34:02 +0000
+++ hooks/storage-provider.d/nfs/data-relation-departed 2018-12-17 14:11:37 +0000
@@ -1,4 +1,4 @@
1#!/usr/bin/python1#!/usr/bin/python3
22
3import common_util3import common_util
44
55
=== modified file 'metadata.yaml'
--- metadata.yaml 2016-07-04 11:34:30 +0000
+++ metadata.yaml 2018-12-17 14:11:37 +0000
@@ -19,5 +19,6 @@
19 block-storage:19 block-storage:
20 interface: volume-request20 interface: volume-request
21series:21series:
22 - bionic
22 - xenial23 - xenial
23 - trusty24 - trusty

Subscribers

People subscribed via source and target branches

to all changes: