From 06d8422a365428e96f4467d40a026ffe65a83677 Mon Sep 17 00:00:00 2001 From: Tim Serong Date: Fri, 19 Nov 2021 15:46:56 +1100 Subject: [PATCH] Fix lint errors Signed-off-by: Tim Serong --- srv/modules/runners/changed.py | 10 ++++-- srv/modules/runners/osd.py | 8 ++++- srv/modules/runners/upgrade.py | 64 +++++++++++++++++++++------------ srv/modules/runners/validate.py | 3 +- srv/salt/_modules/cephdisks.py | 9 ----- srv/salt/_modules/dg.py | 1 + srv/salt/_modules/mds.py | 1 + srv/salt/_modules/osd.py | 7 ++-- 8 files changed, 63 insertions(+), 40 deletions(-) diff --git a/srv/modules/runners/changed.py b/srv/modules/runners/changed.py index b45d35ed7..438a21999 100644 --- a/srv/modules/runners/changed.py +++ b/srv/modules/runners/changed.py @@ -145,6 +145,7 @@ def has_change(self): def any(): + # pylint: disable=redefined-builtin """ Checks whether any configuration has changed, and if it has, sets the various restart grains appropriately, then returns True, otherwise @@ -165,9 +166,12 @@ def any(): Role(role_name='mds'), Role(role_name='storage', conf_filename='osd'), Role(role_name='client'), - Role(role_name='igw', conf_dir='/srv/salt/ceph/igw/cache/', - conf_filename="iscsi-gateway.*", - conf_extension=".cfg"), + Role( + role_name='igw', + conf_dir='/srv/salt/ceph/igw/cache/', + conf_filename="iscsi-gateway.*", + conf_extension=".cfg" + ), ] local = salt.client.LocalClient() diff --git a/srv/modules/runners/osd.py b/srv/modules/runners/osd.py index 7d7b4a2af..5cfe0d1fd 100644 --- a/srv/modules/runners/osd.py +++ b/srv/modules/runners/osd.py @@ -461,7 +461,13 @@ def ok_to_stop_osds(osd_list): message = list(ret.values())[0] try: # ceph 14.2.22 gives something like: - # {"ok_to_stop":true,"osds":[0],"num_ok_pgs":98,"num_not_ok_pgs":0,"ok_become_degraded": [...]} + # { + # "ok_to_stop": true, + # "osds": [0], + # "num_ok_pgs": 98, + # "num_not_ok_pgs": 0, + # "ok_become_degraded": [...] + # } ok_to_stop = json.loads(message) if ok_to_stop['ok_to_stop']: return True diff --git a/srv/modules/runners/upgrade.py b/srv/modules/runners/upgrade.py index 28f2f1f2d..475443de4 100644 --- a/srv/modules/runners/upgrade.py +++ b/srv/modules/runners/upgrade.py @@ -8,13 +8,12 @@ from __future__ import print_function import re from typing import Iterable, List +import json # pylint: disable=import-error,3rd-party-module-not-gated,redefined-builtin import salt.client import salt.utils.error -from packaging import version -import json import yaml - +from packaging import version class UpgradeValidation(object): @@ -111,15 +110,21 @@ def deprecated_straw(self): class OrigDriveGroup: + # pylint: disable=missing-docstring - def migrate(self, name, spec, storage_hosts=[]): + def migrate(self, name, spec, storage_hosts=None): + if storage_hosts is None: + storage_hosts = [] self.__setattr__('service_type', 'osd') self.__setattr__('filter_logic', 'OR') self.__setattr__('service_id', name) + # pylint: disable=invalid-name for k, v in spec.items(): self.__setattr__(k, v, storage_hosts) - def __setattr__(self, key, value, storage_hosts=[]): + def __setattr__(self, key, value, storage_hosts=None): + if storage_hosts is None: + storage_hosts = [] if key == 'format' and value not in ['bluestore']: raise Exception('Only is supported.') if key == 'target': @@ -136,6 +141,7 @@ def to_yaml(self): def upgrade_orig_drive_group(old_dgs: List[dict], storage_hosts) -> Iterable[dict]: + # pylint: disable=invalid-name,missing-docstring for dg in old_dgs: for name, spec in dg.items(): dg_o = OrigDriveGroup() @@ -243,38 +249,52 @@ def status(): # would be SLE 12 SP3 and some would be SLE 15 SP1. So, we check if there's any oses # that aren't a variant on SLE 15 (or are down/Unkown). If the list is empty, we # assume we're on a SLE 15 SP1 or SP2 base, and can sensibly run the SES6->7 checks. - non_sle_15 = [os for os in set(os_codename.values()) - if not os.startswith("SUSE Linux Enterprise Server 15") - and not os.startswith("Unknown")] + non_sle_15 = [ + os for os in set(os_codename.values()) + if not os.startswith("SUSE Linux Enterprise Server 15") + and not os.startswith("Unknown") + ] + # It's easier to understand "if len(non_sle_15) == 0" than "if not non_sle_15" + # pylint: disable=len-as-condition if len(non_sle_15) == 0: - require_osd_release = list(local.cmd(master_minion, 'osd.require_osd_release', []).items())[0][1] + require_osd_release = list(local.cmd( + master_minion, 'osd.require_osd_release', [] + ).items())[0][1] if require_osd_release[0] < 'n': - print("'require-osd-release' is currently '{}', but must be set to 'nautilus'".format(require_osd_release)) + print("'require-osd-release' is currently '{}', but must be set to 'nautilus'".format( + require_osd_release + )) print("before upgrading to SES 7. Please run `ceph osd require-osd-release nautilus`") print("to fix this before proceeding further.") print("") - filestore_osds = list(local.cmd(master_minion, 'cmd.run', - ["ceph osd metadata | jq '.[] | select(.osd_objectstore == \"filestore\") | .id'"]).items())[0][1].split() + filestore_osds = list(local.cmd(master_minion, 'cmd.run', [ + "ceph osd metadata | jq '.[] | select(.osd_objectstore == \"filestore\") | .id'" + ]).items())[0][1].split() if filestore_osds: print("The following OSDs are using FileStore:") print(" {}".format(', '.join(filestore_osds))) print("All FileStore OSDs must be converted to BlueStore before upgrading to SES 7.") print("") - filesystems = list(local.cmd(master_minion, 'cmd.run', - ["ceph fs ls --format=json|jq -r '.[][\"name\"]'"]).items())[0][1].split() + filesystems = list(local.cmd(master_minion, 'cmd.run', [ + "ceph fs ls --format=json|jq -r '.[][\"name\"]'" + ]).items())[0][1].split() if filesystems: + # pylint: disable=invalid-name for fs in filesystems: - max_mds = int(list(local.cmd(master_minion, 'cmd.run', - ["ceph fs get {}|awk '/max_mds/ {{print $2}}'".format(fs)]).items())[0][1]) + max_mds = int(list(local.cmd(master_minion, 'cmd.run', [ + "ceph fs get {}|awk '/max_mds/ {{print $2}}'".format(fs) + ]).items())[0][1]) if max_mds > 1: + # pylint: disable=line-too-long print("The '{}' filesystem has {} MDS daemons.".format(fs, max_mds)) print("Before upgrading MDS daemons, reduce the number of ranks to 1 by running") print("`ceph fs set {} max_mds 1`".format(fs)) print("") - up_standbys = int(list(local.cmd(master_minion, 'cmd.run', - ["ceph status --format=json-pretty|jq -r '.[\"fsmap\"][\"up:standby\"]'"]).items())[0][1]) + up_standbys = int(list(local.cmd(master_minion, 'cmd.run', [ + "ceph status --format=json-pretty|jq -r '.[\"fsmap\"][\"up:standby\"]'" + ]).items())[0][1]) if up_standbys > 0: if up_standbys > 1: print("There are {} standby MDS daemons running.".format(up_standbys)) @@ -385,10 +405,6 @@ def ceph_salt_config(): __salt__ = salt.loader.minion_mods(__opts__, utils=__utils__) master_minion = __salt__['master.minion']() - runner = salt.runner.RunnerClient(__opts__) - mon_ips = dict(runner.cmd('select.public_addresses', - ['cluster=ceph', 'roles=mon', 'tuples=True'], print_event=False)) - local = salt.client.LocalClient() roles = local.cmd("I@cluster:ceph", 'pillar.get', ['roles'], tgt_type="compound") @@ -452,6 +468,7 @@ def generate_service_specs(): # Lifted from srv/modules/runners/select.py def _grain_host(client, minion): + # pylint: disable=missing-docstring return list(client.cmd(minion, 'grains.item', ['host']).values())[0]['host'] for service_type in ['mon', 'mgr', 'prometheus', 'grafana']: @@ -477,7 +494,8 @@ def _grain_host(client, minion): } }) - storage_hosts = [_grain_host(local, node) for node in roles if 'storage' in roles[node]]; + storage_hosts = [_grain_host(local, node) for node in roles if 'storage' in roles[node]] + # pylint: disable=invalid-name with open('/srv/salt/ceph/configuration/files/drive_groups.yml', 'r') as fd: new = upgrade_orig_drive_group(yaml.safe_load_all(fd), storage_hosts) service_specs.extend(new) diff --git a/srv/modules/runners/validate.py b/srv/modules/runners/validate.py index 7db3b13b8..bd196c3ee 100644 --- a/srv/modules/runners/validate.py +++ b/srv/modules/runners/validate.py @@ -719,7 +719,7 @@ def domain(self): A device with the hostname `myhost` in the parent domain example.com has the fully qualified domain name myhost.example.com. The FQDN uniquely distinguishes the device from any other hosts called myhost in other domains. Please make sure 'hostname --fqdn' returns an FQDN, for example, myhost.example.com -Your currently configured FQDN is: <$fqdn> +Your currently configured FQDN is: <{fqdn}> """ if not domain: self.errors.setdefault('domain', []).append(msg) @@ -1037,6 +1037,7 @@ def subvolume(self): btrfs. Skip checks if subvolume state is disabled. """ for node in self.data: + # pylint: disable=line-too-long if 'subvolume_init' in self.data[node] and self.data[node]['subvolume_init'] == "disabled": self.skipped['subvolume'] = "skipping" return diff --git a/srv/salt/_modules/cephdisks.py b/srv/salt/_modules/cephdisks.py index 36a36f6a6..c42bdbd83 100644 --- a/srv/salt/_modules/cephdisks.py +++ b/srv/salt/_modules/cephdisks.py @@ -6,15 +6,6 @@ from __future__ import absolute_import import logging -import re -import os -from subprocess import Popen, PIPE -from shlex import split - -try: - from salt.utils.path import which -except ImportError: - from distutils.spawn import which # pytest: disable=import-error log = logging.getLogger(__name__) diff --git a/srv/salt/_modules/dg.py b/srv/salt/_modules/dg.py index 480b37e4a..b3bd71c9a 100644 --- a/srv/salt/_modules/dg.py +++ b/srv/salt/_modules/dg.py @@ -1319,6 +1319,7 @@ def destroyed_osds(self) -> list: """ Property that lists 'destroyed' osds """ return self.destroyed_osds_map.get(__grains__.get('host', ''), list()) + # pylint: disable=inconsistent-return-statements def generate_c_v_commands(self): """ Generate ceph-volume commands based on the DriveGroup filters """ data_devices = [x.get('path') for x in self.data_device_props.devices] diff --git a/srv/salt/_modules/mds.py b/srv/salt/_modules/mds.py index 8851a1354..ad88520d8 100644 --- a/srv/salt/_modules/mds.py +++ b/srv/salt/_modules/mds.py @@ -28,6 +28,7 @@ def get_name(host, i=0): def get_local_daemon_count(): + # pylint: disable=missing-docstring,invalid-name p = '/var/lib/ceph/mds/' dirs = [d for d in listdir(p) if isdir('{}/{}'.format(p, d)) and isfile('{}/{}/keyring'.format(p, d))] diff --git a/srv/salt/_modules/osd.py b/srv/salt/_modules/osd.py index 9294a433a..ad292ac05 100644 --- a/srv/salt/_modules/osd.py +++ b/srv/salt/_modules/osd.py @@ -718,7 +718,7 @@ def require_osd_release(**kwargs): """ Return the require_osd_release setting """ - require_osd_release = None + osd_release = None settings = { 'conf': "/etc/ceph/ceph.conf", 'keyring': '/etc/ceph/ceph.client.admin.keyring', @@ -734,11 +734,12 @@ def require_osd_release(**kwargs): cmd = json.dumps({"prefix": "osd dump", "format": "json"}) _, output, _ = cluster.mon_command(cmd, b'', timeout=6) output = json.loads(output) - require_osd_release = output['require_osd_release'] + osd_release = output['require_osd_release'] + # pylint: disable=broad-except except Exception as error: log.error("Couldn't get require_osd_release: {}".format(error)) - return require_osd_release + return osd_release class OSDDevices(object):