Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@godofredoc
Copy link
Contributor

@godofredoc godofredoc commented Mar 16, 2021

Updates the docker image to use debian:stretch to match LUCI and Framework. This is in preparation to improve Cirrus CI usage at Flutter.

Bug: flutter/flutter#77624

@godofredoc godofredoc changed the title Test docker image update. Update docker image to match the LUCI and Framework images Mar 18, 2021
Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

LGTM but I think there's an apt-get install google-chome-stable missing.


# Add repo for chrome stable
RUN wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add -
RUN echo 'deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main' | tee /etc/apt/sources.list.d/google-chrome.list
Copy link
Member

Choose a reason for hiding this comment

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

Do we not need to install google-chrome-stable here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

dosfstools ifupdown libedit-dev libglib2.0-dev liblz4-tool libncurses5-dev libssl-dev lsof mtools nasm net-tools python2.7-dev \
python-m2crypto tcpdump texinfo uuid-dev vim xz-utils zlib1g-dev firefox-esr g++-multilib libblkid-dev libc6-dev-i386 \
libfreetype6-dev libegl1-mesa libgles2-mesa-dev libglu1-mesa-dev libgtk-3-dev libx11-dev libxcursor-dev libxinerama-dev libxrandr-dev \
libxxf86vm-dev mesa-common-dev openjdk-8-jdk zip xvfb ca-certificates gnupg
Copy link
Member

Choose a reason for hiding this comment

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

Not for this patch, but out of curiosity does this all need to be a single RUN command?

I wonder if it would be easier to follow if we broke into sections (in a followup patch) like:

  1. apt-get update
  2. Install gpg to support adding repos
  3. Add additional repos like chrome, gcloud
  4. apt-get update
  5. install basic tools like wget/curl/.../autoconf/.../groff/.../build-essential/tcp-dump
  6. Install X/GL build deps like libglib2.0-dev etc.
  7. Install browsers: chrome and firefox
  8. do gcloud config
  9. clone depot_tools
  10. clone buildroot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@cbracken
Copy link
Member

Have another one of these for good measure -- this looks fabulous!

LGTM stamp from a Japanese personal seal

gke_container:
dockerfile: "ci/docker/build/Dockerfile"
builder_image_name: docker-builder # gce vm image
builder_image_project: flutter-cirrus
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this? is this the gcp project that owns the storage bucket we upload images to? or the project that owns the docker-builder vm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the project the docker image is uploaded/downloaded from.

Copy link
Contributor

Choose a reason for hiding this comment

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

cool, i found the project

apt-get update && apt-get install -y google-cloud-sdk google-chrome-stable libx11-dev && \

# Updates the distribution.
RUN apt-get update
Copy link
Contributor

Choose a reason for hiding this comment

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

it's not a big deal, but the reason we did so many steps in a single RUN statement was to reduce the overall docker image size. each dockerfile statement is saved as its own image layer. if you download a file in one layer and then delete it in a subsequent layer, your final image will have the file size times two, whereas if you download and delete it in a single layer, the net file storage will be zero.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, do you know how can I check the sizes to compare? if the increase is significant I'll get it back to multiple statements per run.

Copy link
Contributor

Choose a reason for hiding this comment

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

you can pull the image you just created via: docker pull gcr.io/flutter-cirrus/35f4dd0e84cf343e53d000bce8899df0:latest. when I tried it locally I didn't have permissions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sizes are about the same:

REPOSITORY TAG IMAGE ID CREATED SIZE
gcr.io/flutter-cirrus/35f4dd0e84cf343e53d000bce8899df0 latest 76f5b861a37a 18 hours ago 2.57GB
gcr.io/flutter-cirrus/df42c0c510954e88f1278196db0afdce latest b9ce201299b9 4 months ago 2.38GB

0.19GB increase do you think is a good tradeoff for the readability of the file?

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@godofredoc godofredoc merged commit de59c6f into flutter:master Mar 18, 2021
@godofredoc godofredoc deleted the test_cirrus branch March 18, 2021 20:55
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 19, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 20, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 20, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 20, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 20, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 22, 2021
hjfreyer pushed a commit to hjfreyer/engine that referenced this pull request Mar 22, 2021
…5009)

* Test docker image update.

Do not review.

* Test build in a VM.

* Add project name.

* Update image type.

* Add apt-get dependencies.

* Update dependencies.

* Update gcloud packages installation.

* Update dockerfile.

* Fix errors in dockerfile.

* Fix dockerfile.

* Organize docker file to make it more readable.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 22, 2021
chriscraws pushed a commit to chriscraws/engine that referenced this pull request Mar 23, 2021
…5009)

* Test docker image update.

Do not review.

* Test build in a VM.

* Add project name.

* Update image type.

* Add apt-get dependencies.

* Update dependencies.

* Update gcloud packages installation.

* Update dockerfile.

* Fix errors in dockerfile.

* Fix dockerfile.

* Organize docker file to make it more readable.
duanqz pushed a commit to duanqz/engine that referenced this pull request Apr 16, 2021
…5009)

* Test docker image update.

Do not review.

* Test build in a VM.

* Add project name.

* Update image type.

* Add apt-get dependencies.

* Update dependencies.

* Update gcloud packages installation.

* Update dockerfile.

* Fix errors in dockerfile.

* Fix dockerfile.

* Organize docker file to make it more readable.
@zanderso zanderso mentioned this pull request Apr 5, 2022
8 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants