Merge lp:~james-page/charms/trusty/neutron-openvswitch/optmize-headers into lp:~openstack-charmers-archive/charms/trusty/neutron-openvswitch/next

Proposed by James Page
Status: Merged
Merged at revision: 108
Proposed branch: lp:~james-page/charms/trusty/neutron-openvswitch/optmize-headers
Merge into: lp:~openstack-charmers-archive/charms/trusty/neutron-openvswitch/next
Diff against target: 433 lines (+114/-65)
6 files modified
hooks/charmhelpers/contrib/openstack/amulet/deployment.py (+2/-1)
hooks/charmhelpers/contrib/openstack/neutron.py (+6/-6)
hooks/charmhelpers/contrib/openstack/utils.py (+63/-30)
hooks/charmhelpers/core/host.py (+39/-25)
hooks/charmhelpers/fetch/giturl.py (+2/-2)
tests/charmhelpers/contrib/openstack/amulet/deployment.py (+2/-1)
To merge this branch: bzr merge lp:~james-page/charms/trusty/neutron-openvswitch/optmize-headers
Reviewer Review Type Date Requested Status
Liam Young Approve
Review via email: mp+282931@code.launchpad.net
To post a comment you must log in.
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #17573 neutron-openvswitch-next for james-page mp282931
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/17573/

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

charm_unit_test #16418 neutron-openvswitch-next for james-page mp282931
    UNIT OK: passed

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

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

charm_amulet_test #8880 neutron-openvswitch-next for james-page mp282931
    AMULET OK: passed

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

Revision history for this message
Liam Young (gnuoy) wrote :

