Merge ~niedbalski/ubuntu/+source/cinder:stable/newton into ~ubuntu-server-dev/ubuntu/+source/cinder:stable/newton

Proposed by Jorge Niedbalski
Status: Merged
Merged at revision: 42e7a27b2c1068d6b8f0c103f7c0fb29a8bdff7f
Proposed branch: ~niedbalski/ubuntu/+source/cinder:stable/newton
Merge into: ~ubuntu-server-dev/ubuntu/+source/cinder:stable/newton
Diff against target: 335 lines (+314/-0)
3 files modified
debian/changelog (+7/-0)
debian/patches/log-message-cleanup-for-volume-usage-audit.patch (+306/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Ubuntu Server Developers Pending
Review via email: mp+319623@code.launchpad.net

Description of the change

* d/p/log-message-cleanup-for-volume-usage-audit.patch:
    Fixes LP: #1631561.

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 570a073..cdcd84a 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
1cinder (2:9.1.2-0ubuntu2) yakkety; urgency=medium
2
3 * d/p/log-message-cleanup-for-volume-usage-audit.patch:
4 Fixes LP: #1631561.
5
6 -- Jorge Niedbalski <jorge.niedbalski@canonical.com> Fri, 10 Mar 2017 18:20:41 -0300
7
1cinder (2:9.1.2-0ubuntu1) yakkety; urgency=medium8cinder (2:9.1.2-0ubuntu1) yakkety; urgency=medium
29
3 [ Corey Bryant ]10 [ Corey Bryant ]
diff --git a/debian/patches/log-message-cleanup-for-volume-usage-audit.patch b/debian/patches/log-message-cleanup-for-volume-usage-audit.patch
4new file mode 10064411new file mode 100644
index 0000000..9421d04
--- /dev/null
+++ b/debian/patches/log-message-cleanup-for-volume-usage-audit.patch
@@ -0,0 +1,306 @@
1From 82792be330e3b88aa1f625537ae1d2649e14ba4f Mon Sep 17 00:00:00 2001
2From: Zhiteng Huang <zhithuang@ebaysf.com>
3Date: Sat, 8 Oct 2016 13:03:37 -0700
4Subject: [PATCH] Log message cleanup for volume-usage-audit
5
6volume-usage-audit utlis logs unecessary message at for deleted
7volumes or exception:
8
9 try:
10 LOG.debug('Send notification for vol X')
11 _notify_func
12 except Exception:
13 LOG.exception(...)
14
15In the case when _notify_func raises exception, it logs
16'Send notification' and the exception.
17
18This change reduce logs by only logging 'Sent notification' when
19there is no exception by:
20
21 try:
22 _notify_func
23 LOG.debug('Sent notification for vol X')
24 except Exception:
25 LOG.exception(...)
26
27Also, this change fixes notify_snapshot_usage() for not handling
28the case when source volume for the snapshot is deleted.
29
30Closes-bug: #1631561
31
32Change-Id: Iff5448e888ee39bce1ff99c7bf48581d655fb687
33---
34 cinder/cmd/volume_usage_audit.py | 80 +++++++++++++++++-----------------
35 cinder/tests/unit/test_volume_utils.py | 46 +++++++++++++++++++
36 cinder/volume/utils.py | 14 +++++-
37 3 files changed, 99 insertions(+), 41 deletions(-)
38
39diff --git a/cinder/cmd/volume_usage_audit.py b/cinder/cmd/volume_usage_audit.py
40index ba8d38f..678d4e0 100644
41--- a/cinder/cmd/volume_usage_audit.py
42+++ b/cinder/cmd/volume_usage_audit.py
43@@ -48,7 +48,7 @@ from cinder import i18n
44 i18n.enable_lazy()
45 from cinder import context
46 from cinder import db
47-from cinder.i18n import _, _LE
48+from cinder.i18n import _, _LE, _LI
49 from cinder import objects
50 from cinder import rpc
51 from cinder import utils
52@@ -95,9 +95,9 @@ def main():
53 'end': end}
54 LOG.error(msg)
55 sys.exit(-1)
56- LOG.debug("Starting volume usage audit")
57- msg = _("Creating usages for %(begin_period)s until %(end_period)s")
58- LOG.debug(msg, {"begin_period": str(begin), "end_period": str(end)})
59+ LOG.info(_LI("Starting volume usage audit"))
60+ msg = _LI("Creating usages for %(begin_period)s until %(end_period)s")
61+ LOG.info(msg, {"begin_period": str(begin), "end_period": str(end)})
62
63 extra_info = {
64 'audit_period_beginning': str(begin),
65@@ -107,19 +107,19 @@ def main():
66 volumes = db.volume_get_active_by_window(admin_context,
67 begin,
68 end)
69- LOG.debug("Found %d volumes", len(volumes))
70+ LOG.info(_LI("Found %d volumes"), len(volumes))
71 for volume_ref in volumes:
72 try:
73- LOG.debug("Send exists notification for <volume_id: "
74+ cinder.volume.utils.notify_about_volume_usage(
75+ admin_context,
76+ volume_ref,
77+ 'exists', extra_usage_info=extra_info)
78+ LOG.debug("Sent exists notification for <volume_id: "
79 "%(volume_id)s> <project_id %(project_id)s> "
80 "<%(extra_info)s>",
81 {'volume_id': volume_ref.id,
82 'project_id': volume_ref.project_id,
83 'extra_info': extra_info})
84- cinder.volume.utils.notify_about_volume_usage(
85- admin_context,
86- volume_ref,
87- 'exists', extra_usage_info=extra_info)
88 except Exception as exc_msg:
89 LOG.exception(_LE("Exists volume notification failed: %s"),
90 exc_msg, resource=volume_ref)
91@@ -132,12 +132,6 @@ def main():
92 'audit_period_beginning': str(volume_ref.created_at),
93 'audit_period_ending': str(volume_ref.created_at),
94 }
95- LOG.debug("Send create notification for "
96- "<volume_id: %(volume_id)s> "
97- "<project_id %(project_id)s> <%(extra_info)s>",
98- {'volume_id': volume_ref.id,
99- 'project_id': volume_ref.project_id,
100- 'extra_info': local_extra_info})
101 cinder.volume.utils.notify_about_volume_usage(
102 admin_context,
103 volume_ref,
104@@ -146,6 +140,12 @@ def main():
105 admin_context,
106 volume_ref,
107 'create.end', extra_usage_info=local_extra_info)
108+ LOG.debug("Sent create notification for "
109+ "<volume_id: %(volume_id)s> "
110+ "<project_id %(project_id)s> <%(extra_info)s>",
111+ {'volume_id': volume_ref.id,
112+ 'project_id': volume_ref.project_id,
113+ 'extra_info': local_extra_info})
114 except Exception as exc_msg:
115 LOG.exception(_LE("Create volume notification failed: %s"),
116 exc_msg, resource=volume_ref)
117@@ -158,12 +158,6 @@ def main():
118 'audit_period_beginning': str(volume_ref.deleted_at),
119 'audit_period_ending': str(volume_ref.deleted_at),
120 }
121- LOG.debug("Send delete notification for "
122- "<volume_id: %(volume_id)s> "
123- "<project_id %(project_id)s> <%(extra_info)s>",
124- {'volume_id': volume_ref.id,
125- 'project_id': volume_ref.project_id,
126- 'extra_info': local_extra_info})
127 cinder.volume.utils.notify_about_volume_usage(
128 admin_context,
129 volume_ref,
130@@ -172,24 +166,30 @@ def main():
131 admin_context,
132 volume_ref,
133 'delete.end', extra_usage_info=local_extra_info)
134+ LOG.debug("Sent delete notification for "
135+ "<volume_id: %(volume_id)s> "
136+ "<project_id %(project_id)s> <%(extra_info)s>",
137+ {'volume_id': volume_ref.id,
138+ 'project_id': volume_ref.project_id,
139+ 'extra_info': local_extra_info})
140 except Exception as exc_msg:
141 LOG.exception(_LE("Delete volume notification failed: %s"),
142 exc_msg, resource=volume_ref)
143
144 snapshots = objects.SnapshotList.get_active_by_window(admin_context,
145 begin, end)
146- LOG.debug("Found %d snapshots", len(snapshots))
147+ LOG.info(_LI("Found %d snapshots"), len(snapshots))
148 for snapshot_ref in snapshots:
149 try:
150- LOG.debug("Send notification for <snapshot_id: %(snapshot_id)s> "
151- "<project_id %(project_id)s> <%(extra_info)s>",
152- {'snapshot_id': snapshot_ref.id,
153- 'project_id': snapshot_ref.project_id,
154- 'extra_info': extra_info})
155 cinder.volume.utils.notify_about_snapshot_usage(admin_context,
156 snapshot_ref,
157 'exists',
158 extra_info)
159+ LOG.debug("Sent notification for <snapshot_id: %(snapshot_id)s> "
160+ "<project_id %(project_id)s> <%(extra_info)s>",
161+ {'snapshot_id': snapshot_ref.id,
162+ 'project_id': snapshot_ref.project_id,
163+ 'extra_info': extra_info})
164 except Exception as exc_msg:
165 LOG.exception(_LE("Exists snapshot notification failed: %s"),
166 exc_msg, resource=snapshot_ref)
167@@ -202,12 +202,6 @@ def main():
168 'audit_period_beginning': str(snapshot_ref.created_at),
169 'audit_period_ending': str(snapshot_ref.created_at),
170 }
171- LOG.debug("Send create notification for "
172- "<snapshot_id: %(snapshot_id)s> "
173- "<project_id %(project_id)s> <%(extra_info)s>",
174- {'snapshot_id': snapshot_ref.id,
175- 'project_id': snapshot_ref.project_id,
176- 'extra_info': local_extra_info})
177 cinder.volume.utils.notify_about_snapshot_usage(
178 admin_context,
179 snapshot_ref,
180@@ -216,6 +210,12 @@ def main():
181 admin_context,
182 snapshot_ref,
183 'create.end', extra_usage_info=local_extra_info)
184+ LOG.debug("Sent create notification for "
185+ "<snapshot_id: %(snapshot_id)s> "
186+ "<project_id %(project_id)s> <%(extra_info)s>",
187+ {'snapshot_id': snapshot_ref.id,
188+ 'project_id': snapshot_ref.project_id,
189+ 'extra_info': local_extra_info})
190 except Exception as exc_msg:
191 LOG.exception(_LE("Create snapshot notification failed: %s"),
192 exc_msg, resource=snapshot_ref)
193@@ -228,12 +228,6 @@ def main():
194 'audit_period_beginning': str(snapshot_ref.deleted_at),
195 'audit_period_ending': str(snapshot_ref.deleted_at),
196 }
197- LOG.debug("Send delete notification for "
198- "<snapshot_id: %(snapshot_id)s> "
199- "<project_id %(project_id)s> <%(extra_info)s>",
200- {'snapshot_id': snapshot_ref.id,
201- 'project_id': snapshot_ref.project_id,
202- 'extra_info': local_extra_info})
203 cinder.volume.utils.notify_about_snapshot_usage(
204 admin_context,
205 snapshot_ref,
206@@ -242,6 +236,12 @@ def main():
207 admin_context,
208 snapshot_ref,
209 'delete.end', extra_usage_info=local_extra_info)
210+ LOG.debug("Sent delete notification for "
211+ "<snapshot_id: %(snapshot_id)s> "
212+ "<project_id %(project_id)s> <%(extra_info)s>",
213+ {'snapshot_id': snapshot_ref.id,
214+ 'project_id': snapshot_ref.project_id,
215+ 'extra_info': local_extra_info})
216 except Exception as exc_msg:
217 LOG.exception(_LE("Delete snapshot notification failed: %s"),
218 exc_msg, resource=snapshot_ref)
219diff --git a/cinder/tests/unit/test_volume_utils.py b/cinder/tests/unit/test_volume_utils.py
220index c49b113..bca8c71 100644
221--- a/cinder/tests/unit/test_volume_utils.py
222+++ b/cinder/tests/unit/test_volume_utils.py
223@@ -166,6 +166,52 @@ class NotifyUsageTestCase(test.TestCase):
224 }
225 self.assertDictMatch(expected_snapshot, usage_info)
226
227+ @mock.patch('cinder.objects.Volume.get_by_id')
228+ def test_usage_from_deleted_snapshot(self, volume_get_by_id):
229+ raw_volume = {
230+ 'id': fake.VOLUME_ID,
231+ 'availability_zone': 'nova',
232+ 'deleted': 1
233+ }
234+ ctxt = context.get_admin_context()
235+ volume_obj = fake_volume.fake_volume_obj(ctxt, **raw_volume)
236+ volume_get_by_id.side_effect = exception.VolumeNotFound(
237+ volume_id=fake.VOLUME_ID)
238+
239+ raw_snapshot = {
240+ 'project_id': fake.PROJECT_ID,
241+ 'user_id': fake.USER_ID,
242+ 'volume': volume_obj,
243+ 'volume_id': fake.VOLUME_ID,
244+ 'volume_size': 1,
245+ 'id': fake.SNAPSHOT_ID,
246+ 'display_name': '11',
247+ 'created_at': '2014-12-11T10:10:00',
248+ 'status': fields.SnapshotStatus.ERROR,
249+ 'deleted': '',
250+ 'snapshot_metadata': [{'key': 'fake_snap_meta_key',
251+ 'value': 'fake_snap_meta_value'}],
252+ 'expected_attrs': ['metadata'],
253+ }
254+
255+ snapshot_obj = fake_snapshot.fake_snapshot_obj(ctxt, **raw_snapshot)
256+ usage_info = volume_utils._usage_from_snapshot(snapshot_obj)
257+ expected_snapshot = {
258+ 'tenant_id': fake.PROJECT_ID,
259+ 'user_id': fake.USER_ID,
260+ 'availability_zone': '',
261+ 'volume_id': fake.VOLUME_ID,
262+ 'volume_size': 1,
263+ 'snapshot_id': fake.SNAPSHOT_ID,
264+ 'display_name': '11',
265+ 'created_at': 'DONTCARE',
266+ 'status': fields.SnapshotStatus.ERROR,
267+ 'deleted': '',
268+ 'metadata': six.text_type({'fake_snap_meta_key':
269+ u'fake_snap_meta_value'}),
270+ }
271+ self.assertDictMatch(expected_snapshot, usage_info)
272+
273 @mock.patch('cinder.db.volume_glance_metadata_get')
274 @mock.patch('cinder.db.volume_attachment_get_all_by_volume_id')
275 def test_usage_from_volume(self, mock_attachment, mock_image_metadata):
276diff --git a/cinder/volume/utils.py b/cinder/volume/utils.py
277index 0aec08a..c811e2d 100644
278--- a/cinder/volume/utils.py
279+++ b/cinder/volume/utils.py
280@@ -149,10 +149,22 @@ def notify_about_backup_usage(context, backup, event_suffix,
281
282
283 def _usage_from_snapshot(snapshot, **extra_usage_info):
284+ try:
285+ az = snapshot.volume['availability_zone']
286+ except exception.VolumeNotFound:
287+ # (zhiteng) Snapshot's source volume could have been deleted
288+ # (which means snapshot has been deleted as well),
289+ # lazy-loading volume would raise VolumeNotFound exception.
290+ # In that case, not going any further by abusing low level
291+ # DB API to fetch deleted volume but simply return empty
292+ # string for snapshot's AZ info.
293+ az = ''
294+ LOG.debug("Source volume %s deleted", snapshot.volume_id)
295+
296 usage_info = {
297 'tenant_id': snapshot.project_id,
298 'user_id': snapshot.user_id,
299- 'availability_zone': snapshot.volume['availability_zone'],
300+ 'availability_zone': az,
301 'volume_id': snapshot.volume_id,
302 'volume_size': snapshot.volume_size,
303 'snapshot_id': snapshot.id,
304--
3052.7.4
306
diff --git a/debian/patches/series b/debian/patches/series
index 7abc008..238c9c6 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,2 +1,3 @@
1google-back-requirements.patch1google-back-requirements.patch
2fix-long-casting.patch2fix-long-casting.patch
3log-message-cleanup-for-volume-usage-audit.patch

Subscribers

People subscribed via source and target branches