feat: Update the NodeJS Dockerfilegenerator transformer and Dockerfile template to support multi-arch build #904

Akash-Nayak · 2022-10-21T11:37:44Z

Signed-off-by: Akash Nayak akash19nayak@gmail.com

github-actions · 2022-10-21T11:38:00Z

Thanks for making a pull request! 😃
One of the maintainers will review and advise on the next steps.

codecov · 2022-10-21T11:45:50Z

Codecov Report

Base: 15.30% // Head: 15.30% // No change to project coverage 👍

Coverage data is based on head (4733542) compared to base (9e1e10e).
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #904   +/-   ##
=======================================
  Coverage   15.30%   15.30%           
=======================================
  Files          50       50           
  Lines        4568     4568           
=======================================
  Hits          699      699           
  Misses       3696     3696           
  Partials      173      173

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Akash-Nayak · 2022-11-09T07:53:31Z

Hi @ashokponkumar ji, I have updated this PR to use ubi8/nodejs image in the Dockerfile template for NodeJS and also build the image and tested the deployment on OpenShift cluster.

New Dockerfile generated by Move2Kube.

FROM registry.access.redhat.com/ubi8/nodejs-14
COPY . .
RUN npm install
RUN npm run build
USER root
RUN chown -R 1001:0 /opt/app-root/src/.npm
RUN chmod -R 775 /opt/app-root/src/.npm
USER 1001
EXPOSE 8080
CMD npm run start

Currently, I have not removed the image-tag logic that I added, and I have commented it out in case if we want to switch to docker hub Node image in future.

ashokponkumar

Nice work. Few comments.

ashokponkumar · 2022-11-13T04:27:49Z

transformer/dockerfilegenerator/nodejsdockerfiletransformer.go

