This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new f97d1fbe16 Add better diagnostics when tests fail (#33023)
f97d1fbe16 is described below
commit f97d1fbe1621b09bc36d2b713dde7399e2283335
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.
---
.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
