From eea5a355bcc8b85885621c58cb4420f304016925 Mon Sep 17 00:00:00 2001 From: Edward Hope-Morley Date: Thu, 5 Sep 2024 13:11:55 +0100 Subject: [PATCH] Add tox with pep8,pylint and bashate Tests all python scripts and starts adding coverage for bash scripts. --- .github/workflows/lint.yaml | 36 ------------ .github/workflows/run-tests.yaml | 47 +++++++++++++++ common/generate-bundle.sh | 3 +- common/generate_bundle_base | 52 ++++++++++++----- gh-test-requirements.txt | 1 + openstack/tools/identify_charm_func_tests.py | 43 +++++++++----- pylintrc | 28 +++++++++ requirements.txt | 1 + test-requirements.txt | 6 ++ tools/juju-bundle-applications.py | 24 ++++---- tools/parse-bundle.py | 61 +++++++++++--------- tox.ini | 61 ++++++++++++++++++++ 12 files changed, 257 insertions(+), 106 deletions(-) delete mode 100644 .github/workflows/lint.yaml create mode 100644 .github/workflows/run-tests.yaml create mode 100644 gh-test-requirements.txt create mode 100644 pylintrc create mode 100644 requirements.txt create mode 100644 test-requirements.txt create mode 100644 tox.ini diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml deleted file mode 100644 index d21afffb..00000000 --- a/.github/workflows/lint.yaml +++ /dev/null @@ -1,36 +0,0 @@ -name: Linter -on: - pull_request: - branches: - - main - workflow_dispatch: - -jobs: - lint: - name: Lint bash scripts - runs-on: ubuntu-latest - steps: - - name: Install tools - run: sudo apt install --no-install-recommends --yes python3-bashate - - name: Get sources - uses: actions/checkout@v3 - - name: Lint bash scripts - run: | - bashate --ignore E006 --verbose openstack/tools/create-microceph-vm.sh - bashate --ignore E006 --verbose tools/*.sh tools/juju-lnav - bashate --ignore E006 --verbose openstack/novarc - bashate --ignore E006 --verbose common/ch_channel_map/*.sh - - check-commit-message: - name: Check Commit Message - runs-on: ubuntu-latest - steps: - - name: Get sources - uses: actions/checkout@v3 - with: - fetch-depth: 0 - - name: Check Commit Message - run: | - ./tools/lint-git-messages.sh \ - ${{ github.event.pull_request.base.sha }} \ - ${{ github.event.pull_request.head.sha }} diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml new file mode 100644 index 00000000..20ae97bb --- /dev/null +++ b/.github/workflows/run-tests.yaml @@ -0,0 +1,47 @@ +# This is a templated file and must be kept up-to-date with the original +# from upstream at https://github.com/canonical/se-tooling-ci-common. +name: Run Tests +on: + - push + - pull_request + - workflow_dispatch + +jobs: + test: + strategy: + matrix: + python-version: ['3.8', '3.10', '3.12'] + os: [ubuntu-24.04, ubuntu-22.04, ubuntu-20.04] + exclude: + - os: ubuntu-20.04 + python-version: '3.10' + - os: ubuntu-20.04 + python-version: '3.12' + - os: ubuntu-22.04 + python-version: '3.8' + - os: ubuntu-22.04 + python-version: '3.12' + - os: ubuntu-24.04 + python-version: '3.8' + - os: ubuntu-24.04 + python-version: '3.10' + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r gh-test-requirements.txt + - name: Run pylint + run: tox -e pylint + if: matrix.python-version == '3.10' + - name: Run pep8 + run: tox -e pep8 + if: matrix.python-version == '3.10' + - name: Run bashate + run: tox -e bashate + if: matrix.python-version == '3.10' diff --git a/common/generate-bundle.sh b/common/generate-bundle.sh index 4722cfe1..48f4e78f 100755 --- a/common/generate-bundle.sh +++ b/common/generate-bundle.sh @@ -6,6 +6,7 @@ MOD_DIR=$(realpath $(dirname $0)) . $MOD_DIR/pipeline/02configure . $MOD_DIR/pipeline/03build # Ensure no unrendered variables -out="`grep -r __ $MOD_DIR/b/${MASTER_OPTS[BUNDLE_NAME]} --exclude=config --exclude-dir=p| egrep -v '^.*#'`" || exit 0 +out="`grep -r __ $MOD_DIR/b/${MASTER_OPTS[BUNDLE_NAME]} --exclude=config \ + --exclude-dir=p| egrep -v '^.*#'`" || exit 0 echo -e "ERROR: there are unrendered variables in your bundle:\n$out" exit 1 diff --git a/common/generate_bundle_base b/common/generate_bundle_base index 31091384..25e15082 100644 --- a/common/generate_bundle_base +++ b/common/generate_bundle_base @@ -12,10 +12,13 @@ update_master_opts ${MOD_PASSTHROUGH_OPTS[@]} vip_start=${MASTER_OPTS[VIP_ADDR_START]} if [[ -z $vip_start ]] && [[ -e ~/novarc ]]; then # prodstack - cidr=$(source ~/novarc; openstack subnet show subnet_${OS_USERNAME}-psd -c cidr -f value 2>/dev/null) + cidr=$(source ~/novarc; openstack subnet show subnet_${OS_USERNAME}-psd \ + -c cidr -f value 2>/dev/null) if [[ -z $cidr ]]; then # stsstack - cidr=$(source ~/novarc; openstack subnet show ${OS_USERNAME}_admin_subnet -c cidr -f value 2>/dev/null) + cidr=$(source ~/novarc; openstack subnet show \ + ${OS_USERNAME}_admin_subnet \ + -c cidr -f value 2>/dev/null) if [[ -n $cidr ]]; then vip_start=$(echo $cidr| sed -r 's/([0-9]+\.[0-9]+).+/\1/g').150.0 fi @@ -24,7 +27,9 @@ if [[ -z $vip_start ]] && [[ -e ~/novarc ]]; then # last 20 addresses for vips which is prone to collisions but # we have no alternative currently. net_end=$(awk -F'.' '/HostMax/{print $NF}' <<<$(ipcalc -b $cidr)) - vip_start=$(echo $cidr| sed -r 's/([0-9]+\.[0-9]+\.[0-9]+).+/\1/g').$((net_end - 19)) + vip_start=$(echo $cidr| + sed -r 's/([0-9]+\.[0-9]+\.[0-9]+).+/\1/g').$((net_end - + 19)) fi fi VIP_START_PREFIX=${vip_start%\.*} @@ -67,7 +72,8 @@ finish () { local target - echo "${MOD_DIR}/generate-bundle.sh ${CACHED_STDIN[@]}" > ${bundles_dir}/generate-command + echo "${MOD_DIR}/generate-bundle.sh ${CACHED_STDIN[@]}" > \ + ${bundles_dir}/generate-command if has_opt --replay; then target=${bundles_dir}/command echo -e "INFO: replaying last known command (from $target)\n" @@ -160,20 +166,27 @@ aggregate_mysql_interface_parts $bundles_dir/o # generate bundle list base_bundle=$bundles_dir/`basename $bundle` overlay_list+=( $base_bundle ) -readarray -t application_list < <("$MOD_DIR/../tools/juju-bundle-applications.py" ${overlay_list[@]}) +readarray -t \ + application_list < <("$MOD_DIR/../tools/juju-bundle-applications.py" \ + ${overlay_list[@]}) # Generate placement overlay for use with MAAS provider if ${MASTER_OPTS[HYPERCONVERGED_DEPLOYMENT]}; then - cp $MOD_DIR/overlays/unit_placement/header.yaml.template $bundles_dir/unit-placement.yaml + cp $MOD_DIR/overlays/unit_placement/header.yaml.template \ + $bundles_dir/unit-placement.yaml if [[ $MOD_NAME = openstack ]]; then # these two represent total machines since e.g. ceph is deployed on compute but compute and gateway are never colocated - num_placement_machines=$((${MOD_PARAMS[__NUM_NEUTRON_GATEWAY_UNITS__]}+${MOD_PARAMS[__NUM_COMPUTE_UNITS__]})) + num_placement_machines=$((${MOD_PARAMS[__NUM_NEUTRON_GATEWAY_UNITS__]}+ + ${MOD_PARAMS[__NUM_COMPUTE_UNITS__]})) elif [[ $MOD_NAME = kubernetes ]]; then - num_placement_machines=$((${MOD_PARAMS[__NUM_K8S_CONTROL_PLANE_UNITS__]}+${MOD_PARAMS[__NUM_K8S_WORKER_UNITS__]})) + num_placement_machines=\ + $((${MOD_PARAMS[__NUM_K8S_CONTROL_PLANE_UNITS__]}+ + ${MOD_PARAMS[__NUM_K8S_WORKER_UNITS__]})) elif [[ $MOD_NAME = ceph ]]; then num_placement_machines=$((${MOD_PARAMS[__NUM_CEPH_OSD_UNITS__]})) else - echo "ERROR: module '$MOD_NAME' does not yet have support for hyperconverged mode" 1>&2 + echo -n "ERROR: module '$MOD_NAME'" 1>&2 + echo " does not yet have support for hyperconverged mode" 1>&2 exit 1 fi # detect all apps used and generate placement info by doing: @@ -181,33 +194,40 @@ if ${MASTER_OPTS[HYPERCONVERGED_DEPLOYMENT]}; then # * search unit_placement template with same name for app in ${application_list[@]}; do # filter juju keywords - [[ $app == "options" ]] || [[ $app == "to" ]] || [[ $app == "storage" ]] && continue + [[ $app == "options" ]] || [[ $app == "to" ]] || \ + [[ $app == "storage" ]] && continue app_placement=${app}.yaml t=$MOD_DIR/overlays/unit_placement/${app_placement}.template [ -r "$t" ] || continue # load template cp $t $PLACEMENT_OVERLAYS_DIR/$app_placement # apply all renderers - render_placement_units_lxd $PLACEMENT_OVERLAYS_DIR/$app_placement $num_placement_machines - render_placement_units_metal $PLACEMENT_OVERLAYS_DIR/$app_placement $num_placement_machines + render_placement_units_lxd $PLACEMENT_OVERLAYS_DIR/$app_placement \ + $num_placement_machines + render_placement_units_metal $PLACEMENT_OVERLAYS_DIR/$app_placement \ + $num_placement_machines render $PLACEMENT_OVERLAYS_DIR/$app_placement # add to master placement overlay - cat $PLACEMENT_OVERLAYS_DIR/$app_placement >> $bundles_dir/unit-placement.yaml + cat $PLACEMENT_OVERLAYS_DIR/$app_placement >> \ + $bundles_dir/unit-placement.yaml done # finally render master machine list - render_placement_machines $bundles_dir/unit-placement.yaml $num_placement_machines + render_placement_machines $bundles_dir/unit-placement.yaml \ + $num_placement_machines render $bundles_dir/unit-placement.yaml # and add to list of overlays overlay_opts+=( --overlay $bundles_dir/unit-placement.yaml ) # add default binding to all applications bindings="\ bindings:\n '': ${MASTER_OPTS[DEFAULT_BINDING]}" - find $bundles_dir -name \*.yaml| xargs -l sed -i -r "/^\s+charm:.+/a$bindings" + find $bundles_dir -name \*.yaml|\ + xargs -l sed -i -r "/^\s+charm:.+/a$bindings" fi ((${#overlay_opts[@]})) && overlay_opts+=("") # right padding -echo -e "juju deploy${JUJU_DEPLOY_OPTS} ${base_bundle}${overlay_opts[@]:- }\n " > ${bundles_dir}/command +echo -e "juju deploy${JUJU_DEPLOY_OPTS} \ + ${base_bundle}${overlay_opts[@]:- }\n " > ${bundles_dir}/command finish for f in $INTERNAL_BUNDLE_CONFIG; do diff --git a/gh-test-requirements.txt b/gh-test-requirements.txt new file mode 100644 index 00000000..eb28166c --- /dev/null +++ b/gh-test-requirements.txt @@ -0,0 +1 @@ +tox<4.0.0 diff --git a/openstack/tools/identify_charm_func_tests.py b/openstack/tools/identify_charm_func_tests.py index e9e467c4..138aa098 100644 --- a/openstack/tools/identify_charm_func_tests.py +++ b/openstack/tools/identify_charm_func_tests.py @@ -1,32 +1,45 @@ -# Get names of test targets that OSCI would run for the given charm. Should be -# run from within the charm root. -# -# Outputs space seperated list of target names. -# +""" +Get names of test targets that OSCI would run for the given charm. Should be +run from within the charm root. + +Outputs space separated list of target names. +""" import os + import yaml CLASSIC_TESTS_YAML = 'tests/tests.yaml' REACTIVE_TESTS_YAML = os.path.join('src', CLASSIC_TESTS_YAML) -def extract_values(bundle_list): + +def extract_targets(bundle_list): + """ + Targets are provided as strings or dicts where the target name is the + value so this accounts for both formats. + """ extracted = [] for item in bundle_list: if isinstance(item, dict): extracted.append(list(item.values())[0]) else: extracted.append(item) + return extracted -if os.path.exists(REACTIVE_TESTS_YAML): - bundles = yaml.safe_load(open(REACTIVE_TESTS_YAML)) -else: - bundles = yaml.safe_load(open(CLASSIC_TESTS_YAML)) -smoke_bundles = extract_values(bundles['smoke_bundles']) -gate_bundles = extract_values(bundles['gate_bundles']) -dev_bundles = extract_values(bundles['dev_bundles']) +if __name__ == "__main__": + if os.path.exists(REACTIVE_TESTS_YAML): + TESTS_FILE = REACTIVE_TESTS_YAML + else: + TESTS_FILE = CLASSIC_TESTS_YAML + + with open(TESTS_FILE, encoding='utf-8') as fd: + bundles = yaml.safe_load(fd) + + smoke_bundles = extract_targets(bundles['smoke_bundles']) + gate_bundles = extract_targets(bundles['gate_bundles']) + dev_bundles = extract_targets(bundles['dev_bundles']) -targets = set(smoke_bundles + gate_bundles + dev_bundles) + targets = set(smoke_bundles + gate_bundles + dev_bundles) -print(' '.join(sorted(targets))) + print(' '.join(sorted(targets))) diff --git a/pylintrc b/pylintrc new file mode 100644 index 00000000..0ab2bdfc --- /dev/null +++ b/pylintrc @@ -0,0 +1,28 @@ +# This is a templated file and must be kept up-to-date with the original +# from upstream at https://github.com/canonical/se-tooling-ci-common. +[MAIN] +jobs=0 +ignore=.git + +# List of plugins (as comma separated values of python module names) to load, +# usually to register additional checkers. +load-plugins=pylint.extensions.no_self_use + +# When enabled, pylint would attempt to guess common misconfiguration and emit +# user-friendly hints instead of false-positive error messages. +suggestion-mode=yes + +[FORMAT] +max-line-length=79 +# Allow doctrings containing long urls +ignore-long-lines=^\s+.+?$ + +[REPORTS] +#reports=yes +score=yes + +[MESSAGES CONTROL] +disable= + +[DESIGN] +min-public-methods=1 diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 00000000..c3726e8b --- /dev/null +++ b/requirements.txt @@ -0,0 +1 @@ +pyyaml diff --git a/test-requirements.txt b/test-requirements.txt new file mode 100644 index 00000000..898f13dd --- /dev/null +++ b/test-requirements.txt @@ -0,0 +1,6 @@ +# This is a templated file and must be kept up-to-date with the original +# from upstream at https://github.com/canonical/se-tooling-ci-common. +bashate +flake8==6.1.0 +flake8-import-order==0.18.2 +pylint==3.1.0 diff --git a/tools/juju-bundle-applications.py b/tools/juju-bundle-applications.py index 2ac1600d..96e2b376 100755 --- a/tools/juju-bundle-applications.py +++ b/tools/juju-bundle-applications.py @@ -1,14 +1,18 @@ #!/usr/bin/env python3 - +# pylint: disable=invalid-name +""" +Get Juju Bundle or Overlay Applications. +""" import sys -import yaml -application_list = set() +import yaml -for filename in sys.argv[1:]: - with open(filename, 'r') as f: - data = yaml.load_all(f, Loader=yaml.SafeLoader) - for d in data: - if 'applications' in d: - application_list.update(d['applications'].keys()) -print('\n'.join(application_list)) +if __name__ == "__main__": + application_list = set() + for filename in sys.argv[1:]: + with open(filename, 'r', encoding='utf-8') as f: + data = yaml.load_all(f, Loader=yaml.SafeLoader) + for d in data: + if 'applications' in d: + application_list.update(d['applications'].keys()) + print('\n'.join(application_list)) diff --git a/tools/parse-bundle.py b/tools/parse-bundle.py index b328ad82..c904dfd5 100755 --- a/tools/parse-bundle.py +++ b/tools/parse-bundle.py @@ -1,18 +1,23 @@ #!/usr/bin/env python3 - +# pylint: disable=invalid-name +""" +Parse bundle information. +""" import argparse -import yaml import re import sys -lpid = r"([~a-z0-9\-]+/)?" -charm = r"([a-z0-9\-]+)" -charm_match = re.compile(r".*cs:{}{}-([0-9]+)\s*$".format(lpid, charm)) -status_match = re.compile("^App.*Version.*Status") -empty_line = re.compile("^\s*$") +import yaml + +lpid_expr = r"([~a-z0-9\-]+/)?" +charm_name_expr = r"([a-z0-9\-]+)" +charm_expr = re.compile(fr".*cs:{lpid_expr}{charm_name_expr}-([0-9]+)\s*$") +status_match = re.compile(r"^App.*Version.*Status") +empty_line = re.compile(r"^\s*$") def parse_arguments(): + """ Parse cli args. """ parser = argparse.ArgumentParser() parser.add_argument( "FILE", @@ -26,6 +31,7 @@ def parse_arguments(): def get_charms(bundle): + """ Get charms from bundle. """ charms = {} for app in bundle['applications']: charms[app] = bundle['applications'][app]['charm'] @@ -33,21 +39,23 @@ def get_charms(bundle): def process_bundle(bundle): + """ Extra charm info from bundle. """ versions_found = False charms = get_charms(bundle) - for app in charms: - ret = charm_match.match(charms[app]) + for appinfo in charms.values(): + ret = charm_expr.match(appinfo) if ret: versions_found = True _charm = ret.group(2) if ret.group(1): - _charm = "{}{}".format(ret.group(1), _charm) + _charm = f"{ret.group(1)}{_charm}" - print(_charm, charms[app]) + print(_charm, appinfo) return versions_found def process_status(model_status): + """ Extract charm versions from model status. """ versions_found = False processing = False for line in model_status: @@ -58,19 +66,19 @@ def process_status(model_status): processing = False continue if processing: - ret = [l.strip() for l in line.split()] - version = ret[1] + ret = [chunk.strip() for chunk in line.split()] charm = ret[4] store = ret[5] rev = ret[6] if store == 'jujucharms': versions_found = True - print("{} cs:{}-{}".format(charm, charm, rev)) + print(f"{charm} cs:{charm}-{rev}") return versions_found def process(bundle_file, options): + """ Process a bundle. """ bundle = {} versions_found = False # Process revisions file assuming it is an exported bundle in yaml @@ -78,7 +86,8 @@ def process(bundle_file, options): try: bundle = yaml.load(bundle_file, Loader=yaml.SafeLoader) except yaml.scanner.ScannerError: - sys.stderr.write("INFO: input file does not appear to be in YAML format") + sys.stderr.write("INFO: input file does not appear to be in YAML " + "format\n") if 'applications' in bundle: if options.get_charms: versions_found = process_bundle(bundle) @@ -91,19 +100,15 @@ def process(bundle_file, options): versions_found = process_status(model_status) if not versions_found: - sys.stderr.write("WARNING: no valid charm revisions found in {}\n\n". - format(bundle_file.name)) - - -def main(): - options = parse_arguments() - if options.FILE == "-": - with sys.stdin as bundle: - process(bundle, options) - else: - with open(options.FILE) as bundle: - process(bundle, options) + sys.stderr.write(f"WARNING: no valid charm revisions found in " + f"{bundle_file.name}\n\n") if __name__ == "__main__": - main() + _options = parse_arguments() + if _options.FILE == "-": + with sys.stdin as _bundle: + process(_bundle, _options) + else: + with open(_options.FILE, encoding='utf-8') as _bundle: + process(_bundle, _options) diff --git a/tox.ini b/tox.ini new file mode 100644 index 00000000..e9527c34 --- /dev/null +++ b/tox.ini @@ -0,0 +1,61 @@ +# This is a templated file and must be kept up-to-date with the original +# from upstream at https://github.com/canonical/se-tooling-ci-common. +[tox] +skipsdist = True +envlist = bashate,pep8,pylint +minversion = 3.18.0 + +[flake8] +# H106: Don't put vim configuration in source files +# H203: Use assertIs(Not)None to check for None +# H204: Use assert(Not)Equal to check for equality +# H205: Use assert(Greater|Less)(Equal) for comparison +# H904: Delay string interpolations at logging calls +enable-extensions = H106,H203,H204,H205,H904 +show-source = true +exclude = +import-order-style = pep8 + +[testenv] +basepython = {env:TOX_PYTHON:python3} +pyfiles = + {toxinidir}/openstack/tools/identify_charm_func_tests.py + {toxinidir}/tools/parse-bundle.py + {toxinidir}/tools/juju-bundle-applications.py + +bashfiles = + {toxinidir}/common/generate-bundle.sh + {toxinidir}/common/generate_bundle_base + {toxinidir}/openstack/tools/create-microceph-vm.sh + {toxinidir}/tools/juju-lnav + {toxinidir}/tools/vault-unseal-and-authorise.sh + {toxinidir}/tools/model-poweron.sh + {toxinidir}/tools/model-poweroff.sh + {toxinidir}/tools/lint-git-messages.sh + {toxinidir}/tools/stack-manager/show-all.sh + {toxinidir}/tools/stack-manager/show-managed.sh + {toxinidir}/tools/stack-manager/mark-vms-managed.sh + {toxinidir}/tools/stack-manager/mark-vms-unmanaged.sh + {toxinidir}/tools/stack-manager/mark-model-vms-managed.sh + {toxinidir}/tools/stack-manager/show-unmanaged.sh + {toxinidir}/tools/stack-manager/common.sh + {toxinidir}/tools/stack-manager/mark-model-vms-unmanaged.sh + {toxinidir}/tools/mongo-access.sh + {toxinidir}/common/ch_channel_map/test-channel-map.sh + {toxinidir}/common/ch_channel_map/test-all.sh + {toxinidir}/openstack/novarc +setenv = + PYTHONHASHSEED=0 +deps = + -r{toxinidir}/requirements.txt + -r{toxinidir}/test-requirements.txt + +[testenv:pep8] +commands = flake8 -v {posargs:{[testenv]pyfiles}} + +[testenv:pylint] +commands = pylint -v --rcfile={toxinidir}/pylintrc {posargs:{[testenv]pyfiles}} + +[testenv:bashate] +commands = bashate --ignore E006 --verbose {posargs:{[testenv]bashfiles}} +