Skip to content

Draft: Stop exporting additional abstract executor Prepare*() functions

Arran Walker requested to merge ajwalker/simplify-abstract-executor into main

What does this MR do?

Unexports AbstractExecutor's PrepareConfiguration and PrepareBuildAndShell. Modifies Prepare() to accept an additional setup function as its second argument.

Why was this MR needed?

We have the following situation:

Prepare is called by kubernetes, parallels, shell, ssh, virtualbox.

Prepare is NOT called by custom and docker. These instead call PrepareConfiguration and PrepareBuildAndShell, which is exactly what is called by Prepare, but instead use these because they provide additional setup between the two.

This reduces the exported API and instead allows a setup function to be passed to Prepare().


ALTERNATIVE SOLUTION

An alternative solution would be to remove Prepare() entirely, and update each executor to always call PrepareConfiguration and PrepareBuildAndShell.

I have no real preference, but have code that would prefer if the preparation calls had some consistency across executors.

What's the best way to test this MR?

Existing integration tests should cover this. Manually testing any job in the Docker or Custom executor should also cover this.

What are the relevant issue numbers?

#27635

Edited by Elliot Rushton

Merge request reports