Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
refactor: use only goreleaser to build unsigned chainlink images in one workflow #14034
refactor: use only goreleaser to build unsigned chainlink images in one workflow #14034
Changes from 50 commits
b7c8fc3
e079212
1c3244b
3e2eee5
61a1c66
6309fcf
7d6edc7
d33bcdd
5b01357
c5390a5
1e3222f
c607492
800b5bf
253e74d
a903b20
2073502
793b3a7
3c874d3
e70dbe1
c9aa538
30c0ec7
3c83784
be22f6d
3b0aa83
6d91f10
8765e27
4d17c65
cf60d4c
4aca4de
cc65cf1
8e22ef7
4757406
0b2f3e0
6424fdc
7b29382
da70962
11aa57b
583bf8d
0d9fd36
d33f98d
481ce97
c8e41ce
6a2a19e
83c4b76
9274809
2088c24
42b57c7
f51f0d8
b699543
96829b0
78a0fbc
3a93e0c
1084cb3
c5ec2d8
3fe4b7e
09b3d37
996a83f
37d617b
60e55dd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we want this to be false if it was from a PR unless that PR had a specific label?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are enabling to publish to the private sdlc ecr. it's a custom thing we created because the regular goreleaser release with snapshot does not do publish at all.
or did you mean we don't want to publish for any pr - just pr with the label
build-publish
?chainlink/.github/actions/goreleaser-build-sign-publish/action_utils
Lines 83 to 86 in 96829b0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This. Just to minimize the number of images we publish that won't ever get used. But I'm assuming we would use this workflow to build and then optionally publish. Is there still a separate workflow that builds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh I had created the conditional here:
chainlink/.github/workflows/build-publish-develop-pr.yml
Lines 38 to 43 in 96829b0
so only the label
build-publish
will be built and publishedit starts out with being
false
:chainlink/.github/workflows/build-publish-develop-pr.yml
Line 29 in 96829b0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. That will skip the build and publish docker job if the label is not on the PR, right? I guess I was assuming that we would have this workflow to cover all normal builds too (PR's without the label) and in those cases, we don't want to publish the image (if the label isn't on the PR).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like it if we could hook our E2E tests (which run on almost every PR) into this build step as well and not have our own custom one any more. Is requiring the
build-publish
tag necessary for this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah great point - looking over your ticket I had added this :)
chainlink/.github/workflows/build-publish-develop-pr.yml
Lines 9 to 17 in 37d617b
would this solve your issue?
of course we will need to tweak the
goreleaser-build-sign-publish
composite action a bitThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kalverra that's going to be big (avoid all of those unnecessary builds). Glad you're thinking about that.
We might need to push all images from PRs to ECR though if you're going to use it from tests (unless we used artifacts but...). In that case maybe we have a way to differentiate between docker images from PR's that can be pruned out quickly via lifecycle rules (only used for tests) and docker images from PR's that might need to run in longer tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see QA has different set of secrets for ECR? it pushes to a different repo no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now it does, but I think @kalverra is considering using this image for the tests. If so, we'd need to push all PR images to ECR for your tests, right?
This file was deleted.
This file was deleted.