-
Notifications
You must be signed in to change notification settings - Fork 38
Unreal Horde Source Build Documentation with ARM64 Support #813
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
base: main
Are you sure you want to change the base?
Conversation
b78cc1d to
44ff5cb
Compare
hwkiem
left a comment
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.
Great PR. A few minor changes and a couple discussions for the team.
| }, var.github_credentials_secret_arn != null && !var.is_source_build ? { | ||
| repositoryCredentials = { | ||
| "credentialsParameter" : var.github_credentials_secret_arn | ||
| } | ||
| } : {}, { |
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.
Could move some of this logic to a local
| { | ||
| name = "unreal-horde-docdb-cert", | ||
| image = "public.ecr.aws/docker/library/bash:5.3", | ||
| image = "public.ecr.aws/docker/library/bash:latest", |
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.
It is probably safer to pin to a specific version of bash. :latest opens us up to upstream issues
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 know we do this elsewhere... probably should open an issue and fix those too.
| local.need_p4_trust ? [{ | ||
| name = "unreal-horde-p4-trust", | ||
| image = "ubuntu:noble" | ||
| image = "public.ecr.aws/ubuntu/ubuntu:noble" |
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.
nice fix. I wonder if we should also pin this release to a particular version for the same reason as above.
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.
Do you think we should put this in a nested directory like this? Or just update the Horde README with these instructions?
| ### For devices using ARM64 architecture | ||
| 1. In an IDE, open the Horde Server Dockerfile located at `/Engine/Source/Programs/Horde/HordeServer/Dockerfile`. | ||
| 2. Replace the code between `COPY --from=redis /usr/local/bin/redis-server /usr/local/bin/redis-server` and `COPY Source/Programs/Shared/EpicGames.Core/*.csproj ./Source/Programs/Shared/EpicGames.Core/` with the following: | ||
|
|
||
| ```dockerfile | ||
| # Since the .deb does not install in this image, just download it and extract the static binary | ||
| RUN ARCH=$(dpkg --print-architecture) && \ | ||
| if [ "$ARCH" = "arm64" ]; then \ | ||
| wget https://repo.mongodb.org/apt/ubuntu/dists/jammy/mongodb-org/7.0/multiverse/binary-arm64/mongodb-org-server_7.0.4_arm64.deb && \ | ||
| dpkg -x mongodb-org-server_7.0.4_arm64.deb /tmp/mongodb; \ | ||
| else \ | ||
| wget https://repo.mongodb.org/apt/debian/dists/bookworm/mongodb-org/7.0/main/binary-amd64/mongodb-org-server_7.0.4_amd64.deb && \ | ||
| dpkg -x mongodb-org-server_7.0.4_amd64.deb /tmp/mongodb; \ | ||
| fi && \ | ||
| cp /tmp/mongodb/usr/bin/mongod /usr/local/bin/mongod | ||
| ``` | ||
| 3. Locate the code block with the comment `# Remove native libs not used on Linux x86_64`. We need to modify this so that the build has the libraries associated with the operating system and CPU architecture we are using. To do this, replace that block with the following: | ||
| ```dockerfile | ||
| # Remove native libs not used for current architecture | ||
| RUN ARCH=$(dpkg --print-architecture) && \ | ||
| if [ "$ARCH" = "amd64" ]; then \ | ||
| rm -rf /app/out/runtimes/osx* && \ | ||
| rm -rf /app/out/runtimes/win-x86 && \ | ||
| rm -rf /app/out/runtimes/win-arm* && \ | ||
| rm -rf /app/out/runtimes/linux-arm* && \ | ||
| rm -rf /app/out/runtimes/linux-x86 && \ | ||
| rm -rf /app/out/runtimes/linux/native/libgrpc_csharp_ext.x86.so; \ | ||
| elif [ "$ARCH" = "arm64" ]; then \ | ||
| rm -rf /app/out/runtimes/osx-x64 && \ | ||
| rm -rf /app/out/runtimes/win-x64 && \ | ||
| rm -rf /app/out/runtimes/win-x86 && \ | ||
| rm -rf /app/out/runtimes/linux-x64* && \ | ||
| rm -rf /app/out/runtimes/linux-x86 && \ | ||
| rm -rf /app/out/runtimes/linux/native/libgrpc_csharp_ext.x64.so && \ | ||
| rm -rf /app/out/runtimes/linux/native/libgrpc_csharp_ext.x86.so; \ | ||
| fi | ||
| ``` |
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.
What in the WORLD is going on here? Why are we removing the redis dependencies? I am 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.
This was the original code from the UE repository
# Remove native libs not used on Linux x86_64
RUN rm -rf /app/out/runtimes/osx* && \
rm -rf /app/out/runtimes/win* && \
rm -rf /app/out/runtimes/linux-arm* && \
rm -rf /app/out/runtimes/linux/native/libgrpc_csharp_ext.x86.so
It seems to be removing libs that are not used for the Linux x86_64 because that is the architecture this is built for by default. I think this is to remove unnecessary libraries to remove bloat from the docker image. In order to follow this same principle we make this compatible with all compute architectures and OS by identifying the architecture and OS the build is for, then removing the other libraries.
| <Docker-Build BaseDir="Engine/Source/Programs/Horde/HordeServer" Files="Dockerfile*" UseBuildKit="true" Tag="horde-server" DockerFile="Engine/Source/Programs/Horde/HordeServer/Dockerfile.dashboard" /> | ||
|
|
||
| <!-- Publish the docker image to Amazon ECR --> | ||
| <Docker-Push Repository="<account-id>.dkr.ecr.<region>.amazonaws.com" Image="horde-server" TargetImage="horde/horde-server:$(Version)" AwsEcr="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 do like this... Just wondering if we want to recommend the creation of a new graph. I wonder if you could raise a PR against the the engine itself with modifications / improvements to the Horde buildgraph.
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 agree also would make sense to do the same with the Horde Server Dockerfile since at the moment it only is designed to support AMD64 and I have the fixes to make it more adaptive to the different architectures which you see in the README
| 1. In your IDE, open the main.tf located at `modules/unreal/horde/examples/complete/main.tf`. | ||
| 2. Under the module declaration `"unreal_engine_horde"` add the following line with the value of the image URI you noted earlier. Be sure to also set the value of `is_source_build` to `true` and set the value of `horde_server_architecture` to `ARM64` or `X_86` by passing the value with the `var.horde_server_architecture` variable as shown below: | ||
| ```python | ||
| module "unreal_engine_horde" { | ||
| ... | ||
| image = "<image-uri>" | ||
| is_source_build = true | ||
| horde_server_architecture = "<horde-server-architecture>" | ||
| ... | ||
| } | ||
| ``` |
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 should manage this through variables on the example.
| variable "horde_server_architecture" { | ||
| type = string | ||
| description = "The CPU architecture for Horde server container. Valid values: x86 or arm64" | ||
| default = "X86_64" | ||
| validation { | ||
| condition = contains(["X86_64", "ARM64"], var.horde_server_architecture) | ||
| error_message = "horde_server_architecture must be either 'X_86' or 'ARM64'" | ||
| } | ||
| } |
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.
Can we determine architecture via data block and a lookup against the image?
| variable "is_source_build" { | ||
| type = bool | ||
| description = "Set this flag to true if you are using a custom built Horde Server image from source." | ||
| default = false | ||
| } |
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.
Why do we need to know if its a source build?
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 added that because in ecs.tf we have the current steps to use github credentials as part of the container definition to pull the public horde docker image. I didn't want to break the functionality so created the variable so the user can still use the pre-built docker image if they choose or go for the source build
| variable "operating_system" { | ||
| type = string | ||
| description = "The operating system for the Horde server container. Valid values: linux or windows" | ||
| default = "WINDOWS_SERVER_2019_CORE" | ||
| validation { | ||
| condition = contains(["LINUX", "WINDOWS_SERVER_2025_FULL", "WINDOWS_SERVER_2025_CORE", "WINDOWS_SERVER_2022_FULL", "WINDOWS_SERVER_2022_CORE", "WINDOWS_SERVER_2019_FULL", "WINDOWS_SERVER_2019_CORE"], var.operating_system) | ||
| error_message = "Operating_system must be either LINUX, WINDOWS_SERVER_2025_FULL, WINDOWS_SERVER_2025_CORE, WINDOWS_SERVER_2022_FULL, WINDOWS_SERVER_2022_CORE, WINDOWS_SERVER_2019_FULL, and WINDOWS_SERVER_2019_CORE." | ||
| } | ||
| } |
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.
Also this. I am not sure why we want to support a windows OS for Horde unless a customer explicitly asks for it. Open to your thoughts on 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.
That was something I wanted to ask. What are the most commonly used OS for Horde?
44ff5cb to
f1986bd
Compare
📝 Markdown Linting FailedThe markdown linter has found issues in the changed documentation files. Action RequiredPlease review the inline annotations in the Files changed tab and the workflow logs to fix the linting errors. Common Issues
Auto-fix AvailableMany issues can be automatically fixed by running: npx markdownlint-cli2 --fix "**/*.md"Last checked: dea95d5 |
📚 Documentation Preview✅ Preview deployed successfully! 🔗 Preview URL: https://aws-games.github.io/cloud-game-development-toolkit/preview-pr-813/ 🔒 Maintainer Action RequiredThe preview requires approval before it's accessible. A maintainer must approve the GitHub Pages deployment in the Environments section. Once approved, the preview will be accessible within 1-2 minutes. Build Information
This preview will be automatically deleted when the PR is merged or closed. |
f1986bd to
dea95d5
Compare
dea95d5 to
a4fd601
Compare
Issue number:
closes #743
Summary
Changes
User experience
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created might not be successful.