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

DevOps: Fix json query in reading the docker names to filter out fields not starting with aiida #6573

Merged
merged 7 commits into from
Oct 17, 2024

Conversation

agoscinski
Copy link
Contributor

The BAKE_METADATA ha another field with warning information (buildx.build.warnings) that does not contain image.name so the json query failed (. Now I added a json query filtering out every field name that does not start with "aiida".
bake_metadata.json

@agoscinski agoscinski changed the title Fix json query in reading the docker names to filter out fields not starting with aiida DevOps: Fix json query in reading the docker names to filter out fields not starting with aiida Sep 27, 2024
@agoscinski agoscinski marked this pull request as ready for review September 27, 2024 09:00
@agoscinski
Copy link
Contributor Author

There are further errors in the deployment of the docker image, but maybe another PR would be nicer to separate it. It fixes the build-amd64 CI. Compare
last commit on main https://github.com/aiidateam/aiida-core/actions/runs/11054085180/job/30710031354
this PR https://github.com/aiidateam/aiida-core/actions/runs/11067528280

@danielhollas
Copy link
Collaborator

As a quick short term fix you might want to pin to docker/bake-action to v5.5.0
This is what we did in aiidalab-docker-stack, see aiidalab/aiidalab-docker-stack#493

I am just on my way to holiday and will be out for two weeks so don't have time to look into this more. @unkcpz can review this instead perhaps.

@@ -52,5 +49,7 @@ if [[ -z ${BAKE_METADATA-} ]];then
exit 1
fi

images=$(echo "${BAKE_METADATA}" | jq -c '. as $base |[to_entries[] |{"key": (.key|ascii_upcase|sub("-"; "_"; "g") + "_IMAGE"), "value": [(.value."image.name"|split(",")[0]),.value."containerimage.digest"]|join("@")}] |from_entries')
images=$(echo "${BAKE_METADATA}" |
jq -c 'to_entries | map(select(.key | startswith("aiida"))) | from_entries' | # filters out every key that does not start with aiida
Copy link
Collaborator

Choose a reason for hiding this comment

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

interesting approach, although I am honestly quite unhappy about this obscure jq syntax.
Perhaps we should switch to javascript to make this more readable. This was recommended to me here:

docker/bake-action#239 (comment)

Copy link
Contributor Author

@agoscinski agoscinski Sep 27, 2024

Choose a reason for hiding this comment

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

Yes agree, understanding this json query was very cumbersome. I would prefer some Python code for this repo, but I guess it results in a similar code you linked.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed, if you can figure out some Python one-liner that would be best.

@unkcpz
Copy link
Member

unkcpz commented Sep 27, 2024

Why the CI test is not triggered?

@agoscinski
Copy link
Contributor Author

Why the CI test is not triggered?

Because it only happens if you push to aiida-core with a branch that fulfills

Matches all branch and tag names that don't contain a slash (/). The * character is a special character in YAML. When you start a pattern with *, you must use quotes.

https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#patterns-to-match-branches-and-tags
See workflow file

push:
branches:
- '*'

Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

Looks good to me.

.github/workflows/docker-build-test.yml Outdated Show resolved Hide resolved
.github/workflows/docker-build.yml Outdated Show resolved Hide resolved
@agoscinski agoscinski merged commit e1467ed into main Oct 17, 2024
21 checks passed
@agoscinski agoscinski deleted the fix/docker-build-jq branch October 17, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants