Add arm64 support
#45
Conversation
| # Setup image with nginx if not installed and the corresponding version of | ||
| # the nginx-datadog module. |
There was a problem hiding this comment.
Nice addition. It's a little hard for me to parse, though. Maybe:
# Set up the nginx container image. Install nginx, if necessary, and install the nginx-datadog module.
There was a problem hiding this comment.
with wherever that line break wants to go.
| nginx_modules_path=/usr/lib/nginx/modules | ||
| ;; | ||
| *) | ||
| >&2 echo 'Unsupported platform.' |
There was a problem hiding this comment.
What does this exclude?
Also, consider stating in the error message that it's BASE_IMAGE (which might have come from nginx-version-info) that contains the unsupported value.
There was a problem hiding this comment.
Ditto. Maybe only doing the check in install_datadog.sh is enough. What's docker compose behavior if a service can't be built? What's your thought?
There was a problem hiding this comment.
Oh, now I see. This is a little formatting and an added explicit "else" case, which previously was implicitly a no-op.
If docker compose is told to build an image, then failure to build is an error. If it's not told to build an image, than it will use an existing image if one exists, otherwise it's an error.
Adding this check is a good idea -- it probably makes for easier debugging if someone is trying different values for BASE_IMAGE. Consider this wording:
| >&2 echo 'Unsupported platform.' | |
| >&2 echo 'BASE_IMAGE value "%s" is invalid. See nginx-version-info.example for more information.' "$BASE_IMAGE" |
There was a problem hiding this comment.
ah, my mistake, not yours :)
Please see my more recent comment.
Co-authored-by: David Goffredo <david.goffredo@datadoghq.com>
Co-authored-by: David Goffredo <david.goffredo@datadoghq.com>
Resolves #31