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

Implement Docker best practices - Round 2 #6388

Merged

Conversation

woody-apple
Copy link
Contributor

@woody-apple woody-apple commented Apr 30, 2021

Opening for @electrocucaracha given previous revert...

Problem

Hadolint is a tool which analyzes Dockerfile and provides applicable best practices. Version pinning reduce failures due to unanticipated changes in required packages. The CHIP integration Docker images don't have version pinning implemented.

Summary of Changes

These changes implement version pinning best practice as well as reduce some instructions like using WORKDIR Docker and --no-install-recommends apt instructions

Signed-off-by: Victor Morales <v.morales@samsung.com>
Signed-off-by: Victor Morales <v.morales@samsung.com>
Signed-off-by: Victor Morales <v.morales@samsung.com>
Signed-off-by: Victor Morales <v.morales@samsung.com>
Signed-off-by: Victor Morales <v.morales@samsung.com>
Signed-off-by: Victor Morales <v.morales@samsung.com>
Signed-off-by: Victor Morales <v.morales@samsung.com>
Signed-off-by: Victor Morales <v.morales@samsung.com>
@woody-apple
Copy link
Contributor Author

Hi @electrocucaracha I re-opened this as the docker builds don't work. I have a couple suggestions coming (just running a test build now), but would you be able to run build-all.sh in the integrations/docker/images path beforehand? We don't have great CI to verify them yet unfortunately :(

@electrocucaracha
Copy link
Collaborator

Hi @electrocucaracha I re-opened this as the docker builds don't work. I have a couple suggestions coming (just running a test build now), but would you be able to run build-all.sh in the integrations/docker/images path beforehand? We don't have great CI to verify them yet unfortunately :(

@woody-apple I ran build.sh script on every folder, I'm going to try to run build-all.sh so see if there is a difference.

@woody-apple
Copy link
Contributor Author

@electrocucaracha here's what fixes chip-build from my side, the nxp side still fails...

diff --git a/integrations/docker/images/chip-build/Dockerfile b/integrations/docker/images/chip-build/Dockerfile
index 1da02a0d55..065f752d14 100644
--- a/integrations/docker/images/chip-build/Dockerfile
+++ b/integrations/docker/images/chip-build/Dockerfile
@@ -65,13 +65,30 @@ RUN set -x \
     && rm -rf /var/lib/apt/lists/ \
     && : # last line
 
+# NodeJS: install a newer version than what apt-get would read
+RUN set -x \
+    && apt-get update \
+    && cd tmp \
+    && curl -sL https://deb.nodesource.com/setup_12.x -o nodesource_setup.sh \
+    && bash nodesource_setup.sh \
+    && apt-get install nodejs \
+    && rm -rf /var/lib/apt/lists/ \
+    && node -v \
+    && : # last line
+
+
 # Cmake (Mbed OS requires >=3.19.0-rc3 version which is not available in Ubuntu 20.04 repository)
 RUN set -x \
-    && (cd /tmp \
-    && wget --progress=dot:giga https://github.com/Kitware/CMake/releases/download/v3.19.3/cmake-3.19.3-Linux-x86_64.sh \
-    && sh cmake-3.19.3-Linux-x86_64.sh --exclude-subdir --prefix=/usr/local \
-    && rm -rf cmake-3.19.3-Linux-x86_64.sh) \
-    && exec bash \
+    && cd /tmp \
+    && wget --progress=dot:giga https://github.com/Kitware/CMake/releases/download/v3.19.3/cmake-3.19.3.tar.gz \
+    && tar -zxvf cmake-3.19.3.tar.gz \
+    && cd cmake-3.19.3 \
+    && ./bootstrap \
+    && make \
+    && make install \
+    && cd /tmp \
+    && rm -rf cmake-3.19.3.tar.gz \
+    && rm -rf cmake-3.19.3 \
     && : # last line
 
 # Python 2 and PIP
@@ -120,12 +137,4 @@ RUN set -x \
     && rm -rf bloaty \
     && : # last line
 
-# NodeJS: install a newer version than what apt-get would read
-# This installs the latest LTS version of nodejs
-RUN set -x \
-    && curl -o node.tgz -s https://nodejs.org/dist/v12.19.0/node-v12.19.0-linux-x64.tar.xz \
-    && mkdir -p /opt/node \
-    && tar xfJ node.tgz --strip-components=1 -C /opt/node \
-    && ln -s /opt/node/bin/* /usr/bin \
-    && rm -f node.tgz \
-    && : # last line
+RUN node -v

@electrocucaracha
Copy link
Collaborator

@woody-apple I'm not able to reproduce the error in my environment, do you have a log where I can take a look?

@woody-apple
Copy link
Contributor Author

@woody-apple I'm not able to reproduce the error in my environment, do you have a log where I can take a look?

Building again now, takes about 45 minutes. However TLDR is, it looks like the new references to specific architecture are breaking it...

@woody-apple
Copy link
Contributor Author

woody-apple commented Apr 30, 2021

@woody-apple I'm not able to reproduce the error in my environment, do you have a log where I can take a look?

Building again now, takes about 45 minutes. However TLDR is, it looks like the new references to specific architecture are breaking it...

Actually failed fast:

 => ERROR [7/8] RUN set -x     && git clone --depth=1 https://github.com/google/bloaty.git     && mkdir -p bloaty/build     && cd bloaty/build     && cmake ../     && make -j8     && make install      4.0s
------                                                                                                                                                                                                        
 > [7/8] RUN set -x     && git clone --depth=1 https://github.com/google/bloaty.git     && mkdir -p bloaty/build     && cd bloaty/build     && cmake ../     && make -j8     && make install     && cd ../..     && rm -rf bloaty     && : # last line:                                                                                                                                                                     
#10 2.110 + git clone --depth=1 https://github.com/google/bloaty.git                                                                                                                                          
#10 2.171 Cloning into 'bloaty'...                                                                                                                                                                            
#10 3.590 + mkdir -p bloaty/build                                                                                                                                                                             
#10 3.597 + cd bloaty/build
#10 3.597 + cmake ../
#10 3.605 /lib64/ld-linux-x86-64.so.2: No such file or directory

It's failing because the cmake build doesn't match the docker VM architecture

@electrocucaracha
Copy link
Collaborator

bloaty.git

What about removing the --no-install-recommends apt-get instruction? maybe a missing package was previously installed

@woody-apple
Copy link
Contributor Author

bloaty.git

What about removing the --no-install-recommends apt-get instruction? maybe a missing package was previously installed

Tried that. Had to rebuild cmake (per diffs) that wasn't arch specific to make it work... :\

@bzbarsky-apple bzbarsky-apple merged commit 4f2c389 into project-chip:master May 5, 2021
@woody-apple
Copy link
Contributor Author

Going to verify this... not sure it builds yet...

@woody-apple
Copy link
Contributor Author

nope, doesn't build...

 => ERROR [7/8] RUN set -x     && git clone --depth=1 https://github.com/google/bloaty.git     && mkdir -p bloaty/build     && cd bloaty/build     && cmake ../     && make -j8     && make install      3.0s

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

Successfully merging this pull request may close these issues.

5 participants