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.

Sign up for GitHub

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

Jump to bottom

ROCKS 1452 - Refactor Build Rock workflow to be externally reusable #233

Merged
merged 101 commits into from
Oct 17, 2024
Merged

ROCKS 1452 - Refactor Build Rock workflow to be externally reusable #233

merged 101 commits into from
Oct 17, 2024

Conversation

clay-lake
Copy link
Contributor

@clay-lake clay-lake commented Aug 23, 2024 •
edited
Loading

Note: This PR depends on #224
This pull request refactors the build workflow to enable re-usability by external repositories. There is an example application of this in the rocks-toolbox. A example job run can be found here: https://github.com/canonical/rocks-toolbox/actions/runs/10524164671

@clay-lake clay-lake changed the title DRAFT ROCKS 1452 - Refactor Build Rock workflow to be externally reusable DRAFT - ROCKS 1452 - Refactor Build Rock workflow to be externally reusable Aug 23, 2024
@clay-lake clay-lake changed the title DRAFT - ROCKS 1452 - Refactor Build Rock workflow to be externally reusable ROCKS 1452 - Refactor Build Rock workflow to be externally reusable Aug 26, 2024
@clay-lake clay-lake marked this pull request as ready for review September 3, 2024 08:26
@clay-lake
Copy link
Contributor Author

Since it is not part of the checks, here is an example Test OCI Factory run from e6e3c29
https://github.com/canonical/oci-factory/actions/runs/10678234515

zhijie-yang
zhijie-yang requested changes Sep 3, 2024
View reviewed changes
Copy link
Contributor

@zhijie-yang zhijie-yang left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! Leaving several nitpicks:

.github/workflows/Build-Rock.yaml Outdated Show resolved Hide resolved
src/build_rock/configure/generate_build_matrix.py Outdated Show resolved Hide resolved
.github/workflows/Build-Rock.yaml Outdated Show resolved Hide resolved
.github/workflows/Build-Rock.yaml Show resolved Hide resolved
.github/workflows/Image.yaml Show resolved Hide resolved
src/shared/github_output.py Show resolved Hide resolved
tests/fixtures/__ini__.py Outdated Show resolved Hide resolved
tests/integration/test_convert_junit_xml_to_markdown.py Outdated Show resolved Hide resolved
tests/unit/test_generate_build_matrix.py Outdated Show resolved Hide resolved
tests/unit/test_github_output.py Outdated Show resolved Hide resolved
linostar
linostar requested changes Sep 4, 2024
View reviewed changes
Copy link
Contributor

@linostar linostar left a comment

Choose a reason for hiding this comment

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

Good work. I left few minor remarks and nitpicks.

.github/workflows/Build-Rock.yaml Outdated Show resolved Hide resolved
.github/workflows/Build-Rock.yaml Outdated Show resolved Hide resolved
.github/workflows/Build-Rock.yaml Show resolved Hide resolved
.github/workflows/Build-Rock.yaml Outdated Show resolved Hide resolved
.github/workflows/Build-Rock.yaml Show resolved Hide resolved
src/build_rock/configure/generate_build_matrix.py Show resolved Hide resolved
src/build_rock/configure/generate_build_matrix.py Show resolved Hide resolved
@clay-lake clay-lake force-pushed the ROCKS-1452/oci-factory-refactor-build-rock-to-be-externally-reusable branch from 370b1aa to 134291d Compare September 6, 2024 11:38
clay-lake and others added 8 commits September 6, 2024 11:48
@clay-lake
Merge branch 'main' of https://github.com/canonical/oci-factory into …
f5b8d15 
…ROCKS-1452/oci-factory-refactor-build-rock-to-be-externally-reusable
@clay-lake
Merge branch 'ROCKS-1452/oci-factory-refactor-build-rock-to-be-extern…
41f7d75 
…ally-reusable' of https://github.com/canonical/oci-factory into ROCKS-1452/oci-factory-refactor-build-rock-to-be-externally-reusable
@clay-lake clay-lake requested a review from a team as a code owner September 16, 2024 11:37
zhijie-yang
zhijie-yang approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@zhijie-yang zhijie-yang left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

