Skip to content

Conversation

joshuarli
Copy link
Member

@joshuarli joshuarli commented Oct 14, 2025

redo of #1802; we need to preserve the docker tar building step for the self-hosted e2e tests

discussion here: https://github.com/getsentry/symbolicator/pull/1802/files#r2428050962

tl;dr it would be best to have the option to pass a docker tar so we don't have to rebuild with action-build-push-images, but we're lucky here that the build is so lightweight that this is acceptable in my mind for now. Tracked here: https://linear.app/getsentry/issue/DI-1382/action-build-and-push-images-should-have-the-option-to-take-a-docker

@joshuarli joshuarli requested a review from a team as a code owner October 14, 2025 20:30
image_name: 'symbolicator'
platforms: linux/amd64,linux/arm64
build_context: '/tmp/docker-ctx'
publish_on_pr: true # TEMPORARY FOR TESTING
Copy link
Member Author

Choose a reason for hiding this comment

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

self reminder to remove this before merging

path: /tmp/symbolicator-${{ matrix.arch }}.tar

assemble-ghcr:
assemble:
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there's an argument towards keeping ghcr publishing and gar publishing separate jobs given a scenario where ghcr goes down, the job fails entirely and deploys are blocked. I like the idea of combining all this into one workflow though. WDYT @Dav1dde?

Copy link
Member

Choose a reason for hiding this comment

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

I think it was just easier to deal with conditionals (publish to this or that) and docker credentials in separate jobs. Good idea to consolidate these.

@joshuarli
Copy link
Member Author

joshuarli commented Oct 16, 2025

this is ready for review at this point. multiarch was published to GAR here (will revert publish on prs before merging this PR):

i've pulled all 4 images and they work!

Comment on lines 119 to +123
- name: Prepare Docker Context
run: |
mkdir docker-ctx
mkdir -p docker-ctx/binaries/linux/${{ matrix.arch }}
cp Dockerfile docker-ctx/
mv symbolicator docker-ctx/
mv symbolicator docker-ctx/binaries/linux/${{ matrix.arch }}
Copy link
Member

Choose a reason for hiding this comment

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

Why is this still happening if you upload the binary?

path: /tmp/symbolicator-${{ matrix.arch }}.tar

assemble-ghcr:
assemble:
Copy link
Member

Choose a reason for hiding this comment

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

I think it was just easier to deal with conditionals (publish to this or that) and docker credentials in separate jobs. Good idea to consolidate these.

Comment on lines +14 to +15
# sanity check
RUN ["/bin/symbolicator", "help"]
Copy link
Member

Choose a reason for hiding this comment

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

Should definitely not merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants