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

feat: github action matrix for images build #4712

Merged
merged 4 commits into from
Dec 14, 2023

Conversation

hungran
Copy link
Contributor

@hungran hungran commented Dec 10, 2023

What this PR does / why we need it: better DRY on images build CI
this also prepare for upgrade image base, will have flexible option to put image TAG as ARG
#4702 (comment)

another point, though that this could be better view
image

Which issue(s) this PR fixes:

Part of #4713

  • add matrix image build

Does this PR introduce a user-facing change?:

  • How are users affected by this change: No
  • Is this breaking change: No
  • How to migrate (if breaking change): No

@hungran hungran changed the title feat: github action matrix images build feat: github action matrix for images build Dec 10, 2023
@hungran hungran force-pushed the hungran/matrix-build-images branch 2 times, most recently from 3ba3787 to b43c11c Compare December 10, 2023 08:56
@hungran hungran force-pushed the hungran/matrix-build-images branch from b43c11c to abfa2c6 Compare December 10, 2023 08:57
khanhtc1202
khanhtc1202 previously approved these changes Dec 10, 2023
Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

Great, thank you ❤️

Copy link

codecov bot commented Dec 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (301e367) 30.82% compared to head (deb2fbd) 30.81%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4712      +/-   ##
==========================================
- Coverage   30.82%   30.81%   -0.01%     
==========================================
  Files         221      221              
  Lines       25993    25993              
==========================================
- Hits         8012     8010       -2     
- Misses      17331    17332       +1     
- Partials      650      651       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines -139 to -140
${{ env.GHCR }}/pipe-cd/helloworld:${{ env.PIPECD_VERSION }}
${{ env.GCR }}/pipecd/helloworld:${{ env.PIPECD_VERSION }}
Copy link
Member

Choose a reason for hiding this comment

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

It looks that some container images are hosted by GCR 👀
This may be because Cloud Run can use only with Artifact Registry, GCR, Docker Hub.
So it would be nice to consider for GCR 🤔
WDYT @khanhtc1202 ?

📝 memo
Cloud Run docs: https://cloud.google.com/run/docs/deploying
PRs that maybe related

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ffjlabo good catch!
if we GCR only when Cloud Run, so ideally we could bring all images to GCR
however, not sure if PipeCD want to rely on a specific vendor?
@khanhtc1202

btw, even we won't bring all the images to GCR, we could add more configuration for the matrix by this way
https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs#example-adding-configurations

already push the change base on that, please check below comment

@hungran
Copy link
Contributor Author

hungran commented Dec 13, 2023

Guys, I know that it's hard to test the workflow publish build and push image by matrix from this PR
Please give me sometime on a test then show you how with the current change
@khanhtc1202 @ffjlabo

@hungran
Copy link
Contributor Author

hungran commented Dec 13, 2023

image

image

here you are

jobs:
  artifacts:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        container_registry:
          - ghcr.io
        image:
          - helloworld
          - launcher
          - launcher-okd
          - pipecd
          - piped
          - piped-okd
          - pipectl
        include:
          - image: helloworld
            dockerfile: cmd/helloworld/Dockerfile
            container_registry: gcr.io
          - image: launcher
            dockerfile: cmd/launcher/Dockerfile
            container_registry: gcr.io
          - image: launcher-okd
            dockerfile: cmd/launcher/Dockerfile-okd
          - image: pipecd
            dockerfile: cmd/pipecd/Dockerfile
            container_registry: gcr.io
          - image: piped
            dockerfile: cmd/piped/Dockerfile
          - image: piped-okd
            dockerfile: cmd/piped/Dockerfile-okd
          - image: pipectl
            dockerfile: cmd/pipectl/Dockerfile
    steps:
      - name: Show matrix
        run: |
          echo "${{ matrix.container_registry }}/pipe-cd/${{ matrix.image }}:dummy with dockerfile ${{ matrix.dockerfile }}"

Copy link
Member

@ffjlabo ffjlabo left a comment

Choose a reason for hiding this comment

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

It looks good! Here we go 🚀

@ffjlabo
Copy link
Member

ffjlabo commented Dec 14, 2023

@khanhtc1202 plz re-check 🙏

Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

Great, thank you 🙏

@khanhtc1202 khanhtc1202 merged commit fa62482 into pipe-cd:master Dec 14, 2023
12 of 13 checks passed
sZma5a pushed a commit to sZma5a/pipecd that referenced this pull request Dec 17, 2023
* feat: github action matrix images build

Signed-off-by: hungran <[email protected]>

* naming for each CI should be more visible, add matrix for build_tool image base

Signed-off-by: hungran <[email protected]>

* adding configuration for matrix container registry

Signed-off-by: hungran <[email protected]>

* docker tag with matrix container registry

Signed-off-by: hungran <[email protected]>

---------

Signed-off-by: hungran <[email protected]>
sZma5a pushed a commit to sZma5a/pipecd that referenced this pull request Dec 17, 2023
* feat: github action matrix images build

Signed-off-by: hungran <[email protected]>

* naming for each CI should be more visible, add matrix for build_tool image base

Signed-off-by: hungran <[email protected]>

* adding configuration for matrix container registry

Signed-off-by: hungran <[email protected]>

* docker tag with matrix container registry

Signed-off-by: hungran <[email protected]>

---------

Signed-off-by: hungran <[email protected]>
sZma5a pushed a commit to sZma5a/pipecd that referenced this pull request Dec 17, 2023
* feat: github action matrix images build

Signed-off-by: hungran <[email protected]>

* naming for each CI should be more visible, add matrix for build_tool image base

Signed-off-by: hungran <[email protected]>

* adding configuration for matrix container registry

Signed-off-by: hungran <[email protected]>

* docker tag with matrix container registry

Signed-off-by: hungran <[email protected]>

---------

Signed-off-by: hungran <[email protected]>
Signed-off-by: sZma5a <[email protected]>
@github-actions github-actions bot mentioned this pull request Feb 6, 2024
sZma5a pushed a commit to sZma5a/pipecd that referenced this pull request Feb 12, 2024
* feat: github action matrix images build

Signed-off-by: hungran <[email protected]>

* naming for each CI should be more visible, add matrix for build_tool image base

Signed-off-by: hungran <[email protected]>

* adding configuration for matrix container registry

Signed-off-by: hungran <[email protected]>

* docker tag with matrix container registry

Signed-off-by: hungran <[email protected]>

---------

Signed-off-by: hungran <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants