[wontfix] DRONE_RUNNER_MAX_PROCS changes behaviour of job execution

Drone Bugs
1

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
2

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

3

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.

image
image237×1013 33.2 KB

4

if I am understanding correctly, the behavior described in the linked thread is the expected behavior. If the pipeline is in a failure state, subsequent steps are skipped unless they are configured to run on failure. If you limit max procs and the step has not started yet due to concurrency limits, and the pipeline is in a failure state by the time it starts, it is skipped. The logic is consistently applied. I am happy to discuss further, but let’s move back to the original thread to keep this thread focused on pipeline steps having a passing status.

@bradrydzewski

The problem is obviously that the reason these steps are seen as subsequent is only a technicality of how a specific runner is configured. It is only a subsequent step from the viewpoint of internal runner scheduling.

According to the DAG they are all parallel steps and should maybe execute according to that graph.

If nothing else this could break lots of builds in large installations with many parallel steps just by changing the max procs setting.

5

The system guarantees a pipeline step does not execute if the pipeline is in a failing state. This logic is simple and easy to understand and is consistently applied, regardless of sequential or graph ordering. I understand the point you are making, however, I respectfully disagree that this behavior should be changed.

6