Drone

Docker-runner: DRONE_RUNNER_MAX_PROCS changes behaviour of job execution

when DRONE_RUNNER_MAX_PROCS is set queued jobs parallel jobs get canceled if a previous step fails, IMO the remaining jobs should continue to be executed just like if DRONE_RUNNER_MAX_PROCS was never set in the first place.

I don’t think it’s unreasonable to guess that running different kinds of linters/tests in parallell is a common use case and that you want all linters to finish before someone looks at the runner output.

Example where this is a problem:

all test_step’s should run before html regardless if some of them fails because html will render an html report of all tests and then the whole pipeline should fail.

There are maybe some work arounds but It feels strange having to go through all builds that uses a runner and change them just because I changed this setting in the runner.

"drone ci"


def main(ctx):
    return [
        default_pipeline(),
    ]


def default_pipeline():
    test_steps = []
    for t in tests:
        test_steps.append(test_step(t))

    steps = (
        [
            step(
                "install",
                "cypress/included:3.8.2",
                commands=["npm install --production",],
            )
        ]
        + test_steps
        + [
            step(
                "html",
                "node:10",
                commands=[
                    "npm install -g xunit-viewer",
                    "xunit-viewer -o results/results.html -r results/",
                ],
                depends_on=test_names(),
                when={"status": ["success", "failure"]},
            )
        ]
    )
    return pipeline("default", steps=steps, concurrency={"limit": 3},)


def test_step(filename):
    return step(
        test_name(filename),
        "cypress/included:3.8.2",
        commands=[
            "cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './{}'".format(
                filename
            ),
        ],
        environment={"PROPERTIES": "spec:{}".format(filename),},
        depends_on=["install"],
        # failure="ignore",
    )


def step(
    name,
    image,
    commands=[],
    volumes=[],
    depends_on=[],
    environment={},
    settings={},
    when={},
    failure="",
):
    return {
        "name": name,
        "image": image,
        "settings": settings,
        "environment": environment,
        "commands": commands,
        "depends_on": depends_on,
        "volumes": volumes,
        "when": when,
        "failure": failure,
    }


def pipeline(
    name, steps=[], services=[], volumes=[], trigger={}, concurrency={}, depends_on=[]
):
    return {
        "kind": "pipeline",
        "depends_on": depends_on,
        "name": name,
        "type": "docker",
        "platform": {"os": "linux", "arch": "amd64",},
        "concurrency": concurrency,
        "steps": steps,
        "volumes": volumes,
        "services": services,
        "trigger": trigger,
    }


def test_names():
    names = []
    for name in tests:
        names.append(test_name(name))
    return names


def remove_prefix(s, prefix):
    if s.startswith(prefix):
        return s[len(prefix) :]
    return s


def test_name(name):
    return remove_prefix(name, "tests/")


tests = [
    "tests/foo/a_spec.js",
    "tests/foo/b_spec.js",
    "tests/foo/c_spec.js",
    "tests/foo/d_spec.js",
    "tests/foo/e_spec.js",
    "tests/foo/f_spec.js",
    "tests/foo/g_spec.js",
    "tests/foo/h_spec.js",
    "tests/foo/i_spec.js",
    "tests/foo/j_spec.js",
    "tests/foo/1_spec.js",
    "tests/foo/2_spec.js",
    "tests/foo/3_spec.js",
    "tests/foo/4_spec.js",
    "tests/foo/5_spec.js",
    "tests/foo/6_spec.js",
    "tests/foo/7_spec.js",
    "tests/foo/8_spec.js",
    "tests/foo/9_spec.js",
    "tests/foo/10_spec.js",
    "tests/foo/11_spec.js",
    "tests/foo/12_spec.js",
    "tests/foo/13_spec.js",
    "tests/foo/14_spec.js",
    "tests/foo/15_spec.js",
    "tests/foo/16_spec.js",
    "tests/foo/17_spec.js",
    "tests/foo/18_spec.js",
    "tests/foo/19_spec.js",

]

incidentally this was the job group that prompted me to add DRONE_RUNNER_MAX_PROCS in the first place to not fire up 30 chrome headless instances at once. Now I’m just worried about how this might affect all the other pipelines with parallel jobs that are not tuned for this behaviour.

And the rendered .drone.yml from the starlark script:

---
kind: pipeline
type: docker
name: default

platform:
  os: linux
  arch: amd64

concurrency:
  limit: 3

steps:
- name: install
  image: cypress/included:3.8.2
  commands:
  - npm install --production

- name: foo/a_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/a_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/a_spec.js
  depends_on:
  - install

- name: foo/b_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/b_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/b_spec.js
  depends_on:
  - install

- name: foo/c_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/c_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/c_spec.js
  depends_on:
  - install

- name: foo/d_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/d_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/d_spec.js
  depends_on:
  - install

- name: foo/e_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/e_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/e_spec.js
  depends_on:
  - install

- name: foo/f_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/f_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/f_spec.js
  depends_on:
  - install

- name: foo/g_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/g_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/g_spec.js
  depends_on:
  - install

- name: foo/h_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/h_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/h_spec.js
  depends_on:
  - install

- name: foo/i_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/i_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/i_spec.js
  depends_on:
  - install

- name: foo/j_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/j_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/j_spec.js
  depends_on:
  - install

- name: foo/1_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/1_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/1_spec.js
  depends_on:
  - install

- name: foo/2_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/2_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/2_spec.js
  depends_on:
  - install

- name: foo/3_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/3_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/3_spec.js
  depends_on:
  - install

- name: foo/4_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/4_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/4_spec.js
  depends_on:
  - install

- name: foo/5_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/5_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/5_spec.js
  depends_on:
  - install

- name: foo/6_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/6_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/6_spec.js
  depends_on:
  - install

- name: foo/7_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/7_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/7_spec.js
  depends_on:
  - install

- name: foo/8_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/8_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/8_spec.js
  depends_on:
  - install

- name: foo/9_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/9_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/9_spec.js
  depends_on:
  - install

- name: foo/10_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/10_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/10_spec.js
  depends_on:
  - install

- name: foo/11_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/11_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/11_spec.js
  depends_on:
  - install

- name: foo/12_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/12_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/12_spec.js
  depends_on:
  - install

- name: foo/13_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/13_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/13_spec.js
  depends_on:
  - install

- name: foo/14_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/14_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/14_spec.js
  depends_on:
  - install

- name: foo/15_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/15_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/15_spec.js
  depends_on:
  - install

- name: foo/16_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/16_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/16_spec.js
  depends_on:
  - install

- name: foo/17_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/17_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/17_spec.js
  depends_on:
  - install

- name: foo/18_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/18_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/18_spec.js
  depends_on:
  - install

- name: foo/19_spec.js
  image: cypress/included:3.8.2
  commands:
  - cypress run --barr chrome --headless  --reporter junit --reporter-options 'mochaFile=results/junit-[hash].xml' --spec './tests/foo/19_spec.js'
  environment:
    PROPERTIES: spec:tests/foo/19_spec.js
  depends_on:
  - install

- name: html
  image: node:10
  commands:
  - npm install -g xunit-viewer
  - xunit-viewer -o results/results.html -r results/
  when:
    status:
    - success
    - failure
  depends_on:
  - foo/a_spec.js
  - foo/b_spec.js
  - foo/c_spec.js
  - foo/d_spec.js
  - foo/e_spec.js
  - foo/f_spec.js
  - foo/g_spec.js
  - foo/h_spec.js
  - foo/i_spec.js
  - foo/j_spec.js
  - foo/1_spec.js
  - foo/2_spec.js
  - foo/3_spec.js
  - foo/4_spec.js
  - foo/5_spec.js
  - foo/6_spec.js
  - foo/7_spec.js
  - foo/8_spec.js
  - foo/9_spec.js
  - foo/10_spec.js
  - foo/11_spec.js
  - foo/12_spec.js
  - foo/13_spec.js
  - foo/14_spec.js
  - foo/15_spec.js
  - foo/16_spec.js
  - foo/17_spec.js
  - foo/18_spec.js
  - foo/19_spec.js

this seems odd because the max procs variable is only used to limit parallel steps through a semaphore. It works like this [1]:

	if e.sem != nil {
		// the semaphore limits the number of steps that can run
		// concurrently. acquire the semaphore and release when
		// the pipeline completes.
		if err := e.sem.Acquire(ctx, 1); err != nil {
			return nil
		}

		defer func() {
			// recover from a panic to ensure the semaphore is
			// released to prevent deadlock. we do not expect a
			// panic, however, we are being overly cautious.
			if r := recover(); r != nil {
				// TODO(bradrydzewski) log the panic.
			}
			// release the semaphore
			e.sem.Release(1)
		}()
	}

The semaphore code is pretty simple and it does not look at the pipeline status of previous steps, so it is not immediately clear how the two would be related.

[1] https://github.com/drone/runner-go/blob/master/pipeline/runtime/execer.go#L127:L145

It’s probably easy to create a repository demonstrating the issue, when the first job fails all pending jobs with the same depends_on target are canceled.