FYI the intention is to take logic that previously existed in worker.Client and move it into the exec package - so, exec.{Get,Check,Put,Task}Step would interact directly with the runtime abstractions, which will hopefully be general enough that implementing a k8s or nomad runtime is as simple as implementing a (fairly) simple interface that is unaware of the business logic of these steps

Hey @aoldershaw, not sure if you have noticed #6495 that may impact this refactor.

@evanchaoli thanks, i've seen that PR, seems cool! I'm still not sure what will become of this PR, it's still very much a WIP - but if we decide to go forward with this PR, we'll be sure to bring your commits over

@aoldershaw thanks for that great dependency graph! would it be easy enough to produce the same graph for the refactored code?

@muntac absolutely! I'm planning on writing out more in depth how things are structured, but hopefully this is a decent start:

The primary goal is to find a common interface that can be implemented for other runtimes (e.g. a k8s runtime). One thing I didn't illustrate in the original dependency graph is how external packages interact with the atc/worker package, which was kind of a mixed bag - a ton of functionality was baked into the atc/worker.Worker interface, which wasn't really a general interface, having a lot of Garden/Baggageclaim specifics.

So, the atc/runtime package includes a new set of interfaces that can hopefully be more general - this consists of Worker, Container, Volume, and Process (not included in the diagram). Worker provides the ability to construct Containers and Volumes, and you can start Processes on Containers to run things.

The atc/worker package provides helpers for interacting with the runtime types. Specifically, there are two main components - Pool (which represents the pool of workers, allowing you to select a worker for running a container and locate Volumes and Containers), and Streamer (which understands how to stream data between Volumes, and stream files out of Volumes). Implementations of the atc/runtime types make use of Pool and Streamer to locate external volumes and stream data from them.

atc/worker/gardenruntime is an implementation of the atc/runtime interfaces that uses Garden for running containers and Baggageclaim for managing volumes. External packages aren't aware of any Garden/Baggageclaim specifics - they're only aware of atc/runtime

atc/exec (and atc/resource) implement the logic for all the step types (most relevant here are task, get, put, check, but also steps like load_var and set_pipeline which stream files out of volumes). In a simplified way, these steps select a runtime.Worker using worker.Pool, then create a runtime.Container on that worker, then run a runtime.Process on that container. The GetStep is a bit more complicated, and has a resource.Getter helper to achieve a similar thing).

Still wrapping my head around this. Initially tried to account for the interactions of the atc/worker package with the rest of the atc i.e. what are the files outside the worker that rely on it and which functionality is being used. This is to ensure the refactor accounts for all of this, and to get a high level idea for what functionality the rest of the atc expects from the atc/worker package.

Files that use atc/worker:

(relative to atc/)
api/handler.go (for containerserver)
api/containerserver/hijack.go
api/containerserver/server.go
api/artifactserver/create.go
api/artifactserver/server.go

atccmd/command.go

engine/build_step_delegate.go
engine/builder.go
engine/check_delegate.go
engine/delegate_factory.go
engine/get_delegate.go
engine/put_delegate.go
engine/task_delegate.go

exec/artifact_input_step.go
exec/artifact_output_step.go
exec/build_step_delegate.go
exec/check_step.go
exec/get_step.go
exec/load_var_step.go
exec/put_step.go
exec/set_pipeline_step.go
exec/task_config_source.go
exec/task_step.go

Still figuring out the functionality bit, and will then dive back into commits.

Current interface exposed by pool

pool
-> NewPool(WorkerProvider)
-> FindContainer(lager.Logger, int, string) (Container, bool, error)
-> FindVolume(lager.Logger, int, string) (Volume, bool, error)
-> CreateVolume(lager.Logger, VolumeSpec, WorkerSpec, db.VolumeType) (Volume, error)
-> ContainerInWorker(lager.Logger, db.ContainerOwner, WorkerSpec) (bool, error)
-> SelectWorker(context.Context, db.ContainerOwner,ContainerSpec,WorkerSpec,ContainerPlacementStrategy) (Client, error)

Feel free to suggest a different approach.

Still figuring out the functionality bit

@muntac would it help if I went over each of the interactions with atc/worker in a bit more depth?

api/containerserver/hijack.go - this is the API handler that deals with running fly hijack/fly intercept, which allows you to run a process (typically a shell, but not necessarily) on an existing container. So, it interacts with atc/worker to locate a container, start a process, continually update the TTY (esp. terminal size), and store some state in the DB ("the container was hijacked at this time, so don't GC it")

