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

chore(CI/CD): Add pr-image-builder workflow #54

Merged
merged 6 commits into from
Aug 30, 2024

Conversation

davidkornel
Copy link
Member

This PR adds a workflow that builds a Docker image based on the pull requests. It gets triggered on additional commits also.
The image naming is l7mp/stunner-gateway-operator:pr-<pr-number>.

The workflow comments on the PR with the built image name to notify the user.

Currently, the run unit test job is disabled, might be useful to enable it later.

@davidkornel
Copy link
Member Author

@rg0now As stated the current image format is l7mp/stunner-gateway-operator:pr-<pr-number>. I'm afraid this would result in numerous extra tags under the l7mp/stunner-gateway-operator image which is used for prod. Should we change the image name to l7mp/stunner-gateway-operator-pr:<pr-number>

Copy link

The Docker image for this pull request (#54) has been successfully built and pushed to DockerHub as l7mp/stunner-gateway-operator:pr-54.

@levaitamas
Copy link
Member

Nice work! See some minor comments in the code. Two additional things: 1) IMHO, using a new registry for PR builds requires extra effort from the users, and the missing -pr from the name might be a source of errors. Having many tags in a single image repo is a common practice. 2) Can we add an action similar to stunner?

Note: we should disable docker build/push actions in forks

.github/workflows/pr-image-builder.yaml Outdated Show resolved Hide resolved
.github/workflows/pr-image-builder.yaml Outdated Show resolved Hide resolved
@rg0now
Copy link
Member

rg0now commented Aug 18, 2024

Thx, I like this. Minor wishlist item: is there any chance to garbage-collect the image at dockerhub once a PR is closed?

@davidkornel
Copy link
Member Author

@levaitamas
Alright, let's keep them under the same registry name. And we definitely should add the same action for stunner as well.

@rg0now Garbage collection is a good idea. I will add it.

* workflow name
* keep it from running in all forks
* FQ name in the comment
Copy link

The Docker image for this pull request (#54) has been successfully built and pushed to DockerHub as docker.io/l7mp/stunner-gateway-operator:pr-54.

Copy link

The Docker image for this pull request (#54) has been successfully built and pushed to DockerHub as docker.io/l7mp/stunner-gateway-operator:pr-54.

1 similar comment
Copy link

The Docker image for this pull request (#54) has been successfully built and pushed to DockerHub as docker.io/l7mp/stunner-gateway-operator:pr-54.

@davidkornel
Copy link
Member Author

Testing garbage collection

@davidkornel davidkornel reopened this Aug 19, 2024
@davidkornel davidkornel reopened this Aug 19, 2024
@davidkornel
Copy link
Member Author

Garbage collection tag results in 401. Need to figure out the issue with it and need to evaluate the response code properly.

Copy link

The Docker image for this pull request (#54) has been successfully built and pushed to DockerHub as docker.io/l7mp/stunner-gateway-operator:pr-54.

2 similar comments
Copy link

The Docker image for this pull request (#54) has been successfully built and pushed to DockerHub as docker.io/l7mp/stunner-gateway-operator:pr-54.

Copy link

The Docker image for this pull request (#54) has been successfully built and pushed to DockerHub as docker.io/l7mp/stunner-gateway-operator:pr-54.

@davidkornel
Copy link
Member Author

Garbage collection is fixed
@levaitamas could you take a look?

@davidkornel davidkornel reopened this Aug 27, 2024
Copy link
Member

@levaitamas levaitamas left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @davidkornel!

Copy link

The Docker image for this pull request (#54) has been successfully built and pushed to DockerHub as docker.io/l7mp/stunner-gateway-operator:pr-54.

@davidkornel davidkornel merged commit 735fa16 into main Aug 30, 2024
14 of 15 checks passed
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