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

Add build docker image GitHub action #64

Closed
wants to merge 2 commits into from

Conversation

martinkennelly
Copy link
Member

@martinkennelly martinkennelly commented Feb 16, 2021

For each new PR, we want to build the Dockerfile using GitHub actions.

I attempted to add the dockerfile.rhel7 but the build failed.
Part of issue #68

For each new PR, we want to build the Dockerfile using GitHub actions.
@martinkennelly martinkennelly changed the title Add github action to build docker image for PRs Add build docker image GitHub action Feb 17, 2021
@martinkennelly
Copy link
Member Author

martinkennelly commented Feb 19, 2021

you can also add cron job to these actions - do we want to add this to this PR? Twice a week ?

@zshi-redhat
Copy link
Collaborator

For each new PR, we want to build the Dockerfile using GitHub actions.

I attempted to add the dockerfile.rhel7 but the build failed.
Part of issue #68

I think we might want to just remove the Dockerfile.rhel7 as it is for openshift.

@zshi-redhat
Copy link
Collaborator

you can also add cron job to these actions - do we want to add this to this PR? Twice a week ?

If we run test on every PR, would it be already sufficient?

uses: actions/checkout@v2

- name: Build Dockerfile
run: docker build -f Dockerfile -t ${TAG} .
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I see there is a docker build-and-push github action, do you know if we can use it for build image only?

Copy link
Member Author

Choose a reason for hiding this comment

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

btw, will this workflow also aim to push the docker image to github container registry?

I added it to latest commit with build image only! Thanks for this! I also see there is support for multi arch docker builds. I attempted it but it hung.

@zshi-redhat
Copy link
Collaborator

btw, will this workflow also aim to push the docker image to github container registry?

@martinkennelly
Copy link
Member Author

martinkennelly commented Mar 8, 2021

you can also add cron job to these actions - do we want to add this to this PR? Twice a week ?

If we run test on every PR, would it be already sufficient?

I'd like to run it on a regular time cadence as well in case any dependencies that aren't locked to a version change and break NRI.

Signed-off-by: Kennelly, Martin <[email protected]>
@martinkennelly
Copy link
Member Author

btw, will this workflow also aim to push the docker image to github container registry?

I think it would be better to have this in a separate workflow. The reason is we want a workflow for testing PRs / pushes but we don't want to build an image for each PR / pushes. I think having a separate workflow to accomplish github container pushes is a good idea which will push when we merge anything to master and also for new releases.

@zshi-redhat
Copy link
Collaborator

@martinkennelly Do we still need this PR given #83 is merged?

@martinkennelly
Copy link
Member Author

@zshi-redhat I think #83 should not hinder this PR because it only builds images for tagged releases or merges to master. I think a case can definitely be made for #72 (add e2e tests with KinD) because it will build the change and then run e2e tests. It doesn't seem necessary to also build again with this workflow. I will close because of this. LMK if you disagree.

@martinkennelly martinkennelly deleted the github-action-docker-build branch April 8, 2021 14:58
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.

2 participants