Merge lp:~jason-hobbs/charms/trusty/cinder/log-lvm-info into lp:~openstack-charmers-archive/charms/trusty/cinder/next

Proposed by Jason Hobbs
Status: Merged
Merged at revision: 107
Proposed branch: lp:~jason-hobbs/charms/trusty/cinder/log-lvm-info
Merge into: lp:~openstack-charmers-archive/charms/trusty/cinder/next
Diff against target: 128 lines (+28/-1)
2 files modified
hooks/cinder_utils.py (+11/-0)
unit_tests/test_cinder_utils.py (+17/-1)
To merge this branch: bzr merge lp:~jason-hobbs/charms/trusty/cinder/log-lvm-info
Reviewer Review Type Date Requested Status
Corey Bryant (community) Approve
Review via email: mp+265437@code.launchpad.net

Commit message

Add logging of LVM info at critical points during setup.

Description of the change

This logging was useful for me in debugging LVM issues in our deployments. The state of PVs really determines how the charm is going to behave when setting up LVM, and without knowing what it was when the charm ran, it's hard to debug issues. It only generates a few lines of output everytime it's logged, so there's really no downside.

To post a comment you must log in.
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #6565 cinder-next for jason-hobbs mp265437
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/11916244/
Build: http://10.245.162.77:8080/job/charm_lint_check/6565/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #6197 cinder-next for jason-hobbs mp265437
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/6197/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_amulet_test #5242 cinder-next for jason-hobbs mp265437
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/5242/

Revision history for this message
Corey Bryant (corey.bryant) wrote :

Looks good, thanks for the patch!

review: Approve
Revision history for this message
Jason Hobbs (jason-hobbs) wrote :

