Executor Runtimes by Piszmog · Pull Request #47496 · sourcegraph/sourcegraph-public-snapshot

Piszmog · 2023-02-09T16:12:23Z

Note: Most of the new lines are from adding Unit Tests. Over 120 new unit tests were added.

This PR will help position us into a situation where Executors can understand the environment they are deployed to and handle Jobs with that specific environment.

So for example if docker has been configured, the executor will handle all jobs with docker.

In the future, we will add runtimes for Firecracker, K8s, and more.

Changes

Split command into two packages
- worker/command and worker/runner
- This is to help with getting test coverage and to make integrating the worker/runtime package more easily
General housekeeping (fixing function names, remove variable shadowing when not necessary)
Created an interface Runtime in the executor app
- This interface will prepare the workspace, create a runner (what will actually execute the steps), and build the commands for the runner to execute
- At startup, the runtime is configured based on how the host is configured.
  - At the moment, I commented out this setup call to be backwards compatible
- When a Job is processed, the configured runtime is used to execute the Job
Created a dockerRuntime struct to handle "docker" runtimes
Added a special case for src
- If Job.CLISteps is set, then we fallback to the "legacy" way
Over 120 new Unit Tests

New Issues

Created the following Issue as I was implementing this PR.

Runtime CLI Actions #47819

Demo

The following is using the native SSBC for testing.

This just shows that,

A job is successfully processed
The logs confirm the commands

"Legacy" Flow

Screen.Recording.2023-03-03.at.11.40.51.mov

Runtime flow (docker)

Screen.Recording.2023-03-03.at.11.29.01.mov

Test plan

Added unit tests and did functional testing.

…gEntry

…executor-job-specific-tokens # Conflicts: # enterprise/cmd/executor/internal/apiclient/queue/job/client.go # enterprise/cmd/executor/internal/apiclient/queue/job/client_test.go # enterprise/cmd/executor/internal/worker/store/store.go

# Conflicts: # internal/database/permission_sync_jobs.go # mockgen.test.yaml

…executor-job-specific-tokens

Piszmog · 2023-03-23T20:11:42Z

Pushed these changes to executor-patch-notest/runtime-interface to get an image built without merging to main

Edit (3/30):

Updated GCP dogfood use use image

eseliger · 2023-03-22T19:27:20Z

 	}

-	cmd.Env = env
+	cmd.Env = append(cmd.Env, env...)


does this still unset env vars from the host? We never want os.Environ to spill into this env.

Yea not sure what my brain was thinking. I changed this back to cmd.Env = env

Now I remember why. This was allowing me to inject the ENV vars when running the unit test with the tricky cmd.Exec

sanderginn

I really like the patterns you implemented here. Awesome job 👍

Most of my comments are actually questions I have to understand executors better. A few nits otherwise.

sanderginn · 2023-03-30T16:32:18Z

+	if err != nil {
+		return "", err
+	}
+	cniConfigFile := path.Join(cniConfigDir, "10-sourcegraph-executors.conflist")


What's the significance of the 10-?

Good question 🤔

@eseliger what is the significance of this? (I'll add a comment with the why)

…nterface # Conflicts: # enterprise/cmd/executor/internal/run/util.go # enterprise/cmd/executor/internal/run/validate.go # enterprise/cmd/executor/internal/run/validate_test.go # enterprise/cmd/executor/singlebinary/service.go

…nterface

Piszmog · 2023-04-03T15:38:36Z

If there are no more comments, I will merge this tomorrow (one more day of running in dogfood)

sanderginn · 2023-04-03T15:41:23Z

All good from me 👍

Piszmog added 30 commits January 20, 2023 12:09

Move ExecutionLogEntry to internal/executor

c0d2cad

Remove store.ExecutionLogEntry and update to use executor.ExecutionLo…

ca33957

…gEntry

Split worker and job functionality

b097570

Update job client to implement store.ExecutionLogEntryStore

8c2c60b

Update worker client to implement workerutil.Store

7d17e7d

Update mocks to generate the new location for ExecutionLogEntryStore

29efff6

Delete unneeded tests

4f738ba

Fix tests

923cde9

Fix ExecutionLogEntryStore generated mocks

dcb2331

Fix ExecutionLogEntryStore generated mocks

e2848ad

Fix docs

b54b81f

Fix package import alias

bff1cc1

Split HTTP structs from job

76b0bed

Generate and validate job specific tokens

f697794

Add configuration for executor job access tokens

57e5a02

Use the job token to authenticate with executor queue endpoints

c6e51dc

Pass the record to the worker

0dc1706

Combine clients back together

8f3d27e

Update to use the single client

d57c678

Update generated mocks

768a77d

Merge changes from log worker branch

2440111

Use job token for marking the status of a job

35a53c2

Merge remote-tracking branch 'origin/main' into rc/executor-log-worker

710999b

# Conflicts: # internal/database/permission_sync_jobs.go # mockgen.test.yaml

Fix linting issues

4a0b344

Fix tests

88d9814

Merge remote-tracking branch 'origin/rc/executor-log-worker' into rc/…

faa4e36

…executor-job-specific-tokens

Fix lint issues

122326a

Merge remote-tracking branch 'origin/main' into rc/executor-log-worker

4585d75

Fix lint issues

1750969

Piszmog added 3 commits March 22, 2023 14:56

Add tests for shell runner

218a83f

Enable runtime so we can build an image for testing

612be35

Revert change

18ff6fd

eseliger reviewed Mar 30, 2023

View reviewed changes

sanderginn approved these changes Mar 30, 2023

View reviewed changes

Piszmog added 16 commits March 30, 2023 10:58

Rever command env setting

d65c938

Change parameter orger to have context come first

4fa9ffb

Fix comment

2fb9887

Use default docker auth if no specific auth provided

9de17c3

Fix doc

0153ecf

Merge main

b2fe4e1

Fix test

83e99dc

Merge branch 'main' into rc/executor-runtime-interface

7760057

Fix firecracker command

2df519a

Fix resource options

2a8c9d8

Merge branch 'main' into rc/executor-runtime-interface

8cb5d9b

Fix firecracker issues

2829a86

Merge remote-tracking branch 'origin/main' into rc/executor-runtime-i…

47065e2

…nterface

Fix tests

2502703

Log the working directory to help with any debugging

2789c5e

Piszmog added 2 commits April 3, 2023 11:07

Update bazel files

7d554c0

Merge branch 'main' into rc/executor-runtime-interface

74a8f7e

Piszmog merged commit 00b3bef into main Apr 4, 2023

Piszmog deleted the rc/executor-runtime-interface branch April 4, 2023 15:18

Piszmog mentioned this pull request Apr 4, 2023

Add Firecracker Runtime #48752

Merged

almeidapaulooliveira pushed a commit that referenced this pull request Apr 5, 2023

Executor Runtimes (#47496)

b012683

Conversation

Piszmog commented Feb 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

New Issues

Demo

"Legacy" Flow

Runtime flow (docker)

Test plan

Uh oh!

Piszmog commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eseliger Mar 22, 2023

Choose a reason for hiding this comment

Uh oh!

Piszmog Mar 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Piszmog Mar 30, 2023

Choose a reason for hiding this comment

Uh oh!

sanderginn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sanderginn Mar 30, 2023

Choose a reason for hiding this comment

Uh oh!

Piszmog Mar 30, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Piszmog commented Apr 3, 2023

Uh oh!

sanderginn commented Apr 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Piszmog commented Feb 9, 2023 •

edited

Loading

Piszmog commented Mar 23, 2023 •

edited

Loading

Piszmog Mar 30, 2023 •

edited

Loading