From 3c19dba7921430e20450b16850942be5ca33555d Mon Sep 17 00:00:00 2001 From: Sebastian Kalinowski Date: Wed, 24 Jun 2015 13:34:18 +0200 Subject: [PATCH] Upgrade hacking rules and fix new issues Change-Id: I5a1a6b20198dc13b28698ac9b9e28dbb9a2ddddb Implements: blueprint volume-manager-refactoring --- fuel_agent/__init__.py | 13 ------------- fuel_agent/cmd/__init__.py | 13 ------------- fuel_agent/drivers/__init__.py | 13 ------------- fuel_agent/drivers/base.py | 3 ++- fuel_agent/drivers/nailgun.py | 22 +++++++++++++--------- fuel_agent/manager.py | 6 ++++-- fuel_agent/objects/partition.py | 6 ++++-- fuel_agent/tests/__init__.py | 13 ------------- fuel_agent/tests/test_nailgun.py | 8 ++++---- fuel_agent/utils/__init__.py | 13 ------------- fuel_agent/utils/artifact_utils.py | 5 +++-- fuel_agent/utils/build_utils.py | 9 +++++---- fuel_agent/utils/grub_utils.py | 2 ++ fuel_agent/utils/hardware_utils.py | 7 +++++-- fuel_agent/utils/lvm_utils.py | 4 ++-- fuel_agent/utils/md_utils.py | 4 ++-- fuel_agent/utils/utils.py | 12 +++++++----- tox.ini | 3 +-- 18 files changed, 54 insertions(+), 102 deletions(-) diff --git a/fuel_agent/__init__.py b/fuel_agent/__init__.py index a2bc2f3..e69de29 100644 --- a/fuel_agent/__init__.py +++ b/fuel_agent/__init__.py @@ -1,13 +0,0 @@ -# Copyright 2014 Mirantis, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. diff --git a/fuel_agent/cmd/__init__.py b/fuel_agent/cmd/__init__.py index a2bc2f3..e69de29 100644 --- a/fuel_agent/cmd/__init__.py +++ b/fuel_agent/cmd/__init__.py @@ -1,13 +0,0 @@ -# Copyright 2014 Mirantis, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. diff --git a/fuel_agent/drivers/__init__.py b/fuel_agent/drivers/__init__.py index a2bc2f3..e69de29 100644 --- a/fuel_agent/drivers/__init__.py +++ b/fuel_agent/drivers/__init__.py @@ -1,13 +0,0 @@ -# Copyright 2014 Mirantis, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. diff --git a/fuel_agent/drivers/base.py b/fuel_agent/drivers/base.py index 819132d..9ce4224 100644 --- a/fuel_agent/drivers/base.py +++ b/fuel_agent/drivers/base.py @@ -18,7 +18,8 @@ import six @six.add_metaclass(abc.ABCMeta) class BaseDataDriver(object): - """Data driver API is to be put here. + """Data driver API + For example, data validation methods, methods for getting object schemes, etc. """ diff --git a/fuel_agent/drivers/nailgun.py b/fuel_agent/drivers/nailgun.py index 58b6e22..9c8d64d 100644 --- a/fuel_agent/drivers/nailgun.py +++ b/fuel_agent/drivers/nailgun.py @@ -35,7 +35,9 @@ LOG = logging.getLogger(__name__) def match_device(hu_disk, ks_disk): - """Tries to figure out if hu_disk got from hu.list_block_devices + """Check if hu_disk and ks_disk are the same device + + Tries to figure out if hu_disk got from hu.list_block_devices and ks_spaces_disk given correspond to the same disk device. This is the simplified version of hu.match_device @@ -88,24 +90,26 @@ class Nailgun(BaseDataDriver): @property def ks_disks(self): - disk_filter = lambda x: x['type'] == 'disk' and x['size'] > 0 - return filter(disk_filter, self.partition_data()) + return filter( + lambda x: x['type'] == 'disk' and x['size'] > 0, + self.partition_data()) @property def small_ks_disks(self): - """Get those disks which are smaller than 2T - """ + """Get those disks which are smaller than 2T""" return [d for d in self.ks_disks if d['size'] <= 2097152] @property def ks_vgs(self): - vg_filter = lambda x: x['type'] == 'vg' - return filter(vg_filter, self.partition_data()) + return filter( + lambda x: x['type'] == 'vg', + self.partition_data()) @property def hu_disks(self): """Actual disks which are available on this node - it is a list of dicts which are formatted other way than + + It is a list of dicts which are formatted other way than ks_spaces disks. To match both of those formats use _match_device method. """ @@ -437,7 +441,7 @@ class Nailgun(BaseDataDriver): LOG.debug('--- Preparing image scheme ---') data = self.data image_scheme = objects.ImageScheme() - #FIXME(agordeev): this piece of code for fetching additional image + # FIXME(agordeev): this piece of code for fetching additional image # meta data should be factored out of this particular nailgun driver # into more common and absract data getter which should be able to deal # with various data sources (local file, http(s), etc.) and different diff --git a/fuel_agent/manager.py b/fuel_agent/manager.py index 0e8ccd5..89d4cf4 100644 --- a/fuel_agent/manager.py +++ b/fuel_agent/manager.py @@ -192,7 +192,7 @@ class Manager(object): LOG.debug("Skipping udev rule %s de-blacklisting" % src) utils.execute('udevadm', 'control', '--reload-rules', check_exit_code=[0]) - #NOTE(agordeev): re-create all the links which were skipped by udev + # NOTE(agordeev): re-create all the links which were skipped by udev # while blacklisted # NOTE(agordeev): do subsystem match, otherwise it will stuck utils.execute('udevadm', 'trigger', '--subsystem-match=block', @@ -475,7 +475,9 @@ class Manager(object): # into a set of smaller ones # https://bugs.launchpad.net/fuel/+bug/1444090 def do_build_image(self): - """Building OS images includes the following steps + """Building OS images + + Includes the following steps 1) create temporary sparse files for all images (truncate) 2) attach temporary files to loop devices (losetup) 3) create file systems on these loop devices diff --git a/fuel_agent/objects/partition.py b/fuel_agent/objects/partition.py index aa32266..733c4c4 100644 --- a/fuel_agent/objects/partition.py +++ b/fuel_agent/objects/partition.py @@ -73,7 +73,7 @@ class Parted(object): return 'primary' elif len(self.partitions) == 3 and not self.extended: return 'extended' - #NOTE(agordeev): how to reach that condition? + # NOTE(agordeev): how to reach that condition? else: return 'logical' @@ -291,7 +291,9 @@ class PartitionScheme(object): return found[0] def fs_sorted_by_depth(self, reverse=False): - """Getting file systems sorted by path length. Shorter paths earlier. + """Getting file systems sorted by path length. + + Shorter paths earlier. ['/', '/boot', '/var', '/var/lib/mysql'] :param reverse: Sort backward (Default: False) """ diff --git a/fuel_agent/tests/__init__.py b/fuel_agent/tests/__init__.py index a2bc2f3..e69de29 100644 --- a/fuel_agent/tests/__init__.py +++ b/fuel_agent/tests/__init__.py @@ -1,13 +0,0 @@ -# Copyright 2014 Mirantis, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. diff --git a/fuel_agent/tests/test_nailgun.py b/fuel_agent/tests/test_nailgun.py index aa0b547..e391b28 100644 --- a/fuel_agent/tests/test_nailgun.py +++ b/fuel_agent/tests/test_nailgun.py @@ -356,7 +356,7 @@ LIST_BLOCK_DEVICES_SAMPLE = [ 'sz': '976773168', 'iomin': '4096', 'size64': '500107862016', 'ss': '512', 'ioopt': '0', 'alignoff': '0', 'pbsz': '4096', 'ra': '256', 'ro': '0', 'maxsect': '1024' - }, + }, 'size': 500107862016}, {'uspec': {'DEVLINKS': [ @@ -865,7 +865,7 @@ class TestNailgun(test_base.BaseTestCase): @mock.patch.object(utils, 'init_http_request') @mock.patch.object(hu, 'list_block_devices') def test_partition_scheme_ceph(self, mock_lbd, mock_http_req, mock_yaml): - #TODO(agordeev): perform better testing of ceph logic + # TODO(agordeev): perform better testing of ceph logic p_data = copy.deepcopy(PROVISION_SAMPLE_DATA) for i in range(0, 3): p_data['ks_meta']['pm_data']['ks_spaces'][i]['volumes'].append( @@ -881,8 +881,8 @@ class TestNailgun(test_base.BaseTestCase): self.assertEqual(2, len(p_scheme.vgs)) self.assertEqual(3, len(p_scheme.parteds)) self.assertEqual(3, drv._get_partition_count('ceph')) - #NOTE(agordeev): (-2, -1, -1) is the list of ceph data partition counts - # corresponding to (sda, sdb, sdc) disks respectively. + # NOTE(agordeev): (-2, -1, -1) is the list of ceph data partition + # counts corresponding to (sda, sdb, sdc) disks respectively. for disk, part in enumerate((-2, -1, -1)): self.assertEqual(CEPH_DATA['partition_guid'], p_scheme.parteds[disk].partitions[part].guid) diff --git a/fuel_agent/utils/__init__.py b/fuel_agent/utils/__init__.py index a2bc2f3..e69de29 100644 --- a/fuel_agent/utils/__init__.py +++ b/fuel_agent/utils/__init__.py @@ -1,13 +0,0 @@ -# Copyright 2014 Mirantis, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. diff --git a/fuel_agent/utils/artifact_utils.py b/fuel_agent/utils/artifact_utils.py index 8938113..2cf8eda 100644 --- a/fuel_agent/utils/artifact_utils.py +++ b/fuel_agent/utils/artifact_utils.py @@ -19,6 +19,7 @@ import tempfile import zlib from oslo.config import cfg +import six from fuel_agent import errors from fuel_agent.openstack.common import log as logging @@ -38,8 +39,8 @@ CONF = cfg.CONF CONF.register_opts(au_opts) +@six.add_metaclass(abc.ABCMeta) class Target(object): - __metaclass__ = abc.ABCMeta def __iter__(self): return self @@ -117,7 +118,7 @@ class HttpUrl(Target): class GunzipStream(Target): def __init__(self, stream): self.stream = iter(stream) - #NOTE(agordeev): toggle automatic header detection on + # NOTE(agordeev): toggle automatic header detection on self.decompressor = zlib.decompressobj(zlib.MAX_WBITS | 32) def next(self): diff --git a/fuel_agent/utils/build_utils.py b/fuel_agent/utils/build_utils.py index 2e54cef..4f53fee 100644 --- a/fuel_agent/utils/build_utils.py +++ b/fuel_agent/utils/build_utils.py @@ -153,8 +153,7 @@ def suppress_services_start(chroot): def clean_dirs(chroot, dirs, delete=False): - """Method is used to clean directories content - or remove directories themselfs. + """Removes dirs and recreates them :param chroot: Root directory where to look for subdirectories :param dirs: List of directories to clean/remove (Relative to chroot) @@ -218,6 +217,7 @@ def do_post_inst(chroot): def stop_chrooted_processes(chroot, signal=sig.SIGTERM, attempts=10, attempts_delay=2): """Sends signal to all processes, which are running inside chroot. + It tries several times until all processes die. If at some point there are no running processes found, it returns True. @@ -354,8 +354,9 @@ def strip_filename(name): def get_release_file(uri, suite, section): - """Download repo's Release file, parse it and returns an apt - preferences line for this repo. + """Download and parse repo's Release file + + It and returns an apt preferences line for specified repo. :param repo: a repo as dict :returns: a string with apt preferences rules diff --git a/fuel_agent/utils/grub_utils.py b/fuel_agent/utils/grub_utils.py index a031d69..85e3d31 100644 --- a/fuel_agent/utils/grub_utils.py +++ b/fuel_agent/utils/grub_utils.py @@ -98,6 +98,7 @@ def guess_grub1_datadir(chroot='', arch='x86_64'): def guess_kernel(chroot='', regexp=None): """Tries to guess kernel by regexp + :param chroot: Path to chroot :param regexp: (String) Regular expression (must have python syntax). Default is r'^vmlinuz.*' @@ -115,6 +116,7 @@ def guess_kernel(chroot='', regexp=None): def guess_initrd(chroot='', regexp=None): """Tries to guess initrd by regexp + :param chroot: Path to chroot :param regexp: (String) Regular expression (must have python syntax). Default is r'^(initrd|initramfs).*' diff --git a/fuel_agent/utils/hardware_utils.py b/fuel_agent/utils/hardware_utils.py index 56394b0..db64645 100644 --- a/fuel_agent/utils/hardware_utils.py +++ b/fuel_agent/utils/hardware_utils.py @@ -270,7 +270,9 @@ def get_block_devices_from_udev_db(): def list_block_devices(disks=True): - """Gets list of block devices, tries to guess which of them are disks + """Gets list of block devices + + Tries to guess which of them are disks and returns list of dicts representing those disks. :returns: A list of dict representing disks available on a node. @@ -316,7 +318,8 @@ def list_block_devices(disks=True): def match_device(uspec1, uspec2): - """Tries to find out if uspec1 and uspec2 are uspecs from the same device. + """Tries to find out if uspec1 and uspec2 are uspecs from the same device + It compares only some fields in uspecs (not all of them) which, we believe, is enough to say exactly whether uspecs belong to the same device or not. diff --git a/fuel_agent/utils/lvm_utils.py b/fuel_agent/utils/lvm_utils.py index f5aa8c9..e9ca805 100644 --- a/fuel_agent/utils/lvm_utils.py +++ b/fuel_agent/utils/lvm_utils.py @@ -168,7 +168,7 @@ def lvdisplay(): '-C', '--noheading', '--units', 'm', - #NOTE(agordeev): lv_path had been removed from options + # NOTE(agordeev): lv_path had been removed from options # since versions of lvdisplay prior 2.02.68 don't have it. '--options', 'lv_name,lv_size,vg_name,lv_uuid', '--separator', ';', @@ -188,7 +188,7 @@ def lvdisplay_parse(output): 'size': utils.parse_unit(lv_params[1], 'm'), 'vg': lv_params[2], 'uuid': lv_params[3], - #NOTE(agordeev): simulate lv_path with '/dev/$vg_name/$lv_name' + # NOTE(agordeev): simulate lv_path with '/dev/$vg_name/$lv_name' 'path': '/dev/%s/%s' % (lv_params[2], lv_params[0]) }) LOG.debug('Found logical volumes: {0}'.format(lvs)) diff --git a/fuel_agent/utils/md_utils.py b/fuel_agent/utils/md_utils.py index e2763e0..88c7a8a 100644 --- a/fuel_agent/utils/md_utils.py +++ b/fuel_agent/utils/md_utils.py @@ -106,7 +106,7 @@ def mdcreate(mdname, level, device, *args): 'Error while creating md: at least one of devices is ' 'already in belongs to some md') - #FIXME: mdadm will ask user to continue creating if any device appears to + # FIXME: mdadm will ask user to continue creating if any device appears to # be a part of raid array. Superblock zeroing helps to avoid that. map(mdclean, devices) utils.execute('mdadm', '--create', '--force', mdname, '-e0.90', @@ -120,7 +120,7 @@ def mdremove(mdname): if mdname not in get_mdnames(): raise errors.MDNotFoundError( 'Error while removing md: md %s not found' % mdname) - #FIXME: The issue faced was quiet hard to reproduce and to figure out the + # FIXME: The issue faced was quiet hard to reproduce and to figure out the # root cause. For unknown reason already removed md device is # unexpectedly returning back after a while from time to time making # new md device creation to fail. diff --git a/fuel_agent/utils/utils.py b/fuel_agent/utils/utils.py index aa0a6ad..af04646 100644 --- a/fuel_agent/utils/utils.py +++ b/fuel_agent/utils/utils.py @@ -71,7 +71,7 @@ CONF = cfg.CONF CONF.register_opts(u_opts) -#NOTE(agordeev): signature compatible with execute from oslo +# NOTE(agordeev): signature compatible with execute from oslo def execute(*cmd, **kwargs): command = ' '.join(cmd) LOG.debug('Trying to execute command: %s', command) @@ -137,8 +137,10 @@ def execute(*cmd, **kwargs): def parse_unit(s, unit, ceil=True): - """Converts '123.1unit' string into 124 if ceil is True - and converts '123.9unit' into 123 if ceil is False. + """Converts '123.1unit' string into ints + + If ceil is True it will be rounded up (124) + and and down (123) if ceil is False. """ flt = locale.atof(s.split(unit)[0]) @@ -227,6 +229,7 @@ def init_http_request(url, byte_range=0): def makedirs_if_not_exists(path, mode=0o755): """Create directory if it does not exist + :param path: Directory path :param mode: Directory mode (Default: 0o755) """ @@ -235,8 +238,7 @@ def makedirs_if_not_exists(path, mode=0o755): def grouper(iterable, n, fillvalue=None): - """Collect data into fixed-length chunks or blocks - """ + """Collect data into fixed-length chunks or blocks""" args = [iter(iterable)] * n return zip_longest(*args, fillvalue=fillvalue) diff --git a/tox.ini b/tox.ini index 0156311..b54b3fe 100644 --- a/tox.ini +++ b/tox.ini @@ -16,7 +16,7 @@ commands = downloadcache = ~/cache/pip [testenv:pep8] -deps = hacking==0.7 +deps = hacking==0.10.2 commands = flake8 {posargs:fuel_agent} @@ -33,7 +33,6 @@ envdir = devenv usedevelop = True [flake8] -ignore = H234,H302,H802 exclude = .venv,.git,.tox,dist,doc,*openstack/common*,*lib/python*,*egg,build,tools,docs show-pep8 = True show-source = True