Cool - how does this get landed now?

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hooks/cinder_utils.py'
2--- hooks/cinder_utils.py 2015-06-09 09:57:24 +0000
3+++ hooks/cinder_utils.py 2015-07-21 19:09:53 +0000
4@@ -355,6 +355,12 @@
5 subprocess.check_call(['vgextend', volume_group, block_device])
6
7
8+def log_lvm_info():
9+ """Log some useful information about how LVM is setup."""
10+ pvscan_output = subprocess.check_output(['pvscan'])
11+ juju_log('pvscan: %s' % pvscan_output)
12+
13+
14 def configure_lvm_storage(block_devices, volume_group, overwrite=False,
15 remove_missing=False):
16 ''' Configure LVM storage on the list of block devices provided
17@@ -366,6 +372,7 @@
18 :param remove_missing: bool: Remove missing physical volumes from volume
19 group if logical volume not allocated on them
20 '''
21+ log_lvm_info()
22 devices = []
23 for block_device in block_devices:
24 (block_device, size) = _parse_block_device(block_device)
25@@ -397,6 +404,8 @@
26 # Mark vg as found
27 vg_found = True
28
29+ log_lvm_info()
30+
31 if vg_found is False and len(new_devices) > 0:
32 # Create new volume group from first device
33 create_lvm_volume_group(volume_group, new_devices[0])
34@@ -411,6 +420,8 @@
35 for new_device in new_devices:
36 extend_lvm_volume_group(volume_group, new_device)
37
38+ log_lvm_info()
39+
40
41 def prepare_volume(device):
42 clean_storage(device)
43
44=== modified file 'unit_tests/test_cinder_utils.py'
45--- unit_tests/test_cinder_utils.py 2015-06-09 09:57:24 +0000
46+++ unit_tests/test_cinder_utils.py 2015-07-21 19:09:53 +0000
47@@ -1,4 +1,4 @@
48-from mock import patch, call, MagicMock
49+from mock import patch, call, MagicMock, Mock
50
51 from collections import OrderedDict
52 import os
53@@ -14,6 +14,7 @@
54 # helpers.core.hookenv
55 'config',
56 'log',
57+ 'juju_log',
58 'relation_get',
59 'relation_set',
60 'local_unit',
61@@ -245,6 +246,7 @@
62 cinder_utils.has_partition_table(block_device)
63 _check.assert_called_with(['fdisk', '-l', '/dev/vdb'], stderr=-2)
64
65+ @patch('cinder_utils.log_lvm_info', Mock())
66 @patch.object(cinder_utils, 'clean_storage')
67 @patch.object(cinder_utils, 'reduce_lvm_volume_group_missing')
68 @patch.object(cinder_utils, 'extend_lvm_volume_group')
69@@ -265,6 +267,7 @@
70 reduce_lvm.assert_called_with('test')
71 extend_lvm.assert_called_with('test', '/dev/vdc')
72
73+ @patch('cinder_utils.log_lvm_info', Mock())
74 @patch.object(cinder_utils, 'has_partition_table')
75 @patch.object(cinder_utils, 'clean_storage')
76 @patch.object(cinder_utils, 'reduce_lvm_volume_group_missing')
77@@ -287,6 +290,7 @@
78 reduce_lvm.assert_called_with('test')
79 extend_lvm.assert_called_with('test', '/dev/vdc')
80
81+ @patch('cinder_utils.log_lvm_info', Mock())
82 @patch.object(cinder_utils, 'has_partition_table')
83 @patch.object(cinder_utils, 'reduce_lvm_volume_group_missing')
84 def test_configure_lvm_storage_used_dev(self, reduce_lvm, has_part):
85@@ -296,6 +300,7 @@
86 cinder_utils.configure_lvm_storage(devices, 'test', False, True)
87 reduce_lvm.assert_called_with('test')
88
89+ @patch('cinder_utils.log_lvm_info', Mock())
90 @patch.object(cinder_utils, 'clean_storage')
91 @patch.object(cinder_utils, 'reduce_lvm_volume_group_missing')
92 @patch.object(cinder_utils, 'extend_lvm_volume_group')
93@@ -312,6 +317,7 @@
94 reduce_lvm.assert_called_with('test')
95 self.assertFalse(extend_lvm.called)
96
97+ @patch('cinder_utils.log_lvm_info', Mock())
98 @patch.object(cinder_utils, 'clean_storage')
99 @patch.object(cinder_utils, 'reduce_lvm_volume_group_missing')
100 @patch.object(cinder_utils, 'extend_lvm_volume_group')
101@@ -344,6 +350,7 @@
102 extend_lvm.assert_called_with('test', '/dev/vdc')
103 self.assertFalse(self.create_lvm_volume_group.called)
104
105+ @patch('cinder_utils.log_lvm_info', Mock())
106 @patch.object(cinder_utils, 'clean_storage')
107 @patch.object(cinder_utils, 'reduce_lvm_volume_group_missing')
108 @patch.object(cinder_utils, 'extend_lvm_volume_group')
109@@ -372,6 +379,7 @@
110 extend_lvm.assert_called_with('test', '/dev/vdc')
111 self.assertFalse(self.create_lvm_volume_group.called)
112
113+ @patch('cinder_utils.log_lvm_info', Mock())
114 @patch.object(cinder_utils, 'clean_storage')
115 @patch.object(cinder_utils, 'reduce_lvm_volume_group_missing')
116 @patch.object(cinder_utils, 'extend_lvm_volume_group')
117@@ -709,3 +717,11 @@
118 cinder_utils.check_db_initialised()
119 calls = [call(**{'cinder-db-initialised-echo': 'unit/1-1234'})]
120 self.relation_set.assert_has_calls(calls)
121+
122+ @patch('subprocess.check_output')
123+ def test_log_lvm_info(self, _check):
124+ output = "some output"
125+ _check.return_value = output
126+ cinder_utils.log_lvm_info()
127+ _check.assert_called_with(['pvscan'])
128+ self.juju_log.assert_called_with("pvscan: %s" % output)

Subscribers

People subscribed via source and target branches