feat(ruby): build and release layer for both amd64 and arm64 architec…#2349
feat(ruby): build and release layer for both amd64 and arm64 architec…#2349DCchoudhury15 wants to merge 10 commits into
Conversation
|
@wpessers pr up for review whenever you get time |
wpessers
left a comment
There was a problem hiding this comment.
Hi @DCchoudhury15, first of all appreciate the effort and time that went into the PR. However, I'd like to point out that this dual-arch build logic is something we already have for the collector layer. Maybe it would be wise to check out how the build and release workflow for that one work.
Also I'm not entirely convinced about the QEMU stuff, you're going to be compiling the ruby interpreter from source and that will take a long time. I'm wondering, why can't we just use an arm64 gh action runner instead?
Finally and most importantly, it looks like you disabled integration testing on arm64 for all other language layers. That's not something we want to do, could you explain the thought process here?
|
Hi @wpessers, Thank you for taking the time to review this and for the helpful feedback. Regarding the QEMU setup, I wasn't entirely sure whether native For the integration tests, I got a bit too focused on getting the Ruby artifact naming changes working on my branch and missed the broader impact of the matrix exclusions. You're absolutely right that those changes would have disabled ARM test coverage for the other languages as well, which was not my intention. I've removed the exclusions so the existing test coverage remains unchanged. I'll push the updated commit shortly. When you have a chance, I'd appreciate it if you could take another look and let me know whether the new runner setup is more in line with what you had in mind. Thanks again for the your time and patience! |
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
| runner: ["ubuntu-latest"] | ||
| include: | ||
| - language: ruby | ||
| architecture: amd64 | ||
| runner: ubuntu-latest | ||
| - language: ruby | ||
| architecture: arm64 | ||
| runner: ubuntu-24.04-arm |
There was a problem hiding this comment.
Not sure how this will work when triggered for a single (non-ruby) language like python. I'm guessing this has the unintended side-effect of running the ruby integration test each time a non-ruby integration test is being ran.
| - name: Skip version output for non-amd64 | ||
| if: ${{ inputs.architecture != 'amd64' }} | ||
| id: version-skip | ||
| shell: bash | ||
| run: echo "component-version=" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
I find this quite confusing, you seem to have done a lot of work here and in the release workflow to skip outputting the version. IIRC the collector release has the same matrixed amd64/arm64 build and handles this with no such extra logic. As I said earlier, might be worth taking a look how the collector builds / releases are setup. Let me reiterate that you're basically building the same mechanism that is already in place there.
| - name: Save component version to file (amd64 only) | ||
| if: ${{ matrix.architecture == 'amd64' }} | ||
| shell: bash | ||
| run: echo "${{ steps.build.outputs.component-version }}" > component-version.txt | ||
|
|
||
| - name: Upload component version artifact (amd64 only) | ||
| if: ${{ matrix.architecture == 'amd64' }} | ||
| uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 | ||
| with: | ||
| name: ruby-component-version | ||
| path: component-version.txt | ||
|
|
||
| get-version: | ||
| runs-on: ubuntu-latest | ||
| needs: [build-layer] | ||
| outputs: | ||
| component-version: ${{ steps.read-version.outputs.version }} | ||
| steps: | ||
| - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 | ||
| with: | ||
| name: ruby-component-version | ||
|
|
||
| - id: read-version | ||
| shell: bash | ||
| run: echo "version=$(cat component-version.txt)" >> "$GITHUB_OUTPUT" | ||
|
|
There was a problem hiding this comment.
Same comment here, can we please take a look at the collector release workflow? I'm not convinced that we need all of this stuff.
There was a problem hiding this comment.
Hi @wpessers, Thanks for your patience and for pointing me back to the collector pattern , that was definitely the right approach.
For integration-test.yml, I originally tried using runner + include to assign the native arm64 runner only to Ruby builds. I overlooked the fact that include adds extra matrix combinations rather than modifying existing ones, which would have caused the Ruby tests to run with every non-Ruby language trigger as well. I've switched to using exclude instead, which is much cleaner and avoids that issue entirely.
For the version-skip step and get-version job in the release workflow, I was trying to prevent a matrix race condition where the arm64 shard finishing last might overwrite the component-version output with an empty value. After taking another look at the collector workflow, I realized it already handles this cleanly with an if: architecture == 'amd64' guard on the version step and a direct outputs reference. I was overthinking it, so I've reverted the implementation to match the collector pattern.Again than you for your patience and guidance.
Problem
The Ruby Lambda layer was only built for x86_64. When used with ARM-based Lambda functions, initialization failed because native gems such as
google-protobufwere compiled for the wrong architecture.Changes
ruby/src/build.shand generated architecture-specific ZIP artifacts.architectureinput to the Ruby layer build action and enabled cross-platform builds using QEMU and Buildx.Result
Ruby Lambda layers are now built correctly for both x86_64 and ARM64, preventing architecture-related runtime failures.
Fixes: #2340