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

fix(docker): remove build main action #4747

Closed
wants to merge 11 commits into from

Conversation

oguzkaganozt
Copy link
Contributor

@oguzkaganozt oguzkaganozt commented May 21, 2024

Description

To simplify CI-CD pipeline ,build-main and build-main-self-hosted can be deleted because they are simply using the same docker build mechanism as docker-build-main and docker-build-main-self-hosted.

Also renaming of:

  • docker-build-and-push-main -> docker-build-main
  • docker-build-and-push-main-self-hosted -> docker-build-main-self-hosted

can be done since docker builds does not pushes every time into the registry the naming implies false positive.

Tests performed

Not applicable.

Effects on system behavior

Not applicable.

Interface changes

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

@oguzkaganozt oguzkaganozt self-assigned this May 21, 2024
@oguzkaganozt oguzkaganozt added the type:ci Continuous Integration (CI) processes and testing. label May 21, 2024
@oguzkaganozt oguzkaganozt requested a review from youtalk May 27, 2024 08:38
@youtalk
Copy link
Member

youtalk commented May 27, 2024

We are using docker-build-and-push-main and docker-build-and-push-main-self-hosted for not only main branch but also the other branches.
So are docker-build and docker-build-self-hosted better?

Copy link
Contributor

@xmfcx xmfcx left a comment

Choose a reason for hiding this comment

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

Please keep in mind that build-main runs every day. And docker-build-and-push-main runs twice a month.

build-main is necessary to keep track of the entire health of the autoware.

Also used in the README:

autoware/README.md

Lines 34 to 36 in 27e31e9

<a href="https://github.com/autowarefoundation/autoware/actions/workflows/build-main.yaml?query=branch%3Amain">
<img src="https://img.shields.io/github/actions/workflow/status/autowarefoundation/autoware/build-main.yaml?style=flat&label=build-main"
alt="Build Main CI" /></a>

image

You may remove it but you also need to provide a way to:

  • Build and test entire autoware every day
  • Show the status in readme file.

@youtalk youtalk requested a review from xmfcx June 4, 2024 05:04
@youtalk
Copy link
Member

youtalk commented Jun 4, 2024

@oguzkaganozt I think this PR is important for the workflow refactoring, so I took over and made the necessary modifications.

@xmfcx I've updated the schedule to every day and the README badge. Note that there is no docker-build workflow until this PR is merged, so no badge has been generated.
Please review again.

@xmfcx
Copy link
Contributor

xmfcx commented Jun 4, 2024

But now the question is, do we want to update the base image and push it to registry every day?

I think there was a reason to do this twice a month. @mitsudome-r @youtalk

@youtalk
Copy link
Member

youtalk commented Jun 4, 2024

Oh, I finally understood. The difference between build-main and docker-build-and-push-main is allow-push option.
That's why it might not be suitable to run docker-build-and-push-main every day.
@xmfcx I think so.

oguzkaganozt and others added 7 commits June 24, 2024 13:08
Signed-off-by: Oguz Ozturk <[email protected]>
Signed-off-by: Oguz Ozturk <[email protected]>
Signed-off-by: Yutaka Kondo <[email protected]>
Signed-off-by: Yutaka Kondo <[email protected]>
Signed-off-by: Yutaka Kondo <[email protected]>
Signed-off-by: Yutaka Kondo <[email protected]>
@oguzkaganozt oguzkaganozt force-pushed the fix/remove-build-main-action branch from e28fed2 to 991afca Compare June 24, 2024 10:09
@oguzkaganozt oguzkaganozt requested a review from youtalk June 24, 2024 11:05
@oguzkaganozt
Copy link
Contributor Author

@xmfcx @youtalk @mitsudome-r

Updated the PR so that; we are now using docker-build and docker-build-self-hosted both for daily builds and pushing images to registry twice a month. Please review again.

@youtalk
Copy link
Member

youtalk commented Jul 4, 2024

@oguzkaganozt I'm sorry to review late.

Simplifying the workflow is a very good thing.
However, there is a constraint that unless we execute the docker-build action called in the build-main and build-main-self-hosted workflows, we cannot save the cache of docker build's --mount=type=cache cache to the GitHub Actions cache.
#4865

This might be fixed in a future update of buildkit-cache-dance action, but removing it now would mean disabling the ccache, resulting in CI build times becoming several times slower again.

@youtalk youtalk deleted the fix/remove-build-main-action branch September 5, 2024 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:ci Continuous Integration (CI) processes and testing.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants