Help authors handle concurrent GitHub+You™ tests

[plocket]

Problem

Currently GitHub+You™ tests upload a branch of a repo to a da server. This can cause the server to slow down or, in the case of packages with modules, reload. If multiple GitHub+You™ tests run at the same time, they may interfere with each other, causing tests to fail due to race conditions (an unhelpful result). [We need to prevent overlapping runs of a workflow.]

Ideal behavior imo

When I accidentally push in quick succession (to the same branch on the same repository), I want the old GitHub+You™ workflow to get cancelled and the new workflow to run instead. When I (or others in my org) push to a different branch on the same repository or to different repositories I'd like those GitHub+You™ workflows to queue to run their tests later.

To get all of these features, I believe we'd need a custom solution. Description of such is near the bottom and we should discuss that first before starting research.

Partial solution

@michaelhofrichter is using the concurrency feature of GitHub workflows in his workflow at the workflow level (as opposed to the job level) to avoid this issue. Or to at least mitigate it:

# Avoid running/triggering multiple workflows at a time (in one repository). Instead,
# cancel old test runs in favor of new runs.
concurrency:
  # This "group" value only works for workflows running on the same branch because
  # it uses github.ref, the branch "name". If you work on different branches, one
  # workflow will run on each branch. Also, it would allow workflows with different names
  # to run, which would break the tests of at least one current author.
  group: ${{ github.workflow }}-${{ github.ref }}-ALKiln
  # Deletes previously running workflows. Note: can't have pending workflows _and_ cancelling
  cancel-in-progress: true

I would recommend a slightly different version (though still not ideal) for folks who are developing on multiple branches and multiple workflows:

# Avoid running/triggering multiple workflows at a time (in one repository). Instead,
# cancel old test runs in favor of new runs.
concurrency:
  # Previously running GitHub+You™ tests on other branches get cancelled too.
  # It does cancel tests that I may indeed want to run, but avoids unhelpfully
  # failing tests.
  group: ALKiln
  # Deletes previously running workflows. Note: can't have pending workflows _and_ cancelling
  cancel-in-progress: true

We might be able to add this to the ALKiln composite action or to provide this in ALKlin documentation or in the ALKiln setup form.

Research needed for global GitHub concurrency

Avoid research until we've discussed the proposed custom solution.

Can we somehow implement this globally across the repos for a whole organization, or, even better, for tests running on a specific server, along with some queue handling for unique tests?

It seems likely the current code would be unable to control workflows globally. That is, I believe an org can't use this for all workflows across that organization. Docs muddy the water on that conclusion by referring to organizations and enterprises as a whole. See https://docs.github.com/en/actions/concepts/workflows-and-actions/concurrency.

concurrency does allow using evaluation for at least some values, as we can see above, and can use input and vars (though not GitHub secrets). I'm not sure this can help with cancelling old test runs, but dynamic values brings up more possibilities if we flip the script a little:

Dynamically preventing new tests

Though preventing new tests would be less useful for accidental pushes to the same branch, it would be useful when testing a different branch or repo to allow old tests to finish first. This may be useful within an organization that has multiple active developers. Potentially, these could be used at the job-level so that the workflow could then also tell the developer, via another job in the same workflow, why the test suite didn't run.

• Could ALKiln's composite action use concurrency? If so, can it use inputs or vars to determine whether a new action/workflow can run? Things to test:
  • concurrency in ALKiln's composite action with a hard-coded value
    • Valid YAML composite action code?
    • Effective within one repo?
  • group: ${{ inputs.SERVER_URL }} effective within one repo?
  • group: ${{ inputs.SERVER_URL }} effective within all repos of one org or user?
  • group: ${{ vars.SERVER_URL }} effective within one repo?
  • group: ${{ vars.SERVER_URL }} effective within all repos of one org or user?
• If not from ALKiln's composite action, could an organization do this using its own vars? To test:
  • group: ${{ vars.SERVER_URL }} effective within one repo?
  • group: ${{ vars.SERVER_URL }} effective within all repos of one org or user?
• If not either of those, could ALKiln or an org use flags that the org sets dynamically. That would be vulnerable to race conditions, but may be sufficient for most orgs' needs. For example, use a var value as a flag (vars.ALKILN_ALREADY_RUNNING). When a workflow starts, it's first move will be to set vars.ALKILN_ALREADY_RUNNING to true, thus blocking other tests from running. To test:
  • group: ${{ vars.ALKILN_ALREADY_RUNNING }} effective within one repo?
  • group: ${{ vars.ALKILN_ALREADY_RUNNING }} effective within all repos of one org or user?

Dynamically pending new tests

I believe 'pending' could only work

The values of pending (single and max) imply that changing that value mid-stream (if even possible) may cancel old pending runs. That particular issue may only be a problem per-repository.

• Can we set the value of pending dynamically?
• If so, does that

Both

Could we prevent new test runs on the workflow level and then use dynamicly pending values on the job level? Or visa versa? In combination, could that create a queue of the kind we want? Haven't thought that one through yet.

Our own solution

We might be able to implement the ideal behavior ourselves if a workflow or composite action can change org vars. This would let us or authors could chose to cancel old tests or to prevent new tests.

I believe we could develop a new job for ALKiln's composite action, or offer code for authors' workflows for the same, that checks the org flags and cancels (new or old) or sleeps if appropriate.

If every workflow just waits a hard-coded amount of time (e.g. 5 seconds) we're very likely to run into race conditions We would also need to set specific timing for each workflow/job to prevent multiple workflows from checking at the same time to reduce the chance of race conditions. For example, vars.ALKILN_QUEUE where each test would put its ID, run on its turn in the queue, and then would remove its ID when it was done (regardless of success or failure).

Preventing new tests seems simple enough. I'm not sure how we could cancel old tests. If we can, though, I presume we would identify the old tests within the repository using the repo's workflow and branch name.

GitHub concurrency docs

• An overview of GitHub concurrency
• A second overview of GitHub concurrency
• Workflow concurrency specifics

[michaelhofrichter]

Note that the cancelled tests might already have created projects on the docassemble server that may not get deleted by the cancellation of the workflow. Paying attention for cleanup may be a concern.

Consider the situation where you make a push and the tests auto start. Two minutes later, you realize a typo and follow up with another commit. In those two minutes, Github's actions saw the first push and loaded the package into a playground on your server. The ALKiln tests start running, but your second commit cancels the existing workflow, preventing the cleanup steps from happening.

[plocket]

It's a good point and deserves more research. I'd like to see whether GitHub does interrupt even the test cleanup that way. It does seem likely [simply because things can fall through the cracks sometimes]!

[plocket]

From the GitHub documentation I found, a workflow running ALKiln should be able to get through the cancellation process in a timely manner that will allow ALKiln to delete a Project from a server. [As always,] The Project deletion process should only fail if the server is inaccessible during the Project deletion window. For example, if a server reload is going on and takes too long to finish during the time ALKiln tries to delete the Project, ALKiln will be unable to delete the Project it created.

I think it's still worth taking the time to confirm this with an experiment, though.