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

Split arch and name action as the build repo (infra) #796

Merged
merged 4 commits into from
Oct 26, 2023

Conversation

Hook25
Copy link
Collaborator

@Hook25 Hook25 commented Oct 26, 2023

Description

Building multiple arches per matrix expansion leads the workflow to hang waiting for slower archs even if a build failed and makes us unable to re-run the build for a specific arch if it failed. With this modification each arch is built with its own command.

Note: this is the same PR I proposed previously but I have discovered that --build-id can be used to give the build an id and not only to recover a previously running build

Minor: This also includes a new step that dumps the build URL, this makes it way easier to debug the build if anything goes wrong at runtime
Minor: A few name changes (prettier output for the action menu)

Resolved issues

The probability of a full build is hindered by the fact that every series either builds all arches or retries them all, this greatly increases the probability of retrying the individual arch because it goes from P(arch_failure) to 1-(1-P(arch_failure))**3

Documentation

This improves the build readability by providing the URL where the build repo is allowing for further debugging if needed without searching this information in the log at runtime

Tests

Started the frontend workflow to see if it works at: https://github.com/canonical/checkbox/actions/runs/6652646495/job/18077062870

@kissiel
Copy link
Contributor

kissiel commented Oct 26, 2023

And with that patch how many workers will we occupy when building *?

@Hook25
Copy link
Collaborator Author

Hook25 commented Oct 26, 2023

1 (debs) + 2*4*3 (frontend) + 2*3 (runtime) = 31 (all self hosted runners, all jobs that are not intensive at all and can be swapped in and out frequently)

@kissiel
Copy link
Contributor

kissiel commented Oct 26, 2023

I'd keep frontends together. They snap relatively fast, and provisioning the runner (installing snapcraft) can be slower than snapping everything together.

@Hook25
Copy link
Collaborator Author

Hook25 commented Oct 26, 2023

ok good idea, I agree, I partially roll back the frontend change (keeping the QOL, that is very nice)

Copy link
Contributor

@kissiel kissiel left a comment

Choose a reason for hiding this comment

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

Loop qualifies for removal. See below.

.github/workflows/checkbox-snap-daily-builds.yml Outdated Show resolved Hide resolved
.github/workflows/checkbox-core-snap-daily-builds.yml Outdated Show resolved Hide resolved
.github/workflows/checkbox-core-snap-daily-builds.yml Outdated Show resolved Hide resolved
mz2
mz2 previously approved these changes Oct 26, 2023
Copy link
Collaborator

@mz2 mz2 left a comment

Choose a reason for hiding this comment

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

Please introduce an env var

.github/workflows/checkbox-core-snap-daily-builds.yml Outdated Show resolved Hide resolved
.github/workflows/checkbox-core-snap-daily-builds.yml Outdated Show resolved Hide resolved
.github/workflows/checkbox-core-snap-daily-builds.yml Outdated Show resolved Hide resolved
.github/workflows/checkbox-core-snap-daily-builds.yml Outdated Show resolved Hide resolved
.github/workflows/checkbox-core-snap-daily-builds.yml Outdated Show resolved Hide resolved
.github/workflows/checkbox-snap-daily-builds.yml Outdated Show resolved Hide resolved
.github/workflows/checkbox-snap-daily-builds.yml Outdated Show resolved Hide resolved
.github/workflows/checkbox-snap-daily-builds.yml Outdated Show resolved Hide resolved
.github/workflows/checkbox-snap-daily-builds.yml Outdated Show resolved Hide resolved
.github/workflows/checkbox-snap-daily-builds.yml Outdated Show resolved Hide resolved
@kissiel
Copy link
Contributor

kissiel commented Oct 26, 2023

Let's not mess with that loop right now, the glob will match one snap.
My new concern is: IIRC if there's one file, the action doesn't tarball it, we need to make sure the rest will work ok.

Minor: Spaces, nomenclature, moved comments
Minor: bumped retries for build and upload
Minor: clarified comment
Copy link
Contributor

@kissiel kissiel 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! Now let's see if it runs as well, as it looks :)

@Hook25 Hook25 merged commit 7105814 into main Oct 26, 2023
7 of 19 checks passed
@Hook25 Hook25 deleted the move_to_forked_action branch October 26, 2023 13:34
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