Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Unable to run concurrent Danger instances on a single machine. #1311

Closed
squarefrog opened this issue Sep 16, 2022 · 5 comments
Closed
Labels

Comments

@squarefrog
Copy link
Contributor

squarefrog commented Sep 16, 2022

Describe the bug
We run Danger via Danger Swift, on a self-hosted Mac Mini using GitLab Runners. We have a single runner setup, with concurrent enabled.

Danger Swift passes in the DSL via URL, which DangerJS writes to the temporary directory:

  const sendDSLToSubprocess = () => {
    if (exec.options.passURLForDSL) {
      const resultsPath = join(tmpdir(), "danger-dsl.json")
      writeFileSync(resultsPath, dslJSONString, "utf8")
      const url = `danger://dsl/${resultsPath}`
      d(`Started passing in STDIN via the URL: ${url}`)
      child.stdin.write(url)
      child.stdin.end()
    } else {
      d(`Started passing in STDIN`)
      child.stdin.write(dslJSONString)
      child.stdin.end()
    }
    d(`Passed DSL in via STDIN`)
  }

However, this is problematic as this results in a URL like the following, which both processes try to read/write from at the same time:

danger://dsl//var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger-dsl.json

Instead we should probably suffix a unique directory, for example:

danger://dsl//var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger-js/230ebef4-35ce-11ed-a261-0242ac120002/danger-dsl.json

To Reproduce
Steps to reproduce the behavior:

  1. Setup an environment that lets you test locally
  2. In one Terminal window, kick off a Danger run
  3. In another Terminal window, kick off another Danger run

Expected behavior
The temporary DSL file should be placed in a unique directory, so that one process wont cleanup the DSL while another is using it.

Your Environment

software version
danger.js 11.1.2
node N/A
npm N/A
Operating System macOS 12.6

Additional context
I submitted a similar fix for danger-swift, where we use a UUID for a folder name. This works well in my testing. If you're happy with this approach, I'm happy to try create a PR for this change. I'm unaware if there are any complications for doing this though. Checking the source code there are two places where tmpdir() is used, and this issue could potentially occur:

The disadvantage to this approach is we're then responsible for cleaning up this working directory once the process has finished.

@squarefrog squarefrog added the bug label Sep 16, 2022
@squarefrog
Copy link
Contributor Author

For some additional context, here are some snippets from our pipelines:

This one failed:

2022-09-16T13:26:35.491Z danger:process_runner Preparing to run: .build/debug/danger-swift,runner,/snapshot/danger-js/distribution/commands/danger-ci.js,--process,.build/debug/danger-swift,--passURLForDSL,-i,danger_ci,-f,--cwd,../
2022-09-16T13:26:35.491Z danger:runDangerSubprocess Running sub-process: danger-swift - runner,/snapshot/danger-js/distribution/commands/danger-ci.js,--process,.build/debug/danger-swift,--passURLForDSL,-i,danger_ci,-f,--cwd,../
2022-09-16T13:26:35.500Z danger:runDangerSubprocess Started passing in STDIN via the URL: danger://dsl//var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger-dsl.json
2022-09-16T13:26:35.500Z danger:runDangerSubprocess Passed DSL in via STDIN
Ran with: /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger/8B52C17F-7357-4BF2-BC85-5FB55F2448AC/_tmp_dangerfile.swift runner /snapshot/danger-js/distribution/commands/danger-ci.js --process .build/debug/danger-swift --passURLForDSL -i danger_ci -f --cwd ../ /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger-dsl.json /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger/8B52C17F-7357-4BF2-BC85-5FB55F2448AC/danger-response.json
ERROR: could not find DSL JSON at path: /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger-dsl.json
Launching Danger Swift runner (v3.14.0)
Got URL for JSON: /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger-dsl.json
Created a temporary file for the Dangerfile DSL at: /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger-dsl.json
Running Dangerfile at: Dangerfile.swift
Preparing to compile
Running: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift --driver-mode=swift -L /private/tmp/gitlab-runner-builds/245930/Danger/.build/debug -I /private/tmp/gitlab-runner-builds/245930/Danger/.build/debug -lDangerDeps /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger/8B52C17F-7357-4BF2-BC85-5FB55F2448AC/_tmp_dangerfile.swift runner /snapshot/danger-js/distribution/commands/danger-ci.js --process .build/debug/danger-swift --passURLForDSL -i danger_ci -f --cwd ../ /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger-dsl.json /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger/8B52C17F-7357-4BF2-BC85-5FB55F2448AC/danger-response.json
Completed evaluation
ERROR: Dangerfile eval failed at Dangerfile.swift
ERROR: Could not get the results JSON file at /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger/8B52C17F-7357-4BF2-BC85-5FB55F2448AC/danger-response.json

This one worked fine, then presumably cleaned up the file, which broke the other:

2022-09-16T13:26:35.135Z danger:process_runner Preparing to run: .build/debug/danger-swift,runner,/snapshot/danger-js/distribution/commands/danger-ci.js,--process,.build/debug/danger-swift,--passURLForDSL,-i,danger_ci,-f,--cwd,../
2022-09-16T13:26:35.135Z danger:runDangerSubprocess Running sub-process: danger-swift - runner,/snapshot/danger-js/distribution/commands/danger-ci.js,--process,.build/debug/danger-swift,--passURLForDSL,-i,danger_ci,-f,--cwd,../
2022-09-16T13:26:35.162Z danger:runDangerSubprocess Started passing in STDIN via the URL: danger://dsl//var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger-dsl.json
2022-09-16T13:26:35.162Z danger:runDangerSubprocess Passed DSL in via STDIN
Ran with: /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger/C31A4E15-B2D0-4622-AA41-EC29822D0386/_tmp_dangerfile.swift runner /snapshot/danger-js/distribution/commands/danger-ci.js --process .build/debug/danger-swift --passURLForDSL -i danger_ci -f --cwd ../ /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger-dsl.json /var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger/C31A4E15-B2D0-4622-AA41-EC29822D0386/danger-response.json
Decoding the DSL into Swift types
Setting up to dump results
Sending results back to Danger
Sending results back to Danger
2022-09-16T13:26:59.330Z danger:runDangerSubprocess Got JSON URL from STDOUT, results are at: 
danger-results://var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger/C31A4E15-B2D0-4622-AA41-EC29822D0386/danger-response.json
Launching Danger Swift runner (v3.14.0)

Notice theres a URL which is identical in both runs: danger://dsl//var/folders/0h/c4gthkj13sz3hb1z4fwpnd3c0000gp/T/danger-dsl.json

@orta
Copy link
Member

orta commented Sep 17, 2022

Seems reasonable to me to have the danger-dsl JSON file name be randomized then I guess, I think it was probably only made consistent so that it was easy to look in and verify

@squarefrog
Copy link
Contributor Author

Would a good compromise be something like:

danger-dsl-[unique token].json

Doing some research last night, it seems we’d have to add a new dependency to generate a UUID, which I’m not thrilled about.

Is there another approach to get a unique enough string without external deps?

@orta
Copy link
Member

orta commented Sep 17, 2022

Yeah, the crypto module can get you there:

import  {randomBytes}  from "crypto"

const randomString1 = randomBytes(4).toString('hex');

@squarefrog
Copy link
Contributor Author

Amazing. I’ll do a PR this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants