From 7ae8942b3dd054deecd17c99902296c65491639a Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Fri, 10 Jun 2022 11:07:50 +0200 Subject: [PATCH 1/5] Repaired prepare-job hook without job container --- packages/k8s/src/hooks/prepare-job.ts | 39 +++++++++++++------------- packages/k8s/tests/prepare-job-test.ts | 7 +++++ 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/packages/k8s/src/hooks/prepare-job.ts b/packages/k8s/src/hooks/prepare-job.ts index 7bff8d6..12b24c2 100644 --- a/packages/k8s/src/hooks/prepare-job.ts +++ b/packages/k8s/src/hooks/prepare-job.ts @@ -1,12 +1,6 @@ import * as core from '@actions/core' import * as io from '@actions/io' import * as k8s from '@kubernetes/client-node' -import { - PodPhase, - containerVolumes, - DEFAULT_CONTAINER_ENTRY_POINT, - DEFAULT_CONTAINER_ENTRY_POINT_ARGS -} from '../k8s/utils' import { ContextPorts, prepareJobArgs, writeToResponseFile } from 'hooklib' import path from 'path' import { @@ -19,6 +13,12 @@ import { requiredPermissions, waitForPodPhases } from '../k8s' +import { + containerVolumes, + DEFAULT_CONTAINER_ENTRY_POINT, + DEFAULT_CONTAINER_ENTRY_POINT_ARGS, + PodPhase +} from '../k8s/utils' import { JOB_CONTAINER_NAME } from './constants' export async function prepareJob( @@ -71,24 +71,23 @@ export async function prepareJob( new Set([PodPhase.RUNNING]), new Set([PodPhase.PENDING]) ) + core.debug('Job pod is ready for traffic') + + let isAlpine = false + try { + isAlpine = await isPodContainerAlpine( + createdPod.metadata.name, + JOB_CONTAINER_NAME + ) + } catch (err) { + throw new Error(`Failed to determine if the pod is alpine: ${err}`) + } + core.debug(`Setting isAlpine to ${isAlpine}`) + generateResponseFile(responseFile, createdPod, isAlpine) } catch (err) { await prunePods() throw new Error(`Pod failed to come online with error: ${err}`) } - - core.debug('Job pod is ready for traffic') - - let isAlpine = false - try { - isAlpine = await isPodContainerAlpine( - createdPod.metadata.name, - JOB_CONTAINER_NAME - ) - } catch (err) { - throw new Error(`Failed to determine if the pod is alpine: ${err}`) - } - core.debug(`Setting isAlpine to ${isAlpine}`) - generateResponseFile(responseFile, createdPod, isAlpine) } function generateResponseFile( diff --git a/packages/k8s/tests/prepare-job-test.ts b/packages/k8s/tests/prepare-job-test.ts index 1992da1..253f36e 100644 --- a/packages/k8s/tests/prepare-job-test.ts +++ b/packages/k8s/tests/prepare-job-test.ts @@ -68,4 +68,11 @@ describe('Prepare job', () => { prepareJob(prepareJobData.args, prepareJobOutputFilePath) ).rejects.toThrow() }) + + it('should run prepare job without the job container', async () => { + prepareJobData.args.container = undefined + await expect( + prepareJob(prepareJobData.args, prepareJobOutputFilePath) + ).resolves.not.toThrow() + }) }) From bf362ba0dd68e39ab780ceaea6c289bb590a12b1 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Fri, 10 Jun 2022 11:56:10 +0200 Subject: [PATCH 2/5] fixed prepare-job for docker to allow for no job container --- packages/docker/src/hooks/prepare-job.ts | 10 ++++++++-- packages/docker/tests/prepare-job-test.ts | 10 ++++++++++ packages/k8s/src/hooks/prepare-job.ts | 24 +++++++++++------------ 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/packages/docker/src/hooks/prepare-job.ts b/packages/docker/src/hooks/prepare-job.ts index 581e6d1..5be7f9d 100644 --- a/packages/docker/src/hooks/prepare-job.ts +++ b/packages/docker/src/hooks/prepare-job.ts @@ -94,7 +94,10 @@ export async function prepareJob( ) } - const isAlpine = await isContainerAlpine(containerMetadata!.id) + let isAlpine = false + if (containerMetadata?.id) { + isAlpine = await isContainerAlpine(containerMetadata.id) + } if (containerMetadata?.id) { containerMetadata.ports = await containerPorts(containerMetadata.id) @@ -105,7 +108,10 @@ export async function prepareJob( } } - const healthChecks: Promise[] = [healthCheck(containerMetadata!)] + const healthChecks: Promise[] = [] + if (containerMetadata) { + healthChecks.push(healthCheck(containerMetadata)) + } for (const service of servicesMetadata) { healthChecks.push(healthCheck(service)) } diff --git a/packages/docker/tests/prepare-job-test.ts b/packages/docker/tests/prepare-job-test.ts index d761b7d..d0d80f6 100644 --- a/packages/docker/tests/prepare-job-test.ts +++ b/packages/docker/tests/prepare-job-test.ts @@ -118,4 +118,14 @@ describe('prepare job', () => { expect(redisServicePorts['80']).toBe('8080') expect(redisServicePorts['8080']).toBe('8088') }) + + it('should run prepare job without job container without exception', async () => { + prepareJobDefinition.args.container = null + const prepareJobOutput = testSetup.createOutputFile( + 'prepare-job-output.json' + ) + await expect( + prepareJob(prepareJobDefinition.args, prepareJobOutput) + ).resolves.not.toThrow() + }) }) diff --git a/packages/k8s/src/hooks/prepare-job.ts b/packages/k8s/src/hooks/prepare-job.ts index 12b24c2..83f769c 100644 --- a/packages/k8s/src/hooks/prepare-job.ts +++ b/packages/k8s/src/hooks/prepare-job.ts @@ -72,22 +72,22 @@ export async function prepareJob( new Set([PodPhase.PENDING]) ) core.debug('Job pod is ready for traffic') - - let isAlpine = false - try { - isAlpine = await isPodContainerAlpine( - createdPod.metadata.name, - JOB_CONTAINER_NAME - ) - } catch (err) { - throw new Error(`Failed to determine if the pod is alpine: ${err}`) - } - core.debug(`Setting isAlpine to ${isAlpine}`) - generateResponseFile(responseFile, createdPod, isAlpine) } catch (err) { await prunePods() throw new Error(`Pod failed to come online with error: ${err}`) } + + let isAlpine = false + try { + isAlpine = await isPodContainerAlpine( + createdPod.metadata.name, + JOB_CONTAINER_NAME + ) + } catch (err) { + throw new Error(`Failed to determine if the pod is alpine: ${err}`) + } + core.debug(`Setting isAlpine to ${isAlpine}`) + generateResponseFile(responseFile, createdPod, isAlpine) } function generateResponseFile( From 2b386f7cbdc3802ea7f9c10b0d04abc6aeee1084 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Fri, 10 Jun 2022 12:00:49 +0200 Subject: [PATCH 3/5] returned logging below the try k8s prepare job --- packages/k8s/src/hooks/prepare-job.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/k8s/src/hooks/prepare-job.ts b/packages/k8s/src/hooks/prepare-job.ts index 83f769c..2362523 100644 --- a/packages/k8s/src/hooks/prepare-job.ts +++ b/packages/k8s/src/hooks/prepare-job.ts @@ -71,12 +71,13 @@ export async function prepareJob( new Set([PodPhase.RUNNING]), new Set([PodPhase.PENDING]) ) - core.debug('Job pod is ready for traffic') } catch (err) { await prunePods() throw new Error(`Pod failed to come online with error: ${err}`) } + core.debug('Job pod is ready for traffic') + let isAlpine = false try { isAlpine = await isPodContainerAlpine( From 3e04b45585beaee9a4f5174d482bb5f351c7a5c5 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Fri, 10 Jun 2022 12:45:59 +0200 Subject: [PATCH 4/5] removed leftover todo comments --- packages/docker/src/dockerCommands/container.ts | 2 +- packages/docker/src/hooks/prepare-job.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/docker/src/dockerCommands/container.ts b/packages/docker/src/dockerCommands/container.ts index 89bf737..8e3bc04 100644 --- a/packages/docker/src/dockerCommands/container.ts +++ b/packages/docker/src/dockerCommands/container.ts @@ -149,7 +149,7 @@ export async function containerBuild( dockerArgs.push('-t', tag) dockerArgs.push('-f', args.dockerfile) dockerArgs.push(getBuildContext(args.dockerfile)) - // TODO: figure out build working directory + await runDockerCommand(dockerArgs, { workingDir: getWorkingDir(args.dockerfile) }) diff --git a/packages/docker/src/hooks/prepare-job.ts b/packages/docker/src/hooks/prepare-job.ts index 5be7f9d..599ff97 100644 --- a/packages/docker/src/hooks/prepare-job.ts +++ b/packages/docker/src/hooks/prepare-job.ts @@ -139,7 +139,6 @@ function generateResponseFile( servicesMetadata?: ContainerMetadata[], isAlpine = false ): void { - // todo figure out if we are alpine const response = { state: { network: networkName }, context: {}, From d3ec1c0040d45009d7e2e57d103b982390577c0c Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Fri, 10 Jun 2022 16:38:07 +0200 Subject: [PATCH 5/5] prepare job in k8s does not allow for no job container --- packages/k8s/src/hooks/prepare-job.ts | 4 ++++ packages/k8s/tests/prepare-job-test.ts | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/k8s/src/hooks/prepare-job.ts b/packages/k8s/src/hooks/prepare-job.ts index 2362523..e5ecf6a 100644 --- a/packages/k8s/src/hooks/prepare-job.ts +++ b/packages/k8s/src/hooks/prepare-job.ts @@ -25,6 +25,10 @@ export async function prepareJob( args: prepareJobArgs, responseFile ): Promise { + if (!args.container) { + throw new Error('Job Container is required.') + } + await prunePods() if (!(await isAuthPermissionsOK())) { throw new Error( diff --git a/packages/k8s/tests/prepare-job-test.ts b/packages/k8s/tests/prepare-job-test.ts index 253f36e..00a5759 100644 --- a/packages/k8s/tests/prepare-job-test.ts +++ b/packages/k8s/tests/prepare-job-test.ts @@ -69,10 +69,10 @@ describe('Prepare job', () => { ).rejects.toThrow() }) - it('should run prepare job without the job container', async () => { + it('should not run prepare job without the job container', async () => { prepareJobData.args.container = undefined await expect( prepareJob(prepareJobData.args, prepareJobOutputFilePath) - ).resolves.not.toThrow() + ).rejects.toThrow() }) })