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 support for Node v12 and v14 #556

Merged
merged 4 commits into from
Jul 28, 2020

Conversation

michael-gillett
Copy link
Contributor

@michael-gillett michael-gillett commented Jul 28, 2020

Overview

This PR introduces Node v12 and v14 distroless images. I read through the thread at #442 and believe this meets all of the criteria outlined.

Note: For backwards compatibility I've continued to use Node v10 for the latest and debug tags. I imagine once it reaches end of life we can switch to have latest be the current Node LTS version.

Edit: Given discussion below we have decided to deprecated the latest tag once Node v10 reaches EOL to avoid confusion.

Shout out to @craigthompsonhive @HofmannZ for getting the ball rolling on this

Testing

I generated the Node v14 image locally using Bazel, dropped it into one of my projects that was using the Node v10 distroless image, and everything ran as expected

Future Work

Setting up automatic minor and patch version bumps using GitHub actions

@googlebot googlebot added the cla: yes CLAs look good label Jul 28, 2020
@michael-gillett
Copy link
Contributor Author

michael-gillett commented Jul 28, 2020

@chanseokoh FYI since you were active in the other thread. Let me know if I missed anything, I haven't worked with Bazel before

@HofmannZ
Copy link
Contributor

@chanseokoh what do you think?

experimental/nodejs/README.md Outdated Show resolved Hide resolved
experimental/nodejs/README.md Outdated Show resolved Hide resolved
experimental/nodejs/README.md Show resolved Hide resolved
Copy link
Member

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

Simple enough! Technically and basically LGTM though there are some errors.

cloudbuild.yaml Outdated Show resolved Hide resolved
Fix copy-pasta for debug images
Copy link
Member

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you very much for your contribution!

I'll merge the PR, and then the automated release process will publish the new images typically within an hour or two. If you don't see the images after a few hours, please update here, because it would probably mean that this PR broke cloudbuild.yaml.

@chanseokoh
Copy link
Member

@briandealwis merging this. File a new PR if you want to update the doc further.

@chanseokoh chanseokoh merged commit e1f531a into GoogleContainerTools:master Jul 28, 2020
@michael-gillett
Copy link
Contributor Author

michael-gillett commented Jul 28, 2020

@chanseokoh I checked the node.js container registry and didn't see the images. I tried running the cloudbuild.yaml locally and ran into this error.

code/distroless > cloud-build-local --config=cloudbuild.yaml .
2020/07/28 15:23:56 RUNNER - [docker ps -a -q --filter name=step_[0-9]+|cloudbuild_|metadata]
2020/07/28 15:23:56 RUNNER - [docker network ls -q --filter name=cloudbuild]
2020/07/28 15:23:56 RUNNER - [docker volume ls -q --filter name=homevol|cloudbuild_]
2020/07/28 15:23:58 Error merging substitutions and validating build: Error validating build: invalid .artifacts field: cannot specify more than 100 images to build

It seems like there may be a hard limit of 100 images to be published by a single cloudbuild run. Before my commit there were 97 images being published and after there are 103. However I couldn't find anything in the cloudbuild docs about such a limit.

Not sure what your build pipeline looks like for this behind the scenes, but the easiest thing to do may be to have separate cloudbuild files for different directories java, cc, experimental etc

@chanseokoh
Copy link
Member

What a bummer! I don't even know how the backend release pipeline looks like, but I will try to think of something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLAs look good
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants