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

gha: Add image build check for s390x #576

Merged

Conversation

BbolroC
Copy link
Member

@BbolroC BbolroC commented Nov 15, 2024

Issue #568 highlights the need for an image build check for s390x, similar to what we have for x86_64.
This PR addresses the need by:

  • Extracting the image build steps from the push-{kbs,as}-image-to-ghcr workflow into separate workflows
  • Configuring these workflows to run on PR events or after merging a PR

Notable change:

  • On merge, the workflow now pushes ghcr.io/confidential-containers/staged-images/rhel-ubi

The changes on PR events have been verified with:

The changes on merge needs to be verified after merging this PR. I will keep an eye on how it goes and bring things up immediately when something goes wrong. Thanks.

Signed-off-by: Hyounggyu Choi [email protected]

@BbolroC BbolroC requested a review from a team as a code owner November 15, 2024 06:48
@BbolroC BbolroC force-pushed the add-image-build-check-s390x branch from 4551382 to 40031e8 Compare November 15, 2024 06:51
Copy link
Member

@Xynnn007 Xynnn007 left a comment

Choose a reason for hiding this comment

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

Thanks @BbolroC ! LGTM, only a name thing I am not quite sure

@@ -0,0 +1,84 @@
name: Build CoCo AS/RVPS Image
Copy link
Member

Choose a reason for hiding this comment

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

The name here is different from as-docker-build.yml's. What would this cause? idk do we need to make them the same?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm.. To my best knowledge, a title of the called workflow does not appear on an action section. I prefer to having a different title for each workflow. 😉

.github/workflows/as-docker-build.yml Outdated Show resolved Hide resolved
.github/workflows/build-as-image.yml Show resolved Hide resolved
Comment on lines 11 to 12
with:
build_option:
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, let's make the build_option not required, so we can skip this

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.

Issue confidential-containers#568 highlights the need for an image build check for s390x,
similar to what we have for x86_64. This commit addresses the need by:
- Extracting the image build steps from the push-{kbs,as}-image-to-ghcr
  workflow into separate workflows
- Configuring these workflows to run on PR events or after merging a PR

Notable change:
- On merge, the workflow now pushes `ghcr.io/confidential-containers/staged-images/rhel-ubi`

Signed-off-by: Hyounggyu Choi <[email protected]>
@BbolroC BbolroC force-pushed the add-image-build-check-s390x branch from 40031e8 to b65d78a Compare November 15, 2024 10:37
@BbolroC BbolroC merged commit 1afe92c into confidential-containers:main Nov 15, 2024
24 checks passed
@BbolroC BbolroC deleted the add-image-build-check-s390x branch November 15, 2024 11:11
@BbolroC
Copy link
Member Author

BbolroC commented Nov 15, 2024

It seems that two workflows (e.g., one triggered on merge and another on PR events) are being executed when the PR is merged. This results in a duplicate image build step, similar to what we observed in the guest-components repository.

I will create a follow-up PR similar to the following to deduplicate the image build process:

Thanks.


This behaviour is not something this PR has introduced. The docker build check workflow has been called on merge together with the build-and-push workflow like: https://github.com/confidential-containers/trustee/actions/runs/11854184038/job/33035819254

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