Fix Docker tag logic to prevent undefined in dev tag

e2e/test/config.js

@@ -35,8 +35,7 @@ const {
     HOST_IP = '127.0.0.1',
     GENERATE_ONLY,
     TEST_OPENSEARCH = false,
-    TEST_PLATFORM = 'native',
-    KEEP_OPEN = false
+    TEST_PLATFORM = 'native'
 } = process.env;
 
 const TEST_HOST = TEST_OPENSEARCH ? OPENSEARCH_HOST : ELASTICSEARCH_HOST;
@@ -83,6 +82,5 @@ module.exports = {
     newId,
     TEST_HOST,
     TEST_PLATFORM,
-    KEEP_OPEN,
     TERASLICE_PORT
 };

packages/scripts/package.json

@@ -1,7 +1,7 @@
 {
     "name": "@terascope/scripts",
     "displayName": "Scripts",
-    "version": "0.66.0",
+    "version": "0.67.0",
     "description": "A collection of terascope monorepo scripts",
     "homepage": "https://github.com/terascope/teraslice/tree/master/packages/scripts#readme",
     "bugs": {

packages/scripts/src/helpers/config.ts

@@ -172,8 +172,6 @@ if (testElasticsearch) {
 
 export const SEARCH_TEST_HOST = testHost;
 
-// This should match a node version from the base-docker-image repo:
-// https://github.com/terascope/base-docker-image
 // This overrides the value in the Dockerfile
 export const NODE_VERSION = process.env.NODE_VERSION || '18.18.2';
 

packages/scripts/src/helpers/publish/index.ts

@@ -8,6 +8,7 @@ import {
     shouldNPMPublish,
     formatDailyTag,
     buildDevDockerImage,
+    removeNodeSuffixFromTag
 } from './utils';
 import {
     yarnPublish,
@@ -96,7 +97,7 @@ async function publishToDocker(options: PublishOptions) {
         const nodeVersionSuffix = `nodev${options.nodeVersion}`;
 
         if (options.type === PublishType.Latest) {
-            imageToBuild = `${registry}:latest`;
+            imageToBuild = `${registry}:latest-${nodeVersionSuffix}`;
         } else if (options.type === PublishType.Tag || options.type === PublishType.Prerelease) {
             const mainPkgInfo = getMainPackageInfo();
             if (!mainPkgInfo) {
@@ -110,32 +111,30 @@ async function publishToDocker(options: PublishOptions) {
                 }
             }
 
-            const image = `${registry}:v${mainPkgInfo.version}`;
-            let exists;
-            if (options.nodeSuffix) {
-                exists = await remoteDockerImageExists(`${image}-${nodeVersionSuffix}`);
-            } else {
-                exists = await remoteDockerImageExists(image);
+            let image = `${registry}:v${mainPkgInfo.version}-${nodeVersionSuffix}`;
+            if (!options.nodeSuffix) {
+                image = removeNodeSuffixFromTag(image);
             }
+            const exists = await remoteDockerImageExists(image);
             if (exists) {
                 err = new Error(`Docker Image ${image} already exists`);
                 continue;
             }
             imageToBuild = image;
         } else if (options.type === PublishType.Daily) {
             const tag = await formatDailyTag();
-            imageToBuild = `${registry}:${tag}`;
+            imageToBuild = `${registry}:${tag}-${nodeVersionSuffix}`;
         } else if (options.type === PublishType.Dev) {
-            imageToBuild = getDevDockerImage();
+            imageToBuild = getDevDockerImage(options.nodeVersion);
         }
 
         const startTime = Date.now();
         signale.pending(`building docker for ${options.type} release`);
         // TODO: perhaps this should be moved inside the block above and
         // repeated for each conditional branch to avoid the duplication on line
         // 113, I cant' decide which is worse.
-        if (options.nodeSuffix) {
-            imageToBuild = `${imageToBuild}-${nodeVersionSuffix}`;
+        if (!options.nodeSuffix) {
+            imageToBuild = removeNodeSuffixFromTag(imageToBuild);
         }
         signale.debug(`building docker image ${imageToBuild}`);
 

packages/scripts/src/helpers/publish/utils.ts

@@ -107,3 +107,7 @@ export async function buildDevDockerImage(
     signale.success(`built docker image ${devImage}, took ${toHumanTime(Date.now() - startTime)}`);
     return devImage;
 }
+
+export function removeNodeSuffixFromTag(tag: string) {
+    return tag.split('-').filter((part) => !part.startsWith('nodev')).join('-');
+}

packages/scripts/test/publish-spec.ts

@@ -1,5 +1,5 @@
 import 'jest-extended';
-import { formatDailyTag } from '../src/helpers/publish/utils';
+import { formatDailyTag, removeNodeSuffixFromTag } from '../src/helpers/publish/utils';
 
 describe('Publish', () => {
     describe('->formatDailyTag', () => {
@@ -8,4 +8,18 @@ describe('Publish', () => {
             expect(tag).toMatch(/^daily-\d{4}.\d{2}.\d{2}-[A-Za-z0-9]{5,12}$/);
         });
     });
+
+    describe('->removeNodeSuffixFromTag', () => {
+        it('should return a tag without the node version', async () => {
+            const tagBefore = 'v0.91.0-nodev18.18.2';
+            const tagAfter = await removeNodeSuffixFromTag(tagBefore);
+            expect(tagAfter).toEqual('v0.91.0');
+        });
+
+        it('should not modify a tag without -nodev', async () => {
+            const tagBefore = 'v0.91.0-v18.18.2';
+            const tagAfter = await removeNodeSuffixFromTag(tagBefore);
+            expect(tagAfter).toEqual('v0.91.0-v18.18.2');
+        });
+    });
 });