Add `jq` to Ubuntu Base Image

trudeaua21 commented 2 years ago

Changes

Adds jq to the "toolbox" of apt-get packages in the Ubuntu Base Image
Removes the temporary jq installation/uninstallation from the Ubuntu Editor Image (since the Editor Image is derived from the Base image, and thus will already have jq installed when it's needed)

Checklist

[x] Read the contribution guide and accept the code of conduct
[x] Readme (updated or not needed)

webbertakken commented 2 years ago

Could you please make a screenshot of image size before and after using something like docker build . -t before, after and docker images ls?

trudeaua21 commented 2 years ago

Could you please make a screenshot of image size before and after using something like docker build . -t before, after and docker images ls?

Absolutely! Thanks for the instruction 😄

trudeaua21 commented 2 years ago

Here's the size differential for the base image:

***********myUser docker % docker image ls
REPOSITORY   TAG       IMAGE ID       CREATED         SIZE
after        latest    8e0c007fa634   5 minutes ago   466MB
before       latest    33a6128b8ab5   7 minutes ago   465MB

Here's the specific sizes, if you need those also:

***********myUser docker % docker image inspect before --format='{{.Size}}'
464757106
***********myUser docker % docker image inspect after --format='{{.Size}}'
465628969

I'm going to keep this as a draft until I can at least confirm that the editor image still works for what I'm working on for in the test-runner repo, just to have some confirmation that the editor image didn't break from having the explicit install/uninstall of jq removed! just realized I can't build the editor image locally, so I'm not entirely sure how I would get the editor image on to docker hub to test out.

So I'm going to mark this as ready for review, but I want to note that I have not actually tested the functionality of the image yet. I'm going to do some digging in the docs and online to see if I can find more ideas for ways to test (other than just running the test workflow, which I did on my fork: https://github.com/trudeaua21/docker/actions/runs/2041144388).

I totally understand if you need to wait for me to figure out a way to test this further before you merge it (especially since it says as much in the contribution guide).

I was unable to get the editor image to build in either before or after. It failed with this error on the before:

***********myUser docker % docker build images/ubuntu/editor -t editor-before
[+] Building 4.5s (7/24)
 => [internal] load build definition from Dockerfile                                                                                                                                                                                                                     0.0s
 => => transferring dockerfile: 37B                                                                                                                                                                                                                                      0.0s
 => [internal] load .dockerignore                                                                                                                                                                                                                                        0.0s
 => => transferring context: 2B                                                                                                                                                                                                                                          0.0s
 => [internal] load metadata for docker.io/unityci/base:latest                                                                                                                                                                                                           0.2s
 => [internal] load metadata for docker.io/unityci/hub:latest                                                                                                                                                                                                            0.3s
 => CACHED [builder 1/5] FROM docker.io/unityci/hub@sha256:0f725afe65fafbb3f8d577212ac8ef938d18f95570bc934be205503595f6845e                                                                                                                                              0.0s
 => CACHED [stage-1  1/15] FROM docker.io/unityci/base@sha256:31120b25c42f3861e7cae1169eb0cec7f0d6aab8932a3e2a107581ec7a875881                                                                                                                                           0.0s
 => ERROR [builder 2/5] RUN unity-hub install --version "$version" --changeset "$changeSet" | tee /var/log/install-editor.log && grep 'Error' /var/log/install-editor.log | exit $(wc -l)                                                                                4.1s
------
 > [builder 2/5] RUN unity-hub install --version "$version" --changeset "$changeSet" | tee /var/log/install-editor.log && grep 'Error' /var/log/install-editor.log | exit $(wc -l):
#7 2.980 Failed to execute the command due the following, please see '-- --headless help' for assistance.
#7 2.980 Error: Missing [-v|--version] argument.
#7 2.980
------
executor failed running [/bin/sh -c unity-hub install --version "$version" --changeset "$changeSet" | tee /var/log/install-editor.log && grep 'Error' /var/log/install-editor.log | exit $(wc -l)]: exit code: 1

I'm not super familiar with Docker so I don't know if this is just my environment not being set up correctly or not, but just thought I should note it just in case.

webbertakken commented 2 years ago

just realized I can't build the editor image locally

Our PR workflow will make test builds of all images, which will verify that the build instructions will still work.

(I just noticed that currently it doesn't because your fork also uses the branch main, I think - or maybe it needs another commit since it left draft status)

If you want to test locally you have to build base then hub and then editor. The last image requires some arguments (in a Dockerfile you can recognise them by ARG or ENV instructions), so it knows which version of the editor, which target platform modules to install and the change hash so it knows how to download that editor version. You can find the right parameters on our versions overview page.

trudeaua21 commented 2 years ago

@webbertakken Thank you so much for the help!! 😄

I will get that built locally, get it pushed up to docker hub, and run the unity-test-runner test workflow on a branch on my fork, but with each testing step using this new version of the editor image instead of the default one. I'll post the results here after I finish!

webbertakken commented 2 years ago

Awesome!

Note that unity-test-runner has a public api for using custom images: https://game.ci/docs/github/test-runner#customimage

(it assumes dockerhub, as it's the default from docker cli)

webbertakken commented 2 years ago

Could you please push another (empty) commit and check if that triggers the pipeline?

trudeaua21 commented 2 years ago

Looks like it did!

By the way, I actually haven't gotten around to testing this with unity-test-runner yet - I'll do so either tonight or tomorrow, just in case you were waiting for me to test this before merging.

trudeaua21 commented 2 years ago

Okay, I'm pretty convinced that it works, at least just based on the use-case of the packageMode functionality of the unity-test-runner. Here's the workflow run of it working on my own project: https://github.com/trudeaua21/EasyAccessibilityPackage/actions/runs/2068599396

Not sure what could be causing the 3 failing checks though (it looks like one timed out, but the other two had associated error messages). I'm not sure if that's something that has happened on other workflow runs, or if it's most likely that my change caused the issue.

To provide some more context, the failing checks (or at least, ones that have the same name, but maybe are different somehow?) seemed to pass on my own workflow test on my fork

If your guess would be that my change caused the failing checks, then I'll take a look deeper into the failing checks to see why!

webbertakken commented 2 years ago

Yep let's go!

game-ci / docker

Add `jq` to Ubuntu Base Image #168

Changes

Checklist