From c94a64ca0b9eee287101c945a5c39bb1dbaa2d1b Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 24 Jun 2021 16:58:32 -0700 Subject: [PATCH 1/2] hack/lib/unify-job-properties: Add a helper for preserving mutable properties As discussed in e6b339e301 (ci-operator/jobs/openshift/cluster-version-operator: Metal optional on 4.8 and 4.9, 2021-07-21, #19513) and similar work, the current branch-forking procedure does not track these job properties which are not covered by ci-operator/config/... entries. This script makes it easier to mass-repair suspect branches, based on agreement between master and more reliable sibling branches. --- hack/lib/unify-job-properties.py | 163 +++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100755 hack/lib/unify-job-properties.py diff --git a/hack/lib/unify-job-properties.py b/hack/lib/unify-job-properties.py new file mode 100755 index 0000000000000..d5dab585598d6 --- /dev/null +++ b/hack/lib/unify-job-properties.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python + +import json +import os + +import ruamel.yaml as yaml # ruamel allows us to preserve formatting + + +def unify_job_properties(directory): + jobs = load_jobs_from_directory(directory=directory) + unify_jobs(jobs=jobs) + + +def load_jobs_from_directory(directory): + jobs = [] + for filename in os.listdir(path=directory): + if not filename.endswith('.yaml'): + continue + path = os.path.join(directory, filename) + with open(path) as f: + try: + jobs.extend(load_jobs_from_stream(stream=f, path=path)) + except Exception as error: + raise ValueError('failed to load jobs from {}'.format(path)) from error + return jobs + + +def load_jobs_from_stream(stream, path): + documents = list(yaml.safe_load_all(stream)) + if len(documents) != 1: + raise ValueError('{} YAML documents; only one document is supported'.format(len(documents))) + for job_type, type_data in documents[0].items(): + if job_type == 'periodics': + for job in type_data: + job['_path'] = path + job['_type'] = job_type + yield job + else: # presubmits, etc. + for repo, jobs in type_data.items(): + for job in jobs: + job['_path'] = path + job['_type'] = job_type + job['_repo'] = repo + yield job + + +def unify_jobs(jobs, suspect_branches=None): + if suspect_branches is None: + suspect_branches = {'release-4.8', 'release-4.9'} + for job in jobs: + try: + branch, _ = job_branch_context(job=job) + except ValueError: + continue + if branch not in suspect_branches: + continue # master is a source of canonical data + + siblings = get_siblings(job=job, jobs=jobs, suspect_branches=suspect_branches) + if not siblings: + continue + + unify_job(job=job, siblings=siblings) + + +def job_branch_context(job): + try: + branch = job.get('branches', [])[0] + except IndexError as error: + raise ValueError('job has no branch') from error + if branch.startswith('release-3.') or branch in {'release-4.1', 'release-4.2', 'release-4.3', 'release-4.4', 'release-4.5'}: + raise ValueError('{} is ancient'.format(branch)) + + context = job.get('context') + if not context: + raise ValueError('job has no context') + + return (branch, context) + + +def get_siblings(job, jobs, suspect_branches): + try: + branch, context = job_branch_context(job=job) + except ValueError: + return None + + siblings = {} + for sibling in jobs: + try: + sibling_branch, sibling_context = job_branch_context(job=sibling) + except ValueError: + continue + if sibling_branch in suspect_branches: + continue + if sibling_context == context and sibling_branch != branch: + siblings[sibling_branch] = sibling + return siblings + + +def unify_job(job, siblings, mutable_properties=None): + if mutable_properties is None: + # https://docs.ci.openshift.org/docs/how-tos/contributing-openshift-release/#tolerated-changes-to-generated-jobs + mutable_properties = { + 'always_run', + 'max_concurrency', + 'optional', + 'reporter_config', + 'run_if_changed', + 'skip_if_only_changed', + 'skip_report', + } + branch, context = job_branch_context(job=job) + + updated = False + for prop in mutable_properties: + sibling_values = {json.dumps(sibling.get(prop), sort_keys=True) for sibling in siblings.values()} + if len(sibling_values) > 1: + print('cannot unify {} for {} {}. Sibling values include:'.format(prop, branch, context)) + for sibling_branch, sibling in sorted(siblings.items()): + print(' {}: {}'.format(sibling_branch, json.dumps(sibling.get(prop), sort_keys=True))) + elif len(sibling_values) == 1: + sibling_value = list(sibling_values)[0] + value = json.dumps(job.get(prop), sort_keys=True) + if value != sibling_value: + print('{} {}: change {} from {} to {} to match siblings'.format( + branch, context, prop, value, sibling_value)) + job[prop] = json.loads(sibling_value) + updated = True + if updated: + update_job(job=job) + + +def update_job(job): + with open(job['_path']) as f: + data = yaml.load(f, yaml.RoundTripLoader) + if job['_type'] == 'periodics': + jobs = data[job['_type']] + else: + jobs = data[job['_type']][job['_repo']] + for i, j in enumerate(jobs): + if j['name'] == job['name']: + jobs[i] = {k: v for k,v in job.items() if not k.startswith('_')} + break + with open(job['_path'], 'w') as f: + yaml.dump(data, f, default_flow_style=False, Dumper=yaml.RoundTripDumper) + + +if __name__ == '__main__': + import argparse + + parser = argparse.ArgumentParser( + description='Unify 4.8 and 4.9 job configuration by adjusting mutable properties (optional, etc.) when they diverge from the master, 4.6, and 4.7 configuration.', + ) + + parser.add_argument( + 'dir', + nargs='+', + help='Path to a directory with job-configuration YAML, like ci-operator/jobs/$ORG/$REPO.', + ) + + args = parser.parse_args() + + for directory in args.dir: + unify_job_properties(directory=directory) From 916e7c5f37efde19660915d5c809ecbea272dfbb Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 24 Jun 2021 17:01:13 -0700 Subject: [PATCH 2/2] ci-operator/jobs/openshift/origin: Restore mutable properties for 4.8 and 4.9 Generated with: $ hack/lib/unify-job-properties.py ci-operator/jobs/openshift/origin $ make jobs I haven't gone through and guessed at the mutable properties for which sibling branches disagree, but that shouldn't block us moving ahead to repair where there is agreement. --- .../openshift-origin-release-4.8-presubmits.yaml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ci-operator/jobs/openshift/origin/openshift-origin-release-4.8-presubmits.yaml b/ci-operator/jobs/openshift/origin/openshift-origin-release-4.8-presubmits.yaml index 86ea53e88df3d..fea24e531ede2 100644 --- a/ci-operator/jobs/openshift/origin/openshift-origin-release-4.8-presubmits.yaml +++ b/ci-operator/jobs/openshift/origin/openshift-origin-release-4.8-presubmits.yaml @@ -670,7 +670,7 @@ presubmits: secretName: result-aggregator trigger: (?m)^/test( | .* )e2e-aws-ovn,?($|\s.*) - agent: kubernetes - always_run: true + always_run: false branches: - release-4.8 cluster: build01 @@ -682,6 +682,7 @@ presubmits: ci-operator.openshift.io/prowgen-controlled: "true" pj-rehearse.openshift.io/can-be-rehearsed: "true" name: pull-ci-openshift-origin-release-4.8-e2e-aws-proxy + optional: true rerun_command: /test e2e-aws-proxy spec: containers: @@ -1414,7 +1415,7 @@ presubmits: secretName: result-aggregator trigger: (?m)^/test( | .* )e2e-gcp-upgrade,?($|\s.*) - agent: kubernetes - always_run: true + always_run: false branches: - release-4.8 cluster: build01 @@ -1426,6 +1427,7 @@ presubmits: ci-operator.openshift.io/prowgen-controlled: "true" pj-rehearse.openshift.io/can-be-rehearsed: "true" name: pull-ci-openshift-origin-release-4.8-e2e-metal-ipi + optional: true rerun_command: /test e2e-metal-ipi spec: containers: @@ -1482,7 +1484,7 @@ presubmits: secretName: result-aggregator trigger: (?m)^/test( | .* )e2e-metal-ipi,?($|\s.*) - agent: kubernetes - always_run: true + always_run: false branches: - release-4.8 cluster: build01 @@ -1494,6 +1496,7 @@ presubmits: ci-operator.openshift.io/prowgen-controlled: "true" pj-rehearse.openshift.io/can-be-rehearsed: "true" name: pull-ci-openshift-origin-release-4.8-e2e-metal-ipi-ovn-dualstack + optional: true rerun_command: /test e2e-metal-ipi-ovn-dualstack spec: containers: @@ -1618,7 +1621,7 @@ presubmits: secretName: result-aggregator trigger: (?m)^/test( | .* )e2e-metal-ipi-ovn-ipv6,?($|\s.*) - agent: kubernetes - always_run: true + always_run: false branches: - release-4.8 cluster: build01 @@ -1630,6 +1633,7 @@ presubmits: ci-operator.openshift.io/prowgen-controlled: "true" pj-rehearse.openshift.io/can-be-rehearsed: "true" name: pull-ci-openshift-origin-release-4.8-e2e-metal-ipi-virtualmedia + optional: true rerun_command: /test e2e-metal-ipi-virtualmedia spec: containers: