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

Update container build to only push on merges #717

Merged
merged 10 commits into from
Oct 31, 2023
Merged

Update container build to only push on merges #717

merged 10 commits into from
Oct 31, 2023

Conversation

nithyatsu
Copy link
Contributor

@nithyatsu nithyatsu commented Oct 26, 2023 •
edited
Loading

Enable PR from forks:

  • remove az login from validate-bicep yaml since this is no longer required
  • push to GHCR only when github action is a push. For PRs from forks, the action_name is pull_request.

@nithyatsu nithyatsu changed the base branch from v0.26 to edge October 26, 2023 19:42
@nithyatsu nithyatsu force-pushed the local_regitry branch 2 times, most recently from 927aac6 to ac4a765 Compare October 26, 2023 20:45
@nithyatsu nithyatsu changed the title Allow PRs to use a local registry instead of GHCR for pushing and reading built images. Make push to GHCR conditional in order to enable PR from forks. Oct 30, 2023
@nithyatsu nithyatsu changed the title Make push to GHCR conditional in order to enable PR from forks. Make push to ghcr conditional in order to enable PR from forks. Oct 30, 2023
@nithyatsu nithyatsu marked this pull request as ready for review October 30, 2023 18:16
@nithyatsu nithyatsu requested a review from a team as a code owner October 30, 2023 18:16
vinayada1
vinayada1 reviewed Oct 30, 2023
View reviewed changes
.github/workflows/build.yaml Outdated
@@ -60,5 +68,6 @@ jobs:
uses: docker/build-push-action@v4
with:
context: ./${{ matrix.directory }}
push: ${{ env.PUSH_IMAGE }}
push: ${{ github.event_name == 'push' && true || false }}
# push: ${{ fromJSON(steps.eval_push.outputs.PUSH_IMAGE) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented code

vinayada1
vinayada1 reviewed Oct 30, 2023
View reviewed changes
.github/workflows/build.yaml
@@ -60,5 +68,6 @@ jobs:
uses: docker/build-push-action@v4
with:
context: ./${{ matrix.directory }}
push: ${{ env.PUSH_IMAGE }}
push: ${{ github.event_name == 'push' && true || false }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this just be ${{ github.event_name == 'push' }}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That did not work, which is why I used the fromJSON originally (actions/runner#1483). But Aaron's suggestion works.

@nithyatsu nithyatsu changed the title Make push to ghcr conditional in order to enable PR from forks. [edge PR]Make push to ghcr conditional in order to enable PR from forks. Oct 31, 2023
@AaronCrawfis AaronCrawfis changed the title [edge PR]Make push to ghcr conditional in order to enable PR from forks. Update container build to only push on merges Oct 31, 2023
@AaronCrawfis
Merge branch 'v0.26' into local_regitry
6724e99 
Signed-off-by: Aaron Crawfis <Aaron.Crawfis@microsoft.com>
@AaronCrawfis AaronCrawfis merged commit 4e2561b into edge Oct 31, 2023
19 of 27 checks passed
@AaronCrawfis AaronCrawfis deleted the local_regitry branch October 31, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rynowak rynowak rynowak approved these changes

@vinayada1 vinayada1 vinayada1 approved these changes

@AaronCrawfis AaronCrawfis AaronCrawfis approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@nithyatsu @rynowak @vinayada1 @AaronCrawfis