api/artifactserver/create.go - this is the API handler used by fly execute to upload artifacts to a worker, where artifacts refers to local inputs that you upload to Concourse. It interacts with atc/worker to create an empty volume on a random worker and then stream in the artifact contents. This artifact (and its corresponding volume) can then be used to run steps (specifically, the task step that's run in fly execute). It is used by the ArtifactInput step

exec/artifact_input_step.go - this step runs in builds created by fly execute. It interacts with atc/worker to locate the volume that was previously created in api/artifactserver/create.go and makes it available to other steps.

exec/artifact_output_step.go - this step also runs in builds created by fly execute to mark a volume created by the task as a downloadable artifact. Previously, it interacted with atc/worker to locate the volume - now, it only interacts with the DB to mark a volume as an artifact **. Once an artifact has been "output" by this step, it can be downloaded in the api via api/artifactserver/get.go

** This is due to the refactor...previously, we used "Artifact" to mean two things - the uploadable/downloadable volumes I've been describing that are used in fly execute, but also as a general handle to a volume (formerly runtime.Artifact). If that's confusing, I'm not surprised - runtime.Artifact didn't make much sense to me to be honest, since it just seemed like an abstraction on top of volumes.

api/artifactserver/get.go - as I alluded to, this is the counterpart to exec/artifact_output_step.go - it's used by fly execute to download the task's outputs after it finishes. It interacts with atc/worker to locate the volume and stream the contents out.

So, the high-level flow for fly execute is (was):

fly execute -c task.yml -i some-input=./input -o some-output=./output
  -> POST api/artifactserver/create.go
    -> volume := atc/worker.Pool.CreateVolume() // for the artifact some-input
    -> volume.InitializeArtifact() // mark in the DB that this is an artifact
    -> volume.StreamIn(req.Body)
  -> Create a build with an ArtifactInputStep and ArtifactOutputStep (and a TaskStep of course)
    -> Run ArtifactInputStep
      -> atc/worker.Pool.LocateVolume() // find the artifact created earlier
      -> Register runtime.Artifact on build // basically just keep track of the fact that `some-input` points to that volume
    -> Run TaskStep
      -> Will give more details later, but this also keeps track of the output volume for `some-output`
    -> Run ArtifactOutputStep
      -> volume := atc/worker.Pool.LocateVolume() // find the volume that `some-output`, created in the task step, points to
      -> volume.InitializeArtifact() // mark in the DB that this is an artifact
  -> GET api/artifactserver/get.go
    -> volume := atc/worker.Pool.LocateVolume() // find the artifact volume created in ArtifactOutputStep
    -> volume.StreamOut() // download the contents via baggageclaim

The various delegates in engine/*_delegate.go use an abstraction called the worker.ArtifactSourcer. This abstraction was removed in the refactor. In effect, worker.ArtifactSourcer's only purpose was to convert from a type that the exec package understands (runtime.Artifact) to a type that the worker package understands (worker.ArtifactSource).

worker.ArtifactSource is an interface that simply tells you whether an artifact (volume) is on a worker. It was previously used to model both input volumes (which also implemented worker.StreamableArtifactSource, which allows for streaming the volume to another worker), as well as task caches (which are not streamable, and simply remain on the worker they were created on). These abstraction was removed in favour of runtime.Volume (which can stream in/out) and worker.Streamer (which knows how to stream data to/from runtime.Volumes, optionally using p2p streaming). Now, the exec package just references runtime.Volumes directly, so there's no need for a conversion layer like we had with worker.ArtifactSourcer.

All that to say the engine no longer depends on worker post-refactor (except for basic dependency injection, since the engine package threads the worker dependencies to the exec.*Steps). There is one exception to that, actually - engine/task_delegate.go implements a SelectWorker method which uses the worker.Pool to, well, select a worker! The difference between this method and worker.Pool.SelectWorker is that the latter doesn't retry when no worker is available, while the former does (it loops until a worker is available). Note that with #6635, all steps will share the same codepath (which exists on the worker.Pool itself), so we can get rid of engine.TaskDelegate.SelectWorker.

@aoldershaw saw this but forgot to respond. This is great. Thanks so much!

@muntac glad to hear it! still working on it, haven't gotten to the exec package interactions yet, but will update that comment soon