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

fix: rewrite Dockerfile to resolve architecture mismatch issues #307

Merged
merged 2 commits into from
Apr 25, 2024

Conversation

rbtr
Copy link
Collaborator

@rbtr rbtr commented Apr 24, 2024

Description

This should address the arm/amd64 binary errors in the built image while keeping the Go build stage fast by using the BUILDPLATFORM/TARGETPLATFORM and cross-compiling.

It also moves the tools stage from bullseye to bookworm: bookworm has clang-14 available in the package manager and directly installable, so all of the manual downloads are removed, and the explicit installations are cut down significantly.

Removes some unnecessary Docker cruft which may have been well-intentioned but isn't useful.

Related Issue

This builds on #260 which fixed #130.

Checklist

  • I have read the contributing documentation.
  • I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
  • I have correctly attributed the author(s) of the code.
  • I have tested the changes locally.
  • I have followed the project's style guidelines.
  • I have updated the documentation, if necessary.
  • I have added tests, if applicable.

Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes made.

Additional Notes

Add any additional notes or context about the pull request here.


Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.

@rbtr rbtr requested a review from a team as a code owner April 24, 2024 18:11
@rbtr rbtr requested review from matmerr and alam-tahmid April 24, 2024 18:11
@rbtr
Copy link
Collaborator Author

rbtr commented Apr 24, 2024

@rbtr rbtr changed the title fix: rewrite dockrefile to resolve architecture mismatch issues fix: rewrite Dockerfile to resolve architecture mismatch issues Apr 24, 2024
@rbtr rbtr force-pushed the fix/build-arch branch 2 times, most recently from be8ddc9 to 833f1d5 Compare April 24, 2024 19:13
This should address the arm/amd64 binary errors in the built image
while keeping the Go build stage fast by using the buildplatform
and cross-compiling.

Signed-off-by: Evan Baker <[email protected]>
@rbtr rbtr requested a review from nddq April 24, 2024 20:07
@rbtr rbtr enabled auto-merge April 24, 2024 20:07
@rbtr rbtr disabled auto-merge April 24, 2024 21:04
@rbtr rbtr force-pushed the fix/build-arch branch 2 times, most recently from 781a128 to 8ded7d1 Compare April 25, 2024 17:58
Signed-off-by: Evan Baker <[email protected]>
@rbtr rbtr enabled auto-merge April 25, 2024 20:41
@rbtr
Copy link
Collaborator Author

rbtr commented Apr 25, 2024

Tested by deploying the image from my branch on to a Linux x [x86, arm64] and Windows cluster and verifying that all retina-agents started and plugins loaded without error
cc @anubhabMajumdar

@rbtr rbtr added type/enhancement New feature or request lang/go The Go Programming Language area/infra Test, Release, or CI Infrastructure priority/1 P1 lang/c The C Programming Language scope/L Change is Large labels Apr 25, 2024
@nddq
Copy link
Contributor

nddq commented Apr 25, 2024

what was the cause of the segfault? @rbtr

@rbtr
Copy link
Collaborator Author

rbtr commented Apr 25, 2024

what was the cause of the segfault? @rbtr

upgraded the tools image to debian 12 (and installed clang14 there) but the agent image was still using debian 11 distroless. updated that and it resolved

@rbtr rbtr added this pull request to the merge queue Apr 25, 2024
Merged via the queue into microsoft:main with commit 709ccb0 Apr 25, 2024
21 checks passed
@rbtr rbtr deleted the fix/build-arch branch April 25, 2024 23:38
nddq pushed a commit that referenced this pull request May 6, 2024
# Description

This should address the arm/amd64 binary errors in the built image while
keeping the Go build stage fast by using the
BUILDPLATFORM/TARGETPLATFORM and cross-compiling.

It also moves the tools stage from bullseye to bookworm: bookworm has
clang-14 available in the package manager and directly installable, so
all of the manual downloads are removed, and the explicit installations
are cut down significantly.

Removes some unnecessary Docker cruft which may have been
well-intentioned but isn't useful.

## Related Issue

This builds on #260 which fixed #130.

## Checklist

- [x] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [x] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [x] I have correctly attributed the author(s) of the code.
- [x] I have tested the changes locally.
- [x] I have followed the project's style guidelines.
- [x] I have updated the documentation, if necessary.
- [x] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes
made.

## Additional Notes

Add any additional notes or context about the pull request here.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.

---------

Signed-off-by: Evan Baker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/infra Test, Release, or CI Infrastructure lang/c The C Programming Language lang/go The Go Programming Language priority/1 P1 scope/L Change is Large type/enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Unable to fork /bin/clang.
2 participants