Skip to content

Conversation

@lool
Copy link
Contributor

@lool lool commented Apr 3, 2025

Build ids on the fileserver simply used the GITHUB_RUN_ID, but it is not
guaranteed to be unique, and other repositories and workflows share the
same tree. Change build ids to be prefixed with the GITHUB_REPOSITORY
name, and suffixed with the GITHUB_RUN_ATTEMPT (in case the run has been
given back).

This is still safe from the perspective of directory names and URLs as
GitHub prevents org and repo names to start with a dash, and use only
alphanumeric chars plus single dashes.

@lool
Copy link
Contributor Author

lool commented Apr 3, 2025

build-yocto.yml is the only workflow where the fileserver local path and URL are defined/used directly.

The LAVA test workflow inject the GITHUB_RUN_ID in their files, and also submit jobs using the GITHUB_RUN_ID, but these workflow runs are entirely separate from the build workflow runs. For instance, this recent LAVA job is named after the test.yml workflow run 14129155762 (boot test (qrb2210-rb1-core-kit) 14129155762), but is testing artifact URL from build-yocto workflow run 14127965204 (https://quic-yocto-fileserver-1029608027416.us-central1.run.app/14127965204/qrb2210-rb1-core-kit/core-image-base-qrb2210-rb1-core-kit.rootfs.qcomflash.tar.gz).

@lool
Copy link
Contributor Author

lool commented Apr 3, 2025

Also see qualcomm-linux/qcom-deb-images#4 for motivation

koenkooi
koenkooi previously approved these changes Apr 3, 2025
Copy link
Contributor

@koenkooi koenkooi 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!

@lumag lumag enabled auto-merge April 3, 2025 09:01
@ndechesne ndechesne disabled auto-merge April 3, 2025 09:05
@ndechesne
Copy link
Contributor

Don't you need to update BASE_ARTIFACT_URL as well? That's the variable we use to define where the LAVA job will download the image from, and it's currently using the old path (RUN ID only).

@lool
Copy link
Contributor Author

lool commented Apr 3, 2025

Indeed, I saw this BASE_URL in my first read, but I forgot to update it. I've pushed an updated version, and grep-ed for github.run_id across workflows, this was the only use.

lumag
lumag previously approved these changes Apr 3, 2025
@lumag lumag requested a review from koenkooi April 3, 2025 09:32
@lumag lumag enabled auto-merge April 3, 2025 09:33
@github-actions
Copy link

github-actions bot commented Apr 3, 2025

Test Results

 3 files  ±0   6 suites  ±0   1m 47s ⏱️ ±0s
23 tests ±0  23 ✅ ±0  0 💤 ±0  0 ❌ ±0 
57 runs  ±0  57 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 39a5a31. ± Comparison against base commit 987c99a.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Apr 3, 2025

Build ids on the fileserver simply used the GITHUB_RUN_ID, but it is not
guaranteed to be unique, and other repositories and workflows share the
same tree. Change build ids to be prefixed with the github.repository
name, and suffixed with the github.run_attempt (in case the run has been
given back).

This is still safe from the perspective of directory names and URLs as
GitHub prevents org and repo names to start with a dash, and use only
alphanumeric chars plus single dashes.

Signed-off-by: Loïc Minier <[email protected]>
auto-merge was automatically disabled April 3, 2025 13:52

Head branch was pushed to by a user without write access

@lool lool force-pushed the update-fileserver-urls branch from 31dbdc5 to 39a5a31 Compare April 3, 2025 13:52
@github-actions
Copy link

github-actions bot commented Apr 3, 2025

@ndechesne ndechesne enabled auto-merge April 3, 2025 16:10
@ndechesne ndechesne merged commit 6b3af65 into qualcomm-linux:master Apr 3, 2025
24 of 25 checks passed
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.

4 participants