You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "Nbelles (via GitHub)" <gi...@apache.org> on 2024/03/23 06:22:09 UTC
[PR] Guacamole in Docker on ARM [guacamole-server]
Nbelles opened a new pull request, #499:
URL: https://github.com/apache/guacamole-server/pull/499
Pull request based on discussion [here](https://lists.apache.org/thread/yoqfy9s6bfvcsgccc33kylgyk9h7pwko).
It is difficult to dynamically determine build options based on a given architecture from within a Dockerfile without requiring manually passing in the architecture type as an argument during the build process. Instead, this proposed approach uses the fact that a build script is used to actually build the dependencies and checks the architecture there. With this information it generates a list of overrides which are appended to the end of the `install_from_git` functions. Options are processed in order so by appending them to the end, they will override any options provided in the main Dockerfile. This guarantees the build process succeeds (even if the user provides bad options in the Dockerfile).
I have tested that it works on macOS (arch: amd64) and a Raspberry Pi 4 (arch: aarch64), open to hearing how it works on other architectures!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "Nbelles (via GitHub)" <gi...@apache.org>.
Nbelles commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2038806499
@jmuehlner Thanks for catching the spelling error and I removed the extra print statements in the latest commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2037420861
@Nbelles Thanks for that detailed info.
@mike-jumper or @jmuehlner Thoughts on this approach to modifying flags for the Docker build?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#discussion_r1552464060
##########
src/guacd-docker/bin/build-all.sh:
##########
@@ -95,11 +95,29 @@ install_from_git() {
}
+#
+# Determine any option overrides to guarantee successful build
+#
+
+export BUILD_ARCHITECTURE="$(arch)" # Determine architechutre building on
Review Comment:
Typo: `architechutre`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "Nbelles (via GitHub)" <gi...@apache.org>.
Nbelles commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2033585568
I just confirmed that I was able to connect to a Windows machine with RDP successfully through guacamole 1.5.5 on ARM on a Raspberry Pi 5 with the suggested commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman merged PR #499:
URL: https://github.com/apache/guacamole-server/pull/499
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2030892211
@Nbelles So you confirmed that the build behaves as expected when you specify both `-DWITH_SSE2=ON -DWITH_SSE2=OFF`, as the `OFF` one occurs last?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "Nbelles (via GitHub)" <gi...@apache.org>.
Nbelles commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2030970380
I have verified that the build process completes on ARM when the `OFF` flag comes after the `ON` flag (when just the `ON` flag fails to build on ARM).
I have someone visiting me tomorrow that has a Windows computer that I will be able to verify that RDP still works as expected when those flags are set according to the commit. Will report back with those results late tomorrow evening.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#discussion_r1552474549
##########
src/guacd-docker/bin/build-all.sh:
##########
@@ -95,11 +95,29 @@ install_from_git() {
}
+#
+# Determine any option overrides to guarantee successful build
+#
+
+export BUILD_ARCHITECTURE="$(arch)" # Determine architechutre building on
+echo "Build architecture: $BUILD_ARCHITECTURE"
+
+case $BUILD_ARCHITECTURE in
+ armv6l|armv7l|aarch64)
+ echo "Using ARM overrides"
Review Comment:
These echo statements look useful for debugging this change, but not so useful for general usage. If you look at above logging examples like `"Building $REPO_DIR @ $VERSION ..."`, it includes useful information that varies from build to build, where as "Using ARM overrides" is going to appear verbatim in the build output of every single ARM build ever, and also doesn't add any additional information that the output on line `103` isn't already telling you.
I'd just remove these last 2 log lines entirely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "Nbelles (via GitHub)" <gi...@apache.org>.
Nbelles commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2041288470
@jmuehlner Thanks for reminding me, should be squashed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2040260952
> @jmuehlner Thanks for catching the spelling error and I removed the extra print statements in the latest commit.
Thanks for the cleanup! Note that your second commit message still needs a `GUACAMOLE-1374` prefix, or alternatively you might consider just squashing it into the first commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2031726338
What about this:
https://nielscautaerts.xyz/making-dockerfiles-architecture-independent.html
In particular, using the TARGETPLATFORM build variable:
https://docs.docker.com/reference/dockerfile/#automatic-platform-args-in-the-global-scope
It seems like this would allow for actually conditionally setting the `FREERDP_OPTS` variable inside the Dockerfile, rather than resorting to the extra variable in the build script?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "Nbelles (via GitHub)" <gi...@apache.org>.
Nbelles commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2033390356
I had looked into this methodology and I like the idea of doing the logic directly in the `.Dockerfile` but the downside to this approach is that I don't see a way for a user to easily build it themselves without forcing them to determine their target platform and specify it to the `docker buildx build ...` command.
One route would be to use the `TARGETPLATFORM` argument which then forces all users who want to build the container themselves to use the `docker buildx build ...` command to build the `.Dockerfile`. The build command that pushes images to the docker hub would have to be updated as well. This would also mean the documentation would need to get updated to include instructions on how to build with `docker buildx build ...` because I do not believe that is common knowledge, as compared to the normal `docker build ...` command. We could also create a short script that determines the target platform and passes that to the `docker buildx build ...` command to make it easier for users to use this method but I think this is more work than the method used in the above commit.
The other route (as prescribed in the above commit) would be to override any erroneous build options that the platform doesn't support. Then a user could put desired options (potentially incorrectly) in the `.Dockerfile` and the `build-all.sh` file would account for any discrepancies between what is selected and what is required for the platform. We could add some additional logic to check the variables and print some warning about any inconsistent build options that are being overridden if that helps.
I personally prefer the latter option so that the build process is easy for users (albeit slightly more complex on the backend) but I'm open to hearing what others think about this tradeoff.
Note: The reason I think being able to build the docker image yourself is so important is that for anyone who develops their own extensions and wants to run them in docker (such as myself), there is currently no easy way to build the jar file and load it into the docker image with guacamole properties passed through without building the image yourself. I hope to create another pull request in the near future that allows for easy jar file and guacamole properties loading using the docker hub provided images.
(I still plan to verify the provided commit works as expected shortly)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2038264022
> @Nbelles Thanks for that detailed info.
>
> @mike-jumper or @jmuehlner Thoughts on this approach to modifying flags for the Docker build?
I don't have an issue with this approach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2041575942
> @jmuehlner Good with it if you are...
Looks good to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "Nbelles (via GitHub)" <gi...@apache.org>.
Nbelles commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2016654706
I think that should do the trick. I will also create a separate pull request shortly for the guacamole-client changes that need to be made to make that work on ARM as well.
I think the next step towards providing ARM Docker images on Docker Hub would be to follow something like the direction provided by [this Docker guide](https://www.docker.com/blog/multi-arch-build-and-images-the-simple-way/). I'm not sure how the Docker Hub build process for this repo is set up right now (I'm only seeing the code for building on pull request) but it seems like the next step would be to adjust the `docker build` command to be a `docker buildx build` similar to that guide to include building for multiple architectures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Guacamole in Docker on ARM [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2016479167
@Nbelles Thanks for putting this in. All pull requests and commits need to be associated and tagged with a Jira issue. In this case, you're in luck, because one already exists to cover this:
https://issues.apache.org/jira/browse/GUACAMOLE-1374.
If you could tag both the PR and your commit messages with "GUACAMOLE-1374:", that would be great.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-1374: Guacamole in Docker on ARM [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #499:
URL: https://github.com/apache/guacamole-server/pull/499#issuecomment-2041574710
@jmuehlner Good with it if you are...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org