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

Add jq to Ubuntu Base Image #168

Merged
merged 2 commits into from
Apr 15, 2022
Merged

Add jq to Ubuntu Base Image #168

merged 2 commits into from
Apr 15, 2022

Conversation

trudeaua21
Copy link
Contributor

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

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

@webbertakken
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor Author

trudeaua21 commented Mar 25, 2022

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.

@trudeaua21 trudeaua21 marked this pull request as ready for review March 25, 2022 16:41
@trudeaua21 trudeaua21 marked this pull request as draft March 25, 2022 17:21
@trudeaua21 trudeaua21 marked this pull request as ready for review March 25, 2022 17:21
@webbertakken
Copy link
Member

webbertakken commented Mar 25, 2022

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.

image

@trudeaua21
Copy link
Contributor Author

@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
Copy link
Member

webbertakken commented Mar 25, 2022

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
Copy link
Member

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

@trudeaua21
Copy link
Contributor Author

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
Copy link
Contributor Author

trudeaua21 commented Mar 31, 2022

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!

Copy link
Member

@davidmfinol davidmfinol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-ran the checks, and they all passed now, so it must have been some transient error.

@webbertakken Should be good to merge, right?

@webbertakken
Copy link
Member

Yep let's go!

@webbertakken webbertakken merged commit 573058f into game-ci:main Apr 15, 2022
mob-sakai pushed a commit to mob-sakai/docker that referenced this pull request May 17, 2022
* add jq to the base image

* empty commit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants