1602960 - [worker-runner] more flexible handling of configuration for workers

Status: RESOLVED FIXED

(Taskcluster :: Workers, enhancement)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

We need to rationalize the handling of config between docker-worker and generic-worker. Right now, docker-worker worker-pools have the form

  ...
  workerConfig: {
    someConfigParam: value,
    ...
  }

while generic-worker pools have the form

  ...
  workerConfig: {
    genericWorker: {
      config: {
        someConfigParam: value,
        ...
      }
    }
  }

In bug 1592844, pete made some good arguments for keeping the nested form: it would allow us to configure other things on the instance, such as worker-runner. It also allows specifying files (as a peer to config).

Both workers take their config files without this nesting. So, it falls to worker-runner to flatten.

We had a similar issue with secrets: docker-worker just read its config out of the secret object, while generic-worker read from config and file properties. We consider the latter form to be the "correct" form for a secret, while supporting the former for compatibility.

So, let's do something similar with workerConfig. The only tricky bit is that the identifier genericWorker appears in the files at a place where it really shouldn't matter what worker implementation is involved. So let's call that worker, with genericWorker being a compatibility alias for it. Here goes:

In a worker-pool launchspec, given a workerConfig property value v:

• if it has the form {genericWorker: x}, it is rewritten to {worker: x}
• if it does not have the form {worker: x} is rewritten to {worker: {config: v}}
  Both of these initial, non-nested forms will be considered deprecated and after deployment of worker-runner everywhere (bug 1602946) we will update all configs remove support for the old forms.

Worker-runner providers will then merge the value of workerConfig.worker.config to the config that they pass on to the workers, and will additionally apply workerConfig.worker.files as they already do with files in secrets.

Pete, does this sound OK?

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

I have this about half-baked. Pete, if this doesn't sound OK please let me know.

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: hmm, pie

I finished baking it: https://github.com/taskcluster/taskcluster-worker-runner/pull/78

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

That's kind of lame, bugzilla..

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

I'd like to get some other PRs merged and then release a new tc-worker-runner.

Comment 5

[Pete Moore [:pmoore][:pete]]

In the end dustin and I discussed this in the PR directly.

Nice work!

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

Those are both merged, and this will be out in the next version of tc-worker-runner, 1.0.0.