-
Notifications
You must be signed in to change notification settings - Fork 5
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
dockerize snapshot upload #400
Conversation
ee53148
to
d2ba038
Compare
Forest: Snapshot Service Infrastructure Plan: successShow Plan
|
d2ba038
to
15f51ba
Compare
15f51ba
to
5bcb69a
Compare
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.
LGTM
- name: Build image and push to GitHub Container Registry | ||
uses: docker/build-push-action@v5 | ||
with: | ||
context: ./images/snapshot-service/ |
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.
context: ./images/snapshot-service/ | |
context: ./images/snapshot-service/ | |
load: true |
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 believe without load: true
, docker image ls
steps won't list the output image
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.
|
||
# TODO: Change this once `sync-check` is fully-dockerized as well. | ||
# hadolint ignore=DL3022 | ||
COPY --from=common ruby_common /opt/snapshot-service/ruby_common |
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.
where is common
defined?
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, it's from build-context
. Maybe adding some comments to reduce the confusion
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.
@hanabi1224 done. I had to drop apk
packages pinning, sadly, alpine repos don't keep old packages, so there's no way to meaningfully pin the versions.
14c293b
to
a7cb602
Compare
This reverts commit 8ce48b2.
Summary of changes
Changes introduced in this pull request:
I'll pin the snapshot version in the TF to a specific version once it is built by CI (it needs to reach
main
first).Reference issue to close (if applicable)
Part of #364
Other information and links