cjdcordeiro
cjdcordeiro requested changes Sep 16, 2024
View reviewed changes
.github/workflows/Build-Rock.yaml Outdated Show resolved Hide resolved
clay-lake and others added 12 commits October 11, 2024 08:22
@clay-lake
test build
bc09726
@clay-lake
test build
598dd91
@clay-lake
test build
7f7e605
@clay-lake
test: build with optional build id
5ead99b
@clay-lake
test: same as last build, fixed syntax
7be1b4c
@clay-lake
test build
85a9f29
@clay-lake
test build without build-id
d1ad574
@clay-lake
test build
5f29eb9
ci: automatically update oci/mock-rock/_releases.json, from https://g…
b0c0a07 
…ithub.com/canonical/oci-factory/actions/runs/11324839553
@clay-lake
feat: build-id paramter for identifying worflow jobs
d5d5675
@clay-lake
Merge branch 'ROCKS-1452/oci-factory-refactor-build-rock-to-be-extern…
43931e8 
…ally-reusable' of https://github.com/canonical/oci-factory into ROCKS-1452/oci-factory-refactor-build-rock-to-be-externally-reusable
@clay-lake
Merge branch 'main' of https://github.com/canonical/oci-factory into …
dd3fe16 
…ROCKS-1452/oci-factory-refactor-build-rock-to-be-externally-reusable
@cjdcordeiro
Copy link
Collaborator

@clay-lake https://github.com/canonical/oci-factory/actions/runs/11342675482/job/31545731337 1.0.1 seems to be missing

clay-lake and others added 7 commits October 16, 2024 12:01
ci: automatically update oci/mock-rock/_releases.json, from https://g…
9661423 
…ithub.com/canonical/oci-factory/actions/runs/11365021701
@clay-lake
Revert "feat: add new non-EOL track for mock rock 1.0"
3eb2c73 
This reverts commit 072878a.
@clay-lake
Merge branch 'ROCKS-1452/oci-factory-refactor-build-rock-to-be-extern…
d2c13e8 
…ally-reusable' of https://github.com/canonical/oci-factory into ROCKS-1452/oci-factory-refactor-build-rock-to-be-externally-reusable
ci: automatically update oci/mock-rock/_releases.json, from https://g…
18d5a16 
…ithub.com/canonical/oci-factory/actions/runs/11365421745
@clay-lake
fix(/workspaces/oci-factory/.github/workflows/Build-Rock.yaml): missi…
8edab43 
…ng "needs" item
@clay-lake
Merge branch 'ROCKS-1452/oci-factory-refactor-build-rock-to-be-extern…
8923913 
…ally-reusable' of https://github.com/canonical/oci-factory into ROCKS-1452/oci-factory-refactor-build-rock-to-be-externally-reusable
ci: automatically update oci/mock-rock/_releases.json, from https://g…
f3b2a5a 
…ithub.com/canonical/oci-factory/actions/runs/11365706243
@clay-lake
Copy link
Contributor Author

@cjdcordeiro I found why 1.0.1 is not building. Testing the prepare_single_image_build_matrix.py script (from main) seems to drop a build entirely if any of the releases in the image.yaml are EOL. Is that intended? I'll fix in another PR if not.

Instead of testing mock-rock/1.0 here, I made a multi-arch test build in rocks-toolbox using an external workflow call.
https://github.com/canonical/rocks-toolbox/actions/runs/11365735363

@cjdcordeiro
Copy link
Collaborator

good catch @clay-lake

That is NOT the desired behaviour indeed. We should only ignore the track, not the whole build.

cjdcordeiro
cjdcordeiro approved these changes Oct 17, 2024
View reviewed changes
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

great. thanks for the changes :) looking good.
we need to make sure we multi-arch tests are running again so pls let me know once the fix PR for the bad EOL filtering comes up

@cjdcordeiro cjdcordeiro merged commit 398d4a5 into main Oct 17, 2024
@cjdcordeiro cjdcordeiro deleted the ROCKS-1452/oci-factory-refactor-build-rock-to-be-externally-reusable branch October 17, 2024 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhijie-yang zhijie-yang zhijie-yang approved these changes

@cjdcordeiro cjdcordeiro cjdcordeiro approved these changes

@linostar linostar Awaiting requested review from linostar

Assignees
No one assigned
Labels
priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@clay-lake @cjdcordeiro @linostar @zhijie-yang