From 80d23d5cc90927e1d7ad3c831ebef5cf06721143 Mon Sep 17 00:00:00 2001 From: Yuxiang Zhu Date: Mon, 3 Aug 2020 17:40:51 +0000 Subject: [PATCH] Add 'verify-cvp' verb to verify CVP test results Example: Verify CVP test results for all latest 4.4 image builds, also warn those with failed content_set_check ```sh $ elliott --group openshift-4.4 verify-cvp --all --include-optional-check content_set_check ``` Example 2: Apply patches to ocp-build-data to fix the redundant content sets error: ```sh $ elliott --group openshift-4.4 verify-cvp --all --include-optional-check content_set_check --fix ``` --- elliottlib/cli/__main__.py | 3 + elliottlib/cli/common.py | 2 + elliottlib/cli/verify_cvp_cli.py | 215 +++++++++++++++++++++++++++++++ elliottlib/constants.py | 1 + elliottlib/gitdata.py | 10 +- elliottlib/resultsdb.py | 28 ++++ elliottlib/util.py | 13 +- requirements.txt | 1 + 8 files changed, 265 insertions(+), 8 deletions(-) create mode 100644 elliottlib/cli/verify_cvp_cli.py create mode 100644 elliottlib/resultsdb.py diff --git a/elliottlib/cli/__main__.py b/elliottlib/cli/__main__.py index 0b35b9c5..ca5d667c 100644 --- a/elliottlib/cli/__main__.py +++ b/elliottlib/cli/__main__.py @@ -53,6 +53,7 @@ from elliottlib.cli.advisory_images_cli import advisory_images_cli from elliottlib.cli.advisory_impetus_cli import advisory_impetus_cli from elliottlib.cli.tag_builds_cli import tag_builds_cli +from elliottlib.cli.verify_cvp_cli import verify_cvp_cli # 3rd party import bugzilla @@ -875,6 +876,8 @@ def poll_signed(runtime, minutes, advisory, default_advisory_type, noop): cli.add_command(rpmdiff_cli) cli.add_command(tag_builds_cli) cli.add_command(tarball_sources_cli) +cli.add_command(verify_cvp_cli) + # ----------------------------------------------------------------------------- # CLI Entry point diff --git a/elliottlib/cli/common.py b/elliottlib/cli/common.py index 9aed88ab..679e91e5 100644 --- a/elliottlib/cli/common.py +++ b/elliottlib/cli/common.py @@ -92,3 +92,5 @@ def find_default_advisory(runtime, default_advisory_type, quiet=False): type=click.Choice(constants.standard_advisory_types), help='Use the default value from group.yml for ADVISORY_TYPE [{}]'.format( ', '.join(constants.standard_advisory_types))) + +pass_runtime = click.make_pass_decorator(Runtime) diff --git a/elliottlib/cli/verify_cvp_cli.py b/elliottlib/cli/verify_cvp_cli.py new file mode 100644 index 00000000..e92d86c9 --- /dev/null +++ b/elliottlib/cli/verify_cvp_cli.py @@ -0,0 +1,215 @@ +from typing import Iterator, List + +import click +import koji +import requests +import pathlib +import sys + +import elliottlib +from elliottlib import Runtime, brew, constants +from elliottlib.cli.common import (cli, find_default_advisory, pass_runtime, + use_default_advisory_option) +from elliottlib.imagecfg import ImageMetadata +from elliottlib.resultsdb import ResultsDBAPI +from elliottlib.util import (green_prefix, parallel_results_with_progress, + red_prefix, yellow_print, red_print) + + +@cli.command("verify-cvp", short_help="Verify CVP test results") +@click.option( + '--all', 'all_images', required=False, is_flag=True, + help='Verify all latest image builds (default to False)') +@click.option( + '--build', '-b', 'nvrs', + multiple=True, metavar='NVR_OR_ID', + help='Only verify specified builds') +@click.option( + '--include-optional-check', "optional_checks", + multiple=True, metavar='OPTIONAL_CVP_CHECK_NAME', + help="Also print failed optional CVP checks. e.g. content_set_check") +@click.option( + '--all-optional-checks', "all_optional_checks", is_flag=True, + help="If set, print all failed optional CVP checks") +@click.option( + '--fix', "fix", is_flag=True, + help="Try to fix failed all_optional_checks. Currently only supports fixing redundant content sets. See --help for more details") +@click.option( + '--message', '-m', 'message', metavar='COMMIT_MESSAGE', + help='Commit message for ocp-build-data when using `--fix`. If not given, no changes will be committed.') +@pass_runtime +def verify_cvp_cli(runtime: Runtime, all_images, nvrs, optional_checks, all_optional_checks, fix, message): + """ Verify CVP test results + + Example 1: Verify CVP test results for all latest 4.4 image builds, also warn those with failed content_set_check + + $ elliott --group openshift-4.4 verify-cvp --all --include-optional-check content_set_check + + Example 2: Apply patches to ocp-build-data to fix the redundant content sets error: + + $ elliott --group openshift-4.4 verify-cvp --all --include-optional-check content_set_check --fix + + Note: + 1. If `--message` is not given, `--fix` will leave changed ocp-build-data files uncommitted. + 2. Make sure your ocp-build-data directory is clean before running `--fix`. + """ + if bool(all_images) + bool(nvrs) != 1: + raise click.BadParameter('You must use one of --all or --build.') + if all_optional_checks and optional_checks: + raise click.BadParameter('Use only one of --all-optional-checks or --include-optional-check.') + + runtime.initialize(mode='images') + tag_pv_map = runtime.gitdata.load_data(key='erratatool', replace_vars=runtime.group_config.vars.primitive() if runtime.group_config.vars else {}).data.get('brew_tag_product_version_mapping') + brew_session = koji.ClientSession(runtime.group_config.urls.brewhub or constants.BREW_HUB) + + builds = [] + if all_images: + runtime.logger.info("Getting latest image builds from Brew...") + builds = get_latest_image_builds(brew_session, tag_pv_map.keys(), runtime.image_metas) + elif nvrs: + runtime.logger.info(f"Finding {len(builds)} builds from Brew...") + builds = brew.get_build_objects(nvrs, brew_session) + runtime.logger.info(f"Found {len(builds)} image builds.") + + resultsdb_api = ResultsDBAPI() + nvrs = [b["nvr"] for b in builds] + runtime.logger.info(f"Getting CVP test results for {len(builds)} image builds...") + latest_cvp_results = get_latest_cvp_results(runtime, resultsdb_api, nvrs) + + # print a summary for all CVP results + good_results = [] # good means PASSED or INFO + bad_results = [] # bad means NEEDS_INSPECTION or FAILED + incomplete_nvrs = [] + for nvr, result in zip(nvrs, latest_cvp_results): + if not result: + incomplete_nvrs.append(nvr) + continue + outcome = result.get("outcome") # only PASSED, FAILED, INFO, NEEDS_INSPECTION are now valid outcome values (https://resultsdb20.docs.apiary.io/#introduction/changes-since-1.0) + if outcome in {"PASSED", "INFO"}: + good_results.append(result) + elif outcome in {"NEEDS_INSPECTION", "FAILED"}: + bad_results.append(result) + green_prefix("good: {}".format(len(good_results))) + click.echo(", ", nl=False) + red_prefix("bad: {}".format(len(bad_results))) + click.echo(", ", nl=False) + yellow_print("incomplete: {}".format(len(incomplete_nvrs))) + + if bad_results: + red_print("The following builds didn't pass CVP tests:") + for r in bad_results: + nvr = r["data"]["item"][0] + red_print(f"{nvr} {r['outcome']}: {r['ref_url']}") + + if incomplete_nvrs: + yellow_print("We couldn't find CVP test results for the following builds:") + for nvr in incomplete_nvrs: + yellow_print(nvr) + + if not optional_checks and not all_optional_checks: + return # no need to print failed optional CVP checks + # Find failed optional CVP checks in case some of the tiem *will* become required. + optional_checks = set(optional_checks) + complete_results = good_results + bad_results + runtime.logger.info(f"Getting optional checks for {len(complete_results)} CVP tests...") + optional_check_results = get_optional_checks(runtime, complete_results) + + component_distgit_keys = {} # a dict of brew component names to distgit keys + content_set_repo_names = {} # a map of x86_64 content set names to group.yml repo names + if fix: # Fixing redundant content sets requires those dicts + for image in runtime.image_metas(): + component_distgit_keys[image.get_component_name()] = image.distgit_key + for repo_name, repo_info in runtime.group_config.get("repos", {}).items(): + content_set_name = repo_info.get('content_set', {}).get('x86_64') or repo_info.get('content_set', {}).get('default') + if content_set_name: + content_set_repo_names[content_set_name] = repo_name + + ocp_build_data_updated = False + + for cvp_result, checks in zip(complete_results, optional_check_results): + # example optional checks: http://external-ci-coldstorage.datahub.redhat.com/cvp/cvp-product-test/hive-container-v4.6.0-202008010302.p0/da01e36c-8c69-4a19-be7d-ba4593a7b085/sanity-tests-optional-results.json + bad_checks = [check for check in checks["checks"] if check["status"] != "PASS" and (all_optional_checks or check["name"] in optional_checks)] + if not bad_checks: + continue + nvr = cvp_result["data"]["item"][0] + yellow_print("----------") + yellow_print(f"Build {nvr} has {len(bad_checks)} problematic CVP optional checks:") + for check in bad_checks: + yellow_print(f"* {check['name']} {check['status']}") + if fix and check["name"] == "content_set_check": + if "Some content sets are redundant." in check["logs"]: + # fix redundant content sets + name = nvr.rsplit('-', 2)[0] + distgit_keys = component_distgit_keys.get(name) + if not distgit_keys: + runtime.logger.warning(f"Will not apply the redundant content sets fix to image {name}: We don't know its distgit key.") + continue + amd64_content_sets = list(filter(lambda item: item.get("arch") == "amd64", check["logs"][-1])) # seems only x86_64 (amd64) content sets are defined in ocp-build-data. + if not amd64_content_sets: + runtime.logger.warning(f"Will not apply the redundant content sets fix to image {name}: It doesn't have redundant x86_64 (amd64) content sets") + continue + amd64_redundant_cs = amd64_content_sets[0]["redundant_cs"] + redundant_repos = [content_set_repo_names[cs] for cs in amd64_redundant_cs if cs in content_set_repo_names] + if len(redundant_repos) != len(amd64_redundant_cs): + runtime.logger.error(f"Not all content sets have a repo entry in group.yml: #content_sets is {len(amd64_redundant_cs)}, #repos is {len(redundant_repos)}") + runtime.logger.info(f"Applying redundant content sets fix to {distgit_keys}...") + fix_redundant_content_set(runtime, distgit_keys, redundant_repos) + ocp_build_data_updated = True + runtime.logger.info(f"Fixed redundant content sets for {distgit_keys}") + yellow_print(f"See {cvp_result['ref_url']}sanity-tests-optional-results.json for more details.") + + if message and ocp_build_data_updated: + runtime.gitdata.commit(message) + + +def fix_redundant_content_set(runtime: Runtime, distgit_keys: str, redundant_repos: List[str]): + """ Patch ocp-build-data image config yaml to fix the redundant content sets error. + + Note this function just adds the redundant repos to `non_shipping_repos`. It doesn't really remove repos from enabled_repos because the builder image may require them. + """ + data_obj = runtime.gitdata.load_data(path='images', key=distgit_keys) + data_obj.reload() # reload with ruamel.yaml to preserve the format and comments as much as possible + image_cfg = data_obj.data + non_shipping_repos = image_cfg.get("non_shipping_repos", []) + non_shipping_repos = set(non_shipping_repos) | set(redundant_repos) + if non_shipping_repos: + image_cfg["non_shipping_repos"] = sorted(list(non_shipping_repos)) + elif "non_shipping_repos" in image_cfg: + del image_cfg["non_shipping_repos"] + data_obj.save() + + +def get_latest_image_builds(brew_session: koji.ClientSession, tags: Iterator[str], image_metas: Iterator[ImageMetadata]): + tag_component_tuples = [(tag, image.get_component_name()) for tag in tags for image in image_metas()] + builds = brew.get_latest_builds(tag_component_tuples, brew_session) + return [b[0] for b in builds if b] + + +def get_latest_cvp_results(runtime: Runtime, resultsdb_api: ResultsDBAPI, nvrs: List[str]): + all_results = [] + results = parallel_results_with_progress(nvrs, lambda nvr: resultsdb_api.get_latest_results(["rhproduct.default.sanity"], [nvr]), file=sys.stderr) + for nvr, result in zip(nvrs, results): + data = result.get("data") + if not data: # Couldn't find a CVP test result for the given Brew build + runtime.logger.warning(f"Couldn't find a CVP test result for {nvr}. Is the CVP test still running?") + all_results.append(None) + continue + all_results.append(data[0]) + return all_results + + +def get_optional_checks(runtime: Runtime, test_results): + # Each CVP test result stored in ResultsDB has a link to an external storage with more CVP test details + # e.g. http://external-ci-coldstorage.datahub.redhat.com/cvp/cvp-product-test/ose-insights-operator-container-v4.5.0-202007240519.p0/fb9dd365-e886-46c9-9661-fd13c0d29c49/ + external_urls = [r["ref_url"] + "sanity-tests-optional-results.json" for r in test_results] + optional_check_results = [] + with requests.session() as session: + responses = parallel_results_with_progress(external_urls, lambda url: session.get(url), file=sys.stderr) + for cvp_result, r in zip(test_results, responses): + if r.status_code != 200: + nvr = cvp_result["data"]["item"][0] + runtime.logger.warning(f"Couldn't find sanity-tests-optional-results.json for {nvr}") + optional_check_results.append(None) + continue + optional_check_results.append(r.json()) + return optional_check_results diff --git a/elliottlib/constants.py b/elliottlib/constants.py index ef554f18..1665fd28 100644 --- a/elliottlib/constants.py +++ b/elliottlib/constants.py @@ -5,6 +5,7 @@ BREW_HUB = "https://brewhub.engineering.redhat.com/brewhub" CGIT_URL = "http://pkgs.devel.redhat.com/cgit" +RESULTSDB_API_URL = "https://resultsdb-api.engineering.redhat.com/api/v2.0" VALID_BUG_STATES = ['NEW', 'ASSIGNED', 'POST', 'MODIFIED', 'ON_QA', 'VERIFIED', 'RELEASE_PENDING', 'CLOSED'] diff --git a/elliottlib/gitdata.py b/elliottlib/gitdata.py index 4f2174d7..fdf2ead6 100644 --- a/elliottlib/gitdata.py +++ b/elliottlib/gitdata.py @@ -7,6 +7,8 @@ from urllib.parse import urlparse import yaml +import ruamel.yaml +import ruamel.yaml.util import logging import os import shutil @@ -37,6 +39,8 @@ def __init__(self, key, path, data): self.base_dir = os.path.dirname(self.path) self.filename = self.path.replace(self.base_dir, '').strip('/') self.data = data + self.indent = 2 + self.block_seq_indent = None @as_native_str() def __repr__(self): @@ -49,11 +53,13 @@ def __repr__(self): def reload(self): with open(self.path, 'r') as f: - self.data = yaml.full_load(f) + # Reload with ruamel.yaml and guess the indent. + self.data, self.indent, self.block_seq_indent = ruamel.yaml.util.load_yaml_guess_indent(f, preserve_quotes=True) def save(self): with open(self.path, 'w') as f: - yaml.safe_dump(self.data, f, default_flow_style=False) + # pyyaml doesn't preserve the order of keys or comments when loading and saving yamls. Save with ruamel.yaml instead to keep the format as much as possible. + ruamel.yaml.round_trip_dump(self.data, f, indent=self.indent, block_seq_indent=self.block_seq_indent) class GitData(object): diff --git a/elliottlib/resultsdb.py b/elliottlib/resultsdb.py new file mode 100644 index 00000000..5ce87fb4 --- /dev/null +++ b/elliottlib/resultsdb.py @@ -0,0 +1,28 @@ +from elliottlib import constants +import requests +from itertools import islice + + +class ResultsDBAPI: + def __init__(self): + self.url = constants.RESULTSDB_API_URL + self.session = requests.Session() + + def get_latest_results(self, test_cases, items): + """ Get latest test results from ResultsDB + It takes filter parameters, and returns the most recent result for all the relevant Testcases. Only Testcases with at least one Result that meet the filter are present + https://resultsdb20.docs.apiary.io/#reference/0/results/get-a-list-of-latest-results-for-a-specified-filter + """ + params = {} + if test_cases: + params["testcases"] = ",".join(test_cases) + + results = [] + if items: + params["item"] = ",".join(items) + r = self.session.get(f"{self.url}/results/latest", params=params) + # an example CVP test result for ose-insights-operator-container-v4.5.0-202007240519.p0: + # https://resultsdb-api.engineering.redhat.com/api/v2.0/results/latest?testcases=rhproduct.default.sanity&item=ose-insights-operator-container-v4.5.0-202007240519.p0 + r.raise_for_status() + results = r.json() + return results diff --git a/elliottlib/util.py b/elliottlib/util.py index 5f557923..80dc2ea6 100644 --- a/elliottlib/util.py +++ b/elliottlib/util.py @@ -189,7 +189,7 @@ def pbar_header(msg_prefix='', msg='', seq=[], char='*'): click.echo("[" + (char * len(seq)) + "]") -def progress_func(func, char='*'): +def progress_func(func, char='*', file=None): """Use to wrap functions called in parallel. Prints a character for each function call. @@ -197,15 +197,16 @@ def progress_func(func, char='*'): after printing a progress character :param str char: The character (or multi-char string, if you really wanted to) to print before calling `func` + :param file: the file to print the progress. None means stdout. Usage examples: * See find-builds command """ - click.secho(char, fg='green', nl=False) + click.secho(char, fg='green', nl=False, file=file) return func() -def parallel_results_with_progress(inputs, func): +def parallel_results_with_progress(inputs, func, file=None): """Run a function against a list of inputs with a progress bar :param sequence inputs : A sequence of items to iterate over in parallel @@ -223,16 +224,16 @@ def parallel_results_with_progress(inputs, func): [****************] """ - click.secho('[', nl=False) + click.secho('[', nl=False, file=file) pool = ThreadPool(cpu_count()) results = pool.map( - lambda it: progress_func(lambda: func(it)), + lambda it: progress_func(lambda: func(it), file=file), inputs) # Wait for results pool.close() pool.join() - click.echo(']') + click.echo(']', file=file) return results diff --git a/requirements.txt b/requirements.txt index ba19b486..91699ebb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,5 +10,6 @@ python-bugzilla pyyaml requests requests_kerberos +ruamel.yaml six typing ; python_version <= '3.4'