This is an automated email from the ASF dual-hosted git repository. ephraimanierobi pushed a commit to branch v2-7-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 35b22bf5a72b1e75072152e1cd4047dfdb9f606a Author: Jarek Potiuk <[email protected]> AuthorDate: Thu Aug 3 18:32:31 2023 +0200 Add better diagnostics when tests fail (#33023) The "post_test" action had not worked as intended when there was a test failure and did not upload dump of the container logs. It turned out that the "if" conditions in the sub-action do not propagate from the parent action, so "failure()" condition was never met. This PR spplits post-test actions to "success" and "failure" ones, and triggers each action respectively when the tests succeed or fail. It also includes two fixes that makes CI debugging better: * only logs from failed test suite are includeded in the dump (previously all currently running containers were included) * we need to give some time for logs to propagate in case of errors. (cherry picked from commit f97d1fbe1621b09bc36d2b713dde7399e2283335) --- .github/actions/migration_tests/action.yml | 19 ++++ .../{post_tests => post_tests_failure}/action.yml | 26 +---- .../{post_tests => post_tests_success}/action.yml | 19 +--- .github/workflows/ci.yml | 115 +++++++++++++-------- .../airflow_breeze/commands/testing_commands.py | 28 +++-- scripts/ci/docker-compose/_docker.env | 2 + 6 files changed, 122 insertions(+), 87 deletions(-) diff --git a/.github/actions/migration_tests/action.yml b/.github/actions/migration_tests/action.yml index cd1a424099..a1d098713a 100644 --- a/.github/actions/migration_tests/action.yml +++ b/.github/actions/migration_tests/action.yml @@ -28,6 +28,13 @@ runs: airflow db migrate --to-revision heads && airflow db downgrade -r e959f08ac86c -y && airflow db migrate" + env: + COMPOSE_PROJECT_NAME: "docker-compose" + - name: "Bring composer down" + shell: bash + run: breeze down + env: + COMPOSE_PROJECT_NAME: "docker-compose" - name: "Test downgrade ORM ${{env.BACKEND}}" shell: bash run: > @@ -35,3 +42,15 @@ runs: airflow db migrate && airflow db downgrade -r e959f08ac86c -y && airflow db migrate" + COMPOSE_PROJECT_NAME: "docker-compose" + env: + COMPOSE_PROJECT_NAME: "docker-compose" + - name: "Bring any containers left down" + shell: bash + run: breeze down + env: + COMPOSE_PROJECT_NAME: "docker-compose" + - name: "Dump logs on failure ${{env.BACKEND}}" + shell: bash + run: docker ps -q | xargs docker logs + if: failure() diff --git a/.github/actions/post_tests/action.yml b/.github/actions/post_tests_failure/action.yml similarity index 61% copy from .github/actions/post_tests/action.yml copy to .github/actions/post_tests_failure/action.yml index b7c60aef3b..96e43bbe0e 100644 --- a/.github/actions/post_tests/action.yml +++ b/.github/actions/post_tests_failure/action.yml @@ -16,45 +16,29 @@ # under the License. # --- -name: 'Post tests' -description: 'Run post tests actions' +name: 'Post tests on failure' +description: 'Run post tests actions on failure' runs: using: "composite" steps: - name: "Upload airflow logs" uses: actions/upload-artifact@v3 - if: failure() with: name: airflow-logs-${{env.JOB_ID}} path: './files/airflow_logs*' retention-days: 7 - name: "Upload container logs" uses: actions/upload-artifact@v3 - if: failure() with: name: container-logs-${{env.JOB_ID}} path: "./files/container_logs*" retention-days: 7 - - name: "Upload artifact for warnings" + - name: "Upload other logs" uses: actions/upload-artifact@v3 with: - name: test-warnings-${{env.JOB_ID}} - path: ./files/warnings-*.txt + name: container-logs-${{env.JOB_ID}} + path: "./files/other_logs*" retention-days: 7 - - name: "Move coverage artifacts in separate directory" - if: env.COVERAGE == 'true' && env.TEST_TYPES != 'Helm' - shell: bash - run: | - mkdir ./files/coverage-reposts - mv ./files/coverage*.xml ./files/coverage-reposts/ || true - - name: "Upload all coverage reports to codecov" - uses: codecov/codecov-action@v3 - if: env.COVERAGE == 'true' && env.TEST_TYPES != 'Helm' - with: - name: coverage-${{env.JOB_ID}} - flags: python-${{env.PYTHON_MAJOR_MINOR_VERSION}},${{env.BACKEND}}-${{env.BACKEND_VERSION}} - directory: "./files/coverage-reposts/" - name: "Fix ownership" shell: bash run: breeze ci fix-ownership - if: always() diff --git a/.github/actions/post_tests/action.yml b/.github/actions/post_tests_success/action.yml similarity index 77% rename from .github/actions/post_tests/action.yml rename to .github/actions/post_tests_success/action.yml index b7c60aef3b..0d81eca60f 100644 --- a/.github/actions/post_tests/action.yml +++ b/.github/actions/post_tests_success/action.yml @@ -16,25 +16,11 @@ # under the License. # --- -name: 'Post tests' -description: 'Run post tests actions' +name: 'Post tests on success' +description: 'Run post tests actions on success' runs: using: "composite" steps: - - name: "Upload airflow logs" - uses: actions/upload-artifact@v3 - if: failure() - with: - name: airflow-logs-${{env.JOB_ID}} - path: './files/airflow_logs*' - retention-days: 7 - - name: "Upload container logs" - uses: actions/upload-artifact@v3 - if: failure() - with: - name: container-logs-${{env.JOB_ID}} - path: "./files/container_logs*" - retention-days: 7 - name: "Upload artifact for warnings" uses: actions/upload-artifact@v3 with: @@ -57,4 +43,3 @@ runs: - name: "Fix ownership" shell: bash run: breeze ci fix-ownership - if: always() diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bba72c39e1..eedb6a3665 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -907,8 +907,14 @@ jobs: uses: ./.github/actions/prepare_breeze_and_image - name: "Helm Unit Tests: ${{ matrix.helm-test-package }}" run: breeze testing helm-tests --helm-test-package "${{ matrix.helm-test-package }}" - - name: "Post Helm Tests" - uses: ./.github/actions/post_tests + - name: > + Post Tests success: Helm" + uses: ./.github/actions/post_tests_success + if: success() + - name: > + Post Tests failure: Helm" + uses: ./.github/actions/post_tests_failure + if: failure() tests-postgres: timeout-minutes: 130 @@ -957,9 +963,13 @@ jobs: run: breeze testing tests --run-in-parallel --collect-only --remove-arm-packages if: matrix.postgres-version == needs.build-info.outputs.default-postgres-version - name: > - Post Tests: - ${{matrix.python-version}}:${{needs.build-info.outputs.parallel-test-types-list-as-string}} - uses: ./.github/actions/post_tests + Post Tests success: Postgres" + uses: ./.github/actions/post_tests_success + if: success() + - name: > + Post Tests failure: Postgres" + uses: ./.github/actions/post_tests_failure + if: failure() tests-postgres-boto: timeout-minutes: 130 @@ -1002,9 +1012,13 @@ jobs: ${{needs.build-info.outputs.parallel-test-types-list-as-string}} run: breeze testing tests --run-in-parallel - name: > - Post Tests: ${{needs.build-info.outputs.default-python-version}}: - ${{needs.build-info.outputs.parallel-test-types-list-as-string}} - uses: ./.github/actions/post_tests + Post Tests success: ${{needs.build-info.outputs.default-python-version}}:Boto" + uses: ./.github/actions/post_tests_success + if: success() + - name: > + Post Tests failure: ${{needs.build-info.outputs.default-python-version}}:Boto" + uses: ./.github/actions/post_tests_failure + if: failure() tests-postgres-in-progress-features-disabled: timeout-minutes: 130 @@ -1047,9 +1061,13 @@ jobs: ${{needs.build-info.outputs.parallel-test-types-list-as-string}} run: breeze testing tests --run-in-parallel - name: > - Post Tests: ${{needs.build-info.outputs.default-python-version}}: - ${{needs.build-info.outputs.parallel-test-types-list-as-string}} - uses: ./.github/actions/post_tests + Post Tests success: ${{needs.build-info.outputs.default-python-version}}:FeaturesDisabled" + uses: ./.github/actions/post_tests_success + if: success() + - name: > + Post Tests failure: ${{needs.build-info.outputs.default-python-version}}:FeaturesDisabled" + uses: ./.github/actions/post_tests_failure + if: failure() tests-mysql: timeout-minutes: 130 @@ -1094,9 +1112,14 @@ jobs: Tests: ${{matrix.python-version}}:${{needs.build-info.outputs.parallel-test-types-list-as-string}} run: breeze testing tests --run-in-parallel - name: > - Post Tests: - ${{matrix.python-version}}:${{needs.build-info.outputs.parallel-test-types-list-as-string}} - uses: ./.github/actions/post_tests + Post Tests success: MySQL" + uses: ./.github/actions/post_tests_success + if: success() + - name: > + Post Tests failure: MySQL" + uses: ./.github/actions/post_tests_failure + if: failure() + tests-mssql: timeout-minutes: 130 @@ -1149,9 +1172,13 @@ jobs: Tests: ${{matrix.python-version}}:${{needs.build-info.outputs.parallel-test-types-list-as-string}} run: breeze testing tests --run-in-parallel - name: > - Post Tests: - ${{matrix.python-version}}:${{needs.build-info.outputs.parallel-test-types-list-as-string}} - uses: ./.github/actions/post_tests + Post Tests success: MsSQL" + uses: ./.github/actions/post_tests_success + if: success() + - name: > + Post Tests failure: MsSQL" + uses: ./.github/actions/post_tests_failure + if: failure() tests-sqlite: timeout-minutes: 130 @@ -1194,9 +1221,13 @@ jobs: Tests: ${{matrix.python-version}}:${{needs.build-info.outputs.parallel-test-types-list-as-string}} run: breeze testing tests --run-in-parallel - name: > - Post Tests: - ${{matrix.python-version}}:${{needs.build-info.outputs.parallel-test-types-list-as-string}} - uses: ./.github/actions/post_tests + Post Tests success: Sqlite" + uses: ./.github/actions/post_tests_success + if: success() + - name: > + Post Tests failure: Sqlite" + uses: ./.github/actions/post_tests_failure + if: failure() tests-integration-postgres: timeout-minutes: 130 @@ -1261,9 +1292,14 @@ jobs: run: breeze testing integration-tests --integration all-testable if: needs.build-info.outputs.runs-on == 'self-hosted' - name: > - Post Tests: - ${{matrix.python-version}}:${{needs.build-info.outputs.parallel-test-types-list-as-string}} - uses: ./.github/actions/post_tests + Post Tests success: Integration Postgres" + uses: ./.github/actions/post_tests_success + if: success() + - name: > + Post Tests failure: Integration Postgres" + uses: ./.github/actions/post_tests_failure + if: failure() + tests-integration-mysql: timeout-minutes: 130 @@ -1301,10 +1337,13 @@ jobs: run: breeze testing integration-tests --integration all-testable if: needs.build-info.outputs.runs-on == 'self-hosted' - name: > - Post Tests: - ${{matrix.python-version}}:${{needs.build-info.outputs.parallel-test-types-list-as-string}} - uses: ./.github/actions/post_tests - if: needs.build-info.outputs.runs-on == 'self-hosted' + Post Tests success: Integration MySQL" + uses: ./.github/actions/post_tests_success + if: success() && needs.build-info.outputs.runs-on == 'self-hosted' + - name: > + Post Tests failure: Integration MySQL" + uses: ./.github/actions/post_tests_failure + if: failure() && needs.build-info.outputs.runs-on == 'self-hosted' tests-quarantined: @@ -1340,9 +1379,6 @@ jobs: BACKEND: "postgres" BACKEND_VERSION: "11" POSTGRES_VERSION: "11" - - name: > - Cleaning up ${{needs.build-info.outputs.default-python-version}}:Quarantined - run: breeze down - name: > Tests: mysql:${{needs.build-info.outputs.default-python-version}}:Quarantined run: breeze testing tests || true @@ -1350,9 +1386,6 @@ jobs: BACKEND: "mysql" BACKEND_VERSION: ${{needs.build-info.outputs.default-mysql-version}} MYSQL_VERSION: ${{needs.build-info.outputs.default-mysql-version}} - - name: > - Cleaning up ${{needs.build-info.outputs.default-python-version}}:Quarantined - run: breeze down - name: > Tests: mssql:${{needs.build-info.outputs.default-python-version}}:Quarantined run: breeze testing tests || true @@ -1361,21 +1394,21 @@ jobs: BACKEND_VERSION: ${{needs.build-info.outputs.default-mssql-version}} MSSQL_VERSION: ${{needs.build-info.outputs.default-mssql-version}} - name: > - Cleaning up ${{needs.build-info.outputs.default-python-version}}:Quarantined - run: breeze down - - name: > - Tests: mssql:${{needs.build-info.outputs.default-python-version}}:Quarantined + Tests: sqlite:${{needs.build-info.outputs.default-python-version}}:Quarantined run: breeze testing tests || true env: BACKEND: "sqlite" BACKEND_VERSION: "" MSSQL_VERSION: "" - name: > - Cleaning up ${{needs.build-info.outputs.default-python-version}}:Quarantined - run: breeze down + Post Tests success: Quarantined" + uses: ./.github/actions/post_tests_success + if: success() - name: > - Post Tests: ${{needs.build-info.outputs.default-python-version}}:Quarantined" - uses: ./.github/actions/post_tests + Post Tests failure: Quarantined" + uses: ./.github/actions/post_tests_failure + if: failure() + summarize-warnings: timeout-minutes: 15 diff --git a/dev/breeze/src/airflow_breeze/commands/testing_commands.py b/dev/breeze/src/airflow_breeze/commands/testing_commands.py index 8cf37487a9..d892727ee4 100644 --- a/dev/breeze/src/airflow_breeze/commands/testing_commands.py +++ b/dev/breeze/src/airflow_breeze/commands/testing_commands.py @@ -19,6 +19,7 @@ from __future__ import annotations import os import sys from datetime import datetime +from time import sleep import click from click import IntRange @@ -165,10 +166,13 @@ def _run_test( ) sys.exit(1) project_name = file_name_from_test_type(exec_shell_params.test_type) + compose_project_name = f"airflow-test-{project_name}" + # This is needed for Docker-compose 1 compatibility + env_variables["COMPOSE_PROJECT_NAME"] = compose_project_name down_cmd = [ *DOCKER_COMPOSE_COMMAND, "--project-name", - f"airflow-test-{project_name}", + compose_project_name, "down", "--remove-orphans", ] @@ -176,7 +180,7 @@ def _run_test( run_cmd = [ *DOCKER_COMPOSE_COMMAND, "--project-name", - f"airflow-test-{project_name}", + compose_project_name, "run", "-T", "--service-ports", @@ -185,7 +189,7 @@ def _run_test( ] run_cmd.extend(list(extra_pytest_args)) try: - remove_docker_networks(networks=[f"airflow-test-{project_name}_default"]) + remove_docker_networks(networks=[f"{compose_project_name}_default"]) result = run_command( run_cmd, env=env_variables, @@ -201,22 +205,30 @@ def _run_test( text=True, ) container_ids = ps_result.stdout.splitlines() + get_console(output=output).print("[info]Wait 10 seconds for logs to find their way to stderr.\n") + sleep(10) get_console(output=output).print( - f"[info]Error {ps_result.returncode}. Dumping containers: {container_ids}." + f"[info]Error {result.returncode}. Dumping containers: {container_ids} for {project_name}.\n" ) date_str = datetime.now().strftime("%Y_%d_%m_%H_%M_%S") for container_id in container_ids: + if compose_project_name not in container_id: + continue dump_path = FILES_DIR / f"container_logs_{container_id}_{date_str}.log" - get_console(output=output).print(f"[info]Dumping container {container_id} to {dump_path}") + get_console(output=output).print(f"[info]Dumping container {container_id} to {dump_path}\n") with open(dump_path, "w") as outfile: - run_command(["docker", "logs", container_id], check=False, stdout=outfile) + run_command( + ["docker", "logs", "--details", "--timestamps", container_id], + check=False, + stdout=outfile, + ) finally: if not skip_docker_compose_down: run_command( [ *DOCKER_COMPOSE_COMMAND, "--project-name", - f"airflow-test-{project_name}", + compose_project_name, "rm", "--stop", "--force", @@ -227,7 +239,7 @@ def _run_test( check=False, verbose_override=False, ) - remove_docker_networks(networks=[f"airflow-test-{project_name}_default"]) + remove_docker_networks(networks=[f"{compose_project_name}_default"]) return result.returncode, f"Test: {exec_shell_params.test_type}" diff --git a/scripts/ci/docker-compose/_docker.env b/scripts/ci/docker-compose/_docker.env index 72ecc84f34..2cd4bc0f7d 100644 --- a/scripts/ci/docker-compose/_docker.env +++ b/scripts/ci/docker-compose/_docker.env @@ -31,6 +31,8 @@ CI_TARGET_REPO CI_TARGET_BRANCH COLLECT_ONLY COMMIT_SHA +# Needed for docker-compose 1 compatibility +COMPOSE_PROJECT_NAME DB_RESET DEFAULT_BRANCH DEFAULT_CONSTRAINTS_BRANCH
