chdsbd / kodiak

🔮 A bot to automatically update and merge GitHub PRs
https://kodiakhq.com
GNU Affero General Public License v3.0
1.03k stars 65 forks source link

Added New Required Github Action, Kodiak merged before complete #719

Closed aliwatters closed 2 years ago

aliwatters commented 2 years ago

Hi -- our kodiak installation is not waiting for "All Jobs Complete" (a required action) -- before merging.

It merged as soon as our label was added. (job completed later)

Screen Shot 2021-09-01 at 2 16 30 PM Screen Shot 2021-09-01 at 2 13 04 PM

When does kodiak look for new required actions?

chdsbd commented 2 years ago

Hi @aliwatters

GitHub allows the PR to merge because the "neutral" status check set by "All Jobs Complete" counts as a passing status check for your GitHub Branch Protection rule.

If you update "All Jobs Complete" to set an in-progress status check instead of a neutral check, your branch protection configuration should work correctly and Kodiak will wait for the status check to pass before merging.

Do you have a link to the "All Jobs Complete" action?

fabiendem commented 2 years ago

Hi @chdsbd Sorry what do you mean by "in-progress status check instead of a neutral check"? We can't find any ref to this in the docs.

"All Jobs complete" looks like:

done:
    runs-on: ubuntu-latest
    name: All Jobs Complete
    needs:
      - build
      - lint
      - setup
      - test
    steps:
      - run: echo "Hurrah!"
chdsbd commented 2 years ago

@fabiendem I just tried on a test repository with "All Jobs Completed" set as a required status check and couldn't reproduce the behavior described in this issue.

In my test case, GitHub blocked merging until "All Jobs Complete" had finished.

name: Example CI

on:
  push:
    branches: [ master ]
  pull_request:
    branches: [ master ]
    types: [opened, labeled, unlabeled, synchronize]

jobs:
  build:
    runs-on: ubuntu-latest
    steps:
      - run: echo "yes!!"
  test:
    runs-on: ubuntu-latest
    steps:
      - run: echo "yes!!"
  done:
      runs-on: ubuntu-latest
      name: All Jobs Complete
      needs:
        - build
        - test
      steps:
        - run: echo "Hurrah!"

When does kodiak look for new required actions?

GitHub will prevent Kodiak from merging a PR if there are missing, required actions.

fabiendem commented 2 years ago

Thanks. I think it's because the "All Jobs Complete" is skipped if one test step fails.

Screenshot 2021-09-02 at 17 50 37

We may need to prevent the cancellation of all steps if one fails.

aliwatters commented 2 years ago

right -- we need that final step to always run (and fail if any needs jobs fail) -- or is there a way we can configure kodiak to specifically look for a succes?

chdsbd commented 2 years ago

@aliwatters This config using https://github.com/technote-space/workflow-conclusion-action seems to work for me.

name: Example CI

on:
  push:
    branches: [ master ]
  pull_request:
    branches: [ master ]
    types: [opened, labeled, unlabeled, synchronize]

jobs:
  build:
    runs-on: ubuntu-latest
    steps:
      - run: echo "yes!!"
  test:
    runs-on: ubuntu-latest
    steps:
      - run: exit 1
  done:
    runs-on: ubuntu-latest
    name: All Jobs Complete
    needs:
      - build
      - test
    if: always()
    steps:
      - uses: technote-space/workflow-conclusion-action@v2
      - run: exit 1
        if: env.WORKFLOW_CONCLUSION == 'failure'

Kodiak depends on GitHub Branch Protection for disallowing merging a PR. If GitHub allows the PR to merge, Kodiak will merge it.

aliwatters commented 2 years ago

Thanks @chdsbd -- will try out that action

aliwatters commented 2 years ago

if: env.WORKFLOW_CONCLUSION == 'failure'

ultimately need (to account for skipped / canceled workflows)

if: env.WORKFLOW_CONCLUSION != 'success'

Thanks @chdsbd @fabiendem