👷 Refactor when to rebuild staging images

[shnizzedy]

Related problem

The CI is currently set up to check each staging image's Dockerfile for changes and tag for existence before deciding whether to pull or rebuild that stage.

C-PAC/.github/workflows/build_stages.yml

Lines 38 to 69 in 12c4f74

	- name: Check out C-PAC (depth 0)
	if: github.event.inputs.base_ref != ''
	uses: actions/checkout@v2
	with:
	fetch-depth: 0
	- name: Check out C-PAC (depth 2)
	if: github.event.inputs.base_ref == ''
	uses: actions/checkout@v2
	with:
	fetch-depth: 2
	- name: Get changed files since fork point
	uses: tj-actions/[email protected]
	if: github.event.inputs.base_ref != ''
	id: changed-files-base
	with:
	use_fork_point: "true"
	files: .github/Dockerfiles/*
	- name: Get changed files since last commit
	uses: tj-actions/[email protected]
	if: github.event.inputs.base_ref == ''
	id: changed-files
	with:
	since_last_remote_commit: "true"
	files: .github/Dockerfiles/*
	- name: Set tag & see if it exists
	continue-on-error: true
	run: |
	TAG=$(sed 's/\./:/' <(echo ${{ matrix.Dockerfile }}))
	DOCKER_TAG=$(echo "ghcr.io/${{ github.repository }}/$TAG" | tr '[:upper:]' '[:lower:]')
	echo DOCKER_TAG=$DOCKER_TAG >> $GITHUB_ENV
	docker manifest inspect $DOCKER_TAG >/dev/null
	echo "::set-output name=not_yet_exists::$?"

C-PAC/.github/workflows/build_stages.yml

Line 72 in 12c4f74

if: contains(steps.changed-files-base.outputs.all_changed_files, matrix.Dockerfile) || contains(steps.changed-files.outputs.all_changed_files, matrix.Dockerfile)

This logic falls short on both sides of the idea runs-only-when-necessary:

• It has to check for changes every time
• It doesn't catch changes in external sources, e.g., c-pac_templates:latest

Proposed feature

We could update the CI to take a key string like

[rebuild AFNI.20.0.04]

in the commit message, update the strategies to be something like

    strategy:
      matrix:
        stage:
        - AFNI.16.2.07.neurodocker
        - AFNI.20.0.04
        - ANTs.2.2.0.neurodocker
        - c3d.1.0.0
        - connectome-workbench.1.3.2-1.neurodebian
        - connectome-workbench.1.3.2-2.neurodebian
        - FreeSurfer.6.0.0-min.neurodocker
        - FreeSurfer.6.0.1-min
        - FSL.5.0.9-5.neurodebian
        - FSL.5.0.10
        - ICA-AROMA.0.4.5
        - msm.2.0

and update the workflow logic with something like

if: "contains(github.event.head_commit.message, '[rebuild ${{ matrix.stage }}]')"

plus some logic to handle which downstream stages depend on a given image. Then rebuilds would

• only happen when a relevant [rebuild… is specified
• always happen when a relevant [rebuild… is specified

Alternatives

We could also/additionally add an input parameter to the workflow_call so, for example, a merge to main in FCP-INDI/C-PAC_templates could trigger the relevant rebuilds

Additional context

Ref: Conditional GitHub action based on commit message

[gkiar]

Cool! In another project, I've handled conditional building like this. It has the perk of not requiring us to be careful with commit messages. I imagine something similar could be done, e.g., with commit messages on things like the template repository. Forgive my ignorance of the current approach, but do you think something like this could work for us?

[shnizzedy]

I think this strategy is more or less what's in place now?

fuzzy	C-PAC
build-fuzzy.yml#L54-L56 - name: Check for file changes uses: jitterbit/get-changed-files@v1 id: changes	C-PAC/.github/workflows/build_stages.yml Lines 121 to 134 in 12c4f74 - name: Get changed files since fork point uses: tj-actions/[email protected] if: github.event.inputs.base_ref != '' id: changed-files-base with: use_fork_point: "true" files: .github/Dockerfiles/* - name: Get changed files since last commit uses: tj-actions/[email protected] if: github.event.inputs.base_ref == '' id: changed-files with: since_last_remote_commit: "true" files: .github/Dockerfiles/*
build-fuzzy.yml#L89 if: contains(steps.changes.outputs.all, ${{ env.RUN_IF_CHANGED }})	C-PAC/.github/workflows/build_stages.yml Line 236 in 12c4f74 if: always() && steps.changed-files-base.outputs.any_changed == 'true' || steps.changed-files.outputs.any_changed == 'true' || steps.docker_tag.not_yet_exists == 1

It looks to me like the differences here are fairly superficial:

• fuzzy has 2 tiers of stages; C-PAC has 4
• the workflows use different marketplace actions to check for file changes
• fuzzy's strategy implicitly differentiates between "fork point" and "last commit"; C-PAC's strategy does so implicitly
• fuzzy only rebuilds the final stage if that repo's docker directory has changes; C-PAC rebuild the final stage every time

Am I missing something?

---

not requiring us to be careful with commit messages

Is the concern that we'd update a Dockerfile but forget to rebuild it? I hadn't thought of that, but we could combine approaches here and add the proposed strategy here to the existing strategy instead of replacing it. (I.e, rebuild if the [rebuild… key is given OR the relevant Dockerfile changed)

[gkiar]

Is the concern that we'd update a Dockerfile but forget to rebuild it?

Yes, exactly!

add the proposed strategy here to the existing strategy instead of replacing it. (I.e, rebuild if the [rebuild… key is given OR
the relevant Dockerfile changed)

That sounds great to me — do you think that would solve both of your noted concerns with the current approach?

[shnizzedy]

Yeah, more or less. It would still have to check for changes for each dockerfile, but that's only 3‒4 seconds per in my experience, and it usually does 3 at a time