@@ -135,8 +140,16 @@ func (t *NodejsDockerfileGenerator) Init(tc transformertypes.Transformer, env *e
 	t.Spec = spec
 	if t.NodejsConfig.DefaultNodejsVersion == "" {
 		t.NodejsConfig.DefaultNodejsVersion = t.Spec.NodeVersions[0][versionKey]


We might want to check if the Len of nodeversions is not 0.

ashokponkumar · 2022-11-13T08:31:01Z

transformer/dockerfilegenerator/nodejsdockerfiletransformer.go

+	DefaultNodejsVersion string `yaml:"defaultNodejsVersion"`
+	// DefaultNodejsTag          string            `yaml:"defaultNodejsTag"`
+	DefaultPackageManager   string            `yaml:"defaultPackageManager"`
+	NodejsVersionTagMapping map[string]string `yaml:"nodejsVersionTagMapping"`


Aren't we taking this as a config file?

I have removed this.

ashokponkumar · 2022-11-13T08:31:21Z

transformer/dockerfilegenerator/nodejsdockerfiletransformer.go

 }

 const (
 	packageJSONFile        = "package.json"
 	versionMappingFilePath = "mappings/nodeversions.yaml"
 	defaultPackageManager  = "npm"
-	versionKey             = "version"
+	// tagKey                 = "tag"


what is tag?

This is the image tag which can be used in the Dockerfile, for example- FROM node:14.20.0-alpine. https://github.com/Akash-Nayak/move2kube/blob/13141078de0de81e2c705d85b31f162d06772933/assets/built-in/transformers/dockerfilegenerator/mappings/nodeversions.yaml#L10

The image tag is currently unused as we have moved to the ubi-8/node image, but if we want to revert back to node image from docker hub then it will be just required to uncomment this line https://github.com/Akash-Nayak/move2kube/blob/13141078de0de81e2c705d85b31f162d06772933/transformer/dockerfilegenerator/nodejsdockerfiletransformer.go#L262

ashokponkumar · 2022-11-13T08:31:54Z

assets/built-in/transformers/dockerfilegenerator/nodejs/templates/Dockerfile

-    mv node-{{ .NodeVersion }}-linux-x64 /node && \
-    rm -f node-{{ .NodeVersion }}-linux-x64.tar.xz
-ENV PATH="$PATH:/node/bin"
+FROM registry.access.redhat.com/ubi8/nodejs-{{ .NodeMajorVersion }}
 COPY . .
 RUN {{ .PackageManager }} install


Does the image have both npm and yarn?

The ubi8/nodejs-{version} base image only has npm and doesn't have yarn. Earlier in the ubi8-minimal image, we were downloading node but not installing yarn. So, I have added the command to install yarn if the PackageManager is yarn. https://github.com/Akash-Nayak/move2kube/blob/03df5e77bd6734eb18dfd6645b3c108549655203/assets/built-in/transformers/dockerfilegenerator/nodejs/templates/Dockerfile#L18

ashokponkumar · 2022-11-13T08:33:59Z

transformer/dockerfilegenerator/nodejsdockerfiletransformer.go

+			Port:        port,
+			NodeVersion: nodeVersion,
+			// NodeTag:               nodeTag,
+			NodeMajorVersion:      strings.TrimPrefix(semver.Major(nodeVersion), "v"),


We might not have all the node versions in the images, isn't it? So we might want to have a list of versions that are present and go to the nearest that fits the version criteria.

Yes, currently there are 4 node versions in the ubi8/node images (10, 12, 14 and 16) that can be pulled.

docker run --rm -it -p 8080:8080 registry.access.redhat.com/ubi8/nodejs-10 bash bash-4.4$ node -v v10.24.0

docker run --rm -it -p 8080:8080 registry.access.redhat.com/ubi8/nodejs-12 bash bash-4.4$ node -v v12.22.5

docker run --rm -it -p 8080:8080 registry.access.redhat.com/ubi8/nodejs-14 bash bash-4.4$ node -v v14.20.1

docker run --rm -it -p 8080:8080 registry.access.redhat.com/ubi8/nodejs-16 bash bash-4.4$ node -v v16.17.1

We have the updated list of available versions here- https://github.com/Akash-Nayak/move2kube/blob/13141078de0de81e2c705d85b31f162d06772933/assets/built-in/transformers/dockerfilegenerator/mappings/nodeversions.yaml#L6 and we select the node version that fits the version criteria here https://github.com/Akash-Nayak/move2kube/blob/03df5e77bd6734eb18dfd6645b3c108549655203/transformer/dockerfilegenerator/nodejsdockerfiletransformer.go#L222.

The node version selection heuristic was added in the PR #834 and we are still using the same heuristic for selecting the nearest node version that fits the version constraint if it's there in the package.json.

…lti-arch build Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

…port multi-arch build Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

… to support multi-arch build Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

…sformer to support multi-arch build Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

… used in the ubi8/node images Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

github-actions bot added the feat label Oct 21, 2022

Akash-Nayak requested a review from ashokponkumar October 21, 2022 12:12

Akash-Nayak force-pushed the update-nodejs-transformer branch 3 times, most recently from 5462666 to 9354507 Compare November 9, 2022 06:39

Akash-Nayak force-pushed the update-nodejs-transformer branch 2 times, most recently from f962644 to df34887 Compare November 11, 2022 07:51

ashokponkumar requested changes Nov 13, 2022

View reviewed changes

Akash-Nayak force-pushed the update-nodejs-transformer branch from 2b73233 to 03df5e7 Compare November 16, 2022 11:10

Akash-Nayak added 5 commits November 22, 2022 11:49

feat: Update the NodeJS Dockerfilegenerator transformer to support mu…

510a806

…lti-arch build Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

fixup! feat: Update the NodeJS Dockerfilegenerator transformer to sup…

840d687

…port multi-arch build Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

fixup! fixup! feat: Update the NodeJS Dockerfilegenerator transformer…

d63201d

… to support multi-arch build Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

fixup! fixup! fixup! feat: Update the NodeJS Dockerfilegenerator tran…

3e52469

…sformer to support multi-arch build Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

feat: Update the node version mappings corresponding to version being…

4733542

… used in the ubi8/node images Signed-off-by: Akash Nayak <akash19nayak@gmail.com>

Akash-Nayak force-pushed the update-nodejs-transformer branch from 1314107 to 4733542 Compare November 22, 2022 06:21

ashokponkumar approved these changes Nov 28, 2022

View reviewed changes

ashokponkumar merged commit ae2b33c into konveyor:main Nov 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Update the NodeJS Dockerfilegenerator transformer and Dockerfile template to support multi-arch build #904

feat: Update the NodeJS Dockerfilegenerator transformer and Dockerfile template to support multi-arch build #904

Akash-Nayak commented Oct 21, 2022

github-actions bot commented Oct 21, 2022

codecov bot commented Oct 21, 2022 •

edited

Loading

Akash-Nayak commented Nov 9, 2022

ashokponkumar left a comment

ashokponkumar Nov 13, 2022

Akash-Nayak Nov 16, 2022

ashokponkumar Nov 13, 2022

Akash-Nayak Nov 16, 2022

ashokponkumar Nov 13, 2022

Akash-Nayak Nov 16, 2022

ashokponkumar Nov 13, 2022

Akash-Nayak Nov 16, 2022

ashokponkumar Nov 13, 2022

Akash-Nayak Nov 16, 2022

Akash-Nayak Nov 22, 2022

feat: Update the NodeJS Dockerfilegenerator transformer and Dockerfile template to support multi-arch build #904

feat: Update the NodeJS Dockerfilegenerator transformer and Dockerfile template to support multi-arch build #904

Conversation

Akash-Nayak commented Oct 21, 2022

github-actions bot commented Oct 21, 2022

codecov bot commented Oct 21, 2022 • edited Loading

Codecov Report

Akash-Nayak commented Nov 9, 2022

ashokponkumar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 21, 2022 •

edited

Loading