Approved

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hooks/charmhelpers/contrib/openstack/amulet/deployment.py'
2--- hooks/charmhelpers/contrib/openstack/amulet/deployment.py 2016-01-08 13:11:21 +0000
3+++ hooks/charmhelpers/contrib/openstack/amulet/deployment.py 2016-01-18 12:00:43 +0000
4@@ -125,7 +125,8 @@
5
6 # Charms which can not use openstack-origin, ie. many subordinates
7 no_origin = ['cinder-ceph', 'hacluster', 'neutron-openvswitch', 'nrpe',
8- 'openvswitch-odl', 'neutron-api-odl', 'odl-controller']
9+ 'openvswitch-odl', 'neutron-api-odl', 'odl-controller',
10+ 'cinder-backup']
11
12 if self.openstack:
13 for svc in services:
14
15=== modified file 'hooks/charmhelpers/contrib/openstack/neutron.py'
16--- hooks/charmhelpers/contrib/openstack/neutron.py 2016-01-08 13:11:21 +0000
17+++ hooks/charmhelpers/contrib/openstack/neutron.py 2016-01-18 12:00:43 +0000
18@@ -50,7 +50,7 @@
19 if kernel_version() >= (3, 13):
20 return []
21 else:
22- return ['openvswitch-datapath-dkms']
23+ return [headers_package(), 'openvswitch-datapath-dkms']
24
25
26 # legacy
27@@ -70,7 +70,7 @@
28 relation_prefix='neutron',
29 ssl_dir=QUANTUM_CONF_DIR)],
30 'services': ['quantum-plugin-openvswitch-agent'],
31- 'packages': [[headers_package()] + determine_dkms_package(),
32+ 'packages': [determine_dkms_package(),
33 ['quantum-plugin-openvswitch-agent']],
34 'server_packages': ['quantum-server',
35 'quantum-plugin-openvswitch'],
36@@ -111,7 +111,7 @@
37 relation_prefix='neutron',
38 ssl_dir=NEUTRON_CONF_DIR)],
39 'services': ['neutron-plugin-openvswitch-agent'],
40- 'packages': [[headers_package()] + determine_dkms_package(),
41+ 'packages': [determine_dkms_package(),
42 ['neutron-plugin-openvswitch-agent']],
43 'server_packages': ['neutron-server',
44 'neutron-plugin-openvswitch'],
45@@ -155,7 +155,7 @@
46 relation_prefix='neutron',
47 ssl_dir=NEUTRON_CONF_DIR)],
48 'services': [],
49- 'packages': [[headers_package()] + determine_dkms_package(),
50+ 'packages': [determine_dkms_package(),
51 ['neutron-plugin-cisco']],
52 'server_packages': ['neutron-server',
53 'neutron-plugin-cisco'],
54@@ -174,7 +174,7 @@
55 'neutron-dhcp-agent',
56 'nova-api-metadata',
57 'etcd'],
58- 'packages': [[headers_package()] + determine_dkms_package(),
59+ 'packages': [determine_dkms_package(),
60 ['calico-compute',
61 'bird',
62 'neutron-dhcp-agent',
63@@ -219,7 +219,7 @@
64 relation_prefix='neutron',
65 ssl_dir=NEUTRON_CONF_DIR)],
66 'services': [],
67- 'packages': [[headers_package()] + determine_dkms_package()],
68+ 'packages': [determine_dkms_package()],
69 'server_packages': ['neutron-server',
70 'python-neutron-plugin-midonet'],
71 'server_services': ['neutron-server']
72
73=== modified file 'hooks/charmhelpers/contrib/openstack/utils.py'
74--- hooks/charmhelpers/contrib/openstack/utils.py 2016-01-08 21:04:45 +0000
75+++ hooks/charmhelpers/contrib/openstack/utils.py 2016-01-18 12:00:43 +0000
76@@ -103,29 +103,28 @@
77 ('2016.1', 'mitaka'),
78 ])
79
80-# The ugly duckling
81+# The ugly duckling - must list releases oldest to newest
82 SWIFT_CODENAMES = OrderedDict([
83- ('1.4.3', 'diablo'),
84- ('1.4.8', 'essex'),
85- ('1.7.4', 'folsom'),
86- ('1.8.0', 'grizzly'),
87- ('1.7.7', 'grizzly'),
88- ('1.7.6', 'grizzly'),
89- ('1.10.0', 'havana'),
90- ('1.9.1', 'havana'),
91- ('1.9.0', 'havana'),
92- ('1.13.1', 'icehouse'),
93- ('1.13.0', 'icehouse'),
94- ('1.12.0', 'icehouse'),
95- ('1.11.0', 'icehouse'),
96- ('2.0.0', 'juno'),
97- ('2.1.0', 'juno'),
98- ('2.2.0', 'juno'),
99- ('2.2.1', 'kilo'),
100- ('2.2.2', 'kilo'),
101- ('2.3.0', 'liberty'),
102- ('2.4.0', 'liberty'),
103- ('2.5.0', 'liberty'),
104+ ('diablo',
105+ ['1.4.3']),
106+ ('essex',
107+ ['1.4.8']),
108+ ('folsom',
109+ ['1.7.4']),
110+ ('grizzly',
111+ ['1.7.6', '1.7.7', '1.8.0']),
112+ ('havana',
113+ ['1.9.0', '1.9.1', '1.10.0']),
114+ ('icehouse',
115+ ['1.11.0', '1.12.0', '1.13.0', '1.13.1']),
116+ ('juno',
117+ ['2.0.0', '2.1.0', '2.2.0']),
118+ ('kilo',
119+ ['2.2.1', '2.2.2']),
120+ ('liberty',
121+ ['2.3.0', '2.4.0', '2.5.0']),
122+ ('mitaka',
123+ ['2.5.0']),
124 ])
125
126 # >= Liberty version->codename mapping
127@@ -227,6 +226,33 @@
128 error_out(e)
129
130
131+def get_os_version_codename_swift(codename):
132+ '''Determine OpenStack version number of swift from codename.'''
133+ for k, v in six.iteritems(SWIFT_CODENAMES):
134+ if k == codename:
135+ return v[-1]
136+ e = 'Could not derive swift version for '\
137+ 'codename: %s' % codename
138+ error_out(e)
139+
140+
141+def get_swift_codename(version):
142+ '''Determine OpenStack codename that corresponds to swift version.'''
143+ codenames = [k for k, v in six.iteritems(SWIFT_CODENAMES) if version in v]
144+ if len(codenames) > 1:
145+ # If more than one release codename contains this version we determine
146+ # the actual codename based on the highest available install source.
147+ for codename in reversed(codenames):
148+ releases = UBUNTU_OPENSTACK_RELEASE
149+ release = [k for k, v in six.iteritems(releases) if codename in v]
150+ ret = subprocess.check_output(['apt-cache', 'policy', 'swift'])
151+ if codename in ret or release[0] in ret:
152+ return codename
153+ elif len(codenames) == 1:
154+ return codenames[0]
155+ return None
156+
157+
158 def get_os_codename_package(package, fatal=True):
159 '''Derive OpenStack release codename from an installed package.'''
160 import apt_pkg as apt
161@@ -270,7 +296,7 @@
162 # < Liberty co-ordinated project versions
163 try:
164 if 'swift' in pkg.name:
165- return SWIFT_CODENAMES[vers]
166+ return get_swift_codename(vers)
167 else:
168 return OPENSTACK_CODENAMES[vers]
169 except KeyError:
170@@ -289,12 +315,14 @@
171
172 if 'swift' in pkg:
173 vers_map = SWIFT_CODENAMES
174+ for cname, version in six.iteritems(vers_map):
175+ if cname == codename:
176+ return version[-1]
177 else:
178 vers_map = OPENSTACK_CODENAMES
179-
180- for version, cname in six.iteritems(vers_map):
181- if cname == codename:
182- return version
183+ for version, cname in six.iteritems(vers_map):
184+ if cname == codename:
185+ return version
186 # e = "Could not determine OpenStack version for package: %s" % pkg
187 # error_out(e)
188
189@@ -460,11 +488,16 @@
190 cur_vers = get_os_version_package(package)
191 if "swift" in package:
192 codename = get_os_codename_install_source(src)
193- available_vers = get_os_version_codename(codename, SWIFT_CODENAMES)
194+ avail_vers = get_os_version_codename_swift(codename)
195 else:
196- available_vers = get_os_version_install_source(src)
197+ avail_vers = get_os_version_install_source(src)
198 apt.init()
199- return apt.version_compare(available_vers, cur_vers) == 1
200+ if "swift" in package:
201+ major_cur_vers = cur_vers.split('.', 1)[0]
202+ major_avail_vers = avail_vers.split('.', 1)[0]
203+ major_diff = apt.version_compare(major_avail_vers, major_cur_vers)
204+ return avail_vers > cur_vers and (major_diff == 1 or major_diff == 0)
205+ return apt.version_compare(avail_vers, cur_vers) == 1
206
207
208 def ensure_block_device(block_device):
209
210=== modified file 'hooks/charmhelpers/core/host.py'
211--- hooks/charmhelpers/core/host.py 2016-01-08 21:04:45 +0000
212+++ hooks/charmhelpers/core/host.py 2016-01-18 12:00:43 +0000
213@@ -160,13 +160,13 @@
214
215
216 def init_is_systemd():
217+ """Return True if the host system uses systemd, False otherwise."""
218 return os.path.isdir(SYSTEMD_SYSTEM)
219
220
221 def adduser(username, password=None, shell='/bin/bash', system_user=False,
222 primary_group=None, secondary_groups=None):
223- """
224- Add a user to the system.
225+ """Add a user to the system.
226
227 Will log but otherwise succeed if the user already exists.
228
229@@ -174,7 +174,7 @@
230 :param str password: Password for user; if ``None``, create a system user
231 :param str shell: The default shell for the user
232 :param bool system_user: Whether to create a login or system user
233- :param str primary_group: Primary group for user; defaults to their username
234+ :param str primary_group: Primary group for user; defaults to username
235 :param list secondary_groups: Optional list of additional groups
236
237 :returns: The password database entry struct, as returned by `pwd.getpwnam`
238@@ -300,14 +300,12 @@
239
240
241 def fstab_remove(mp):
242- """Remove the given mountpoint entry from /etc/fstab
243- """
244+ """Remove the given mountpoint entry from /etc/fstab"""
245 return Fstab.remove_by_mountpoint(mp)
246
247
248 def fstab_add(dev, mp, fs, options=None):
249- """Adds the given device entry to the /etc/fstab file
250- """
251+ """Adds the given device entry to the /etc/fstab file"""
252 return Fstab.add(dev, mp, fs, options=options)
253
254
255@@ -363,8 +361,7 @@
256
257
258 def file_hash(path, hash_type='md5'):
259- """
260- Generate a hash checksum of the contents of 'path' or None if not found.
261+ """Generate a hash checksum of the contents of 'path' or None if not found.
262
263 :param str hash_type: Any hash alrgorithm supported by :mod:`hashlib`,
264 such as md5, sha1, sha256, sha512, etc.
265@@ -379,10 +376,9 @@
266
267
268 def path_hash(path):
269- """
270- Generate a hash checksum of all files matching 'path'. Standard wildcards
271- like '*' and '?' are supported, see documentation for the 'glob' module for
272- more information.
273+ """Generate a hash checksum of all files matching 'path'. Standard
274+ wildcards like '*' and '?' are supported, see documentation for the 'glob'
275+ module for more information.
276
277 :return: dict: A { filename: hash } dictionary for all matched files.
278 Empty if none found.
279@@ -394,8 +390,7 @@
280
281
282 def check_hash(path, checksum, hash_type='md5'):
283- """
284- Validate a file using a cryptographic checksum.
285+ """Validate a file using a cryptographic checksum.
286
287 :param str checksum: Value of the checksum used to validate the file.
288 :param str hash_type: Hash algorithm used to generate `checksum`.
289@@ -410,6 +405,7 @@
290
291
292 class ChecksumError(ValueError):
293+ """A class derived from Value error to indicate the checksum failed."""
294 pass
295
296
297@@ -515,7 +511,7 @@
298
299
300 def list_nics(nic_type=None):
301- '''Return a list of nics of given type(s)'''
302+ """Return a list of nics of given type(s)"""
303 if isinstance(nic_type, six.string_types):
304 int_types = [nic_type]
305 else:
306@@ -557,12 +553,13 @@
307
308
309 def set_nic_mtu(nic, mtu):
310- '''Set MTU on a network interface'''
311+ """Set the Maximum Transmission Unit (MTU) on a network interface."""
312 cmd = ['ip', 'link', 'set', nic, 'mtu', mtu]
313 subprocess.check_call(cmd)
314
315
316 def get_nic_mtu(nic):
317+ """Return the Maximum Transmission Unit (MTU) for a network interface."""
318 cmd = ['ip', 'addr', 'show', nic]
319 ip_output = subprocess.check_output(cmd).decode('UTF-8').split('\n')
320 mtu = ""
321@@ -574,6 +571,7 @@
322
323
324 def get_nic_hwaddr(nic):
325+ """Return the Media Access Control (MAC) for a network interface."""
326 cmd = ['ip', '-o', '-0', 'addr', 'show', nic]
327 ip_output = subprocess.check_output(cmd).decode('UTF-8')
328 hwaddr = ""
329@@ -584,7 +582,7 @@
330
331
332 def cmp_pkgrevno(package, revno, pkgcache=None):
333- '''Compare supplied revno with the revno of the installed package
334+ """Compare supplied revno with the revno of the installed package
335
336 * 1 => Installed revno is greater than supplied arg
337 * 0 => Installed revno is the same as supplied arg
338@@ -593,7 +591,7 @@
339 This function imports apt_cache function from charmhelpers.fetch if
340 the pkgcache argument is None. Be sure to add charmhelpers.fetch if
341 you call this function, or pass an apt_pkg.Cache() instance.
342- '''
343+ """
344 import apt_pkg
345 if not pkgcache:
346 from charmhelpers.fetch import apt_cache
347@@ -603,19 +601,27 @@
348
349
350 @contextmanager
351-def chdir(d):
352+def chdir(directory):
353+ """Change the current working directory to a different directory for a code
354+ block and return the previous directory after the block exits. Useful to
355+ run commands from a specificed directory.
356+
357+ :param str directory: The directory path to change to for this context.
358+ """
359 cur = os.getcwd()
360 try:
361- yield os.chdir(d)
362+ yield os.chdir(directory)
363 finally:
364 os.chdir(cur)
365
366
367 def chownr(path, owner, group, follow_links=True, chowntopdir=False):
368- """
369- Recursively change user and group ownership of files and directories
370+ """Recursively change user and group ownership of files and directories
371 in given path. Doesn't chown path itself by default, only its children.
372
373+ :param str path: The string path to start changing ownership.
374+ :param str owner: The owner string to use when looking up the uid.
375+ :param str group: The group string to use when looking up the gid.
376 :param bool follow_links: Also Chown links if True
377 :param bool chowntopdir: Also chown path itself if True
378 """
379@@ -639,15 +645,23 @@
380
381
382 def lchownr(path, owner, group):
383+ """Recursively change user and group ownership of files and directories
384+ in a given path, not following symbolic links. See the documentation for
385+ 'os.lchown' for more information.
386+
387+ :param str path: The string path to start changing ownership.
388+ :param str owner: The owner string to use when looking up the uid.
389+ :param str group: The group string to use when looking up the gid.
390+ """
391 chownr(path, owner, group, follow_links=False)
392
393
394 def get_total_ram():
395- '''The total amount of system RAM in bytes.
396+ """The total amount of system RAM in bytes.
397
398 This is what is reported by the OS, and may be overcommitted when
399 there are multiple containers hosted on the same machine.
400- '''
401+ """
402 with open('/proc/meminfo', 'r') as f:
403 for line in f.readlines():
404 if line:
405
406=== modified file 'hooks/charmhelpers/fetch/giturl.py'
407--- hooks/charmhelpers/fetch/giturl.py 2016-01-08 21:04:45 +0000
408+++ hooks/charmhelpers/fetch/giturl.py 2016-01-18 12:00:43 +0000
409@@ -49,8 +49,8 @@
410 cmd = ['git', '-C', dest, 'pull', source, branch]
411 else:
412 cmd = ['git', 'clone', source, dest, '--branch', branch]
413- if depth:
414- cmd.extend(['--depth', depth])
415+ if depth:
416+ cmd.extend(['--depth', depth])
417 check_call(cmd)
418
419 def install(self, source, branch="master", dest=None, depth=None):
420
421=== modified file 'tests/charmhelpers/contrib/openstack/amulet/deployment.py'
422--- tests/charmhelpers/contrib/openstack/amulet/deployment.py 2016-01-08 13:11:21 +0000
423+++ tests/charmhelpers/contrib/openstack/amulet/deployment.py 2016-01-18 12:00:43 +0000
424@@ -125,7 +125,8 @@
425
426 # Charms which can not use openstack-origin, ie. many subordinates
427 no_origin = ['cinder-ceph', 'hacluster', 'neutron-openvswitch', 'nrpe',
428- 'openvswitch-odl', 'neutron-api-odl', 'odl-controller']
429+ 'openvswitch-odl', 'neutron-api-odl', 'odl-controller',
430+ 'cinder-backup']
431
432 if self.openstack:
433 for svc in services:

Subscribers

People subscribed via source and target branches