From 5c1cea1601a9e310604363b5b4d50149c510eca6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Doktor?= Date: Fri, 19 Apr 2024 10:36:10 +0200 Subject: [PATCH 1/7] ci: Select jobs by touched code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit to allow selective testing as well as selective list of required tests let's add a mapping of required jobs/tests in "skips.py" and a "gatekeaper" workflow that will ensure the expected required jobs were successful. Then we can only mark the "gatekeaper" as the required job and modify the logic to suit our needs. Fixes: #9237 Signed-off-by: Lukáš Doktor --- .github/workflows/ci-on-push.yaml | 11 +- .github/workflows/ci.yaml | 14 ++ .github/workflows/gatekeeper-skipper.yaml | 52 +++++ .github/workflows/gatekeeper.yaml | 36 ++++ .../workflows/static-checks-self-hosted.yaml | 10 +- .github/workflows/static-checks.yaml | 14 ++ tools/testing/gatekeeper/jobs.py | 190 ++++++++++++++++++ tools/testing/gatekeeper/required-tests.yaml | 36 ++++ tools/testing/gatekeeper/skips.py | 105 ++++++++++ 9 files changed, 466 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/gatekeeper-skipper.yaml create mode 100644 .github/workflows/gatekeeper.yaml create mode 100644 tools/testing/gatekeeper/jobs.py create mode 100644 tools/testing/gatekeeper/required-tests.yaml create mode 100644 tools/testing/gatekeeper/skips.py diff --git a/.github/workflows/ci-on-push.yaml b/.github/workflows/ci-on-push.yaml index c58b2330c1..e09905f74d 100644 --- a/.github/workflows/ci-on-push.yaml +++ b/.github/workflows/ci-on-push.yaml @@ -19,12 +19,21 @@ concurrency: cancel-in-progress: true jobs: - kata-containers-ci-on-push: + skipper: if: ${{ contains(github.event.pull_request.labels.*.name, 'ok-to-test') }} + uses: ./.github/workflows/gatekeeper-skipper.yaml + with: + commit-hash: ${{ github.event.pull_request.head.sha }} + target-branch: ${{ github.event.pull_request.base.ref }} + + kata-containers-ci-on-push: + needs: skipper + if: ${{ needs.skipper.outputs.skip_build != 'yes' }} uses: ./.github/workflows/ci.yaml with: commit-hash: ${{ github.event.pull_request.head.sha }} pr-number: ${{ github.event.pull_request.number }} tag: ${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} target-branch: ${{ github.event.pull_request.base.ref }} + skip-test: ${{ needs.skipper.outputs.skip_test }} secrets: inherit diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 3528f86e04..285e2ecc5c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -15,6 +15,10 @@ on: required: false type: string default: "" + skip-test: + required: false + type: string + default: no jobs: build-kata-static-tarball-amd64: @@ -132,6 +136,7 @@ jobs: file: tests/integration/kubernetes/runtimeclass_workloads/confidential/unencrypted/Dockerfile run-kata-monitor-tests: + if: ${{ inputs.skip-test != 'yes' }} needs: build-kata-static-tarball-amd64 uses: ./.github/workflows/run-kata-monitor-tests.yaml with: @@ -140,6 +145,7 @@ jobs: target-branch: ${{ inputs.target-branch }} run-k8s-tests-on-aks: + if: ${{ inputs.skip-test != 'yes' }} needs: publish-kata-deploy-payload-amd64 uses: ./.github/workflows/run-k8s-tests-on-aks.yaml with: @@ -153,6 +159,7 @@ jobs: secrets: inherit run-k8s-tests-on-amd64: + if: ${{ inputs.skip-test != 'yes' }} needs: publish-kata-deploy-payload-amd64 uses: ./.github/workflows/run-k8s-tests-on-amd64.yaml with: @@ -165,6 +172,7 @@ jobs: secrets: inherit run-kata-coco-tests: + if: ${{ inputs.skip-test != 'yes' }} needs: [publish-kata-deploy-payload-amd64, build-and-publish-tee-confidential-unencrypted-image] uses: ./.github/workflows/run-kata-coco-tests.yaml with: @@ -177,6 +185,7 @@ jobs: secrets: inherit run-k8s-tests-on-zvsi: + if: ${{ inputs.skip-test != 'yes' }} needs: [publish-kata-deploy-payload-s390x, build-and-publish-tee-confidential-unencrypted-image] uses: ./.github/workflows/run-k8s-tests-on-zvsi.yaml with: @@ -189,6 +198,7 @@ jobs: secrets: inherit run-k8s-tests-on-ppc64le: + if: ${{ inputs.skip-test != 'yes' }} needs: publish-kata-deploy-payload-ppc64le uses: ./.github/workflows/run-k8s-tests-on-ppc64le.yaml with: @@ -200,6 +210,7 @@ jobs: target-branch: ${{ inputs.target-branch }} run-metrics-tests: + if: ${{ inputs.skip-test != 'yes' }} needs: build-kata-static-tarball-amd64 uses: ./.github/workflows/run-metrics.yaml with: @@ -208,6 +219,7 @@ jobs: target-branch: ${{ inputs.target-branch }} run-basic-amd64-tests: + if: ${{ inputs.skip-test != 'yes' }} needs: build-kata-static-tarball-amd64 uses: ./.github/workflows/basic-ci-amd64.yaml with: @@ -216,6 +228,7 @@ jobs: target-branch: ${{ inputs.target-branch }} run-cri-containerd-tests-s390x: + if: ${{ inputs.skip-test != 'yes' }} needs: build-kata-static-tarball-s390x uses: ./.github/workflows/run-cri-containerd-tests-s390x.yaml with: @@ -224,6 +237,7 @@ jobs: target-branch: ${{ inputs.target-branch }} run-cri-containerd-tests-ppc64le: + if: ${{ inputs.skip-test != 'yes' }} needs: build-kata-static-tarball-ppc64le uses: ./.github/workflows/run-cri-containerd-tests-ppc64le.yaml with: diff --git a/.github/workflows/gatekeeper-skipper.yaml b/.github/workflows/gatekeeper-skipper.yaml new file mode 100644 index 0000000000..b3e269575d --- /dev/null +++ b/.github/workflows/gatekeeper-skipper.yaml @@ -0,0 +1,52 @@ +name: Skipper + +# This workflow sets various "skip_*" output values that can be used to +# determine what workflows/jobs are expected to be executed. Sample usage: +# +# skipper: +# uses: ./.github/workflows/gatekeeper-skipper.yaml +# with: +# commit-hash: ${{ github.event.pull_request.head.sha }} +# target-branch: ${{ github.event.pull_request.base.ref }} +# +# your-workflow: +# needs: skipper +# if: ${{ needs.skipper.outputs.skip_build != 'yes' }} + +on: + workflow_call: + inputs: + commit-hash: + required: true + type: string + target-branch: + required: false + type: string + default: "" + outputs: + skip_build: + value: ${{ jobs.skipper.outputs.skip_build }} + skip_test: + value: ${{ jobs.skipper.outputs.skip_test }} + skip_static: + value: ${{ jobs.skipper.outputs.skip_static }} + + +jobs: + skipper: + runs-on: ubuntu-latest + outputs: + skip_build: ${{ steps.skipper.outputs.skip_build }} + skip_test: ${{ steps.skipper.outputs.skip_test }} + skip_static: ${{ steps.skipper.outputs.skip_static }} + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ inputs.commit-hash }} + fetch-depth: 0 + - id: skipper + env: + TARGET_BRANCH: ${{ inputs.target-branch }} + run: | + python3 tools/testing/gatekeeper/skips.py | tee -a "$GITHUB_OUTPUT" + shell: /usr/bin/bash -x {0} diff --git a/.github/workflows/gatekeeper.yaml b/.github/workflows/gatekeeper.yaml new file mode 100644 index 0000000000..50adc6be88 --- /dev/null +++ b/.github/workflows/gatekeeper.yaml @@ -0,0 +1,36 @@ +name: Gatekeeper + +# Gatekeeper uses the "skips.py" to determine which job names/regexps are +# required for given PR and waits for them to either complete or fail +# reporting the status. + +on: + pull_request_target: + types: + - opened + - synchronize + - reopened + - labeled + +jobs: + gatekeeper: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + fetch-depth: 0 + - id: gatekeeper + env: + TARGET_BRANCH: ${{ github.event.pull_request.base.ref }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + #!/usr/bin/env bash -x + mapfile -t lines < <(python3 tools/testing/gatekeeper/skips.py -t) + export REQUIRED_JOBS="${lines[0]}" + export REQUIRED_REGEXPS="${lines[1]}" + echo "REQUIRED_JOBS: $REQUIRED_JOBS" + echo "REQUIRED_REGEXPS: $REQUIRED_REGEXPS" + python3 tools/testing/gatekeeper/jobs.py + exit $? + shell: /usr/bin/bash -x {0} diff --git a/.github/workflows/static-checks-self-hosted.yaml b/.github/workflows/static-checks-self-hosted.yaml index 2b47bb6a6a..943240df31 100644 --- a/.github/workflows/static-checks-self-hosted.yaml +++ b/.github/workflows/static-checks-self-hosted.yaml @@ -12,8 +12,16 @@ concurrency: name: Static checks self-hosted jobs: - build-checks: + skipper: if: ${{ contains(github.event.pull_request.labels.*.name, 'ok-to-test') }} + uses: ./.github/workflows/gatekeeper-skipper.yaml + with: + commit-hash: ${{ github.event.pull_request.head.sha }} + target-branch: ${{ github.event.pull_request.base.ref }} + + build-checks: + needs: skipper + if: ${{ needs.skipper.outputs.skip_static != 'yes' }} strategy: fail-fast: false matrix: diff --git a/.github/workflows/static-checks.yaml b/.github/workflows/static-checks.yaml index 0b3291e82c..cf33405967 100644 --- a/.github/workflows/static-checks.yaml +++ b/.github/workflows/static-checks.yaml @@ -12,7 +12,15 @@ concurrency: name: Static checks jobs: + skipper: + uses: ./.github/workflows/gatekeeper-skipper.yaml + with: + commit-hash: ${{ github.event.pull_request.head.sha }} + target-branch: ${{ github.event.pull_request.base.ref }} + check-kernel-config-version: + needs: skipper + if: ${{ needs.skipper.outputs.skip_static != 'yes' }} runs-on: ubuntu-22.04 steps: - name: Checkout the code @@ -35,12 +43,16 @@ jobs: fi build-checks: + needs: skipper + if: ${{ needs.skipper.outputs.skip_static != 'yes' }} uses: ./.github/workflows/build-checks.yaml with: instance: ubuntu-22.04 build-checks-depending-on-kvm: runs-on: ubuntu-22.04 + needs: skipper + if: ${{ needs.skipper.outputs.skip_static != 'yes' }} strategy: fail-fast: false matrix: @@ -78,6 +90,8 @@ jobs: static-checks: runs-on: ubuntu-22.04 + needs: skipper + if: ${{ needs.skipper.outputs.skip_static != 'yes' }} strategy: fail-fast: false matrix: diff --git a/tools/testing/gatekeeper/jobs.py b/tools/testing/gatekeeper/jobs.py new file mode 100644 index 0000000000..e7eb835418 --- /dev/null +++ b/tools/testing/gatekeeper/jobs.py @@ -0,0 +1,190 @@ +#!/usr/bin/env python3 +# +# Copyright (c) 2024 Red Hat Inc. +# +# SPDX-License-Identifier: Apache-2.0 + +""" +Keeps checking the current PR until all required jobs pass +Env variables: +* REQUIRED_JOBS: comma separated list of required jobs (in form of + "$workflow / $job") +* REQUIRED_REGEXPS: comma separated list of regexps for required jobs +* COMMIT_HASH: Full commit hash we want to be watching +* GITHUB_REPOSITORY: Github repository (user/repo) +Sample execution (GH token can be excluded): +GITHUB_TOKEN="..." REQUIRED_JOBS="skipper / skipper" +REQUIRED_REGEXPS=".*" +COMMIT_HASH=b8382cea886ad9a8f77d237bcfc0eba0c98775dd +GITHUB_REPOSITORY=kata-containers/kata-containers +python3 jobs.py +""" + +import os +import re +import sys +import time +import requests + + +PASS = 0 +FAIL = 1 +RUNNING = 127 + + +_GH_HEADERS = {"Accept": "application/vnd.github.v3+json"} +if os.environ.get("GITHUB_TOKEN"): + _GH_HEADERS["Authorization"] = f"token {os.environ['GITHUB_TOKEN']}" +_GH_RUNS_URL = ("https://api.github.com/repos/" + f"{os.environ['GITHUB_REPOSITORY']}/actions/runs") + + +class Checker: + """Object to keep watching required GH action workflows""" + def __init__(self): + required_jobs = os.getenv("REQUIRED_JOBS") + if required_jobs: + required_jobs = required_jobs.split(",") + else: + required_jobs = [] + required_regexps = os.getenv("REQUIRED_REGEXPS") + self.required_regexps = [] + # TODO: Add way to specify minimum amount of tests + # (eg. via \d+: prefix) and check it in status + if required_regexps: + for regexp in required_regexps.split(","): + self.required_regexps.append(re.compile(regexp)) + if not required_jobs and not self.required_regexps: + raise RuntimeError("No REQUIRED_JOBS or REQUIRED_REGEXPS defined") + # Set all required jobs as RUNNING to enforce waiting for them + self.results = {job: RUNNING for job in required_jobs} + + def record(self, workflow, job): + """ + Records a job run + """ + job_name = f"{workflow} / {job['name']}" + if job_name not in self.results: + for re_job in self.required_regexps: + # Required job via regexp + if re_job.match(job_name): + break + else: + # Not a required job + return + if job["status"] != "completed": + self.results[job_name] = RUNNING + return + if job["conclusion"] != "success": + self.results[job_name] = job['conclusion'] + return + self.results[job_name] = PASS + + def status(self): + """ + :returns: 0 - all tests passing; 127 - no failures but some + tests in progress; 1 - any failure + """ + running = False + if not self.results: + # No results reported so far + return FAIL + for status in self.results.values(): + if status == RUNNING: + running |= True + elif status != PASS: + # Status not passed + return FAIL + if running: + return RUNNING + return PASS + + def __str__(self): + """Sumarize the current status""" + good = [] + bad = [] + warn = [] + for job, status in self.results.items(): + if status == RUNNING: + warn.append(f"WARN: {job} - Still running") + elif status == PASS: + good.append(f"PASS: {job} - success") + else: + bad.append(f"FAIL: {job} - Not passed - {status}") + out = '\n'.join(sorted(good) + sorted(warn) + sorted(bad)) + stat = self.status() + if stat == RUNNING: + status = "Some jobs are still running." + elif stat == PASS: + status = "All required jobs passed" + elif not self.results: + status = ("No required jobs for regexps: " + + ";".join([_.pattern for _ in self.required_regexps])) + else: + status = "Not all required jobs passed!" + return f"{out}\n\n{status}" + + def get_jobs_for_workflow_run(self, run_id): + """Get jobs from a workflow id""" + total_count = -1 + jobs = [] + page = 1 + while True: + response = requests.get( + f"{_GH_RUNS_URL}/{run_id}/jobs?per_page=100&page={page}", + headers=_GH_HEADERS, + timeout=60 + ) + response.raise_for_status() + output = response.json() + jobs.extend(output["jobs"]) + total_count = max(total_count, output["total_count"]) + if len(jobs) >= total_count: + break + page += 1 + return jobs + + def check_workflow_runs_status(self): + """ + Checks if all required jobs passed + + :returns: 0 - all passing; 1 - any failure; 127 some jobs running + """ + # TODO: Check if we need pagination here as well + latest_commit_sha = os.getenv("COMMIT_HASH") + response = requests.get( + _GH_RUNS_URL, + params={"head_sha": latest_commit_sha}, + headers=_GH_HEADERS, + timeout=60 + ) + response.raise_for_status() + workflow_runs = response.json()["workflow_runs"] + + for run in workflow_runs: + jobs = self.get_jobs_for_workflow_run(run["id"]) + for job in jobs: + self.record(run["name"], job) + print(self) + return self.status() + + def run(self): + """ + Keep checking the PR until all required jobs finish + + :returns: 0 on success; 1 on failure + """ + while True: + ret = self.check_workflow_runs_status() + if ret == RUNNING: + running_jobs = len([job + for job, status in self.results.items() + if status == RUNNING]) + print(f"{running_jobs} jobs are still running...") + time.sleep(180) + continue + sys.exit(ret) + + +if __name__ == "__main__": + Checker().run() diff --git a/tools/testing/gatekeeper/required-tests.yaml b/tools/testing/gatekeeper/required-tests.yaml new file mode 100644 index 0000000000..943af6e3fc --- /dev/null +++ b/tools/testing/gatekeeper/required-tests.yaml @@ -0,0 +1,36 @@ +required_tests: + # Always required tests + - "Commit Message Check / Commit Message Check" +required_regexps: + # Always required regexps + +paths: + # Mapping of path (python) regexps to set-of-tests (sort by order of importance) + # CI + - "^ci/openshift-ci/": [] + - "^\\.github/workflows/": [] + # TODO: Expand filters + # Documentation + #- "\\.rst$": ["build"] + #- "\\.md$": ["build"] + # Sources + #- "^src/": ["static", "build", "test"] + +mapping: + # Mapping of set-of-tests to required test names and/or test name regexps + # TODO: Modify this according to actual required tests + test: + # Checks the basic functional tests work + regexps: "Kata Containers CI / .*run-basic-amd64-tests.*|Kata Containers CI / .*run-metrics-tests.*" + names: + - Kata Containers CI / kata-containers-ci-on-push / run-k8s-tests-on-ppc64le / run-k8s-tests (qemu, kubeadm) + - Kata Containers CI / kata-containers-ci-on-push / run-k8s-tests-on-aks / run-k8s-tests (ubuntu, qemu, small) + - Kata Containers CI / kata-containers-ci-on-push / run-k8s-tests-with-crio-on-garm / run-k8s-tests (qemu, k0s, garm-ubuntu-2204) + # TODO: Add support for "depends" to automatically add dependant set-of-tests + # (eg. "build" is required for "test") + build: + # Checks that the kata-containers static tarball is created + regexps: "Kata Containers CI / .*build-kata-static-tarball.*" + static: + # Checks that static checks are passing + regexps: "Static checks.*" diff --git a/tools/testing/gatekeeper/skips.py b/tools/testing/gatekeeper/skips.py new file mode 100644 index 0000000000..dcb6920094 --- /dev/null +++ b/tools/testing/gatekeeper/skips.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python3 +# +# Copyright (c) 2024 Red Hat Inc. +# +# SPDX-License-Identifier: Apache-2.0 + +""" +Gets changes of the current git to env variable TARGET_BRANCH +and reports feature skips in form of "skip_$feature=yes|no" +or list of required tests (based on argv[1]) +""" + +from collections import OrderedDict +import os +import re +import subprocess +import sys + +import yaml + + +class Checks: + def __init__(self): + config_path = os.path.join(os.path.dirname(__file__), "required-tests.yaml") + with open(config_path, "r", encoding="utf8") as config_fd: + config = yaml.load(config_fd, Loader=yaml.SafeLoader) + if config.get('required_tests'): + self.required_tests = config['required_tests'] + else: + self.required_tests = [] + if config.get('required_regexps'): + self.required_regexps = config['required_regexps'] + else: + self.required_regexps = [] + if config.get('paths'): + self.paths = OrderedDict((re.compile(key), value) + for items in config['paths'] + for key, value in items.items()) + if config.get('mapping'): + self.mapping = config['mapping'] + else: + self.mapping = {} + self.all_set_of_tests = set(self.mapping.keys()) + + def run(self, tests, target_branch): + """ + Find the required features/tests + + :param: tests: report required tests+regexps (bool) + :param: target_branch: branch/commit to compare to + """ + enabled_features = self.get_features(target_branch) + if not tests: + for feature in self.all_set_of_tests: + # Print all features status in "$key=$value" format to allow + # usage with $GITHUB_OUTPUT + print(f"skip_{feature}=" + + ('no' if feature in enabled_features else 'yes')) + return 0 + required_tests = set(self.required_tests) + required_regexps = set(self.required_regexps) + for feature in enabled_features: + values = self.mapping.get(feature, {}) + if values.get("names"): + required_tests.update(values["names"]) + if values.get("regexps"): + required_regexps.add(values["regexps"]) + print(';'.join(required_tests)) + print('|'.join(required_regexps)) + return 0 + + def get_features(self, target_branch): + """ + Get changed file to `target_branch` and map them to the + to-be-tested set-of-tests + + :param target_branch: branch/commit to compare to + :returns: List of set-of-tests + """ + cmd = ["git", "diff", "--name-only", f"origin/{target_branch}"] + changed_files = [_.decode("utf-8") + for _ in subprocess.check_output(cmd).split(b'\n') + if _.strip()] + print('\n'.join(changed_files), file=sys.stderr) + enabled_features = set() + # Go through lines and find what features should be covered + for changed_file in changed_files: + for regexp, features in self.paths.items(): + if regexp.search(changed_file): + for feature in features: + enabled_features.add(feature) + # this changed_file was treated, ignore other regexps + break + else: + # Untreated changed_file, run all tests + return self.all_set_of_tests + return enabled_features + + +if __name__ == "__main__": + if len(sys.argv) == 2: + _TESTS = sys.argv[1] == '-t' + else: + _TESTS = False + sys.exit(Checks().run(_TESTS, os.getenv("TARGET_BRANCH", "main"))) From 4abfc11b4f71203417c95eee21586ff65cad3e3b Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Wed, 3 Jul 2024 18:00:25 -0300 Subject: [PATCH 2/7] workflows/gatekeeper: configure concurrency properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will allow to cancel-in-progress the gatekeeper jobs. Signed-off-by: Wainer dos Santos Moschetta Signed-off-by: Lukáš Doktor --- .github/workflows/gatekeeper.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/gatekeeper.yaml b/.github/workflows/gatekeeper.yaml index 50adc6be88..547b9e0fb3 100644 --- a/.github/workflows/gatekeeper.yaml +++ b/.github/workflows/gatekeeper.yaml @@ -12,6 +12,10 @@ on: - reopened - labeled +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: gatekeeper: runs-on: ubuntu-latest From fdcfac0641e89d8dbe45fa0cad55d9d3811f3e38 Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Fri, 5 Jul 2024 20:17:02 -0300 Subject: [PATCH 3/7] workflows/gatekeeper: export COMMIT_HASH variable The Github SHA of triggering PR should be exported in the environment so that gatekeeper can fetch the right workflows/jobs. Note: by default github will export GITHUB_SHA in the job's environment but that value cannot be used if the gatekeeper was triggered from a pull_request_target event, because the SHA correspond to the push branch. Signed-off-by: Wainer dos Santos Moschetta --- .github/workflows/gatekeeper.yaml | 1 + tools/testing/gatekeeper/jobs.py | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/gatekeeper.yaml b/.github/workflows/gatekeeper.yaml index 547b9e0fb3..6dc741c255 100644 --- a/.github/workflows/gatekeeper.yaml +++ b/.github/workflows/gatekeeper.yaml @@ -28,6 +28,7 @@ jobs: env: TARGET_BRANCH: ${{ github.event.pull_request.base.ref }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + COMMIT_HASH: ${{ github.event.pull_request.head.sha }} run: | #!/usr/bin/env bash -x mapfile -t lines < <(python3 tools/testing/gatekeeper/skips.py -t) diff --git a/tools/testing/gatekeeper/jobs.py b/tools/testing/gatekeeper/jobs.py index e7eb835418..94b6904293 100644 --- a/tools/testing/gatekeeper/jobs.py +++ b/tools/testing/gatekeeper/jobs.py @@ -42,6 +42,7 @@ _GH_RUNS_URL = ("https://api.github.com/repos/" class Checker: """Object to keep watching required GH action workflows""" def __init__(self): + self.latest_commit_sha = os.getenv("COMMIT_HASH") required_jobs = os.getenv("REQUIRED_JOBS") if required_jobs: required_jobs = required_jobs.split(",") @@ -151,10 +152,9 @@ class Checker: :returns: 0 - all passing; 1 - any failure; 127 some jobs running """ # TODO: Check if we need pagination here as well - latest_commit_sha = os.getenv("COMMIT_HASH") response = requests.get( _GH_RUNS_URL, - params={"head_sha": latest_commit_sha}, + params={"head_sha": self.latest_commit_sha}, headers=_GH_HEADERS, timeout=60 ) @@ -174,6 +174,8 @@ class Checker: :returns: 0 on success; 1 on failure """ + print(f"Gatekeeper for project={os.environ['GITHUB_REPOSITORY']} and " + f"SHA={self.latest_commit_sha}") while True: ret = self.check_workflow_runs_status() if ret == RUNNING: From dd2878a9c85cff14b1ce43b1a3571b582f5c7fd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Doktor?= Date: Thu, 18 Jul 2024 13:05:51 +0200 Subject: [PATCH 4/7] ci: Unify character for separating items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the test names are using `;` and regexps were designed to use `,` but during development simply joined the expressions by `|`. This should work but might be confusing so let's go with the semi-colon separator everywhere. Signed-off-by: Lukáš Doktor --- tools/testing/gatekeeper/jobs.py | 4 ++-- tools/testing/gatekeeper/skips.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/testing/gatekeeper/jobs.py b/tools/testing/gatekeeper/jobs.py index 94b6904293..5a5db4e723 100644 --- a/tools/testing/gatekeeper/jobs.py +++ b/tools/testing/gatekeeper/jobs.py @@ -45,7 +45,7 @@ class Checker: self.latest_commit_sha = os.getenv("COMMIT_HASH") required_jobs = os.getenv("REQUIRED_JOBS") if required_jobs: - required_jobs = required_jobs.split(",") + required_jobs = required_jobs.split(";") else: required_jobs = [] required_regexps = os.getenv("REQUIRED_REGEXPS") @@ -53,7 +53,7 @@ class Checker: # TODO: Add way to specify minimum amount of tests # (eg. via \d+: prefix) and check it in status if required_regexps: - for regexp in required_regexps.split(","): + for regexp in required_regexps.split(";"): self.required_regexps.append(re.compile(regexp)) if not required_jobs and not self.required_regexps: raise RuntimeError("No REQUIRED_JOBS or REQUIRED_REGEXPS defined") diff --git a/tools/testing/gatekeeper/skips.py b/tools/testing/gatekeeper/skips.py index dcb6920094..1f08af18d7 100644 --- a/tools/testing/gatekeeper/skips.py +++ b/tools/testing/gatekeeper/skips.py @@ -66,7 +66,7 @@ class Checks: if values.get("regexps"): required_regexps.add(values["regexps"]) print(';'.join(required_tests)) - print('|'.join(required_regexps)) + print(';'.join(required_regexps)) return 0 def get_features(self, target_branch): From 2440a39c50757d055bde0a402befeb34d1279519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Doktor?= Date: Thu, 18 Jul 2024 13:42:40 +0200 Subject: [PATCH 5/7] ci: Check required lables before checking tests in gatekeeper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit some tests require certain labels before they are executed. When our PR is not labeled appropriately the gatekeeper detects skipped required tests and reports a failure. With this change we add "required-labeles" to the tests mapping and check the expected labels first informing the user about the missing labeles before even checking the test statuses. Signed-off-by: Lukáš Doktor --- .github/workflows/gatekeeper.yaml | 3 + tools/testing/gatekeeper/jobs.py | 73 ++++++++++++++++---- tools/testing/gatekeeper/required-tests.yaml | 6 ++ tools/testing/gatekeeper/skips.py | 4 ++ 4 files changed, 71 insertions(+), 15 deletions(-) diff --git a/.github/workflows/gatekeeper.yaml b/.github/workflows/gatekeeper.yaml index 6dc741c255..0cb7396d70 100644 --- a/.github/workflows/gatekeeper.yaml +++ b/.github/workflows/gatekeeper.yaml @@ -29,13 +29,16 @@ jobs: TARGET_BRANCH: ${{ github.event.pull_request.base.ref }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} COMMIT_HASH: ${{ github.event.pull_request.head.sha }} + PR_NUMBER: ${{ github.event.pull_request.number }} run: | #!/usr/bin/env bash -x mapfile -t lines < <(python3 tools/testing/gatekeeper/skips.py -t) export REQUIRED_JOBS="${lines[0]}" export REQUIRED_REGEXPS="${lines[1]}" + export REQUIRED_LABELS="${lines[2]}" echo "REQUIRED_JOBS: $REQUIRED_JOBS" echo "REQUIRED_REGEXPS: $REQUIRED_REGEXPS" + echo "REQUIRED_LABELS: $REQUIRED_LABELS" python3 tools/testing/gatekeeper/jobs.py exit $? shell: /usr/bin/bash -x {0} diff --git a/tools/testing/gatekeeper/jobs.py b/tools/testing/gatekeeper/jobs.py index 5a5db4e723..f62b43c13e 100644 --- a/tools/testing/gatekeeper/jobs.py +++ b/tools/testing/gatekeeper/jobs.py @@ -14,10 +14,10 @@ Env variables: * GITHUB_REPOSITORY: Github repository (user/repo) Sample execution (GH token can be excluded): GITHUB_TOKEN="..." REQUIRED_JOBS="skipper / skipper" -REQUIRED_REGEXPS=".*" +REQUIRED_REGEXPS=".*" REQUIRED_LABELS="ok-to-test;bar" COMMIT_HASH=b8382cea886ad9a8f77d237bcfc0eba0c98775dd GITHUB_REPOSITORY=kata-containers/kata-containers -python3 jobs.py +PR_NUMBER=123 python3 jobs.py """ import os @@ -35,14 +35,20 @@ RUNNING = 127 _GH_HEADERS = {"Accept": "application/vnd.github.v3+json"} if os.environ.get("GITHUB_TOKEN"): _GH_HEADERS["Authorization"] = f"token {os.environ['GITHUB_TOKEN']}" -_GH_RUNS_URL = ("https://api.github.com/repos/" - f"{os.environ['GITHUB_REPOSITORY']}/actions/runs") +_GH_API_URL = f"https://api.github.com/repos/{os.environ['GITHUB_REPOSITORY']}" +_GH_RUNS_URL = f"{_GH_API_URL}/actions/runs" class Checker: """Object to keep watching required GH action workflows""" def __init__(self): self.latest_commit_sha = os.getenv("COMMIT_HASH") + self.pr_number = os.getenv("PR_NUMBER") + required_labels = os.getenv("REQUIRED_LABELS") + if required_labels: + self.required_labels = set(required_labels.split(";")) + else: + self.required_labels = [] required_jobs = os.getenv("REQUIRED_JOBS") if required_jobs: required_jobs = required_jobs.split(";") @@ -131,11 +137,9 @@ class Checker: jobs = [] page = 1 while True: - response = requests.get( - f"{_GH_RUNS_URL}/{run_id}/jobs?per_page=100&page={page}", - headers=_GH_HEADERS, - timeout=60 - ) + url = f"{_GH_RUNS_URL}/{run_id}/jobs?per_page=100&page={page}" + print(url, file=sys.stderr) + response = requests.get(url, headers=_GH_HEADERS, timeout=60) response.raise_for_status() output = response.json() jobs.extend(output["jobs"]) @@ -168,14 +172,12 @@ class Checker: print(self) return self.status() - def run(self): + def wait_for_required_tests(self): """ - Keep checking the PR until all required jobs finish + Wait for all required tests to pass or failfast - :returns: 0 on success; 1 on failure + :return: 0 - all passing; 1 - any failure """ - print(f"Gatekeeper for project={os.environ['GITHUB_REPOSITORY']} and " - f"SHA={self.latest_commit_sha}") while True: ret = self.check_workflow_runs_status() if ret == RUNNING: @@ -185,7 +187,48 @@ class Checker: print(f"{running_jobs} jobs are still running...") time.sleep(180) continue - sys.exit(ret) + return ret + + def check_required_labels(self): + """ + Check if all expected labels are present + + :return: True on success, False on failure + """ + if not self.required_labels: + # No required labels, exit + return True + + if not self.pr_number: + print("The PR_NUMBER not specified, skipping the " + f"required-labels-check ({self.required_labels})") + return True + + response = requests.get( + f"{_GH_API_URL}/issues/{self.pr_number}", + headers=_GH_HEADERS, + timeout=60 + ) + response.raise_for_status() + labels = set(_["name"] for _ in response.json()["labels"]) + if self.required_labels.issubset(labels): + return True + print(f"To run all required tests the PR{self.pr_number} must have " + f"{', '.join(self.required_labels.difference(labels))} labels " + "set.") + return False + + def run(self): + """ + Keep checking the PR until all required jobs finish + + :returns: 0 on success; 1 on check failure; 2 when PR missing labels + """ + print(f"Gatekeeper for project={os.environ['GITHUB_REPOSITORY']} and " + f"SHA={self.latest_commit_sha} PR={self.pr_number}") + if not self.check_required_labels(): + sys.exit(2) + sys.exit(self.wait_for_required_tests()) if __name__ == "__main__": diff --git a/tools/testing/gatekeeper/required-tests.yaml b/tools/testing/gatekeeper/required-tests.yaml index 943af6e3fc..db34033347 100644 --- a/tools/testing/gatekeeper/required-tests.yaml +++ b/tools/testing/gatekeeper/required-tests.yaml @@ -26,11 +26,17 @@ mapping: - Kata Containers CI / kata-containers-ci-on-push / run-k8s-tests-on-ppc64le / run-k8s-tests (qemu, kubeadm) - Kata Containers CI / kata-containers-ci-on-push / run-k8s-tests-on-aks / run-k8s-tests (ubuntu, qemu, small) - Kata Containers CI / kata-containers-ci-on-push / run-k8s-tests-with-crio-on-garm / run-k8s-tests (qemu, k0s, garm-ubuntu-2204) + required-labels: + - ok-to-test # TODO: Add support for "depends" to automatically add dependant set-of-tests # (eg. "build" is required for "test") build: # Checks that the kata-containers static tarball is created regexps: "Kata Containers CI / .*build-kata-static-tarball.*" + required-labels: + - ok-to-test static: # Checks that static checks are passing regexps: "Static checks.*" + required-labels: + - ok-to-test diff --git a/tools/testing/gatekeeper/skips.py b/tools/testing/gatekeeper/skips.py index 1f08af18d7..d639f9be8a 100644 --- a/tools/testing/gatekeeper/skips.py +++ b/tools/testing/gatekeeper/skips.py @@ -59,14 +59,18 @@ class Checks: return 0 required_tests = set(self.required_tests) required_regexps = set(self.required_regexps) + required_labels = set() for feature in enabled_features: values = self.mapping.get(feature, {}) if values.get("names"): required_tests.update(values["names"]) if values.get("regexps"): required_regexps.add(values["regexps"]) + if values.get("required-labels"): + required_labels.update(values["required-labels"]) print(';'.join(required_tests)) print(';'.join(required_regexps)) + print(';'.join(required_labels)) return 0 def get_features(self, target_branch): From 2ae090b44b128c88fa93050ed53dda989c41a1d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Doktor?= Date: Thu, 18 Jul 2024 13:44:10 +0200 Subject: [PATCH 6/7] ci: Add extra gatekeeper debug output to stderr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit which might be useful to assess the amount of querries. Signed-off-by: Lukáš Doktor --- tools/testing/gatekeeper/jobs.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/testing/gatekeeper/jobs.py b/tools/testing/gatekeeper/jobs.py index f62b43c13e..9fcaa85dbb 100644 --- a/tools/testing/gatekeeper/jobs.py +++ b/tools/testing/gatekeeper/jobs.py @@ -79,6 +79,8 @@ class Checker: else: # Not a required job return + print(f"{job_name} - {job['status']} {job['conclusion']} {job['id']}", + file=sys.stderr) if job["status"] != "completed": self.results[job_name] = RUNNING return @@ -156,6 +158,7 @@ class Checker: :returns: 0 - all passing; 1 - any failure; 127 some jobs running """ # TODO: Check if we need pagination here as well + print(_GH_RUNS_URL, file=sys.stderr) response = requests.get( _GH_RUNS_URL, params={"head_sha": self.latest_commit_sha}, From 63b6e8a21589393b5e66d45914dda34384dfa5e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Doktor?= Date: Wed, 24 Jul 2024 09:10:05 +0200 Subject: [PATCH 7/7] ci: Ensure we check the latest workflow run in gatekeeper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit with multiple iterations/reruns we need to use the latest run of each workflow. For that we can use the "run_id" and only update results of the same or newer run_ids. To do that we need to store the "run_id". To avoid adding individual attributes this commit stores the full job object that contains the status, conclussion as well as other attributes of the individual jobs, which might come handy in the future in exchange for slightly bigger memory overhead (still we only store the latest run of required jobs only). Signed-off-by: Lukáš Doktor --- tools/testing/gatekeeper/jobs.py | 46 ++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/tools/testing/gatekeeper/jobs.py b/tools/testing/gatekeeper/jobs.py index 9fcaa85dbb..e6cc120801 100644 --- a/tools/testing/gatekeeper/jobs.py +++ b/tools/testing/gatekeeper/jobs.py @@ -63,8 +63,9 @@ class Checker: self.required_regexps.append(re.compile(regexp)) if not required_jobs and not self.required_regexps: raise RuntimeError("No REQUIRED_JOBS or REQUIRED_REGEXPS defined") - # Set all required jobs as RUNNING to enforce waiting for them - self.results = {job: RUNNING for job in required_jobs} + # Set all required jobs as EXPECTED to enforce waiting for them + self.results = {job: {"status": "EXPECTED", "run_id": -1} + for job in required_jobs} def record(self, workflow, job): """ @@ -79,15 +80,25 @@ class Checker: else: # Not a required job return + # TODO: Check if multiple re-runs use the same "run_id". If so use + # job['run_attempt'] in case of matching "run_id". + elif job['run_id'] <= self.results[job_name]['run_id']: + # Newer results already stored + print(f"older {job_name} - {job['status']} {job['conclusion']} " + f"{job['id']}", file=sys.stderr) + return print(f"{job_name} - {job['status']} {job['conclusion']} {job['id']}", file=sys.stderr) + self.results[job_name] = job + + @staticmethod + def _job_status(job): + """Map job status to our status""" if job["status"] != "completed": - self.results[job_name] = RUNNING - return + return RUNNING if job["conclusion"] != "success": - self.results[job_name] = job['conclusion'] - return - self.results[job_name] = PASS + return job['conclusion'] + return PASS def status(self): """ @@ -98,7 +109,8 @@ class Checker: if not self.results: # No results reported so far return FAIL - for status in self.results.values(): + for job in self.results.values(): + status = self._job_status(job) if status == RUNNING: running |= True elif status != PASS: @@ -113,13 +125,14 @@ class Checker: good = [] bad = [] warn = [] - for job, status in self.results.items(): + for name, job in self.results.items(): + status = self._job_status(job) if status == RUNNING: - warn.append(f"WARN: {job} - Still running") + warn.append(f"WARN: {name} - Still running") elif status == PASS: - good.append(f"PASS: {job} - success") + good.append(f"PASS: {name} - success") else: - bad.append(f"FAIL: {job} - Not passed - {status}") + bad.append(f"FAIL: {name} - Not passed - {status}") out = '\n'.join(sorted(good) + sorted(warn) + sorted(bad)) stat = self.status() if stat == RUNNING: @@ -167,8 +180,7 @@ class Checker: ) response.raise_for_status() workflow_runs = response.json()["workflow_runs"] - - for run in workflow_runs: + for i, run in enumerate(workflow_runs): jobs = self.get_jobs_for_workflow_run(run["id"]) for job in jobs: self.record(run["name"], job) @@ -184,9 +196,9 @@ class Checker: while True: ret = self.check_workflow_runs_status() if ret == RUNNING: - running_jobs = len([job - for job, status in self.results.items() - if status == RUNNING]) + running_jobs = len([name + for name, job in self.results.items() + if self._job_status(job) == RUNNING]) print(f"{running_jobs} jobs are still running...") time.sleep(180) continue