chore: automate Docker build & push #36

theneva · 2021-02-02T23:58:42Z

Hi! 👋

This is a big step towards closing #35.

This introduces some npm scripts to automate Docker-related stuff in the repo: npm run docker:build-default and npm run docker:publish-default.

Using npm run docker:publish-default as an example, the scripts invoke a CLI app (./tools/release.js) that…

gathers relevant information
generates Dockerfiles with the specified base images (12-alpine and 14-alpine by default) & writes them to a temporary directory
builds Docker images for each specified base image, and tags them on the form unleashorg/unleash-server:3.10.1-node14-alpine
tags the image with unleash-server's default Node base image (currently node:12-alpine) as latest and without the -node<version> suffix (so for example, unleashorg/unleash-server:3.10.1)
asks if you want to push the new tags
if approved, pushes the new tags

npm run docker:build-default does the same thing, but stops after step 4.

Here's a quick demo:

I hope this makes sense! It should be relatively easy to run this code on any kind of CI by doing echo 'y' | npm run publish-default, assuming dev dependencies get installed first.

I'll leave some comments in the code 😄

I don't know how the GitHub Actions and Docker Hub flows work in this project. How should I document this in the readme?

Edit: By the way, I've formatted all the code in ./tools/ using $ npx prettier --single-quote --trailing-comma all --write ./tools

theneva · 2021-02-02T23:59:24Z

.dockerignore

@@ -0,0 +1,2 @@
+tools/
+node_modules/


the build process runs npm ci anyways, so there's no real point to copying over 50 MB of node modules on every build

theneva · 2021-02-02T23:59:36Z

Dockerfile

@@ -1,5 +1,7 @@
 FROM node:12-alpine

+ENV NODE_ENV production


this prevents dev deps from getting installed 😄

README.md

theneva · 2021-02-03T00:02:28Z

package.json

+    "docker:build": "./tools/release.js build",
+    "docker:build-default": "./tools/release.js build --node-docker-versions 12-alpine,14-alpine --default-node-docker-version 12-alpine",
+    "docker:publish": "./tools/release.js publish",
+    "docker:publish-default": "./tools/release.js publish --node-docker-versions 12-alpine,14-alpine --default-node-docker-version 12-alpine"


here are the scripts. the CLI will yell at you if you invoke it ~~would~~ without the proper arguments:

theneva · 2021-02-03T00:03:00Z

package.json

+  "devDependencies": {
+    "execa": "^5.0.0",
+    "tempy": "^1.0.0",
+    "yargs": "^16.2.0"


this introduces some dev dependencies. this functionality is obviously doable without them, but it would require a fair bit more code. is introducing dev deps a problem?

I'm ok with dev-deps here.

theneva · 2021-02-03T00:03:33Z

tools/docker-utils.js

+
+const tempDirPath = tempy.directory();
+
+/** @type {string} */


IntelliJ yells at me because it thinks this is a Buffer (which is not true, as I pass { encoding: 'utf8' })

theneva · 2021-02-03T00:08:38Z

tools/release.js

+const readline = require('readline').createInterface({
+  input: process.stdin,
+  output: process.stdout,
+});


readline is used to prompt the user for confirmation about pushing the tags later on

theneva · 2021-02-03T00:09:09Z

tools/release.js

+async function main() {
+  const argv = yargs(hideBin(process.argv))
+    .option('defaultNodeDockerVersion', {
+      alias: 'default-node-docker-version',


I strongly dislike having to use --camelCasedNames on the command line, so I figured I might as well allow both camel and kebab case

I don't really mind either way, now if you started debating spaces vs tabs, we'd have to have a serious conversation :sarcasm:

theneva · 2021-02-03T00:10:16Z

tools/release.js

+
+      throw new Error(
+        `defaultNodeDockerVersion "${defaultNodeDockerVersion}" was not in list of versions "${nodeDockerVersions}"`,
+      );


I started writing code to trigger a build if the defaultNodeDockerVersion wasn't present in the nodeDockerVersions, but it turned into a big mess so I decided to fail early instead

Failing early seems logical :)

theneva · 2021-02-03T00:11:46Z

tools/docker-utils.js

+  nodeDockerVersions,
+  defaultNodeDockerVersion,
+}) {
+  const unleashServerVersion = await getUnleashServerVersion();


it doesn't really make sense for a "docker util" do get information about an npm package, but I couldn't find a nice way to pass it in as context without a bunch of duplicated code.

theneva · 2021-02-03T00:12:58Z

tools/unleash-server-utils.js

+  );
+  const versionLine = npmListOutput.split('\n')[1];
+  return versionLine.split('@')[1].trim();
+}


this is to include the unleash-server version in the Docker tag. It's not pretty, but I don't know a better way to find the installed version. Do you?

(the .trim() gets rid of a trailing space)

require('unleash-server/package.json').version

yep that's approximately 78 times better. b6e864c

