From ec8131abb7d7fbd043d5cc08928549bf889a3f38 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Mon, 6 Jun 2022 00:21:44 -0400 Subject: [PATCH 1/6] setup ci to run k8s tests --- .github/workflows/build.yaml | 1 + package.json | 2 +- packages/docker/tests/e2e-test.ts | 1 - packages/hooklib/src/interfaces.ts | 2 +- packages/k8s/README.md | 21 +++- packages/k8s/src/hooks/prepare-job.ts | 2 +- packages/k8s/src/hooks/run-container-step.ts | 5 +- packages/k8s/src/hooks/run-script-step.ts | 4 +- packages/k8s/src/k8s/index.ts | 12 ++- packages/k8s/src/k8s/utils.ts | 6 +- packages/k8s/tests/cleanup-job-test.ts | 17 ++-- packages/k8s/tests/e2e-test.ts | 28 +++--- packages/k8s/tests/prepare-job-test.ts | 23 ++--- packages/k8s/tests/run-container-step-test.ts | 12 ++- packages/k8s/tests/run-script-step-test.ts | 19 ++-- packages/k8s/tests/test-setup.ts | 97 +++++++++++++++++-- 16 files changed, 177 insertions(+), 75 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index a13f05a..e3b1580 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -10,6 +10,7 @@ jobs: build: runs-on: ubuntu-latest steps: + - uses: helm/kind-action@v1.2.0 - uses: actions/checkout@v3 - run: npm install name: Install dependencies diff --git a/package.json b/package.json index aa224c3..a2c6e95 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "doc": "docs" }, "scripts": { - "test": "npm run test --prefix packages/docker", + "test": "npm run test --prefix packages/docker && npm run test --prefix packages/k8s", "bootstrap": "npm install --prefix packages/hooklib && npm install --prefix packages/k8s && npm install --prefix packages/docker", "format": "prettier --write '**/*.ts'", "format-check": "prettier --check '**/*.ts'", diff --git a/packages/docker/tests/e2e-test.ts b/packages/docker/tests/e2e-test.ts index cdc889b..77dfbd8 100644 --- a/packages/docker/tests/e2e-test.ts +++ b/packages/docker/tests/e2e-test.ts @@ -108,7 +108,6 @@ ENTRYPOINT [ "tail", "-f", "/dev/null" ] process.env.GITHUB_WORKSPACE = tmpOutputDir containerStepDataCopy.args.dockerfile = 'Dockerfile' containerStepDataCopy.args.context = '.' - console.log(containerStepDataCopy.args) await expect( runContainerStep(containerStepDataCopy.args, resp.state) ).resolves.not.toThrow() diff --git a/packages/hooklib/src/interfaces.ts b/packages/hooklib/src/interfaces.ts index dabfcd6..f29dcbd 100644 --- a/packages/hooklib/src/interfaces.ts +++ b/packages/hooklib/src/interfaces.ts @@ -76,7 +76,7 @@ export enum Protocol { export enum PodPhase { PENDING = 'Pending', RUNNING = 'Running', - SUCCEEDED = 'Succeded', + SUCCEEDED = 'Succeeded', FAILED = 'Failed', UNKNOWN = 'Unknown' } diff --git a/packages/k8s/README.md b/packages/k8s/README.md index 12e8105..981e9b3 100644 --- a/packages/k8s/README.md +++ b/packages/k8s/README.md @@ -6,7 +6,24 @@ This implementation provides a way to dynamically spin up jobs to run container ## Pre-requisites Some things are expected to be set when using these hooks - The runner itself should be running in a pod, with a service account with the following permissions - - The `ACTIONS_RUNNER_REQUIRE_JOB_CONTAINER=true` should be set to true +``` +- apiGroups: [""] + resources: ["pods"] + verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] +- apiGroups: [""] + resources: ["pods/exec"] + verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] +- apiGroups: [""] + resources: ["pods/log"] + verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] +- apiGroups: ["batch"] + resources: ["jobs"] + verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] +``` - The `ACTIONS_RUNNER_POD_NAME` env should be set to the name of the pod +- The `ACTIONS_RUNNER_REQUIRE_JOB_CONTAINER` env should be set to true to prevent the runner from running any jobs outside of a container - The runner pod should map a persistent volume claim into the `_work` directory - - The `ACTIONS_RUNNER_CLAIM_NAME` should be set to the persistent volume claim that contains the runner's working directory + - The `ACTIONS_RUNNER_CLAIM_NAME` env should be set to the persistent volume claim that contains the runner's working directory +- Some actions runner env's are expected to be set. These are set automatically by the runner. + - `RUNNER_WORKSPACE` is expected to be set to the workspace of the runner + - `GITHUB_WORKSPACE` is expected to be set to the workspace of the job diff --git a/packages/k8s/src/hooks/prepare-job.ts b/packages/k8s/src/hooks/prepare-job.ts index df92e44..9a62f93 100644 --- a/packages/k8s/src/hooks/prepare-job.ts +++ b/packages/k8s/src/hooks/prepare-job.ts @@ -59,7 +59,7 @@ export async function prepareJob( createdPod = await createPod(container, services, args.registry) } catch (err) { await podPrune() - throw new Error(`failed to create job pod: ${err}`) + throw new Error(`failed to create job pod: ${JSON.stringify(err)}`) } if (!createdPod?.metadata?.name) { diff --git a/packages/k8s/src/hooks/run-container-step.ts b/packages/k8s/src/hooks/run-container-step.ts index d3a3cf6..244f438 100644 --- a/packages/k8s/src/hooks/run-container-step.ts +++ b/packages/k8s/src/hooks/run-container-step.ts @@ -25,12 +25,11 @@ export async function runContainerStep(stepContainer): Promise { )} to have correctly set the metadata.name` ) } - const podName = await getContainerJobPodName(job.metadata.name) await waitForPodPhases( podName, - new Set([PodPhase.COMPLETED, PodPhase.RUNNING]), - new Set([PodPhase.PENDING]) + new Set([PodPhase.COMPLETED, PodPhase.RUNNING, PodPhase.SUCCEEDED]), + new Set([PodPhase.PENDING, PodPhase.UNKNOWN]) ) await getPodLogs(podName, JOB_CONTAINER_NAME) await waitForJobToComplete(job.metadata.name) diff --git a/packages/k8s/src/hooks/run-script-step.ts b/packages/k8s/src/hooks/run-script-step.ts index 794fd75..e00c209 100644 --- a/packages/k8s/src/hooks/run-script-step.ts +++ b/packages/k8s/src/hooks/run-script-step.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ import { RunScriptStepArgs } from 'hooklib' import { execPodStep } from '../k8s' -import { JOB_CONTAINER_NAME } from './constants' +import { getJobPodName, JOB_CONTAINER_NAME } from './constants' export async function runScriptStep( args: RunScriptStepArgs, @@ -13,7 +13,7 @@ export async function runScriptStep( args.entryPointArgs, args.environmentVariables ) - await execPodStep(cb.command, state.jobPod, JOB_CONTAINER_NAME) + await execPodStep(cb.command, getJobPodName(), JOB_CONTAINER_NAME) } class CommandsBuilder { diff --git a/packages/k8s/src/k8s/index.ts b/packages/k8s/src/k8s/index.ts index b490091..fef66ab 100644 --- a/packages/k8s/src/k8s/index.ts +++ b/packages/k8s/src/k8s/index.ts @@ -5,6 +5,7 @@ import { v4 as uuidv4 } from 'uuid' import { getJobPodName, getRunnerPodName, + getStepPodName, getVolumeClaimName, RunnerInstanceLabel } from '../hooks/constants' @@ -119,7 +120,7 @@ export async function createJob( job.apiVersion = 'batch/v1' job.kind = 'Job' job.metadata = new k8s.V1ObjectMeta() - job.metadata.name = getJobPodName() + job.metadata.name = getStepPodName() job.metadata.labels = { 'runner-pod': getRunnerPodName() } job.spec = new k8s.V1JobSpec() @@ -173,7 +174,13 @@ export async function getContainerJobPodName(jobName: string): Promise { } export async function deletePod(podName: string): Promise { - await k8sApi.deleteNamespacedPod(podName, namespace()) + await k8sApi.deleteNamespacedPod( + podName, + namespace(), + undefined, + undefined, + 0 + ) } export async function execPodStep( @@ -273,7 +280,6 @@ export async function waitForPodPhases( try { while (true) { phase = await getPodPhase(podName) - if (awaitingPhases.has(phase)) { return } diff --git a/packages/k8s/src/k8s/utils.ts b/packages/k8s/src/k8s/utils.ts index b08bec5..0d8402b 100644 --- a/packages/k8s/src/k8s/utils.ts +++ b/packages/k8s/src/k8s/utils.ts @@ -1,6 +1,5 @@ import * as k8s from '@kubernetes/client-node' import { Mount } from 'hooklib' -import * as path from 'path' import { POD_VOLUME_NAME } from './index' export const DEFAULT_CONTAINER_ENTRY_POINT_ARGS = [`-f`, `/dev/null`] @@ -43,6 +42,9 @@ export function containerVolumes( return mounts } + // TODO: we need to ensure this is a local path under the github workspace or fail/skip + // subpath only accepts a local path under the runner workspace + /* for (const userVolume of userMountVolumes) { const sourceVolumePath = `${ path.isAbsolute(userVolume.sourceVolumePath) @@ -52,7 +54,6 @@ export function containerVolumes( userVolume.sourceVolumePath ) }` - mounts.push({ name: POD_VOLUME_NAME, mountPath: userVolume.targetVolumePath, @@ -60,6 +61,7 @@ export function containerVolumes( readOnly: userVolume.readOnly }) } + */ return mounts } diff --git a/packages/k8s/tests/cleanup-job-test.ts b/packages/k8s/tests/cleanup-job-test.ts index 2f9b3cc..7a5d8c8 100644 --- a/packages/k8s/tests/cleanup-job-test.ts +++ b/packages/k8s/tests/cleanup-job-test.ts @@ -1,9 +1,9 @@ import * as path from 'path' import * as fs from 'fs' import { prepareJob, cleanupJob } from '../src/hooks' -import { TestTempOutput } from './test-setup' +import { TestHelper } from './test-setup' -let testTempOutput: TestTempOutput +let testHelper: TestHelper const prepareJobJsonPath = path.resolve( `${__dirname}/../../../examples/prepare-job.json` @@ -16,16 +16,15 @@ describe('Cleanup Job', () => { const prepareJobJson = fs.readFileSync(prepareJobJsonPath) let prepareJobData = JSON.parse(prepareJobJson.toString()) - testTempOutput = new TestTempOutput() - testTempOutput.initialize() - prepareJobOutputFilePath = testTempOutput.createFile( - 'prepare-job-output.json' - ) + testHelper = new TestHelper() + await testHelper.initialize() + prepareJobOutputFilePath = testHelper.createFile('prepare-job-output.json') await prepareJob(prepareJobData.args, prepareJobOutputFilePath) }) it('should not throw', async () => { - const outputJson = fs.readFileSync(prepareJobOutputFilePath) - const outputData = JSON.parse(outputJson.toString()) await expect(cleanupJob()).resolves.not.toThrow() }) + afterEach(async () => { + await testHelper.cleanup() + }) }) diff --git a/packages/k8s/tests/e2e-test.ts b/packages/k8s/tests/e2e-test.ts index 2439c14..9fab718 100644 --- a/packages/k8s/tests/e2e-test.ts +++ b/packages/k8s/tests/e2e-test.ts @@ -6,38 +6,36 @@ import { runContainerStep, runScriptStep } from '../src/hooks' -import { TestTempOutput } from './test-setup' +import { TestHelper } from './test-setup' jest.useRealTimers() -let testTempOutput: TestTempOutput +let testHelper: TestHelper const prepareJobJsonPath = path.resolve( - `${__dirname}/../../../../examples/prepare-job.json` + `${__dirname}/../../../examples/prepare-job.json` ) const runScriptStepJsonPath = path.resolve( - `${__dirname}/../../../../examples/run-script-step.json` + `${__dirname}/../../../examples/run-script-step.json` ) let runContainerStepJsonPath = path.resolve( - `${__dirname}/../../../../examples/run-container-step.json` + `${__dirname}/../../../examples/run-container-step.json` ) let prepareJobData: any let prepareJobOutputFilePath: string describe('e2e', () => { - beforeEach(() => { + beforeEach(async () => { const prepareJobJson = fs.readFileSync(prepareJobJsonPath) prepareJobData = JSON.parse(prepareJobJson.toString()) - testTempOutput = new TestTempOutput() - testTempOutput.initialize() - prepareJobOutputFilePath = testTempOutput.createFile( - 'prepare-job-output.json' - ) + testHelper = new TestHelper() + await testHelper.initialize() + prepareJobOutputFilePath = testHelper.createFile('prepare-job-output.json') }) afterEach(async () => { - testTempOutput.cleanup() + await testHelper.cleanup() }) it('should prepare job, run script step, run container step then cleanup without errors', async () => { await expect( @@ -57,9 +55,9 @@ describe('e2e', () => { const runContainerStepContent = fs.readFileSync(runContainerStepJsonPath) const runContainerStepData = JSON.parse(runContainerStepContent.toString()) - await expect( - runContainerStep(runContainerStepData.args) - ).resolves.not.toThrow() + // await expect( + // runContainerStep(runContainerStepData.args) + // ).resolves.not.toThrow() await expect(cleanupJob()).resolves.not.toThrow() }) diff --git a/packages/k8s/tests/prepare-job-test.ts b/packages/k8s/tests/prepare-job-test.ts index 5636790..e53bfc8 100644 --- a/packages/k8s/tests/prepare-job-test.ts +++ b/packages/k8s/tests/prepare-job-test.ts @@ -2,11 +2,11 @@ import * as fs from 'fs' import * as path from 'path' import { cleanupJob } from '../src/hooks' import { prepareJob } from '../src/hooks/prepare-job' -import { TestTempOutput } from './test-setup' +import { TestHelper } from './test-setup' jest.useRealTimers() -let testTempOutput: TestTempOutput +let testHelper: TestHelper const prepareJobJsonPath = path.resolve( `${__dirname}/../../../examples/prepare-job.json` @@ -16,21 +16,17 @@ let prepareJobData: any let prepareJobOutputFilePath: string describe('Prepare job', () => { - beforeEach(() => { + beforeEach(async () => { const prepareJobJson = fs.readFileSync(prepareJobJsonPath) prepareJobData = JSON.parse(prepareJobJson.toString()) - testTempOutput = new TestTempOutput() - testTempOutput.initialize() - prepareJobOutputFilePath = testTempOutput.createFile( - 'prepare-job-output.json' - ) + testHelper = new TestHelper() + await testHelper.initialize() + prepareJobOutputFilePath = testHelper.createFile('prepare-job-output.json') }) afterEach(async () => { - const outputJson = fs.readFileSync(prepareJobOutputFilePath) - const outputData = JSON.parse(outputJson.toString()) await cleanupJob() - testTempOutput.cleanup() + await testHelper.cleanup() }) it('should not throw exception', async () => { @@ -38,10 +34,11 @@ describe('Prepare job', () => { prepareJob(prepareJobData.args, prepareJobOutputFilePath) ).resolves.not.toThrow() }) - + /* it('should generate output file in JSON format', async () => { + await prepareJob(prepareJobData.args, prepareJobOutputFilePath) const content = fs.readFileSync(prepareJobOutputFilePath) expect(() => JSON.parse(content.toString())).not.toThrow() - }) + }) */ }) diff --git a/packages/k8s/tests/run-container-step-test.ts b/packages/k8s/tests/run-container-step-test.ts index 901a9f9..a847b51 100644 --- a/packages/k8s/tests/run-container-step-test.ts +++ b/packages/k8s/tests/run-container-step-test.ts @@ -1,11 +1,11 @@ -import { TestTempOutput } from './test-setup' +import { TestHelper } from './test-setup' import * as path from 'path' import { runContainerStep } from '../src/hooks' import * as fs from 'fs' jest.useRealTimers() -let testTempOutput: TestTempOutput +let testHelper: TestHelper let runContainerStepJsonPath = path.resolve( `${__dirname}/../../../examples/run-container-step.json` @@ -14,14 +14,18 @@ let runContainerStepJsonPath = path.resolve( let runContainerStepData: any describe('Run container step', () => { - beforeAll(() => { + beforeAll(async () => { const content = fs.readFileSync(runContainerStepJsonPath) runContainerStepData = JSON.parse(content.toString()) - process.env.RUNNER_NAME = 'testjob' + testHelper = new TestHelper() + await testHelper.initialize() }) it('should not throw', async () => { await expect( runContainerStep(runContainerStepData.args) ).resolves.not.toThrow() }) + afterEach(async () => { + await testHelper.cleanup() + }) }) diff --git a/packages/k8s/tests/run-script-step-test.ts b/packages/k8s/tests/run-script-step-test.ts index 03c69b2..64c05b7 100644 --- a/packages/k8s/tests/run-script-step-test.ts +++ b/packages/k8s/tests/run-script-step-test.ts @@ -1,11 +1,11 @@ import { prepareJob, cleanupJob, runScriptStep } from '../src/hooks' -import { TestTempOutput } from './test-setup' +import { TestHelper } from './test-setup' import * as path from 'path' import * as fs from 'fs' jest.useRealTimers() -let testTempOutput: TestTempOutput +let testHelper: TestHelper const prepareJobJsonPath = path.resolve( `${__dirname}/../../../examples/prepare-job.json` @@ -19,13 +19,10 @@ describe('Run script step', () => { beforeEach(async () => { const prepareJobJson = fs.readFileSync(prepareJobJsonPath) prepareJobData = JSON.parse(prepareJobJson.toString()) - console.log(prepareJobData) - testTempOutput = new TestTempOutput() - testTempOutput.initialize() - prepareJobOutputFilePath = testTempOutput.createFile( - 'prepare-job-output.json' - ) + testHelper = new TestHelper() + await testHelper.initialize() + prepareJobOutputFilePath = testHelper.createFile('prepare-job-output.json') await prepareJob(prepareJobData.args, prepareJobOutputFilePath) const outputContent = fs.readFileSync(prepareJobOutputFilePath) prepareJobOutputData = JSON.parse(outputContent.toString()) @@ -33,7 +30,7 @@ describe('Run script step', () => { afterEach(async () => { await cleanupJob() - testTempOutput.cleanup() + await testHelper.cleanup() }) // NOTE: To use this test, do kubectl apply -f podspec.yaml (from podspec examples) @@ -42,8 +39,8 @@ describe('Run script step', () => { it('should not throw an exception', async () => { const args = { - entryPointArgs: ['echo "test"'], - entryPoint: '/bin/bash', + entryPointArgs: ['-c', 'echo "test"'], + entryPoint: 'bash', environmentVariables: { NODE_ENV: 'development' }, diff --git a/packages/k8s/tests/test-setup.ts b/packages/k8s/tests/test-setup.ts index 7c7b1ab..168d71a 100644 --- a/packages/k8s/tests/test-setup.ts +++ b/packages/k8s/tests/test-setup.ts @@ -1,20 +1,64 @@ import * as fs from 'fs' import { v4 as uuidv4 } from 'uuid' +import * as k8s from '@kubernetes/client-node' +import { V1PersistentVolumeClaim } from '@kubernetes/client-node' -export class TestTempOutput { +const kc = new k8s.KubeConfig() + +kc.loadFromDefault() + +const k8sApi = kc.makeApiClient(k8s.CoreV1Api) + +export class TestHelper { private tempDirPath: string + private podName: string constructor() { - this.tempDirPath = `${__dirname}/_temp/${uuidv4()}` + this.tempDirPath = `${__dirname}/_temp/runner` + this.podName = uuidv4().replace('-', '') } - public initialize(): void { - fs.mkdirSync(this.tempDirPath, { recursive: true }) + public async initialize(): Promise { + await this.cleanupK8sResources() + await this.createTestVolume() + await this.createTestJobPod() + fs.mkdirSync(`${this.tempDirPath}/work/repo/repo`, { recursive: true }) + fs.mkdirSync(`${this.tempDirPath}/externals`, { recursive: true }) + process.env['ACTIONS_RUNNER_POD_NAME'] = `${this.podName}` + process.env['ACTIONS_RUNNER_CLAIM_NAME'] = `${this.podName}-work` + process.env['RUNNER_WORKSPACE'] = `${this.tempDirPath}/work/repo` + process.env['GITHUB_WORKSPACE'] = `${this.tempDirPath}/work/repo/repo` + process.env['ACTIONS_RUNNER_KUBERNETES_NAMESPACE'] = 'default' } - public cleanup(): void { - fs.rmSync(this.tempDirPath, { recursive: true }) + public async cleanup(): Promise { + try { + await this.cleanupK8sResources() + fs.rmSync(this.tempDirPath, { recursive: true }) + } catch {} + } + public async cleanupK8sResources() { + await k8sApi + .deleteNamespacedPersistentVolumeClaim( + `${this.podName}-work`, + 'default', + undefined, + undefined, + 0 + ) + .catch(e => {}) + await k8sApi + .deleteNamespacedPod(this.podName, 'default', undefined, undefined, 0) + .catch(e => {}) + await k8sApi + .deleteNamespacedPod( + `${this.podName}-workflow`, + 'default', + undefined, + undefined, + 0 + ) + .catch(e => {}) } - public createFile(fileName?: string): string { const filePath = `${this.tempDirPath}/${fileName || uuidv4()}` fs.writeFileSync(filePath, '') @@ -25,4 +69,43 @@ export class TestTempOutput { const filePath = `${this.tempDirPath}/${fileName}` fs.rmSync(filePath) } + + public async createTestJobPod() { + const container = { + name: 'nginx', + image: 'nginx:latest', + imagePullPolicy: 'IfNotPresent' + } as k8s.V1Container + + const pod: k8s.V1Pod = { + metadata: { + name: this.podName + }, + spec: { + restartPolicy: 'Never', + containers: [container] + } + } as k8s.V1Pod + await k8sApi.createNamespacedPod('default', pod) + } + + public async createTestVolume() { + var volume: V1PersistentVolumeClaim = { + metadata: { + name: `${this.podName}-work` + }, + spec: { + accessModes: ['ReadWriteOnce'], + + volumeMode: 'Filesystem', + + resources: { + requests: { + storage: '1Gi' + } + } + } + } + await k8sApi.createNamespacedPersistentVolumeClaim('default', volume) + } } From 0ebccbd8c6fc9761440260cef17817e5996f34e8 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Mon, 6 Jun 2022 12:56:50 +0200 Subject: [PATCH 2/6] fixed testing adding storage class and persistent volume and timeout to wait for cleanup --- packages/docker/jest.setup.js | 2 +- packages/k8s/jest.setup.js | 2 +- packages/k8s/tests/e2e-test.ts | 6 +-- packages/k8s/tests/run-container-step-test.ts | 6 ++- packages/k8s/tests/test-setup.ts | 53 ++++++++++++++----- 5 files changed, 50 insertions(+), 19 deletions(-) diff --git a/packages/docker/jest.setup.js b/packages/docker/jest.setup.js index 2be7bb0..fca3957 100644 --- a/packages/docker/jest.setup.js +++ b/packages/docker/jest.setup.js @@ -1 +1 @@ -jest.setTimeout(90000) \ No newline at end of file +jest.setTimeout(500000) diff --git a/packages/k8s/jest.setup.js b/packages/k8s/jest.setup.js index 2be7bb0..fca3957 100644 --- a/packages/k8s/jest.setup.js +++ b/packages/k8s/jest.setup.js @@ -1 +1 @@ -jest.setTimeout(90000) \ No newline at end of file +jest.setTimeout(500000) diff --git a/packages/k8s/tests/e2e-test.ts b/packages/k8s/tests/e2e-test.ts index 9fab718..458eaa3 100644 --- a/packages/k8s/tests/e2e-test.ts +++ b/packages/k8s/tests/e2e-test.ts @@ -55,9 +55,9 @@ describe('e2e', () => { const runContainerStepContent = fs.readFileSync(runContainerStepJsonPath) const runContainerStepData = JSON.parse(runContainerStepContent.toString()) - // await expect( - // runContainerStep(runContainerStepData.args) - // ).resolves.not.toThrow() + await expect( + runContainerStep(runContainerStepData.args) + ).resolves.not.toThrow() await expect(cleanupJob()).resolves.not.toThrow() }) diff --git a/packages/k8s/tests/run-container-step-test.ts b/packages/k8s/tests/run-container-step-test.ts index a847b51..d92984f 100644 --- a/packages/k8s/tests/run-container-step-test.ts +++ b/packages/k8s/tests/run-container-step-test.ts @@ -1,7 +1,7 @@ -import { TestHelper } from './test-setup' +import * as fs from 'fs' import * as path from 'path' import { runContainerStep } from '../src/hooks' -import * as fs from 'fs' +import { TestHelper } from './test-setup' jest.useRealTimers() @@ -27,5 +27,7 @@ describe('Run container step', () => { }) afterEach(async () => { await testHelper.cleanup() + // wait for the job cleanup + await new Promise(resolve => setTimeout(resolve, 300 * 1000)) }) }) diff --git a/packages/k8s/tests/test-setup.ts b/packages/k8s/tests/test-setup.ts index 168d71a..89c9b5a 100644 --- a/packages/k8s/tests/test-setup.ts +++ b/packages/k8s/tests/test-setup.ts @@ -1,33 +1,34 @@ +import * as k8s from '@kubernetes/client-node' import * as fs from 'fs' import { v4 as uuidv4 } from 'uuid' -import * as k8s from '@kubernetes/client-node' -import { V1PersistentVolumeClaim } from '@kubernetes/client-node' const kc = new k8s.KubeConfig() kc.loadFromDefault() const k8sApi = kc.makeApiClient(k8s.CoreV1Api) +const k8sStorageApi = kc.makeApiClient(k8s.StorageV1Api) export class TestHelper { private tempDirPath: string private podName: string constructor() { this.tempDirPath = `${__dirname}/_temp/runner` - this.podName = uuidv4().replace('-', '') + this.podName = uuidv4().replace(/-/g, '') } public async initialize(): Promise { - await this.cleanupK8sResources() - await this.createTestVolume() - await this.createTestJobPod() - fs.mkdirSync(`${this.tempDirPath}/work/repo/repo`, { recursive: true }) - fs.mkdirSync(`${this.tempDirPath}/externals`, { recursive: true }) process.env['ACTIONS_RUNNER_POD_NAME'] = `${this.podName}` process.env['ACTIONS_RUNNER_CLAIM_NAME'] = `${this.podName}-work` process.env['RUNNER_WORKSPACE'] = `${this.tempDirPath}/work/repo` process.env['GITHUB_WORKSPACE'] = `${this.tempDirPath}/work/repo/repo` process.env['ACTIONS_RUNNER_KUBERNETES_NAMESPACE'] = 'default' + + await this.cleanupK8sResources() + await this.createTestVolume() + await this.createTestJobPod() + fs.mkdirSync(`${this.tempDirPath}/work/repo/repo`, { recursive: true }) + fs.mkdirSync(`${this.tempDirPath}/externals`, { recursive: true }) } public async cleanup(): Promise { @@ -46,6 +47,8 @@ export class TestHelper { 0 ) .catch(e => {}) + await k8sApi.deletePersistentVolume('work').catch(e => {}) + await k8sStorageApi.deleteStorageClass('local-storage').catch(e => {}) await k8sApi .deleteNamespacedPod(this.podName, 'default', undefined, undefined, 0) .catch(e => {}) @@ -90,15 +93,41 @@ export class TestHelper { } public async createTestVolume() { - var volume: V1PersistentVolumeClaim = { + var sc: k8s.V1StorageClass = { + metadata: { + name: 'local-storage' + }, + provisioner: 'kubernetes.io/no-provisioner', + volumeBindingMode: 'Immediate' + } + await k8sStorageApi.createStorageClass(sc) + + var volume: k8s.V1PersistentVolume = { + metadata: { + name: 'work' + }, + spec: { + storageClassName: 'local-storage', + capacity: { + storage: '2Gi' + }, + volumeMode: 'Filesystem', + accessModes: ['ReadWriteOnce'], + hostPath: { + path: this.tempDirPath + } + } + } + await k8sApi.createPersistentVolume(volume) + var volumeClaim: k8s.V1PersistentVolumeClaim = { metadata: { name: `${this.podName}-work` }, spec: { accessModes: ['ReadWriteOnce'], - volumeMode: 'Filesystem', - + storageClassName: 'local-storage', + volumeName: 'work', resources: { requests: { storage: '1Gi' @@ -106,6 +135,6 @@ export class TestHelper { } } } - await k8sApi.createNamespacedPersistentVolumeClaim('default', volume) + await k8sApi.createNamespacedPersistentVolumeClaim('default', volumeClaim) } } From 55c9198ada9aa32c8e01fa2b9973c4cc996da168 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Mon, 6 Jun 2022 14:15:14 -0400 Subject: [PATCH 3/6] new pv for each pod --- packages/k8s/tests/test-setup.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/k8s/tests/test-setup.ts b/packages/k8s/tests/test-setup.ts index 89c9b5a..ae8dc6e 100644 --- a/packages/k8s/tests/test-setup.ts +++ b/packages/k8s/tests/test-setup.ts @@ -25,8 +25,15 @@ export class TestHelper { process.env['ACTIONS_RUNNER_KUBERNETES_NAMESPACE'] = 'default' await this.cleanupK8sResources() - await this.createTestVolume() - await this.createTestJobPod() + try + { + await this.createTestVolume() + await this.createTestJobPod() + } + catch + { + + } fs.mkdirSync(`${this.tempDirPath}/work/repo/repo`, { recursive: true }) fs.mkdirSync(`${this.tempDirPath}/externals`, { recursive: true }) } @@ -47,7 +54,7 @@ export class TestHelper { 0 ) .catch(e => {}) - await k8sApi.deletePersistentVolume('work').catch(e => {}) + await k8sApi.deletePersistentVolume(`${this.podName}-pv`).catch(e => {}) await k8sStorageApi.deleteStorageClass('local-storage').catch(e => {}) await k8sApi .deleteNamespacedPod(this.podName, 'default', undefined, undefined, 0) @@ -104,7 +111,7 @@ export class TestHelper { var volume: k8s.V1PersistentVolume = { metadata: { - name: 'work' + name: `${this.podName}-pv` }, spec: { storageClassName: 'local-storage', @@ -127,7 +134,7 @@ export class TestHelper { accessModes: ['ReadWriteOnce'], volumeMode: 'Filesystem', storageClassName: 'local-storage', - volumeName: 'work', + volumeName: `${this.podName}-pv`, resources: { requests: { storage: '1Gi' From 689a74e352c0a48dcb2f18f982f05b49ad46ab43 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Mon, 6 Jun 2022 14:27:06 -0400 Subject: [PATCH 4/6] run format --- packages/k8s/tests/test-setup.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/k8s/tests/test-setup.ts b/packages/k8s/tests/test-setup.ts index ae8dc6e..40d7ebb 100644 --- a/packages/k8s/tests/test-setup.ts +++ b/packages/k8s/tests/test-setup.ts @@ -25,15 +25,10 @@ export class TestHelper { process.env['ACTIONS_RUNNER_KUBERNETES_NAMESPACE'] = 'default' await this.cleanupK8sResources() - try - { + try { await this.createTestVolume() await this.createTestJobPod() - } - catch - { - - } + } catch {} fs.mkdirSync(`${this.tempDirPath}/work/repo/repo`, { recursive: true }) fs.mkdirSync(`${this.tempDirPath}/externals`, { recursive: true }) } From e928fa3252e29a6e001d9212f384e2f2175f4f77 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Mon, 6 Jun 2022 18:43:57 -0400 Subject: [PATCH 5/6] Pass secrets more securely for container action --- packages/k8s/src/hooks/cleanup-job.ts | 5 +- packages/k8s/src/hooks/constants.ts | 9 ++- packages/k8s/src/hooks/prepare-job.ts | 8 +-- packages/k8s/src/hooks/run-container-step.ts | 29 ++++++---- packages/k8s/src/k8s/index.ts | 58 ++++++++++++++++--- packages/k8s/tests/run-container-step-test.ts | 2 - 6 files changed, 84 insertions(+), 27 deletions(-) diff --git a/packages/k8s/src/hooks/cleanup-job.ts b/packages/k8s/src/hooks/cleanup-job.ts index cecb88e..b36dd57 100644 --- a/packages/k8s/src/hooks/cleanup-job.ts +++ b/packages/k8s/src/hooks/cleanup-job.ts @@ -1,5 +1,6 @@ -import { podPrune } from '../k8s' +import { pruneSecrets, prunePods } from '../k8s' export async function cleanupJob(): Promise { - await podPrune() + await prunePods() + await pruneSecrets() } diff --git a/packages/k8s/src/hooks/constants.ts b/packages/k8s/src/hooks/constants.ts index 8218293..5df2257 100644 --- a/packages/k8s/src/hooks/constants.ts +++ b/packages/k8s/src/hooks/constants.ts @@ -20,7 +20,7 @@ export function getJobPodName(): string { export function getStepPodName(): string { return `${getRunnerPodName().substring( 0, - MAX_POD_NAME_LENGTH - ('-step'.length + STEP_POD_NAME_SUFFIX_LENGTH) + MAX_POD_NAME_LENGTH - ('-step-'.length + STEP_POD_NAME_SUFFIX_LENGTH) )}-step-${uuidv4().substring(0, STEP_POD_NAME_SUFFIX_LENGTH)}` } @@ -34,6 +34,13 @@ export function getVolumeClaimName(): string { return name } +export function getSecretName(): string { + return `${getRunnerPodName().substring( + 0, + MAX_POD_NAME_LENGTH - ('-secret-'.length + STEP_POD_NAME_SUFFIX_LENGTH) + )}-secret-${uuidv4().substring(0, STEP_POD_NAME_SUFFIX_LENGTH)}` +} + const MAX_POD_NAME_LENGTH = 63 const STEP_POD_NAME_SUFFIX_LENGTH = 8 export const JOB_CONTAINER_NAME = 'job' diff --git a/packages/k8s/src/hooks/prepare-job.ts b/packages/k8s/src/hooks/prepare-job.ts index 9a62f93..0549563 100644 --- a/packages/k8s/src/hooks/prepare-job.ts +++ b/packages/k8s/src/hooks/prepare-job.ts @@ -14,7 +14,7 @@ import { isAuthPermissionsOK, isPodContainerAlpine, namespace, - podPrune, + prunePods, requiredPermissions, waitForPodPhases } from '../k8s' @@ -29,7 +29,7 @@ export async function prepareJob( args: prepareJobArgs, responseFile ): Promise { - await podPrune() + await prunePods() if (!(await isAuthPermissionsOK())) { throw new Error( `The Service account needs the following permissions ${JSON.stringify( @@ -58,7 +58,7 @@ export async function prepareJob( try { createdPod = await createPod(container, services, args.registry) } catch (err) { - await podPrune() + await prunePods() throw new Error(`failed to create job pod: ${JSON.stringify(err)}`) } @@ -73,7 +73,7 @@ export async function prepareJob( new Set([PodPhase.PENDING]) ) } catch (err) { - await podPrune() + await prunePods() throw new Error(`Pod failed to come online with error: ${err}`) } diff --git a/packages/k8s/src/hooks/run-container-step.ts b/packages/k8s/src/hooks/run-container-step.ts index 244f438..7de4dfa 100644 --- a/packages/k8s/src/hooks/run-container-step.ts +++ b/packages/k8s/src/hooks/run-container-step.ts @@ -3,6 +3,7 @@ import * as core from '@actions/core' import { PodPhase } from 'hooklib' import { createJob, + createSecretForEnvs, getContainerJobPodName, getPodLogs, getPodStatus, @@ -16,7 +17,13 @@ export async function runContainerStep(stepContainer): Promise { if (stepContainer.dockerfile) { throw new Error('Building container actions is not currently supported') } - const container = createPodSpec(stepContainer) + let secretName: string | undefined = undefined + if (stepContainer['environmentVariables']) { + secretName = await createSecretForEnvs( + stepContainer['environmentVariables'] + ) + } + const container = createPodSpec(stepContainer, secretName) const job = await createJob(container) if (!job.metadata?.name) { throw new Error( @@ -39,28 +46,28 @@ export async function runContainerStep(stepContainer): Promise { core.warning(`Can't determine container status`) return 0 } - const exitCode = status.containerStatuses[status.containerStatuses.length - 1].state ?.terminated?.exitCode return Number(exitCode) || 0 } -function createPodSpec(container): k8s.V1Container { +function createPodSpec(container, secretName?: string): k8s.V1Container { const podContainer = new k8s.V1Container() podContainer.name = JOB_CONTAINER_NAME podContainer.image = container.image if (container.entryPoint) { podContainer.command = [container.entryPoint, ...container.entryPointArgs] } - - podContainer.env = [] - for (const [key, value] of Object.entries( - container['environmentVariables'] - )) { - if (value && key !== 'HOME') { - podContainer.env.push({ name: key, value: value as string }) - } + if (secretName) { + podContainer.envFrom = [ + { + secretRef: { + name: secretName, + optional: false + } + } + ] } podContainer.volumeMounts = containerVolumes() diff --git a/packages/k8s/src/k8s/index.ts b/packages/k8s/src/k8s/index.ts index fef66ab..d5175de 100644 --- a/packages/k8s/src/k8s/index.ts +++ b/packages/k8s/src/k8s/index.ts @@ -1,10 +1,10 @@ import * as k8s from '@kubernetes/client-node' import { ContainerInfo, PodPhase, Registry } from 'hooklib' import * as stream from 'stream' -import { v4 as uuidv4 } from 'uuid' import { getJobPodName, getRunnerPodName, + getSecretName, getStepPodName, getVolumeClaimName, RunnerInstanceLabel @@ -251,12 +251,13 @@ export async function createDockerSecret( } } } - const secretName = generateSecretName() + const secretName = getSecretName() const secret = new k8s.V1Secret() secret.immutable = true secret.apiVersion = 'v1' secret.metadata = new k8s.V1ObjectMeta() secret.metadata.name = secretName + secret.metadata.labels = { 'runner-pod': getRunnerPodName() } secret.kind = 'Secret' secret.data = { '.dockerconfigjson': Buffer.from( @@ -269,6 +270,53 @@ export async function createDockerSecret( return body } +export async function createSecretForEnvs(envs: { + [key: string]: string +}): Promise { + const secret = new k8s.V1Secret() + const secretName = getSecretName() + secret.immutable = true + secret.apiVersion = 'v1' + secret.metadata = new k8s.V1ObjectMeta() + secret.metadata.name = secretName + secret.metadata.labels = { 'runner-pod': getRunnerPodName() } + secret.kind = 'Secret' + secret.data = {} + for (const [key, value] of Object.entries(envs)) { + secret.data[key] = Buffer.from(value).toString('base64') + } + try { + await k8sApi.createNamespacedSecret(namespace(), secret) + } catch (e) { + throw e + } + return secretName +} + +export async function deleteSecret(secretName: string): Promise { + await k8sApi.deleteNamespacedSecret(secretName, namespace()) +} + +export async function pruneSecrets(): Promise { + const secretList = await k8sApi.listNamespacedSecret( + namespace(), + undefined, + undefined, + undefined, + undefined, + new RunnerInstanceLabel().toString() + ) + if (!secretList.body.items.length) { + return + } + + await Promise.all( + secretList.body.items.map( + secret => secret.metadata?.name && deleteSecret(secret.metadata.name) + ) + ) +} + export async function waitForPodPhases( podName: string, awaitingPhases: Set, @@ -346,7 +394,7 @@ export async function getPodLogs( await new Promise(resolve => r.on('close', () => resolve(null))) } -export async function podPrune(): Promise { +export async function prunePods(): Promise { const podList = await k8sApi.listNamespacedPod( namespace(), undefined, @@ -460,10 +508,6 @@ export function namespace(): string { return context.namespace } -function generateSecretName(): string { - return `github-secret-${uuidv4()}` -} - function runnerName(): string { const name = process.env.ACTIONS_RUNNER_POD_NAME if (!name) { diff --git a/packages/k8s/tests/run-container-step-test.ts b/packages/k8s/tests/run-container-step-test.ts index d92984f..a28512a 100644 --- a/packages/k8s/tests/run-container-step-test.ts +++ b/packages/k8s/tests/run-container-step-test.ts @@ -27,7 +27,5 @@ describe('Run container step', () => { }) afterEach(async () => { await testHelper.cleanup() - // wait for the job cleanup - await new Promise(resolve => setTimeout(resolve, 300 * 1000)) }) }) From cc90cd2361e167970ad5bd53fb4bc25cc54406b9 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Tue, 7 Jun 2022 16:47:45 -0400 Subject: [PATCH 6/6] pr feedback --- packages/k8s/src/k8s/index.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/k8s/src/k8s/index.ts b/packages/k8s/src/k8s/index.ts index d5175de..c60ec41 100644 --- a/packages/k8s/src/k8s/index.ts +++ b/packages/k8s/src/k8s/index.ts @@ -285,11 +285,8 @@ export async function createSecretForEnvs(envs: { for (const [key, value] of Object.entries(envs)) { secret.data[key] = Buffer.from(value).toString('base64') } - try { - await k8sApi.createNamespacedSecret(namespace(), secret) - } catch (e) { - throw e - } + + await k8sApi.createNamespacedSecret(namespace(), secret) return secretName }