[v2] refactor step plugin api for backwards compat

[mimir-d]

Currently, test step plugins are required to have the following interface:

Load() (
    string, // plugin identifier
    test.TestStepFactory, // instance factory: func() test.TestStep
    []event.name // events allowed to be emitted
)

Run(
    ctx xcontext.Context, // cancellation context
    ch test.TestStepChannels, // input, output chans
    ev testevent.Emitter, // event emitter interface
    stepsVars test.StepsVariables, // step output variables accessor
    params test.TestStepParameters, // map[string] -> list of Param (raw json)
    resumeState json.RawMessage, // input serialized plugin state, when resuming
) (
    json.RawMessage, // output serialized plugin state, when pausing
    error
)

ValidateParameters(
    ctx xcontext.Context,
    params test.TestStepParameters // map[string] -> list of Param (raw json)
) error

// required but unused
Name() string

Most also define (but not required by contest server):

New() test.TestStep // passed in test.TestStepFactory in Load()

The lifecycle of a plugin is: instance is created before main loop and put into a "step bundle" object (owned by test, and then job), that gets carried around until it reaches StepRunner (field of StepState, should also be removed). Step runner calls Run() once (per job, new TestRunner made in JobRunner), and leaves it running in a goroutine. Step runner is stopped in stepState.DecreaseLeftTargets, when number of targets remaining to be injected into a step plugin is 0 (note, this whole thing needs refactoring, but that's for another tracking issue).

Issue 1: The validate params method is usually implemented by another method in plugins, that also gets called at the beginning of Run(), resulting in a double parsing of the input json.
Instead, the plugin instance should be valid by construction. This implies removal of the ValidateParameters method, and plugin config structs should be created along with the plugin instance. Config param interpolation can be done in Run() on a per target basis.

Issue 2: The Run method requires a number of dependencies from inside the server core, and this list may increase later (as was the case with StepsVariables). These dependencies may also depend on other objects, which will in effect explode the signature of Run.
Instead, an interface should be presented to the plugin (we dont require ABI compat) which contains getter methods for these dependencies (called services here).
Proposal

type PluginServices interface {
    EventEmitter() testevent.Emitter
    StepsVariables() test.StepsVariables
    Logger() xcontext.Logger // with any tags needed
    // maybe Metrics() xcontext.Metrics
    // maybe Tracer() xcontext.Tracer
    // maybe something to provide state pause/resume services
    // other
}

// note that xcontext.Context is also removed, for the standard golang Context
Run(
    ctx context.Context,
    chans test.TestStepChannels,
    params test.TestStepParameters,
    svc PluginServices, // new
    resumeState json.RawMessage,
) (json.RawMessage, error)

Issue 3: The plugin currently accepts both a target input chan and an output one. It is generally advised that chans are created by the code that knows most about said channel. In this case, the input chan should be created by caller, but the output should be created by the plugin.
Proposal (provisional, may instead require Run to launch async itself and return an output chan)

Run(
    ctx xcontext.Context,
    input <-chan *target.Target,
    // ...
) (json.RawMessage, error)

Output() chan<- step.TestStepResult

[rihter007]

Good idea!