Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes retrieval of correct branch in non-master related builds #10912

Merged
merged 1 commit into from
Sep 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 31 additions & 5 deletions .github/workflows/build-images-workflow-run.yml
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ jobs:
needs: [cancel-workflow-runs]
strategy:
matrix:
python-version: [3.6, 3.7, 3.8]
python-version: [2.7, 3.5, 3.6, 3.7, 3.8]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you tell more about it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. It was added to the commit which I updated before but I have not updated the PR description. Synced now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, see the comment below :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In short workfow_run always runs from master - no matter which is the branch the PR originates from, so we have to have all the python version supported there, but I exclude those, that are not needed for the given "branch" (I base it on the DEFAULT_BRANCH which is in the incoming PR) -> if DEFAULT_BRANCH = master -> 3.6, 3.7, 3.8 are build, otherwise all five.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the dark side of "workffow_run" flow. We have to make sure that scripts in "master" can be used to build all the images that are used in all other branches, that's a bit annoying, but that's the price to pay for optimisation of the builds and "security".

image-type: [CI, PROD]
fail-fast: true
if: >
Expand All @@ -209,35 +209,61 @@ jobs:
uses: actions/checkout@v2
with:
ref: ${{ needs.cancel-workflow-runs.outputs.sourceHeadSha }}
- name: "Retrieve DEFAULTS from the _initialization.sh"
# We cannot "source" the script here because that would be a security problem (we cannot run
# any code that comes from the sources coming from the PR. Therefore we extract the
# DEFAULT_BRANCH and DEFAULT_CONSTRAINTS_BRANCH via custom grep/awk/sed commands
# Also 2.7 and 3.5 versions are not allowed to proceed on master
id: defaults
run: |
DEFAULT_BRANCH=$(grep "export DEFAULT_BRANCH" scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
ashb marked this conversation as resolved.
Show resolved Hide resolved
ashb marked this conversation as resolved.
Show resolved Hide resolved
echo "::set-env name=DEFAULT_BRANCH::${DEFAULT_BRANCH}"
DEFAULT_CONSTRAINTS_BRANCH=$(grep "export DEFAULT_CONSTRAINTS_BRANCH" \
scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "::set-env name=DEFAULT_CONSTRAINTS_BRANCH::${DEFAULT_CONSTRAINTS_BRANCH}"
if [[ \
${DEFAULT_BRANCH} != "master" || \
( ${PYTHON_MAJOR_MINOR_VERSION} != "2.7" && ${PYTHON_MAJOR_MINOR_VERSION} != "3.5" ) \
]]; then
echo "::set-output name=proceed::true"
else
echo "::set-output name=proceed::false"
fi
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} ) to 'main-airflow' to use main scripts"
uses: actions/checkout@v2
with:
path: "main-airflow"
if: steps.defaults.outputs.proceed == 'true'
- name: "Setup python"
uses: actions/setup-python@v2
with:
python-version: 3.6
if: steps.defaults.outputs.proceed == 'true'
- name: "Override 'scripts/ci' with the ${{ github.ref }} version so that the PR cannot override it."
# We should not override those scripts which become part of the image as they will not be
# changed in the image built - we should only override those that are executed to build
# the image.
run: |
rm -rf "scripts/ci"
mv "main-airflow/scripts/ci" "scripts"
if: steps.defaults.outputs.proceed == 'true'
- name: "Free space"
run: ./scripts/ci/tools/ci_free_space_on_ci.sh
if: steps.defaults.outputs.proceed == 'true'
- name: "Build CI images ${{ matrix.python-version }}:${{ github.event.workflow_run.id }}"
run: ./scripts/ci/images/ci_prepare_ci_image_on_ci.sh
if: matrix.image-type == 'CI'
if: matrix.image-type == 'CI' && steps.defaults.outputs.proceed == 'true'
- name: "Push CI images ${{ matrix.python-version }}:${{ github.event.workflow_run.id }}"
run: ./scripts/ci/images/ci_push_ci_images.sh
if: matrix.image-type == 'CI'
if: matrix.image-type == 'CI' && steps.defaults.outputs.proceed == 'true'
- name: "Build PROD images ${{ matrix.python-version }}:${{ github.event.workflow_run.id }}"
run: ./scripts/ci/images/ci_prepare_prod_image_on_ci.sh
if: matrix.image-type == 'PROD'
if: matrix.image-type == 'PROD' && steps.defaults.outputs.proceed == 'true'
- name: "Push PROD images ${{ matrix.python-version }}:${{ github.event.workflow_run.id }}"
run: ./scripts/ci/images/ci_push_production_images.sh
if: matrix.image-type == 'PROD'
if: matrix.image-type == 'PROD' && steps.defaults.outputs.proceed == 'true'
- name: "Canceling the CI Build source workflow in case of failure!"
if: cancelled() || failure()
uses: potiuk/cancel-workflow-runs@v2
Expand Down
6 changes: 6 additions & 0 deletions scripts/ci/libraries/_build_images.sh
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,10 @@ function build_images::build_ci_image() {
echo >&2
exit 1
fi
EXTRA_DOCKER_CI_BUILD_FLAGS=(
"--build-arg" "AIRFLOW_CONSTRAINTS_REFERENCE=${DEFAULT_CONSTRAINTS_BRANCH}"
)

if [[ -n ${SPIN_PID=} ]]; then
kill -HUP "${SPIN_PID}" || true
wait "${SPIN_PID}" || true
Expand All @@ -546,6 +550,7 @@ Docker building ${AIRFLOW_CI_IMAGE}.
fi
set +u
docker build \
"${EXTRA_DOCKER_CI_BUILD_FLAGS[@]}" \
--build-arg PYTHON_BASE_IMAGE="${PYTHON_BASE_IMAGE}" \
--build-arg PYTHON_MAJOR_MINOR_VERSION="${PYTHON_MAJOR_MINOR_VERSION}" \
--build-arg AIRFLOW_VERSION="${AIRFLOW_VERSION}" \
Expand Down Expand Up @@ -594,6 +599,7 @@ function build_images::prepare_prod_build() {
else
# When no airflow version/reference is specified, production image is built from local sources
EXTRA_DOCKER_PROD_BUILD_FLAGS=(
"--build-arg" "AIRFLOW_CONSTRAINTS_REFERENCE=${DEFAULT_CONSTRAINTS_BRANCH}"
)
fi
if [[ "${DEFAULT_PYTHON_MAJOR_MINOR_VERSION}" == "${PYTHON_MAJOR_MINOR_VERSION}" ]]; then
Expand Down
5 changes: 3 additions & 2 deletions scripts/ci/libraries/_initialization.sh
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ function initialization::initialize_base_variables() {
# Determine current branch
function initialization::initialize_branch_variables() {
# Default branch used - this will be different in different branches
export DEFAULT_BRANCH="master"
export DEFAULT_CONSTRAINTS_BRANCH="constraints-master"
export DEFAULT_BRANCH=${DEFAULT_BRANCH="master"}
export DEFAULT_CONSTRAINTS_BRANCH=${DEFAULT_CONSTRAINTS_BRANCH="constraints-master"}
readonly DEFAULT_BRANCH
readonly DEFAULT_CONSTRAINTS_BRANCH

# Default branch name for triggered builds is the one configured in default branch
Expand Down