tools/docker-utils.js

SimenB · 2021-02-03T10:12:08Z

tools/release.js

+
+const yargs = require('yargs/yargs');
+const { hideBin } = require('yargs/helpers');
+const readline = require('readline').createInterface({


I prefer prompts over readline, but no biggie of course

I think this is fine. I tried to keep the dependencies to a minimum, although I guess it doesn't really matter since the jump from zero to one dependency is way greater than the jump from 3 to 4

SimenB · 2021-02-03T10:12:34Z

tools/release.js

+    .demandCommand(1, 'You have to specify a command.')
+    .strict(true)
+    .version(false)
+    .wrap(120)


it defaults to 80 characters which causes some dumb wrapping since the option names are pretty long

SimenB · 2021-02-03T10:13:30Z

tools/unleash-server-utils.js

+  );
+  const versionLine = npmListOutput.split('\n')[1];
+  return versionLine.split('@')[1].trim();
+}


require('unleash-server/package.json').version

Co-authored-by: Simen Bekkhus <[email protected]>

chriswk

Thanks for this, good job, particularly doing it in an additive manner, so the rest of our build process is undisturbed.

Be aware though, for now, the Unleash team is not running the server on node14 for any customers so it does not receive the same amount of production experience as the default (node-12) build does.

ivarconr · 2021-02-03T16:03:33Z

Be aware though, for now, the Unleash team is not running the server on node14 for any customers so it does not receive the same amount of production experience as the default (node-12) build does.

But we do run all our unit and integration tests on both node-12 and node-14, so should work in theory. Also our OSS demo instance is using node-14.

theneva · 2021-02-03T21:33:01Z

Cool! I'm still unsure about if & how to document the release flow in the readme, and I don't think I'm the right person to configure github actions. I've also not written anything changelog-related.

We already run a Node 14-based image because I had to override the logger configuration, so I guess we're voluntary testers, too 😄

chriswk · 2021-02-04T11:46:33Z

For now, I'd say it's fine running the release flow manually, once we get to automating as much of our release flows as possible, we'll revisit a GHA configuration

chore: automate Docker build & push

976a925

theneva commented Feb 2, 2021

View reviewed changes

oops

2889f77

theneva force-pushed the automate-docker-build branch from 18f9fb4 to 2889f77 Compare February 3, 2021 00:01

theneva commented Feb 3, 2021

View reviewed changes

theneva mentioned this pull request Feb 3, 2021

fix: upgrade Node from 12 to 14 #35

Closed

SimenB reviewed Feb 3, 2021

View reviewed changes

theneva and others added 3 commits February 3, 2021 12:06

get unleash-server version in a less complicated way

b6e864c

Update tools/docker-utils.js

2df24ae

Co-authored-by: Simen Bekkhus <[email protected]>

formatting

e678467

chriswk approved these changes Feb 3, 2021

View reviewed changes

chriswk approved these changes Feb 4, 2021

View reviewed changes

chriswk merged commit 68632bd into Unleash:master Feb 4, 2021

theneva deleted the automate-docker-build branch February 4, 2021 21:10

theneva mentioned this pull request Feb 8, 2021

fix: The default log levels for different NODE_ENVs are surprising Unleash/unleash#704

Closed

chriswk mentioned this pull request Feb 9, 2021

chore: Setup GHA to perform our publishes to Docker #37

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: automate Docker build & push #36

chore: automate Docker build & push #36

theneva commented Feb 2, 2021 •

edited

Loading

theneva Feb 2, 2021

theneva Feb 2, 2021

theneva Feb 3, 2021 •

edited

Loading

theneva Feb 3, 2021

chriswk Feb 3, 2021

theneva Feb 3, 2021

theneva Feb 3, 2021

theneva Feb 3, 2021

chriswk Feb 3, 2021

theneva Feb 3, 2021

theneva Feb 3, 2021

chriswk Feb 3, 2021

theneva Feb 3, 2021

theneva Feb 3, 2021

SimenB Feb 3, 2021

theneva Feb 3, 2021

SimenB Feb 3, 2021

theneva Feb 3, 2021

SimenB Feb 3, 2021

theneva Feb 3, 2021

SimenB Feb 3, 2021

chriswk left a comment

ivarconr commented Feb 3, 2021

theneva commented Feb 3, 2021

chriswk commented Feb 4, 2021

		@@ -0,0 +1,2 @@
		tools/
		node_modules/

chore: automate Docker build & push #36

chore: automate Docker build & push #36

Conversation

theneva commented Feb 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theneva Feb 3, 2021 • edited Loading

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

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

chriswk left a comment

Choose a reason for hiding this comment

ivarconr commented Feb 3, 2021

theneva commented Feb 3, 2021

chriswk commented Feb 4, 2021

theneva commented Feb 2, 2021 •

edited

Loading

theneva Feb 3, 2021 •

edited

Loading