From ea116e378bae9c6e503b8c83bced12a3e0924843 Mon Sep 17 00:00:00 2001 From: williams-jack Date: Wed, 26 Jun 2024 18:47:20 -0400 Subject: [PATCH 01/11] feat: typing on orchestrator for job result fix: inconsistent key for errors in result --- orchestrator/packages/common/src/types/index.ts | 1 + .../packages/common/src/types/job-result.ts | 14 ++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 orchestrator/packages/common/src/types/job-result.ts diff --git a/orchestrator/packages/common/src/types/index.ts b/orchestrator/packages/common/src/types/index.ts index 23b33737..4b97bf8b 100644 --- a/orchestrator/packages/common/src/types/index.ts +++ b/orchestrator/packages/common/src/types/index.ts @@ -1,2 +1,3 @@ export * from "./grading-queue"; export * from "./image-build-service"; +export * from "./job-result"; diff --git a/orchestrator/packages/common/src/types/job-result.ts b/orchestrator/packages/common/src/types/job-result.ts new file mode 100644 index 00000000..a93fbf45 --- /dev/null +++ b/orchestrator/packages/common/src/types/job-result.ts @@ -0,0 +1,14 @@ +export interface GradingJobResult { + shell_responses: Array, + errors: Array, + output?: string +} + +interface GradingScriptCommandResponse { + cmd: string | Array, + stdout: string, + stderr: string, + did_timeout: boolean, + is_error: boolean, + status_code: number +} From bc467b556dabaa2fc9f5d33ac34b3f309b347ece Mon Sep 17 00:00:00 2001 From: williams-jack Date: Wed, 26 Jun 2024 19:04:59 -0400 Subject: [PATCH 02/11] feat: image service notifies client of failed build fix: send key with job result to client --- .../packages/image-build-service/src/index.ts | 16 +++++++--- .../packages/image-build-service/src/utils.ts | 32 ++++++++++++++----- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/orchestrator/packages/image-build-service/src/index.ts b/orchestrator/packages/image-build-service/src/index.ts index fdda2611..0b20967c 100644 --- a/orchestrator/packages/image-build-service/src/index.ts +++ b/orchestrator/packages/image-build-service/src/index.ts @@ -2,9 +2,9 @@ import { GraderImageBuildRequest, toMilliseconds, } from "@codegrade-orca/common"; -import { getNextImageBuild, handleCompletedImageBuild } from "@codegrade-orca/db"; +import { getNextImageBuild, handleCompletedImageBuild } from "@codegrade-orca/db"; import { createAndStoreGraderImage, removeStaleImageFiles } from "./process-request"; -import { cleanUpDockerFiles, removeImageFromDockerIfExists } from "./utils"; +import { cleanUpDockerFiles, notifyClientOfServiceFailure, removeImageFromDockerIfExists } from "./utils"; const LOOP_SLEEP_TIME = 5; // Seconds @@ -26,12 +26,20 @@ const main = async () => { dockerfileSHASum: nextBuildReq.dockerfileSHA, dockerfileContents: nextBuildReq.dockerfileContent } as GraderImageBuildRequest); + await handleCompletedImageBuild(nextBuildReq.dockerfileSHA, true); console.info(`Successfully build image with SHA ${nextBuildReq.dockerfileSHA}.`); } catch (err) { if (currentDockerSHASum) { - // TODO: Send GradingJobResults back to clients on build failure. - await handleCompletedImageBuild(currentDockerSHASum, false); + const cancelledJobInfoList = await handleCompletedImageBuild(currentDockerSHASum, false); + if (cancelledJobInfoList !== null) { + await Promise.all(cancelledJobInfoList.map((cancelInfo) => { + notifyClientOfServiceFailure( + cancelInfo, + `Failed to build image with SHA sum ${currentDockerSHASum} for this job.` + ).catch((notifyError) => console.error(notifyError)); // At this point we can't really do anything, but we should at least log out what happened. + })); + } await cleanUpDockerFiles(currentDockerSHASum); } console.error(err); diff --git a/orchestrator/packages/image-build-service/src/utils.ts b/orchestrator/packages/image-build-service/src/utils.ts index 7877eefc..e974fc46 100644 --- a/orchestrator/packages/image-build-service/src/utils.ts +++ b/orchestrator/packages/image-build-service/src/utils.ts @@ -1,4 +1,5 @@ -import { getConfig } from "@codegrade-orca/common"; +import { GradingJobConfig, GradingJobResult, getConfig } from "@codegrade-orca/common"; +import { CancelJobInfo } from "@codegrade-orca/db/dist/image-builder-operations/handle-completed-image-build"; import { execFile } from "child_process"; import { existsSync, rmSync } from "fs"; import path from "path"; @@ -35,13 +36,28 @@ const deleteImage = (dockerfileSHASum: string): Promise => { const imageExistsInDocker = (dockerfileSHASum: string): Promise => { return new Promise((resolve, reject) => { - execFile("docker", ["image", "ls", "--format", "{{.Repository}}:{{.Tag}}"],(err, stdout, _stderr) => { - if (err) { - reject(err); - } else { - const images = stdout.split("\n"); - resolve(images.includes(`${dockerfileSHASum}:latest`)); - } + execFile("docker", ["image", "ls", "--format", "{{.Repository}}:{{.Tag}}"], (err, stdout, _stderr) => { + if (err) { + reject(err); + } else { + const images = stdout.split("\n"); + resolve(images.includes(`${dockerfileSHASum}:latest`)); + } }); }); }; + +export const notifyClientOfServiceFailure = async (cancelInfo: CancelJobInfo, reason: string) => { + const result: GradingJobResult = { + shell_responses: [], + errors: [reason], + } + await fetch(cancelInfo.response_url, { + method: "POST", + headers: { + "Accept": "application/json", + "Content-Type": "application/json" + }, + body: JSON.stringify({...result, key: cancelInfo.key}) + }) +} From 78cc8a4ee0ab857a5e5ba13dda9cdf25c2770a81 Mon Sep 17 00:00:00 2001 From: williams-jack Date: Wed, 26 Jun 2024 19:18:45 -0400 Subject: [PATCH 03/11] feat: notify client of cancelled job --- .../controllers/grading-queue-controller.ts | 7 +++++-- .../packages/api/src/controllers/utils.ts | 19 +++++++++++++++++++ .../db/src/server-operations/delete-job.ts | 3 ++- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/orchestrator/packages/api/src/controllers/grading-queue-controller.ts b/orchestrator/packages/api/src/controllers/grading-queue-controller.ts index c3b949e0..4bc86ad4 100644 --- a/orchestrator/packages/api/src/controllers/grading-queue-controller.ts +++ b/orchestrator/packages/api/src/controllers/grading-queue-controller.ts @@ -5,9 +5,10 @@ import { getPageFromGradingQueue as getPageFromGradingJobs, } from "../utils/pagination"; import { validateFilterRequest } from "../utils/validate"; -import { errorResponse } from "./utils"; +import { errorResponse, notifyClientOfCancelledJob } from "./utils"; import { GradingJob, + GradingJobConfig, filterGradingJobs, getFilterInfo, getGradingQueueStats, @@ -163,7 +164,9 @@ export const deleteJob = async (req: Request, res: Response) => { if (isNaN(jobID)) { return errorResponse(res, 400, ["Given job ID is not a number."]); } - await deleteJobInQueue(jobID); + const deletedJob = await deleteJobInQueue(jobID); + const deletedJobConfig = deletedJob.config as object as GradingJobConfig; + await notifyClientOfCancelledJob(deletedJobConfig) return res.status(200).json({ message: "OK" }); } catch (err) { if (err instanceof GradingQueueOperationException) { diff --git a/orchestrator/packages/api/src/controllers/utils.ts b/orchestrator/packages/api/src/controllers/utils.ts index 29dee054..fcf75b3a 100644 --- a/orchestrator/packages/api/src/controllers/utils.ts +++ b/orchestrator/packages/api/src/controllers/utils.ts @@ -1,3 +1,4 @@ +import { GradingJobConfig, GradingJobResult } from "@codegrade-orca/common"; import { Response } from "express"; export const errorResponse = ( @@ -7,3 +8,21 @@ export const errorResponse = ( ) => { return res.status(status).json({ errors: errors }); }; + +export const notifyClientOfCancelledJob = async (jobConfig: GradingJobConfig) => { + const result: GradingJobResult = { + shell_responses: [], + errors: ["Job cancelled by a course professor or Orca admin."] + }; + await fetch(jobConfig.response_url, { + method: "POST", + headers: { + "Accept": "application/json", + "Content-Type": "application/json" + }, + body: JSON.stringify({ result, key: jobConfig.key }) + }).catch((err) => + console.error( + `Encountered the following error while attempting to notify client of Job cancellation: ${err}` + )); +} diff --git a/orchestrator/packages/db/src/server-operations/delete-job.ts b/orchestrator/packages/db/src/server-operations/delete-job.ts index 9ba8b2a3..7c2be4fe 100644 --- a/orchestrator/packages/db/src/server-operations/delete-job.ts +++ b/orchestrator/packages/db/src/server-operations/delete-job.ts @@ -2,7 +2,7 @@ import { getAssociatedReservation, retireReservationAndJob } from "../utils"; import prismaInstance from "../prisma-instance"; import { Job } from "@prisma/client"; -const deleteJob = (jobID: number) => +const deleteJob = (jobID: number): Promise => prismaInstance.$transaction(async (tx) => { const job = await tx.job.findUnique({ where: { @@ -11,6 +11,7 @@ const deleteJob = (jobID: number) => }) as Job; const reservation = await getAssociatedReservation(job, tx); await retireReservationAndJob(job, reservation, tx); + return job; }); export default deleteJob; From b4d52ebf964b153e7a92aaa59abaf7acf722c307 Mon Sep 17 00:00:00 2001 From: williams-jack Date: Mon, 1 Jul 2024 20:05:44 -0400 Subject: [PATCH 04/11] feat: better image service failure comms --- .../packages/common/src/types/job-result.ts | 29 +++++ .../packages/image-build-service/src/index.ts | 6 +- .../src/process-request/image-creation.ts | 117 ++++++++++++------ .../packages/image-build-service/src/utils.ts | 19 +-- 4 files changed, 120 insertions(+), 51 deletions(-) diff --git a/orchestrator/packages/common/src/types/job-result.ts b/orchestrator/packages/common/src/types/job-result.ts index a93fbf45..b3b3843a 100644 --- a/orchestrator/packages/common/src/types/job-result.ts +++ b/orchestrator/packages/common/src/types/job-result.ts @@ -1,4 +1,33 @@ +export interface GraderImageBuildResult { + type: "GraderImageBuildResult", + build_logs: Array, + was_successful: boolean + server_exception?: string, + dockerfile_sha_sum: string +} + +export interface ImageBuildFailure { + error: Error, + logs: Array, +}; + +export type ImageBuildStep = "Write request contents to Dockerfile." | + "Run docker build on Dockerfile." | + "Save image to .tgz file." | + "Remove residual Dockerfile."; + +export interface ImageBuildLog { + step: ImageBuildStep, + cmd?: string | Array, + stderr: string +} + +export const isImageBuildFailure = (object: unknown): object is ImageBuildFailure => { + return !!object && typeof object === "object" && "error" in object && "logs" in object; +} + export interface GradingJobResult { + type: "GradingJobResult", shell_responses: Array, errors: Array, output?: string diff --git a/orchestrator/packages/image-build-service/src/index.ts b/orchestrator/packages/image-build-service/src/index.ts index 0b20967c..41634996 100644 --- a/orchestrator/packages/image-build-service/src/index.ts +++ b/orchestrator/packages/image-build-service/src/index.ts @@ -1,5 +1,6 @@ import { GraderImageBuildRequest, + isImageBuildFailure, toMilliseconds, } from "@codegrade-orca/common"; import { getNextImageBuild, handleCompletedImageBuild } from "@codegrade-orca/db"; @@ -30,13 +31,14 @@ const main = async () => { await handleCompletedImageBuild(nextBuildReq.dockerfileSHA, true); console.info(`Successfully build image with SHA ${nextBuildReq.dockerfileSHA}.`); } catch (err) { - if (currentDockerSHASum) { + if (isImageBuildFailure(err) && currentDockerSHASum) { const cancelledJobInfoList = await handleCompletedImageBuild(currentDockerSHASum, false); if (cancelledJobInfoList !== null) { await Promise.all(cancelledJobInfoList.map((cancelInfo) => { notifyClientOfServiceFailure( cancelInfo, - `Failed to build image with SHA sum ${currentDockerSHASum} for this job.` + `Failed to build image with SHA sum ${currentDockerSHASum} for this job.`, + err ).catch((notifyError) => console.error(notifyError)); // At this point we can't really do anything, but we should at least log out what happened. })); } diff --git a/orchestrator/packages/image-build-service/src/process-request/image-creation.ts b/orchestrator/packages/image-build-service/src/process-request/image-creation.ts index 497d6737..229e1c8b 100644 --- a/orchestrator/packages/image-build-service/src/process-request/image-creation.ts +++ b/orchestrator/packages/image-build-service/src/process-request/image-creation.ts @@ -1,7 +1,6 @@ -import { GraderImageBuildRequest, getConfig } from "@codegrade-orca/common"; +import { GraderImageBuildRequest, ImageBuildFailure, ImageBuildLog, getConfig } from "@codegrade-orca/common"; import { execFile } from "child_process"; -import { writeFile } from "fs"; -import { rm } from "fs/promises"; +import { writeFile, rm } from "fs"; import path from "path"; const CONFIG = getConfig(); @@ -9,32 +8,34 @@ const CONFIG = getConfig(); export const createAndStoreGraderImage = ( buildRequest: GraderImageBuildRequest, ) => { - // TODO: validate dockerfileContent - return writeDockerfileContentsToFile(buildRequest) - .then((_) => buildImage(buildRequest)) + const buildLogs: Array = []; + return writeDockerfileContentsToFile(buildRequest, buildLogs) + .then((_) => buildImage(buildRequest, buildLogs)) .then((_) => - rm( - path.join( - CONFIG.dockerImageFolder, - `${buildRequest.dockerfileSHASum}.Dockerfile`, - ), - ), - ) - .then((_) => saveImageToTgz(buildRequest.dockerfileSHASum)); + removeDockerfileAfterBuild( + path.join(CONFIG.dockerImageFolder, `${buildRequest.dockerfileSHASum}.Dockerfile`), + buildLogs + )) + .then((_) => saveImageToTgz(buildRequest.dockerfileSHASum, buildLogs)); }; -const writeDockerfileContentsToFile = ({ - dockerfileContents, - dockerfileSHASum, -}: GraderImageBuildRequest): Promise => { +const writeDockerfileContentsToFile = ({ dockerfileContents, dockerfileSHASum }: GraderImageBuildRequest, buildLogs: Array): Promise => { return new Promise((resolve, reject) => { const dockerfilePath = path.join( CONFIG.dockerImageFolder, `${dockerfileSHASum}.Dockerfile`, ); writeFile(dockerfilePath, dockerfileContents, (err) => { + buildLogs.push({ + step: "Write request contents to Dockerfile.", + stderr: "" + }); if (err) { - reject(err); + const buildFailure: ImageBuildFailure = { + logs: buildLogs, + error: err + }; + reject(buildFailure); } else { resolve(dockerfilePath); } @@ -42,23 +43,31 @@ const writeDockerfileContentsToFile = ({ }); }; -const buildImage = ({ - dockerfileSHASum, -}: GraderImageBuildRequest): Promise => { +const buildImage = ({ dockerfileSHASum }: GraderImageBuildRequest, buildLogs: Array): Promise => { + const dockerBuildArgs = [ + "build", + "-t", + dockerfileSHASum, + "-f", + path.join(CONFIG.dockerImageFolder, `${dockerfileSHASum}.Dockerfile`), + ".", + ]; return new Promise((resolve, reject) => { execFile( "docker", - [ - "build", - "-t", - dockerfileSHASum, - "-f", - path.join(CONFIG.dockerImageFolder, `${dockerfileSHASum}.Dockerfile`), - ".", - ], - (err, _stdout, _stderr) => { + dockerBuildArgs, + (err, _stdout, stderr) => { + buildLogs.push({ + step: "Run docker build on Dockerfile.", + cmd: ["docker", ...dockerBuildArgs], + stderr + }); if (err) { - reject(err); + const buildFailure: ImageBuildFailure = { + error: err, + logs: buildLogs + }; + reject(buildFailure); } else { resolve(); } @@ -67,19 +76,45 @@ const buildImage = ({ }); }; -const saveImageToTgz = (imageName: string): Promise => { +const removeDockerfileAfterBuild = (dockerfilePath: string, buildLogs: Array): Promise => { + buildLogs.push({ + stderr: "", + step: "Remove residual Dockerfile." + }); + return new Promise((resolve, reject) => { + rm(dockerfilePath, (err) => { + if (err) { + const buildFailure: ImageBuildFailure = { error: err, logs: buildLogs }; + reject(buildFailure); + } else { + resolve(); + } + }) + }); +}; + +const saveImageToTgz = (imageName: string, buildLogs: Array): Promise => { + const dockerSaveCommandArgs = [ + "save", + "-o", + path.join(CONFIG.dockerImageFolder, `${imageName}.tgz`), + imageName, + ]; return new Promise((resolve, reject) => { execFile( "docker", - [ - "save", - "-o", - path.join(CONFIG.dockerImageFolder, `${imageName}.tgz`), - imageName, - ], - (err, _stdout, _stderr) => { + dockerSaveCommandArgs, + (err, _stdout, stderr) => { if (err) { - reject(err); + const buildFailure: ImageBuildFailure = { + error: err, + logs: [...buildLogs, { + step: "Save image to .tgz file.", + cmd: ["docker", ...dockerSaveCommandArgs], + stderr + }], + }; + reject(buildFailure); } else { resolve(); } diff --git a/orchestrator/packages/image-build-service/src/utils.ts b/orchestrator/packages/image-build-service/src/utils.ts index e974fc46..d2114cd7 100644 --- a/orchestrator/packages/image-build-service/src/utils.ts +++ b/orchestrator/packages/image-build-service/src/utils.ts @@ -1,4 +1,4 @@ -import { GradingJobConfig, GradingJobResult, getConfig } from "@codegrade-orca/common"; +import { GraderImageBuildResult, ImageBuildFailure, getConfig } from "@codegrade-orca/common"; import { CancelJobInfo } from "@codegrade-orca/db/dist/image-builder-operations/handle-completed-image-build"; import { execFile } from "child_process"; import { existsSync, rmSync } from "fs"; @@ -47,17 +47,20 @@ const imageExistsInDocker = (dockerfileSHASum: string): Promise => { }); }; -export const notifyClientOfServiceFailure = async (cancelInfo: CancelJobInfo, reason: string) => { - const result: GradingJobResult = { - shell_responses: [], - errors: [reason], - } +export const notifyClientOfServiceFailure = async (cancelInfo: CancelJobInfo, dockerfileSHASum: string, buildFailure: ImageBuildFailure) => { + const result: GraderImageBuildResult = { + type: "GraderImageBuildResult", + build_logs: buildFailure.logs, + dockerfile_sha_sum: dockerfileSHASum, + was_successful: false, + server_exception: buildFailure.error.message + }; await fetch(cancelInfo.response_url, { method: "POST", headers: { "Accept": "application/json", "Content-Type": "application/json" }, - body: JSON.stringify({...result, key: cancelInfo.key}) - }) + body: JSON.stringify({ ...result, key: cancelInfo.key }) + }); } From eaf190e5c0c410bb41ec9637e6d99403232490d2 Mon Sep 17 00:00:00 2001 From: williams-jack Date: Wed, 3 Jul 2024 19:50:42 -0400 Subject: [PATCH 05/11] fix: add type prop to job result dicts --- .../packages/api/src/controllers/utils.ts | 1 + .../common/grading_job/grading_job_result.py | 61 +++++++++++-------- .../common/types/grading_job_json_types.py | 2 +- 3 files changed, 36 insertions(+), 28 deletions(-) diff --git a/orchestrator/packages/api/src/controllers/utils.ts b/orchestrator/packages/api/src/controllers/utils.ts index fcf75b3a..15d3217c 100644 --- a/orchestrator/packages/api/src/controllers/utils.ts +++ b/orchestrator/packages/api/src/controllers/utils.ts @@ -11,6 +11,7 @@ export const errorResponse = ( export const notifyClientOfCancelledJob = async (jobConfig: GradingJobConfig) => { const result: GradingJobResult = { + type: "GradingJobResult", shell_responses: [], errors: ["Job cancelled by a course professor or Orca admin."] }; diff --git a/worker/orca_grader/common/grading_job/grading_job_result.py b/worker/orca_grader/common/grading_job/grading_job_result.py index 40942d4e..14f46b9b 100644 --- a/worker/orca_grader/common/grading_job/grading_job_result.py +++ b/worker/orca_grader/common/grading_job/grading_job_result.py @@ -1,31 +1,38 @@ from typing import List, Optional from orca_grader.container.grading_script.grading_script_command_response import GradingScriptCommandResponse -from orca_grader.common.types.grading_job_json_types import GradingJobOutputJSON, GradingJobOutputJSON +from orca_grader.common.types.grading_job_json_types import GradingJobResultJSON, GradingJobResultJSON + class GradingJobResult: - - def __init__(self, command_responses: List[GradingScriptCommandResponse], - execution_errors: List[Exception] = [], - output: str = None) -> None: - self.__command_responses = command_responses - self.__execution_errors = execution_errors - self.__output = output - - def get_command_responses(self) -> List[GradingScriptCommandResponse]: - return self.__command_responses - - def get_output(self) -> Optional[str]: - return self.__output - - def get_execution_errors(self) -> List[Exception]: - return self.__execution_errors - - def to_json(self) -> GradingJobOutputJSON: - result = dict() - json_responses = list(map(lambda c: c.to_json(), self.__command_responses)) - result["shell_responses"] = json_responses - if self.__execution_errors is not None: - result["errors"] = list(map(lambda e: f"{e.__class__.__name__}: {e}", self.__execution_errors)) - if self.__output is not None: - result["output"] = self.__output - return result + + def __init__(self, command_responses: List[GradingScriptCommandResponse], + execution_errors: List[Exception] = [], + output: str = None) -> None: + self.__command_responses = command_responses + self.__execution_errors = execution_errors + self.__output = output + + def get_command_responses(self) -> List[GradingScriptCommandResponse]: + return self.__command_responses + + def get_output(self) -> Optional[str]: + return self.__output + + def get_execution_errors(self) -> List[Exception]: + return self.__execution_errors + + def to_json(self) -> GradingJobResultJSON: + result = {"type": "GradingJobResult"} + json_responses = list( + map(lambda c: c.to_json(), self.__command_responses)) + result["shell_responses"] = json_responses + if self.__execution_errors is not None: + result["errors"] = list( + map( + lambda e: f"{e.__class__.__name__}: {e}", + self.__execution_errors + ) + ) + if self.__output is not None: + result["output"] = self.__output + return result diff --git a/worker/orca_grader/common/types/grading_job_json_types.py b/worker/orca_grader/common/types/grading_job_json_types.py index 547350eb..333aebb8 100644 --- a/worker/orca_grader/common/types/grading_job_json_types.py +++ b/worker/orca_grader/common/types/grading_job_json_types.py @@ -2,5 +2,5 @@ GradingJobJSON = Dict[Any, Any] GradingScriptCommandJSON = Dict[str, Any] -GradingJobOutputJSON = Dict[str, Any] +GradingJobResultJSON = Dict[str, Any] CodeFileInfoJSON = Dict[str, str] From 1ca1bec7648b51ebf9262a0686f94dbc39a80150 Mon Sep 17 00:00:00 2001 From: williams-jack Date: Thu, 4 Jul 2024 15:09:40 -0400 Subject: [PATCH 06/11] fix: remove duped import --- worker/orca_grader/common/grading_job/grading_job_result.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/worker/orca_grader/common/grading_job/grading_job_result.py b/worker/orca_grader/common/grading_job/grading_job_result.py index 14f46b9b..16ff947c 100644 --- a/worker/orca_grader/common/grading_job/grading_job_result.py +++ b/worker/orca_grader/common/grading_job/grading_job_result.py @@ -1,6 +1,6 @@ from typing import List, Optional from orca_grader.container.grading_script.grading_script_command_response import GradingScriptCommandResponse -from orca_grader.common.types.grading_job_json_types import GradingJobResultJSON, GradingJobResultJSON +from orca_grader.common.types.grading_job_json_types import GradingJobResultJSON class GradingJobResult: From 301c750dc8114568d25a38c08a4e8db12ed8ebc0 Mon Sep 17 00:00:00 2001 From: williams-jack Date: Sun, 14 Jul 2024 14:21:24 -0400 Subject: [PATCH 07/11] feat: add response url to image build request Allows image build service to notify client when image build fails. Additionally, swapped local port mappings in migration script and init script to be compatible with the separate postgres instance running for Bottlenose for local development specifically. --- orchestrator/init_dev.sh | 2 +- .../__mocks__/grader-image-build-request.ts | 1 + .../packages/api/src/controllers/utils.ts | 1 - .../common/src/types/image-build-service.ts | 28 +++++++++++++++++ .../packages/common/src/types/job-result.ts | 29 ------------------ .../__mocks__/grader-image-build-request.ts | 1 + .../schemas/grader-image-build-request.ts | 1 + .../migration.sql | 1 + orchestrator/packages/db/prisma/schema.prisma | 1 + orchestrator/packages/db/run-migration | 4 ++- .../db/src/image-builder-operations/index.ts | 2 +- .../server-operations/enqueue-image-build.ts | 8 +++-- .../packages/image-build-service/src/index.ts | 30 +++++++++---------- .../src/process-request/index.ts | 1 - .../packages/image-build-service/src/utils.ts | 28 ++++++++++++----- 15 files changed, 79 insertions(+), 59 deletions(-) rename orchestrator/packages/db/prisma/migrations/{20240327003906_init => 20240714175155_init}/migration.sql (98%) diff --git a/orchestrator/init_dev.sh b/orchestrator/init_dev.sh index c235bf88..3091adda 100755 --- a/orchestrator/init_dev.sh +++ b/orchestrator/init_dev.sh @@ -1,7 +1,7 @@ #!/bin/bash prisma_migration () { docker pull postgres:10 - docker run -p 5432:5432 -v db-data:/var/lib/postgresql/data -e POSTGRES_PASSWORD=password \ + docker run -p 5434:5432 -v db-data:/var/lib/postgresql/data -e POSTGRES_PASSWORD=password \ -d --name postgres postgres:10 npx prisma migrate dev --name init } diff --git a/orchestrator/packages/api/src/__mocks__/grader-image-build-request.ts b/orchestrator/packages/api/src/__mocks__/grader-image-build-request.ts index bd2ce112..56aeb601 100644 --- a/orchestrator/packages/api/src/__mocks__/grader-image-build-request.ts +++ b/orchestrator/packages/api/src/__mocks__/grader-image-build-request.ts @@ -3,4 +3,5 @@ import { GraderImageBuildRequest } from "@codegrade-orca/common"; export const defaultGraderImageBuildRequest: GraderImageBuildRequest = { dockerfileContents: `FROM hello-world:latest`, dockerfileSHASum: "generated-sha-sum", + responseURL: "http://example.com/response" }; diff --git a/orchestrator/packages/api/src/controllers/utils.ts b/orchestrator/packages/api/src/controllers/utils.ts index 15d3217c..fcf75b3a 100644 --- a/orchestrator/packages/api/src/controllers/utils.ts +++ b/orchestrator/packages/api/src/controllers/utils.ts @@ -11,7 +11,6 @@ export const errorResponse = ( export const notifyClientOfCancelledJob = async (jobConfig: GradingJobConfig) => { const result: GradingJobResult = { - type: "GradingJobResult", shell_responses: [], errors: ["Job cancelled by a course professor or Orca admin."] }; diff --git a/orchestrator/packages/common/src/types/image-build-service.ts b/orchestrator/packages/common/src/types/image-build-service.ts index 279b4c12..24dac60a 100644 --- a/orchestrator/packages/common/src/types/image-build-service.ts +++ b/orchestrator/packages/common/src/types/image-build-service.ts @@ -1,4 +1,32 @@ +export interface GraderImageBuildResult { + build_logs: Array, + was_successful: boolean + server_exception?: string, + dockerfile_sha_sum: string +} + +export interface ImageBuildFailure { + error: Error, + logs: Array, +}; + +export type ImageBuildStep = "Write request contents to Dockerfile." | + "Run docker build on Dockerfile." | + "Save image to .tgz file." | + "Remove residual Dockerfile."; + +export interface ImageBuildLog { + step: ImageBuildStep, + cmd?: string | Array, + stderr: string +} + +export const isImageBuildFailure = (object: unknown): object is ImageBuildFailure => { + return !!object && typeof object === "object" && "error" in object && "logs" in object; +} + export interface GraderImageBuildRequest { dockerfileContents: string; dockerfileSHASum: string; + responseURL: string; } diff --git a/orchestrator/packages/common/src/types/job-result.ts b/orchestrator/packages/common/src/types/job-result.ts index b3b3843a..a93fbf45 100644 --- a/orchestrator/packages/common/src/types/job-result.ts +++ b/orchestrator/packages/common/src/types/job-result.ts @@ -1,33 +1,4 @@ -export interface GraderImageBuildResult { - type: "GraderImageBuildResult", - build_logs: Array, - was_successful: boolean - server_exception?: string, - dockerfile_sha_sum: string -} - -export interface ImageBuildFailure { - error: Error, - logs: Array, -}; - -export type ImageBuildStep = "Write request contents to Dockerfile." | - "Run docker build on Dockerfile." | - "Save image to .tgz file." | - "Remove residual Dockerfile."; - -export interface ImageBuildLog { - step: ImageBuildStep, - cmd?: string | Array, - stderr: string -} - -export const isImageBuildFailure = (object: unknown): object is ImageBuildFailure => { - return !!object && typeof object === "object" && "error" in object && "logs" in object; -} - export interface GradingJobResult { - type: "GradingJobResult", shell_responses: Array, errors: Array, output?: string diff --git a/orchestrator/packages/common/src/validations/__mocks__/grader-image-build-request.ts b/orchestrator/packages/common/src/validations/__mocks__/grader-image-build-request.ts index a1b0ae15..07d0105d 100644 --- a/orchestrator/packages/common/src/validations/__mocks__/grader-image-build-request.ts +++ b/orchestrator/packages/common/src/validations/__mocks__/grader-image-build-request.ts @@ -3,4 +3,5 @@ import { GraderImageBuildRequest } from "../../types/image-build-service"; export const defaultGraderImageBuildRequest: GraderImageBuildRequest = { dockerfileContents: `FROM hello-world:latest`, dockerfileSHASum: "generated-sha-sum", + responseURL: "http://example.com/response" }; diff --git a/orchestrator/packages/common/src/validations/schemas/grader-image-build-request.ts b/orchestrator/packages/common/src/validations/schemas/grader-image-build-request.ts index d9e75684..66acb02a 100644 --- a/orchestrator/packages/common/src/validations/schemas/grader-image-build-request.ts +++ b/orchestrator/packages/common/src/validations/schemas/grader-image-build-request.ts @@ -4,5 +4,6 @@ export const graderImageBuildRequestSchema = { properties: { dockerfileContents: { type: "string" }, dockerfileSHASum: { type: "string" }, + responseURL: { type: "string" } }, } as const; diff --git a/orchestrator/packages/db/prisma/migrations/20240327003906_init/migration.sql b/orchestrator/packages/db/prisma/migrations/20240714175155_init/migration.sql similarity index 98% rename from orchestrator/packages/db/prisma/migrations/20240327003906_init/migration.sql rename to orchestrator/packages/db/prisma/migrations/20240714175155_init/migration.sql index 89510678..0d15730d 100644 --- a/orchestrator/packages/db/prisma/migrations/20240327003906_init/migration.sql +++ b/orchestrator/packages/db/prisma/migrations/20240714175155_init/migration.sql @@ -38,6 +38,7 @@ CREATE TABLE "Job" ( CREATE TABLE "ImageBuildInfo" ( "dockerfileSHA" TEXT NOT NULL, "dockerfileContent" TEXT NOT NULL, + "responseURL" TEXT NOT NULL, "inProgress" BOOLEAN NOT NULL DEFAULT false, "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, diff --git a/orchestrator/packages/db/prisma/schema.prisma b/orchestrator/packages/db/prisma/schema.prisma index 8beeb329..934c6210 100644 --- a/orchestrator/packages/db/prisma/schema.prisma +++ b/orchestrator/packages/db/prisma/schema.prisma @@ -60,6 +60,7 @@ model Job { model ImageBuildInfo { dockerfileSHA String @id dockerfileContent String + responseURL String inProgress Boolean @default(false) createdAt DateTime @default(now()) diff --git a/orchestrator/packages/db/run-migration b/orchestrator/packages/db/run-migration index 87dc9ee8..1885b6e6 100755 --- a/orchestrator/packages/db/run-migration +++ b/orchestrator/packages/db/run-migration @@ -1,5 +1,7 @@ #!/bin/bash +set -e + if [[ "$#" -ne 1 ]]; then echo "Please pass a *single* argument for migration name." exit 1 @@ -8,7 +10,7 @@ fi migration_name="$1" container_name='postgres-migration' -docker run --rm -p 5432:5432 -v db-data:/var/lib/postgresql/data \ +docker run --rm -p 5434:5432 -v db-data:/var/lib/postgresql/data \ -e POSTGRES_PASSWORD=password --name "$container_name" -d postgres:10 > /dev/null if [[ $? -ne 0 ]]; then diff --git a/orchestrator/packages/db/src/image-builder-operations/index.ts b/orchestrator/packages/db/src/image-builder-operations/index.ts index 22d79d0e..35bac9ff 100644 --- a/orchestrator/packages/db/src/image-builder-operations/index.ts +++ b/orchestrator/packages/db/src/image-builder-operations/index.ts @@ -2,4 +2,4 @@ import getNextImageBuild from "./get-next-image-build"; import handleCompletedImageBuild from "./handle-completed-image-build"; export { getNextImageBuild }; -export { handleCompletedImageBuild }; +export { handleCompletedImageBuild }; diff --git a/orchestrator/packages/db/src/server-operations/enqueue-image-build.ts b/orchestrator/packages/db/src/server-operations/enqueue-image-build.ts index 4522c482..1953c9cc 100644 --- a/orchestrator/packages/db/src/server-operations/enqueue-image-build.ts +++ b/orchestrator/packages/db/src/server-operations/enqueue-image-build.ts @@ -1,12 +1,13 @@ import { GraderImageBuildRequest } from '@codegrade-orca/common'; import prismaInstance from '../prisma-instance'; -const enqueueImageBuild = ({ dockerfileSHASum, dockerfileContents }: GraderImageBuildRequest): Promise => { +const enqueueImageBuild = ({ dockerfileSHASum, dockerfileContents, responseURL }: GraderImageBuildRequest): Promise => { return prismaInstance.$transaction(async (tx) => { const buildInfoAlreadyExists = (await tx.imageBuildInfo.count({ where: { dockerfileSHA: dockerfileSHASum, - dockerfileContent: dockerfileContents + dockerfileContent: dockerfileContents, + responseURL } })) > 0; if (buildInfoAlreadyExists) { @@ -15,7 +16,8 @@ const enqueueImageBuild = ({ dockerfileSHASum, dockerfileContents }: GraderImage await tx.imageBuildInfo.create({ data: { dockerfileSHA: dockerfileSHASum, - dockerfileContent: dockerfileContents + dockerfileContent: dockerfileContents, + responseURL } }); return true; diff --git a/orchestrator/packages/image-build-service/src/index.ts b/orchestrator/packages/image-build-service/src/index.ts index 41634996..3c3f82b8 100644 --- a/orchestrator/packages/image-build-service/src/index.ts +++ b/orchestrator/packages/image-build-service/src/index.ts @@ -1,18 +1,18 @@ import { - GraderImageBuildRequest, + GraderImageBuildRequest, isImageBuildFailure, toMilliseconds, } from "@codegrade-orca/common"; import { getNextImageBuild, handleCompletedImageBuild } from "@codegrade-orca/db"; import { createAndStoreGraderImage, removeStaleImageFiles } from "./process-request"; -import { cleanUpDockerFiles, notifyClientOfServiceFailure, removeImageFromDockerIfExists } from "./utils"; +import { cleanUpDockerFiles, sendJobResultForBuildFail, removeImageFromDockerIfExists, notifyClientOfBuildFail } from "./utils"; const LOOP_SLEEP_TIME = 5; // Seconds const main = async () => { console.info("Build service initialized."); while (true) { - let currentDockerSHASum: string | undefined = undefined; + let infoAsBuildReq: GraderImageBuildRequest | undefined = undefined; try { const nextBuildReq = await getNextImageBuild(); @@ -21,33 +21,33 @@ const main = async () => { continue; } - currentDockerSHASum = nextBuildReq.dockerfileSHA; console.info(`Attempting to build image with SHA ${nextBuildReq.dockerfileSHA}.`); - await createAndStoreGraderImage({ + infoAsBuildReq = { dockerfileSHASum: nextBuildReq.dockerfileSHA, - dockerfileContents: nextBuildReq.dockerfileContent - } as GraderImageBuildRequest); + dockerfileContents: nextBuildReq.dockerfileContent, + responseURL: nextBuildReq.responseURL + }; + await createAndStoreGraderImage(infoAsBuildReq); await handleCompletedImageBuild(nextBuildReq.dockerfileSHA, true); console.info(`Successfully build image with SHA ${nextBuildReq.dockerfileSHA}.`); } catch (err) { - if (isImageBuildFailure(err) && currentDockerSHASum) { - const cancelledJobInfoList = await handleCompletedImageBuild(currentDockerSHASum, false); + if (isImageBuildFailure(err) && infoAsBuildReq) { + const cancelledJobInfoList = await handleCompletedImageBuild(infoAsBuildReq.dockerfileSHASum, false); if (cancelledJobInfoList !== null) { await Promise.all(cancelledJobInfoList.map((cancelInfo) => { - notifyClientOfServiceFailure( + sendJobResultForBuildFail( cancelInfo, - `Failed to build image with SHA sum ${currentDockerSHASum} for this job.`, - err ).catch((notifyError) => console.error(notifyError)); // At this point we can't really do anything, but we should at least log out what happened. })); } - await cleanUpDockerFiles(currentDockerSHASum); + await notifyClientOfBuildFail(err, infoAsBuildReq).catch((notifyError) => console.error(notifyError)); + await cleanUpDockerFiles(infoAsBuildReq.dockerfileSHASum); } console.error(err); } finally { - if (currentDockerSHASum) { - await removeImageFromDockerIfExists(currentDockerSHASum); + if (infoAsBuildReq) { + await removeImageFromDockerIfExists(infoAsBuildReq.dockerfileSHASum); } await removeStaleImageFiles(); } diff --git a/orchestrator/packages/image-build-service/src/process-request/index.ts b/orchestrator/packages/image-build-service/src/process-request/index.ts index 6a918b8a..50a0b007 100644 --- a/orchestrator/packages/image-build-service/src/process-request/index.ts +++ b/orchestrator/packages/image-build-service/src/process-request/index.ts @@ -2,7 +2,6 @@ import path from "path"; import { getConfig, } from "@codegrade-orca/common"; -import { } from "@codegrade-orca/db"; import { readdir, rm, stat } from "fs/promises"; const CONFIG = getConfig(); diff --git a/orchestrator/packages/image-build-service/src/utils.ts b/orchestrator/packages/image-build-service/src/utils.ts index d2114cd7..7b1a2771 100644 --- a/orchestrator/packages/image-build-service/src/utils.ts +++ b/orchestrator/packages/image-build-service/src/utils.ts @@ -1,4 +1,4 @@ -import { GraderImageBuildResult, ImageBuildFailure, getConfig } from "@codegrade-orca/common"; +import { GraderImageBuildRequest, GraderImageBuildResult, GradingJobResult, ImageBuildFailure, getConfig } from "@codegrade-orca/common"; import { CancelJobInfo } from "@codegrade-orca/db/dist/image-builder-operations/handle-completed-image-build"; import { execFile } from "child_process"; import { existsSync, rmSync } from "fs"; @@ -47,20 +47,34 @@ const imageExistsInDocker = (dockerfileSHASum: string): Promise => { }); }; -export const notifyClientOfServiceFailure = async (cancelInfo: CancelJobInfo, dockerfileSHASum: string, buildFailure: ImageBuildFailure) => { +export const sendJobResultForBuildFail = async (cancelInfo: CancelJobInfo) => { + const result: GradingJobResult = { + shell_responses: [], + errors: ["The grader image for this job failed to build. Please contact a Professor or Admin."] + }; + await fetch(cancelInfo.response_url, { + method: "POST", + headers: { + "Accept": "application/json", + "Content-Type": "application/json" + }, + body: JSON.stringify({ ...result, key: cancelInfo.key }) + }); +} + +export const notifyClientOfBuildFail = async (buildFailure: ImageBuildFailure, originalReq: GraderImageBuildRequest) => { const result: GraderImageBuildResult = { - type: "GraderImageBuildResult", build_logs: buildFailure.logs, - dockerfile_sha_sum: dockerfileSHASum, + server_exception: buildFailure.error.toString(), was_successful: false, - server_exception: buildFailure.error.message + dockerfile_sha_sum: originalReq.dockerfileSHASum }; - await fetch(cancelInfo.response_url, { + await fetch(originalReq.responseURL, { method: "POST", headers: { "Accept": "application/json", "Content-Type": "application/json" }, - body: JSON.stringify({ ...result, key: cancelInfo.key }) + body: JSON.stringify(result) }); } From dd966d638d3823761833218bc5321531469ed0d1 Mon Sep 17 00:00:00 2001 From: williams-jack Date: Sun, 14 Jul 2024 21:43:24 -0400 Subject: [PATCH 08/11] fix: missing response_url in image creation test --- .../src/process-request/__tests__/image-creation.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/orchestrator/packages/image-build-service/src/process-request/__tests__/image-creation.test.ts b/orchestrator/packages/image-build-service/src/process-request/__tests__/image-creation.test.ts index dc1345ac..0606d9da 100644 --- a/orchestrator/packages/image-build-service/src/process-request/__tests__/image-creation.test.ts +++ b/orchestrator/packages/image-build-service/src/process-request/__tests__/image-creation.test.ts @@ -7,9 +7,9 @@ const CONFIG = getConfig(); describe("grader image functionality", () => { const graderImageBuildReq: GraderImageBuildRequest = { - dockerfileContents: `FROM hello-world - `, + dockerfileContents: `FROM hello-world`, dockerfileSHASum: "generated-sha-sum", + responseURL: "http://example.com/response" }; beforeAll(() => { From 670af5c32456868b142d915da861585167b8a581 Mon Sep 17 00:00:00 2001 From: williams-jack Date: Sat, 20 Jul 2024 17:12:17 -0400 Subject: [PATCH 09/11] refactor: simpler build result types feat: build key for responding with build result --- .../__mocks__/grader-image-build-request.ts | 7 +- .../common/src/types/image-build-service.ts | 29 +++--- .../__mocks__/grader-image-build-request.ts | 7 +- .../schemas/grader-image-build-request.ts | 7 +- .../migration.sql | 4 + orchestrator/packages/db/prisma/schema.prisma | 3 + .../server-operations/enqueue-image-build.ts | 13 ++- .../packages/image-build-service/src/index.ts | 20 ++--- .../__tests__/image-creation.test.ts | 13 +-- .../src/process-request/image-creation.ts | 89 ++++++++++--------- .../packages/image-build-service/src/utils.ts | 18 ++-- 11 files changed, 107 insertions(+), 103 deletions(-) rename orchestrator/packages/db/prisma/migrations/{20240714175155_init => 20240720201049_init}/migration.sql (94%) diff --git a/orchestrator/packages/api/src/__mocks__/grader-image-build-request.ts b/orchestrator/packages/api/src/__mocks__/grader-image-build-request.ts index 56aeb601..f8e78bea 100644 --- a/orchestrator/packages/api/src/__mocks__/grader-image-build-request.ts +++ b/orchestrator/packages/api/src/__mocks__/grader-image-build-request.ts @@ -1,7 +1,8 @@ import { GraderImageBuildRequest } from "@codegrade-orca/common"; export const defaultGraderImageBuildRequest: GraderImageBuildRequest = { - dockerfileContents: `FROM hello-world:latest`, - dockerfileSHASum: "generated-sha-sum", - responseURL: "http://example.com/response" + dockerfile_contents: `FROM hello-world:latest`, + dockerfile_sha_sum: "generated-sha-sum", + response_url: "http://example.com/response", + build_key: "{\"grader_id\": 1}" }; diff --git a/orchestrator/packages/common/src/types/image-build-service.ts b/orchestrator/packages/common/src/types/image-build-service.ts index 24dac60a..8cc78b67 100644 --- a/orchestrator/packages/common/src/types/image-build-service.ts +++ b/orchestrator/packages/common/src/types/image-build-service.ts @@ -1,32 +1,27 @@ export interface GraderImageBuildResult { - build_logs: Array, - was_successful: boolean - server_exception?: string, - dockerfile_sha_sum: string + was_successful: boolean, + logs: Array } -export interface ImageBuildFailure { - error: Error, - logs: Array, -}; - export type ImageBuildStep = "Write request contents to Dockerfile." | "Run docker build on Dockerfile." | "Save image to .tgz file." | - "Remove residual Dockerfile."; + "Remove Dockerfile."; export interface ImageBuildLog { step: ImageBuildStep, - cmd?: string | Array, - stderr: string + output?: string, + error?: string } -export const isImageBuildFailure = (object: unknown): object is ImageBuildFailure => { - return !!object && typeof object === "object" && "error" in object && "logs" in object; +export const isImageBuildResult = (o: unknown): o is GraderImageBuildResult => { + if (typeof o !== "object" || o === null) return false; + return Object.keys(o).length == 2 && ["was_successful", "logs"].every((k) => k in o); } export interface GraderImageBuildRequest { - dockerfileContents: string; - dockerfileSHASum: string; - responseURL: string; + dockerfile_contents: string, + dockerfile_sha_sum: string, + response_url: string, + build_key: string, } diff --git a/orchestrator/packages/common/src/validations/__mocks__/grader-image-build-request.ts b/orchestrator/packages/common/src/validations/__mocks__/grader-image-build-request.ts index 07d0105d..84176eb9 100644 --- a/orchestrator/packages/common/src/validations/__mocks__/grader-image-build-request.ts +++ b/orchestrator/packages/common/src/validations/__mocks__/grader-image-build-request.ts @@ -1,7 +1,8 @@ import { GraderImageBuildRequest } from "../../types/image-build-service"; export const defaultGraderImageBuildRequest: GraderImageBuildRequest = { - dockerfileContents: `FROM hello-world:latest`, - dockerfileSHASum: "generated-sha-sum", - responseURL: "http://example.com/response" + dockerfile_contents: `FROM hello-world:latest`, + dockerfile_sha_sum: "generated-sha-sum", + response_url: "http://example.com/response", + build_key: "{\"grader_id\": 1}" }; diff --git a/orchestrator/packages/common/src/validations/schemas/grader-image-build-request.ts b/orchestrator/packages/common/src/validations/schemas/grader-image-build-request.ts index 66acb02a..f033fb80 100644 --- a/orchestrator/packages/common/src/validations/schemas/grader-image-build-request.ts +++ b/orchestrator/packages/common/src/validations/schemas/grader-image-build-request.ts @@ -2,8 +2,9 @@ export const graderImageBuildRequestSchema = { $id: "https://orca-schemas.com/grader-image-build-request", type: "object", properties: { - dockerfileContents: { type: "string" }, - dockerfileSHASum: { type: "string" }, - responseURL: { type: "string" } + dockerfile_contents: { type: "string" }, + dockerfile_sha_sum: { type: "string" }, + response_url: { type: "string" }, + build_key: { type: "string" }, }, } as const; diff --git a/orchestrator/packages/db/prisma/migrations/20240714175155_init/migration.sql b/orchestrator/packages/db/prisma/migrations/20240720201049_init/migration.sql similarity index 94% rename from orchestrator/packages/db/prisma/migrations/20240714175155_init/migration.sql rename to orchestrator/packages/db/prisma/migrations/20240720201049_init/migration.sql index 0d15730d..f11a1e9e 100644 --- a/orchestrator/packages/db/prisma/migrations/20240714175155_init/migration.sql +++ b/orchestrator/packages/db/prisma/migrations/20240720201049_init/migration.sql @@ -38,6 +38,7 @@ CREATE TABLE "Job" ( CREATE TABLE "ImageBuildInfo" ( "dockerfileSHA" TEXT NOT NULL, "dockerfileContent" TEXT NOT NULL, + "buildKey" TEXT NOT NULL, "responseURL" TEXT NOT NULL, "inProgress" BOOLEAN NOT NULL DEFAULT false, "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, @@ -66,6 +67,9 @@ CREATE UNIQUE INDEX "Submitter_clientURL_collationType_collationID_key" ON "Subm -- CreateIndex CREATE UNIQUE INDEX "Job_clientURL_clientKey_key" ON "Job"("clientURL", "clientKey"); +-- CreateIndex +CREATE UNIQUE INDEX "ImageBuildInfo_buildKey_responseURL_key" ON "ImageBuildInfo"("buildKey", "responseURL"); + -- CreateIndex CREATE UNIQUE INDEX "JobConfigAwaitingImage_clientKey_clientURL_key" ON "JobConfigAwaitingImage"("clientKey", "clientURL"); diff --git a/orchestrator/packages/db/prisma/schema.prisma b/orchestrator/packages/db/prisma/schema.prisma index 934c6210..64ddf051 100644 --- a/orchestrator/packages/db/prisma/schema.prisma +++ b/orchestrator/packages/db/prisma/schema.prisma @@ -60,11 +60,14 @@ model Job { model ImageBuildInfo { dockerfileSHA String @id dockerfileContent String + buildKey String responseURL String inProgress Boolean @default(false) createdAt DateTime @default(now()) jobConfigs JobConfigAwaitingImage[] + + @@unique([buildKey, responseURL]) } model JobConfigAwaitingImage { diff --git a/orchestrator/packages/db/src/server-operations/enqueue-image-build.ts b/orchestrator/packages/db/src/server-operations/enqueue-image-build.ts index 1953c9cc..fdd57b15 100644 --- a/orchestrator/packages/db/src/server-operations/enqueue-image-build.ts +++ b/orchestrator/packages/db/src/server-operations/enqueue-image-build.ts @@ -1,13 +1,11 @@ import { GraderImageBuildRequest } from '@codegrade-orca/common'; import prismaInstance from '../prisma-instance'; -const enqueueImageBuild = ({ dockerfileSHASum, dockerfileContents, responseURL }: GraderImageBuildRequest): Promise => { +const enqueueImageBuild = ({ dockerfile_sha_sum, dockerfile_contents, response_url, build_key }: GraderImageBuildRequest): Promise => { return prismaInstance.$transaction(async (tx) => { const buildInfoAlreadyExists = (await tx.imageBuildInfo.count({ where: { - dockerfileSHA: dockerfileSHASum, - dockerfileContent: dockerfileContents, - responseURL + dockerfileSHA: dockerfile_sha_sum, } })) > 0; if (buildInfoAlreadyExists) { @@ -15,9 +13,10 @@ const enqueueImageBuild = ({ dockerfileSHASum, dockerfileContents, responseURL } } else { await tx.imageBuildInfo.create({ data: { - dockerfileSHA: dockerfileSHASum, - dockerfileContent: dockerfileContents, - responseURL + dockerfileSHA: dockerfile_sha_sum, + dockerfileContent: dockerfile_contents, + responseURL: response_url, + buildKey: build_key } }); return true; diff --git a/orchestrator/packages/image-build-service/src/index.ts b/orchestrator/packages/image-build-service/src/index.ts index 3c3f82b8..9ddcc6dc 100644 --- a/orchestrator/packages/image-build-service/src/index.ts +++ b/orchestrator/packages/image-build-service/src/index.ts @@ -1,7 +1,7 @@ import { - GraderImageBuildRequest, - isImageBuildFailure, + GraderImageBuildRequest, toMilliseconds, + isImageBuildResult } from "@codegrade-orca/common"; import { getNextImageBuild, handleCompletedImageBuild } from "@codegrade-orca/db"; import { createAndStoreGraderImage, removeStaleImageFiles } from "./process-request"; @@ -23,17 +23,17 @@ const main = async () => { console.info(`Attempting to build image with SHA ${nextBuildReq.dockerfileSHA}.`); infoAsBuildReq = { - dockerfileSHASum: nextBuildReq.dockerfileSHA, - dockerfileContents: nextBuildReq.dockerfileContent, - responseURL: nextBuildReq.responseURL + dockerfile_sha_sum: nextBuildReq.dockerfileSHA, + dockerfile_contents: nextBuildReq.dockerfileContent, + response_url: nextBuildReq.responseURL, + build_key: nextBuildReq.buildKey }; await createAndStoreGraderImage(infoAsBuildReq); - await handleCompletedImageBuild(nextBuildReq.dockerfileSHA, true); console.info(`Successfully build image with SHA ${nextBuildReq.dockerfileSHA}.`); } catch (err) { - if (isImageBuildFailure(err) && infoAsBuildReq) { - const cancelledJobInfoList = await handleCompletedImageBuild(infoAsBuildReq.dockerfileSHASum, false); + if (isImageBuildResult(err) && infoAsBuildReq) { + const cancelledJobInfoList = await handleCompletedImageBuild(infoAsBuildReq.dockerfile_sha_sum, false); if (cancelledJobInfoList !== null) { await Promise.all(cancelledJobInfoList.map((cancelInfo) => { sendJobResultForBuildFail( @@ -42,12 +42,12 @@ const main = async () => { })); } await notifyClientOfBuildFail(err, infoAsBuildReq).catch((notifyError) => console.error(notifyError)); - await cleanUpDockerFiles(infoAsBuildReq.dockerfileSHASum); + await cleanUpDockerFiles(infoAsBuildReq.dockerfile_sha_sum); } console.error(err); } finally { if (infoAsBuildReq) { - await removeImageFromDockerIfExists(infoAsBuildReq.dockerfileSHASum); + await removeImageFromDockerIfExists(infoAsBuildReq.dockerfile_sha_sum); } await removeStaleImageFiles(); } diff --git a/orchestrator/packages/image-build-service/src/process-request/__tests__/image-creation.test.ts b/orchestrator/packages/image-build-service/src/process-request/__tests__/image-creation.test.ts index 0606d9da..b5243690 100644 --- a/orchestrator/packages/image-build-service/src/process-request/__tests__/image-creation.test.ts +++ b/orchestrator/packages/image-build-service/src/process-request/__tests__/image-creation.test.ts @@ -7,9 +7,10 @@ const CONFIG = getConfig(); describe("grader image functionality", () => { const graderImageBuildReq: GraderImageBuildRequest = { - dockerfileContents: `FROM hello-world`, - dockerfileSHASum: "generated-sha-sum", - responseURL: "http://example.com/response" + dockerfile_contents: `FROM hello-world`, + dockerfile_sha_sum: "generated-sha-sum", + response_url: "http://example.com/response", + build_key: "{\"grader_id\": 1}" }; beforeAll(() => { @@ -22,7 +23,7 @@ describe("grader image functionality", () => { rmSync( path.join( CONFIG.dockerImageFolder, - `${graderImageBuildReq.dockerfileSHASum}.tgz`, + `${graderImageBuildReq.dockerfile_sha_sum}.tgz`, ), { force: true, @@ -37,7 +38,7 @@ describe("grader image functionality", () => { existsSync( path.join( CONFIG.dockerImageFolder, - `${graderImageBuildReq.dockerfileSHASum}.Dockerfile`, + `${graderImageBuildReq.dockerfile_sha_sum}.Dockerfile`, ), ), ).toBe(false); @@ -46,7 +47,7 @@ describe("grader image functionality", () => { path.join( path.join( CONFIG.dockerImageFolder, - `${graderImageBuildReq.dockerfileSHASum}.tgz`, + `${graderImageBuildReq.dockerfile_sha_sum}.tgz`, ), ), ), diff --git a/orchestrator/packages/image-build-service/src/process-request/image-creation.ts b/orchestrator/packages/image-build-service/src/process-request/image-creation.ts index 229e1c8b..49eed8d9 100644 --- a/orchestrator/packages/image-build-service/src/process-request/image-creation.ts +++ b/orchestrator/packages/image-build-service/src/process-request/image-creation.ts @@ -1,4 +1,4 @@ -import { GraderImageBuildRequest, ImageBuildFailure, ImageBuildLog, getConfig } from "@codegrade-orca/common"; +import { GraderImageBuildRequest, GraderImageBuildResult, ImageBuildLog, ImageBuildStep, getConfig } from "@codegrade-orca/common"; import { execFile } from "child_process"; import { writeFile, rm } from "fs"; import path from "path"; @@ -7,68 +7,76 @@ const CONFIG = getConfig(); export const createAndStoreGraderImage = ( buildRequest: GraderImageBuildRequest, -) => { +): Promise => { const buildLogs: Array = []; return writeDockerfileContentsToFile(buildRequest, buildLogs) .then((_) => buildImage(buildRequest, buildLogs)) .then((_) => removeDockerfileAfterBuild( - path.join(CONFIG.dockerImageFolder, `${buildRequest.dockerfileSHASum}.Dockerfile`), + path.join(CONFIG.dockerImageFolder, `${buildRequest.dockerfile_sha_sum}.Dockerfile`), buildLogs )) - .then((_) => saveImageToTgz(buildRequest.dockerfileSHASum, buildLogs)); + .then((_) => saveImageToTgz(buildRequest.dockerfile_sha_sum, buildLogs)) + .then((_) => ({ logs: buildLogs, was_successful: true })); }; -const writeDockerfileContentsToFile = ({ dockerfileContents, dockerfileSHASum }: GraderImageBuildRequest, buildLogs: Array): Promise => { +const writeDockerfileContentsToFile = ({ dockerfile_contents, dockerfile_sha_sum }: GraderImageBuildRequest, buildLogs: Array): Promise => { return new Promise((resolve, reject) => { const dockerfilePath = path.join( CONFIG.dockerImageFolder, - `${dockerfileSHASum}.Dockerfile`, + `${dockerfile_sha_sum}.Dockerfile`, ); - writeFile(dockerfilePath, dockerfileContents, (err) => { - buildLogs.push({ - step: "Write request contents to Dockerfile.", - stderr: "" - }); + writeFile(dockerfilePath, dockerfile_contents, (err) => { + const step: ImageBuildStep = "Write request contents to Dockerfile."; if (err) { - const buildFailure: ImageBuildFailure = { + buildLogs.push({ + step, + error: err.toString(), + }); + reject({ logs: buildLogs, - error: err - }; - reject(buildFailure); + was_successful: false, + }); } else { - resolve(dockerfilePath); + buildLogs.push({ + step, + output: "Contents written to Dockerfile." + }); + resolve(); } }); }); }; -const buildImage = ({ dockerfileSHASum }: GraderImageBuildRequest, buildLogs: Array): Promise => { +const buildImage = ({ dockerfile_sha_sum }: GraderImageBuildRequest, buildLogs: Array): Promise => { const dockerBuildArgs = [ "build", "-t", - dockerfileSHASum, + dockerfile_sha_sum, "-f", - path.join(CONFIG.dockerImageFolder, `${dockerfileSHASum}.Dockerfile`), + path.join(CONFIG.dockerImageFolder, `${dockerfile_sha_sum}.Dockerfile`), ".", ]; return new Promise((resolve, reject) => { execFile( "docker", dockerBuildArgs, - (err, _stdout, stderr) => { - buildLogs.push({ - step: "Run docker build on Dockerfile.", - cmd: ["docker", ...dockerBuildArgs], - stderr - }); + (err, stdout, stderr) => { + const step: ImageBuildStep = "Run docker build on Dockerfile."; if (err) { - const buildFailure: ImageBuildFailure = { - error: err, + buildLogs.push({ + step, + error: stderr + }); + reject({ + was_successful: false, logs: buildLogs - }; - reject(buildFailure); + }); } else { + buildLogs.push({ + step, + output: stdout + }); resolve(); } }, @@ -77,16 +85,14 @@ const buildImage = ({ dockerfileSHASum }: GraderImageBuildRequest, buildLogs: Ar }; const removeDockerfileAfterBuild = (dockerfilePath: string, buildLogs: Array): Promise => { - buildLogs.push({ - stderr: "", - step: "Remove residual Dockerfile." - }); return new Promise((resolve, reject) => { rm(dockerfilePath, (err) => { + const step: ImageBuildStep = "Remove Dockerfile."; if (err) { - const buildFailure: ImageBuildFailure = { error: err, logs: buildLogs }; - reject(buildFailure); + buildLogs.push({ step, error: err.toString() }); + reject({ was_successful: false, logs: buildLogs }); } else { + buildLogs.push({ step, output: "Successfully cleaned up Dockerfile." }); resolve(); } }) @@ -105,17 +111,12 @@ const saveImageToTgz = (imageName: string, buildLogs: Array): Pro "docker", dockerSaveCommandArgs, (err, _stdout, stderr) => { + const step: ImageBuildStep = "Save image to .tgz file."; if (err) { - const buildFailure: ImageBuildFailure = { - error: err, - logs: [...buildLogs, { - step: "Save image to .tgz file.", - cmd: ["docker", ...dockerSaveCommandArgs], - stderr - }], - }; - reject(buildFailure); + buildLogs.push({ step, error: stderr }); + reject({ was_successful: false, logs: buildLogs }); } else { + buildLogs.push({ step, output: `Successfully saved image to ${imageName}.tgz.` }); resolve(); } }, diff --git a/orchestrator/packages/image-build-service/src/utils.ts b/orchestrator/packages/image-build-service/src/utils.ts index 7b1a2771..a2bf5e3c 100644 --- a/orchestrator/packages/image-build-service/src/utils.ts +++ b/orchestrator/packages/image-build-service/src/utils.ts @@ -1,4 +1,4 @@ -import { GraderImageBuildRequest, GraderImageBuildResult, GradingJobResult, ImageBuildFailure, getConfig } from "@codegrade-orca/common"; +import { GraderImageBuildRequest, GraderImageBuildResult, GradingJobResult, getConfig } from "@codegrade-orca/common"; import { CancelJobInfo } from "@codegrade-orca/db/dist/image-builder-operations/handle-completed-image-build"; import { execFile } from "child_process"; import { existsSync, rmSync } from "fs"; @@ -62,19 +62,17 @@ export const sendJobResultForBuildFail = async (cancelInfo: CancelJobInfo) => { }); } -export const notifyClientOfBuildFail = async (buildFailure: ImageBuildFailure, originalReq: GraderImageBuildRequest) => { - const result: GraderImageBuildResult = { - build_logs: buildFailure.logs, - server_exception: buildFailure.error.toString(), - was_successful: false, - dockerfile_sha_sum: originalReq.dockerfileSHASum - }; - await fetch(originalReq.responseURL, { +export const notifyClientOfBuildFail = async (buildFailure: GraderImageBuildResult, originalReq: GraderImageBuildRequest) => { + const { response_url, build_key } = originalReq; + await fetch(response_url, { method: "POST", headers: { "Accept": "application/json", "Content-Type": "application/json" }, - body: JSON.stringify(result) + body: JSON.stringify({ + ...buildFailure, + build_key + }) }); } From 2e92da6328ae20a63add6a6882fafce5360119db Mon Sep 17 00:00:00 2001 From: williams-jack Date: Sat, 20 Jul 2024 17:29:31 -0400 Subject: [PATCH 10/11] fix: notify client of build result in successful cases too --- orchestrator/packages/image-build-service/src/index.ts | 9 +++++---- .../src/process-request/image-creation.ts | 4 +++- orchestrator/packages/image-build-service/src/utils.ts | 4 ++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/orchestrator/packages/image-build-service/src/index.ts b/orchestrator/packages/image-build-service/src/index.ts index 9ddcc6dc..eeb44997 100644 --- a/orchestrator/packages/image-build-service/src/index.ts +++ b/orchestrator/packages/image-build-service/src/index.ts @@ -5,7 +5,7 @@ import { } from "@codegrade-orca/common"; import { getNextImageBuild, handleCompletedImageBuild } from "@codegrade-orca/db"; import { createAndStoreGraderImage, removeStaleImageFiles } from "./process-request"; -import { cleanUpDockerFiles, sendJobResultForBuildFail, removeImageFromDockerIfExists, notifyClientOfBuildFail } from "./utils"; +import { cleanUpDockerFiles, sendJobResultForBuildFail, removeImageFromDockerIfExists, notifyClientOfBuildResult } from "./utils"; const LOOP_SLEEP_TIME = 5; // Seconds @@ -28,9 +28,10 @@ const main = async () => { response_url: nextBuildReq.responseURL, build_key: nextBuildReq.buildKey }; - await createAndStoreGraderImage(infoAsBuildReq); + const result = await createAndStoreGraderImage(infoAsBuildReq); await handleCompletedImageBuild(nextBuildReq.dockerfileSHA, true); - console.info(`Successfully build image with SHA ${nextBuildReq.dockerfileSHA}.`); + await notifyClientOfBuildResult(result, infoAsBuildReq); + console.info(`Successfully built image with SHA ${nextBuildReq.dockerfileSHA}.`); } catch (err) { if (isImageBuildResult(err) && infoAsBuildReq) { const cancelledJobInfoList = await handleCompletedImageBuild(infoAsBuildReq.dockerfile_sha_sum, false); @@ -41,7 +42,7 @@ const main = async () => { ).catch((notifyError) => console.error(notifyError)); // At this point we can't really do anything, but we should at least log out what happened. })); } - await notifyClientOfBuildFail(err, infoAsBuildReq).catch((notifyError) => console.error(notifyError)); + await notifyClientOfBuildResult(err, infoAsBuildReq).catch((notifyError) => console.error(notifyError)); await cleanUpDockerFiles(infoAsBuildReq.dockerfile_sha_sum); } console.error(err); diff --git a/orchestrator/packages/image-build-service/src/process-request/image-creation.ts b/orchestrator/packages/image-build-service/src/process-request/image-creation.ts index 49eed8d9..53ac7123 100644 --- a/orchestrator/packages/image-build-service/src/process-request/image-creation.ts +++ b/orchestrator/packages/image-build-service/src/process-request/image-creation.ts @@ -73,9 +73,11 @@ const buildImage = ({ dockerfile_sha_sum }: GraderImageBuildRequest, buildLogs: logs: buildLogs }); } else { + // All docker build information, regardless of status code, + // is logged to STDERR. buildLogs.push({ step, - output: stdout + output: stderr }); resolve(); } diff --git a/orchestrator/packages/image-build-service/src/utils.ts b/orchestrator/packages/image-build-service/src/utils.ts index a2bf5e3c..7d280c0f 100644 --- a/orchestrator/packages/image-build-service/src/utils.ts +++ b/orchestrator/packages/image-build-service/src/utils.ts @@ -62,7 +62,7 @@ export const sendJobResultForBuildFail = async (cancelInfo: CancelJobInfo) => { }); } -export const notifyClientOfBuildFail = async (buildFailure: GraderImageBuildResult, originalReq: GraderImageBuildRequest) => { +export const notifyClientOfBuildResult = async (result: GraderImageBuildResult, originalReq: GraderImageBuildRequest) => { const { response_url, build_key } = originalReq; await fetch(response_url, { method: "POST", @@ -71,7 +71,7 @@ export const notifyClientOfBuildFail = async (buildFailure: GraderImageBuildResu "Content-Type": "application/json" }, body: JSON.stringify({ - ...buildFailure, + ...result, build_key }) }); From 0e7a99f0990fc04f34bc97ee3940be24452fe23d Mon Sep 17 00:00:00 2001 From: williams-jack Date: Sat, 20 Jul 2024 17:30:13 -0400 Subject: [PATCH 11/11] feat: utilize echo-server for both build and job results --- worker/images/testing/echo-server/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/worker/images/testing/echo-server/index.js b/worker/images/testing/echo-server/index.js index 2bcc9dbe..f1187af0 100644 --- a/worker/images/testing/echo-server/index.js +++ b/worker/images/testing/echo-server/index.js @@ -24,10 +24,10 @@ app.get("/job-output/:key", (req, res) => { }); app.post("/job-output", (req, res) => { - const { key, output } = req.body; + const { key, build_key } = req.body; console.log(req.body); - if (key) { - responses[key] = req.body; + if (key || build_key) { + responses[key || build_key] = req.body; return res.sendStatus(200); } res.sendStatus(400);