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

Updated Node Js Dockerfiles to Node 10.15.0 (Alpine) #117

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@Templum
Copy link

Templum commented Jan 15, 2019

As described in #99 the new version of NodeJs 10.X.X does not introduce breaking changes, but instead provides performance gains and bug fixes.

Description

During my work I discovered the following things:

  • Both arm Templates don't use an user, but instead run with the root user. Is this inteded, otherwise I can also fix that

  • node-armhf installed node and npm for some reason switched over to use `arm32v6/node:10.15.0-alpine

  • node-arm64 was not based on alpine, like the others, switched over to arm64v8/node:10.15.0-alpine

  • node-armhf & node-arm64 differed much from the other. So I adjusted the code so it is similiar

Motivation and Context

  • I have raised an issue to propose this change (required)

Which issue(s) this PR fixes

Fixes #99

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Templum added some commits Jan 15, 2019

Updated node:8.9.1-alpine => node:10.15.0-alpine
Signed-off-by: Simon Pelczer <templum.dev@gmail.com>
Updated Node Version of Arm Images to 10.15.0-alpine
Discovered that the arm images had different base images (now both based on alpine).
Discovered both templates had differences in overall structure

Signed-off-by: Simon Pelczer <templum.dev@gmail.com>
Updated Readme to reflect changes
Signed-off-by: Simon Pelczer <templum.dev@gmail.com>

@derek derek bot added the new-contributor label Jan 15, 2019

@alexellis
Copy link
Member

alexellis left a comment

On the whole LGTM, we just need to test on ARM32 and ARM64

Show resolved Hide resolved template/node-armhf/Dockerfile Outdated
@rgee0

This comment has been minimized.

Copy link
Member

rgee0 commented Jan 18, 2019

On a pi3:

$ cd /tmp ; mkdir fn ; cd fn ; faas template pull https://github.com/Templum/templates#feature/99 ; faas new --lang=node-armhf burt ; faas build -f burt.yml ; faas deploy -f burt.yml

Fetch templates from repository: https://github.com/Templum/templates at feature/99
2019/01/18 21:08:13 Attempting to expand templates from https://github.com/Templum/templates
2019/01/18 21:08:17 Fetched 15 template(s) : [csharp csharp-armhf dockerfile go go-armhf java8 node node-arm64 node-armhf php7 python python-armhf python3 python3-armhf ruby] from https://github.com/Templum/templates
Folder: burt created.
  ___                   _____           ____
 / _ \ _ __   ___ _ __ |  ___|_ _  __ _/ ___|
| | | | '_ \ / _ \ '_ \| |_ / _` |/ _` \___ \
| |_| | |_) |  __/ | | |  _| (_| | (_| |___) |
 \___/| .__/ \___|_| |_|_|  \__,_|\__,_|____/
      |_|


Function created in folder: burt
Stack file written: burt.yml
[0] > Building burt.
Clearing temporary build folder: ./build/burt/
Preparing ./burt/ ./build/burt/function
Building: burt:latest with node-armhf template. Please wait..
Sending build context to Docker daemon  9.728kB
Step 1/19 : FROM arm32v6/node:10.15.0-alpine
 ---> 2db954e1e5b4
Step 2/19 : RUN apk --no-cache add ca-certificates curl     && echo "Pulling watchdog binary from Github."     && curl -sSL https://github.com/openfaas/faas/releases/download/0.9.14/fwatchdog-armhf > /usr/bin/fwatchdog     && chmod +x /usr/bin/fwatchdog     && apk del curl --no-cache
 ---> Using cache
 ---> 6cefc8b20986
Step 3/19 : WORKDIR /root/
 ---> Using cache
 ---> 3cfaa55026b0
Step 4/19 : ENV NPM_CONFIG_LOGLEVEL warn
 ---> Using cache
 ---> ee6f0f907a3a
Step 5/19 : COPY package.json       .
 ---> Using cache
 ---> e322c2855ca2
Step 6/19 : RUN npm i --production
 ---> Using cache
 ---> aa174c832921
Step 7/19 : COPY index.js           .
 ---> Using cache
 ---> 38ce414b380a
Step 8/19 : RUN mkdir -p ./root/function
 ---> Running in f9d3980c31ca
Removing intermediate container f9d3980c31ca
 ---> 2a0b5043c3ae
Step 9/19 : COPY function/*.json    ./function/
 ---> a916a9424901
Step 10/19 : WORKDIR /root/function
 ---> Running in 28c09017597a
Removing intermediate container 28c09017597a
 ---> 8f5268ce3530
Step 11/19 : RUN npm i --production || :
 ---> Running in c404f4a65068
npm WARN function@1.0.0 No description
npm WARN function@1.0.0 No repository field.

up to date in 4.347s
found 0 vulnerabilities

Removing intermediate container c404f4a65068
 ---> b739c669dded
Step 12/19 : WORKDIR /root/
 ---> Running in 5f5f03d7aede
Removing intermediate container 5f5f03d7aede
 ---> a910c4dd2880
Step 13/19 : COPY function           function
 ---> 899fc3574531
Step 14/19 : WORKDIR /root/
 ---> Running in 5723b033f049
Removing intermediate container 5723b033f049
 ---> 2584d804bf6d
Step 15/19 : ENV cgi_headers="true"
 ---> Running in 9f0e82c8c3a1
Removing intermediate container 9f0e82c8c3a1
 ---> 4673f7102b7c
Step 16/19 : ENV fprocess="node index.js"
 ---> Running in 85d6e41c7fe4
Removing intermediate container 85d6e41c7fe4
 ---> a5d4ce3fa9ad
Step 17/19 : EXPOSE 8080
 ---> Running in c107442f55c3
Removing intermediate container c107442f55c3
 ---> 6b0189e5fb6d
Step 18/19 : HEALTHCHECK --interval=3s CMD [ -e /tmp/.lock ] || exit 1
 ---> Running in 0de76e1002b1
Removing intermediate container 0de76e1002b1
 ---> 86b8201b347b
Step 19/19 : CMD ["fwatchdog"]
 ---> Running in 79c88a3c92cb
Removing intermediate container 79c88a3c92cb
 ---> f30e09042116
Successfully built f30e09042116
Successfully tagged burt:latest
Image: burt:latest built.
[0] < Building burt done.
[0] worker done.

Deploying: burt.

Deployed. 202 Accepted.
URL: http://127.0.0.1:8080/function/burt

Invoking the function:

$ echo '' | faas invoke burt
{"status":"done"}

Rebuild with the base image prefix omitted:

$ faas build --no-cache -f burt.yml && faas deploy -f burt.yml
[0] > Building burt.
Clearing temporary build folder: ./build/burt/
Preparing ./burt/ ./build/burt/function
Building: rgee0/burt:latest with node-armhf template. Please wait..
Sending build context to Docker daemon  9.728kB
Step 1/19 : FROM node:10.15.0-alpine
 ---> 2db954e1e5b4
Step 2/19 : RUN apk --no-cache add ca-certificates curl     && echo "Pulling watchdog binary from Github."     && curl -sSL https://github.com/openfaas/faas/releases/download/0.9.14/fwatchdog-armhf > /usr/bin/fwatchdog     && chmod +x /usr/bin/fwatchdog     && apk del curl --no-cache
 ---> Running in bcadf6512ed9
fetch http://dl-cdn.alpinelinux.org/alpine/v3.8/main/armhf/APKINDEX.tar.gz
fetch http://dl-cdn.alpinelinux.org/alpine/v3.8/community/armhf/APKINDEX.tar.gz
(1/5) Installing ca-certificates (20171114-r3)
(2/5) Installing nghttp2-libs (1.32.0-r0)
(3/5) Installing libssh2 (1.8.0-r3)
(4/5) Installing libcurl (7.61.1-r1)
(5/5) Installing curl (7.61.1-r1)
Executing busybox-1.28.4-r2.trigger
Executing ca-certificates-20171114-r3.trigger
OK: 6 MiB in 20 packages
Pulling watchdog binary from Github.
fetch http://dl-cdn.alpinelinux.org/alpine/v3.8/main/armhf/APKINDEX.tar.gz
fetch http://dl-cdn.alpinelinux.org/alpine/v3.8/community/armhf/APKINDEX.tar.gz
(1/4) Purging curl (7.61.1-r1)
(2/4) Purging libcurl (7.61.1-r1)
(3/4) Purging nghttp2-libs (1.32.0-r0)
(4/4) Purging libssh2 (1.8.0-r3)
Executing busybox-1.28.4-r2.trigger
OK: 5 MiB in 16 packages
Removing intermediate container bcadf6512ed9
 ---> 99391060c297
Step 3/19 : WORKDIR /root/
 ---> Running in a887e706f680
Removing intermediate container a887e706f680
 ---> b96768e2a09c
Step 4/19 : ENV NPM_CONFIG_LOGLEVEL warn
 ---> Running in 5fff077d1152
Removing intermediate container 5fff077d1152
 ---> 04bb3dc8a007
Step 5/19 : COPY package.json       .
 ---> e2afc56052b5
Step 6/19 : RUN npm i --production
 ---> Running in 73f48d1baa7a
npm WARN NodejsBase@1.0.0 No description
npm WARN NodejsBase@1.0.0 No repository field.

added 1 package from 1 contributor and audited 1 package in 4.032s
found 0 vulnerabilities

Removing intermediate container 73f48d1baa7a
 ---> afb42cbeaf3b
Step 7/19 : COPY index.js           .
 ---> 708af143941b
Step 8/19 : RUN mkdir -p ./root/function
 ---> Running in bdbf3e1c18b2
Removing intermediate container bdbf3e1c18b2
 ---> 3874cd46dddf
Step 9/19 : COPY function/*.json    ./function/
 ---> 89b54f307c68
Step 10/19 : WORKDIR /root/function
 ---> Running in 9b8a35a28b8c
Removing intermediate container 9b8a35a28b8c
 ---> 3efba54fd7eb
Step 11/19 : RUN npm i --production || :
 ---> Running in c4d006975b27
npm WARN function@1.0.0 No description
npm WARN function@1.0.0 No repository field.

up to date in 3.287s
found 0 vulnerabilities

Removing intermediate container c4d006975b27
 ---> ef983f25183f
Step 12/19 : WORKDIR /root/
 ---> Running in ddc9645d24ba
Removing intermediate container ddc9645d24ba
 ---> cf16c00f2233
Step 13/19 : COPY function           function
 ---> f0274142eb47
Step 14/19 : WORKDIR /root/
 ---> Running in ff6675804ab8
Removing intermediate container ff6675804ab8
 ---> 0c27fa91758d
Step 15/19 : ENV cgi_headers="true"
 ---> Running in d5a52603ee36
Removing intermediate container d5a52603ee36
 ---> 4167fc36bf38
Step 16/19 : ENV fprocess="node index.js"
 ---> Running in f7d05a3b05c0
Removing intermediate container f7d05a3b05c0
 ---> 6004e5e0c12f
Step 17/19 : EXPOSE 8080
 ---> Running in b8cd2d3e145f
Removing intermediate container b8cd2d3e145f
 ---> 7ccfa9f61559
Step 18/19 : HEALTHCHECK --interval=3s CMD [ -e /tmp/.lock ] || exit 1
 ---> Running in 23aacf036565
Removing intermediate container 23aacf036565
 ---> 2b63d963319a
Step 19/19 : CMD ["fwatchdog"]
 ---> Running in 79a6ff103449
Removing intermediate container 79a6ff103449
 ---> cbc72a2421ec
Successfully built cbc72a2421ec
Successfully tagged rgee0/burt:latest
Image: rgee0/burt:latest built.
[0] < Building burt done.
[0] worker done.
Deploying: burt.

Deployed. 202 Accepted.
URL: http://127.0.0.1:8080/function/burt

Invoking the function:

$ echo '' | faas invoke burt
{"status":"done"}
@@ -1,13 +1,10 @@
FROM arm64v8/node:8.9.1
FROM arm64v8/node:10.15.0-alpine

This comment has been minimized.

@alexellis

alexellis Jan 19, 2019

Member

Can we please use the non-prefixed versions? These should all be multi arch now

This comment has been minimized.

@Templum

Templum Jan 19, 2019

Author

I will change this. And what about the open issue with the root user ? (Aka that arm based images don't use an app user)

This comment has been minimized.

@alexellis

alexellis Jan 19, 2019

Member

I think they should all be non root and matching. The only change I can think of is the watchdog binary name.

This comment has been minimized.

@Templum

Templum Jan 19, 2019

Author

Alright then I will fix this as well. Today I'm on Ng girls event as an mentor. I believe in the evening I should find time make the suggested changes

This comment has been minimized.

@Templum

Templum Jan 19, 2019

Author

@alexellis in 4f4525b switched over to the non-prefixed versions and further aligned all images to the "node" templates.

This comment has been minimized.

@alexellis

alexellis Feb 9, 2019

Member

👍 thanks for explaining, this is fine for the arm64 template, let's mirror the x86_64 one.

Show resolved Hide resolved template/node-armhf/Dockerfile Outdated
Adjusted ARM Docker Files
They now use the non prefixed versions and further are based on the "node" template

Signed-off-by: Simon Pelczer <templum.dev@gmail.com>
@alexellis
Copy link
Member

alexellis left a comment

Please can we do this in two steps?

I'd like you to revert all changes which are not:

  • switching the base-image version in this PR
  • updating the README.md table
  • moving arm64 to Alpine

Thanks.

@Templum Templum closed this Feb 9, 2019

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Feb 9, 2019

I want to keep the changes as atomic as possible, because immediately after merge all users are impacted.

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