Skip to content

Commit

Permalink
Optimize-away PROD image build in many cases (apache#35439)
Browse files Browse the repository at this point in the history
We were always building PROD image when we built CI image on CI as
we had only one flag "build-images" in selective checks. This was
based on an assumption that we want to still run docker compose
tests with airflow when any of our sources change - just to see if
it still works.

While "docker-compose" tests are the only end-to-end tests it is
very unlikely that unit and integration tests passing will trigger
a docker compose test failure.

This of course might happen, but if it happens, we will find out during
our canary builds that run all the tests and building PROD image in this
case is a waste of resources and time - waiting for docker-compose tests
to complete when we really do not have to do it makes no sense either.
Even if those tests are relatively quick, just waiting for PROD images
being built (PROD images have to wait for CI images in order to get
constraints from CI builds) adds quite a bit of elapsed test time for
many smaller PRs that do not change k8s or Helm in any way.

This change splits "image-build" into two outputs of selective checks:
ci-image-build and prod-image-build. The CI-image build is the same
as before - we will build CI image whenever any test need to use the
image and run our code (so for example it is false when only
README files change). PROD image build is set to true now only when
we need kubernetes tests (i.e. when k8s provider changes) or when chart
changes - because there we want to test basic functionality of k8s
integration and Helm deployment/running.
  • Loading branch information
potiuk authored Nov 4, 2023
1 parent f50a34b commit d54f302
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 52 deletions.
9 changes: 5 additions & 4 deletions .github/workflows/build-images.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ jobs:
default-python-version: ${{ steps.selective-checks.outputs.default-python-version }}
run-tests: ${{ steps.selective-checks.outputs.run-tests }}
run-kubernetes-tests: ${{ steps.selective-checks.outputs.run-kubernetes-tests }}
image-build: ${{ steps.selective-checks.outputs.image-build }}
ci-image-build: ${{ steps.selective-checks.outputs.ci-image-build }}
prod-image-build: ${{ steps.selective-checks.outputs.prod-image-build }}
cache-directive: ${{ steps.selective-checks.outputs.cache-directive }}
default-branch: ${{ steps.selective-checks.outputs.default-branch }}
default-constraints-branch: ${{ steps.selective-checks.outputs.default-constraints-branch }}
Expand Down Expand Up @@ -192,7 +193,7 @@ jobs:
runs-on: ${{fromJSON(needs.build-info.outputs.runs-on)}}
needs: [build-info]
if: |
needs.build-info.outputs.image-build == 'true' &&
needs.build-info.outputs.ci-image-build == 'true' &&
github.event.pull_request.head.repo.full_name != 'apache/airflow' &&
github.repository == 'apache/airflow'
env:
Expand Down Expand Up @@ -265,7 +266,7 @@ jobs:
runs-on: ${{fromJSON(needs.build-info.outputs.runs-on)}}
needs: [build-info, build-ci-images]
if: |
needs.build-info.outputs.image-build == 'true' &&
needs.build-info.outputs.prod-image-build == 'true' &&
github.event.pull_request.head.repo.full_name != 'apache/airflow' &&
github.repository == 'apache/airflow'
env:
Expand Down Expand Up @@ -322,7 +323,7 @@ jobs:
needs: [build-info, build-prod-images]
# We can change the job to run on ASF ARM runners and do not start our instance once we enable ASF runners
if: |
needs.build-info.outputs.image-build == 'true' &&
needs.build-info.outputs.ci-image-build == 'true' &&
needs.build-info.outputs.upgrade-to-newer-dependencies != 'false' &&
github.event.pull_request.head.repo.full_name != 'apache/airflow' &&
needs.build-info.outputs.is-self-hosted-runner == 'true' &&
Expand Down
16 changes: 8 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ jobs:
run-www-tests: ${{ steps.selective-checks.outputs.run-www-tests }}
run-kubernetes-tests: ${{ steps.selective-checks.outputs.run-kubernetes-tests }}
basic-checks-only: ${{ steps.selective-checks.outputs.basic-checks-only }}
image-build: ${{ steps.selective-checks.outputs.image-build }}
ci-image-build: ${{ steps.selective-checks.outputs.ci-image-build }}
prod-image-build: ${{ steps.selective-checks.outputs.prod-image-build }}
docs-build: ${{ steps.selective-checks.outputs.docs-build }}
needs-helm-tests: ${{ steps.selective-checks.outputs.needs-helm-tests }}
needs-api-tests: ${{ steps.selective-checks.outputs.needs-api-tests }}
Expand Down Expand Up @@ -344,7 +345,7 @@ jobs:
RUNS_ON: "${{ needs.build-info.outputs.runs-on }}"
PYTHON_VERSIONS: ${{needs.build-info.outputs.all-python-versions-list-as-string}}
DEBUG_RESOURCES: ${{needs.build-info.outputs.debug-resources}}
if: needs.build-info.outputs.image-build == 'true'
if: needs.build-info.outputs.ci-image-build == 'true'
steps:
- name: Cleanup repo
run: docker run -v "${GITHUB_WORKSPACE}:/workspace" -u 0:0 bash -c "rm -rf /workspace/*"
Expand Down Expand Up @@ -505,7 +506,7 @@ jobs:
name: "Test examples of production image building"
runs-on: ${{fromJSON(needs.build-info.outputs.runs-on)}}
needs: [build-info]
if: needs.build-info.outputs.image-build == 'true'
if: needs.build-info.outputs.ci-image-build == 'true'
steps:
- name: Cleanup repo
run: docker run -v "${GITHUB_WORKSPACE}:/workspace" -u 0:0 bash -c "rm -rf /workspace/*"
Expand Down Expand Up @@ -543,7 +544,7 @@ jobs:
name: "Wait for CI images"
runs-on: "ubuntu-22.04"
needs: [build-info, build-ci-images]
if: needs.build-info.outputs.image-build == 'true'
if: needs.build-info.outputs.ci-image-build == 'true'
env:
RUNS_ON: "${{needs.build-info.outputs.runs-on}}"
BACKEND: sqlite
Expand Down Expand Up @@ -1613,7 +1614,7 @@ jobs:
name: "Wait for & verify PROD images"
runs-on: ${{fromJSON(needs.build-info.outputs.runs-on)}}
needs: [build-info, wait-for-ci-images, build-prod-images]
if: needs.build-info.outputs.image-build == 'true'
if: needs.build-info.outputs.prod-image-build == 'true'
env:
RUNS_ON: "${{needs.build-info.outputs.runs-on}}"
BACKEND: sqlite
Expand Down Expand Up @@ -1655,7 +1656,7 @@ jobs:
name: "Test docker-compose quick start"
runs-on: ${{fromJSON(needs.build-info.outputs.runs-on)}}
needs: [build-info, wait-for-prod-images]
if: needs.build-info.outputs.image-build == 'true'
if: needs.build-info.outputs.prod-image-build == 'true'
env:
PYTHON_MAJOR_MINOR_VERSION: "${{needs.build-info.outputs.default-python-version}}"
steps:
Expand Down Expand Up @@ -1695,8 +1696,7 @@ jobs:
DEBUG_RESOURCES: ${{needs.build-info.outputs.debug-resources}}
if: >
( needs.build-info.outputs.run-kubernetes-tests == 'true' ||
needs.build-info.outputs.needs-helm-tests == 'true' ) &&
needs.build-info.outputs.default-branch == 'main'
needs.build-info.outputs.needs-helm-tests == 'true')
steps:
- name: Cleanup repo
run: docker run -v "${GITHUB_WORKSPACE}:/workspace" -u 0:0 bash -c "rm -rf /workspace/*"
Expand Down
3 changes: 2 additions & 1 deletion dev/breeze/SELECTIVE_CHECKS.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ Github Actions to pass the list of parameters to a command to execute
| docs-list-as-string | What filter to apply to docs building - based on which documentation packages should be built | apache-airflow helm-chart google | |
| full-tests-needed | Whether this build runs complete set of tests or only subset (for faster PR builds) | false | |
| helm-version | Which Helm version to use for tests | v3.9.4 | |
| image-build | Whether CI image build is needed | true | |
| ci-image-build | Whether CI image build is needed | true | |
| prod-image-build | Whether PROD image build is needed | true | |
| kind-version | Which Kind version to use for tests | v0.16.0 | |
| kubernetes-combos-list-as-string | All combinations of Python version and Kubernetes version to use for tests as space-separated string | 3.8-v1.25.2 3.9-v1.26.4 | * |
| kubernetes-versions | All Kubernetes versions to use for tests as JSON array | ['v1.25.2'] | |
Expand Down
8 changes: 6 additions & 2 deletions dev/breeze/src/airflow_breeze/utils/selective_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,9 +563,13 @@ def run_tests(self) -> bool:
return self._should_be_run(FileGroupForCi.ALL_SOURCE_FILES)

@cached_property
def image_build(self) -> bool:
def ci_image_build(self) -> bool:
return self.run_tests or self.docs_build or self.run_kubernetes_tests or self.needs_helm_tests

@cached_property
def prod_image_build(self) -> bool:
return self.run_kubernetes_tests or self.needs_helm_tests

def _select_test_type_if_matching(
self, test_types: set[str], test_type: SelectiveUnitTestTypes
) -> list[str]:
Expand Down Expand Up @@ -691,7 +695,7 @@ def parallel_test_types_list_as_string(self) -> str | None:

@cached_property
def basic_checks_only(self) -> bool:
return not self.image_build
return not self.ci_image_build

@cached_property
def upgrade_to_newer_dependencies(self) -> bool:
Expand Down
Loading

0 comments on commit d54f302

Please sign in to comment.