dailymotion-oss / octopilot

Automate your Gitops workflow, by automatically creating/merging GitHub Pull Requests
https://dailymotion-oss.github.io/octopilot/
MIT License
175 stars 23 forks source link

Octopilot fails to wait for Github checks to merge PR #268

Open jfoechsler opened 1 year ago

jfoechsler commented 1 year ago

Hello. Have issue merging PR where Required Checks are defined on repository. Octopilot seems to determine PR can be merged before letting checks do its thing.

$ octopilot --repo <myrepo> --update 'yaml(file=testing/ci/js-web.yaml,path='\''spec.chart.spec.version'\'')=>0-testing' --git-commit-title 'Debug CI/CD 3' --git-branch-prefix octopilot-update- --pr-merge true --pr-merge-method squash --pr-base-branch main --fail-on-error --log-level=debug --pr-merge-retry-count 10
DEBU[0000] Updaters ready                                updaters="[YAML[path=spec.chart.spec.version,file=testing/ci/js-web.yaml,style=,create=false,trim=false,indent=2]]"
DEBU[0000] Repositories ready                            repositories="[{<myrepo> map[]}]"
DEBU[0000] Using 'reset' strategy                        repository=<myrepo>
DEBU[0003] Git repository cloned                         git-reference=HEAD git-url="<myrepo>" local-path=/tmp/octopilot3413811827<myrepo>
DEBU[0003] No existing Pull Request found                labels="[octopilot-update]" repository=<myrepo>
DEBU[0003] Switched Git branch                           branch=octopilot-update-ckg174rel7e882q72gpg repository-name=<myrepo>
DEBU[0003] Updater finished                              changes=true repository=<myrepo> updater="YAML[path=spec.chart.spec.version,file=testing/ci/js-web.yaml,style=,create=false,trim=false,indent=2]"
DEBU[0003] All updaters finished                         repository=<myrepo>
DEBU[0003] Git status                                    repository-name=<myrepo> status=" M testing/ci/js-web.yaml\n"
DEBU[0003] Git commit                                    commit=dfc910b6a89f15dd93643f8e3cb91f38cca21554 repository-name=<myrepo>
DEBU[0005] Git changes pushed                            branch=not-octopilot-update-ckg174rel7e882q72gpg repository-name=<myrepo>
INFO[0006] New Pull Request created                      pull-request="https://github.com/<myrepo>/pull/496" repository=<myrepo>
DEBU[0008] Labels added to Pull Request                  pull-request="https://github.com/<myrepo>/pull/496" repository=<myrepo>
DEBU[0008] No comments to add to the Pull Request        pull-request="https://github.com/<myrepo>/pull/496" repository=<myrepo>
DEBU[0008] Pull Request is mergeable                     pull-request="https://github.com/<myrepo>/pull/496" repository=<myrepo>
DEBU[0009] Pull Request can be merged                    pull-request="https://github.com/<myrepo>/pull/496" repository=<myrepo>
ERRO[0010] Repository update failed                      error="failed to merge Pull Request https://github.com/<myrepo>/pull/496: failed to merge Pull Request https://github.com/<myrepo>/pull/496: PUT https://api.github.com/repos/<myrepo>/pulls/496/merge: 405 Required status check \"image-revision-tests\" is expected. []" repository=<myrepo>
INFO[0010] Updates finished                              repositories-count=1
FATA[0010] Some repository updates failed

Please advise if I may be doing something wrong or what. Thanks!

jfoechsler commented 1 year ago

Any plans to add support for GitHub auto-merge by the way? Maybe that could be nice addition to let GitHub merge PR when checks are done instead of doing it in code. I'm using that in some workflows like this: gh pr merge --squash --auto <PR>

jashandeep-sohi commented 12 months ago

I ran into similar problems with auto merging PRs. Maybe this will help others.

First, I had to give the Github App used by octopilot read-only admin permissions to solve auth issues. This is need because the call here leads to GET requests to repos/%v/%v/branches/%v/protection/required_status_checks. URLs of this format are listed under https://docs.github.com/en/rest/overview/permissions-required-for-github-apps?apiVersion=2022-11-28#repository-permissions-for-administration

Second, and this one was the most confusing to figure out: Github Workflows don't post "status" updates they post "checks" when completed/failed. Those are completely different APIs. You can get your Github Workflow to manually post a "status" as a work around. For example:

on:
  push:
    branches:
      - octopilot-*

permissions:
  contents: read
  statuses: write

jobs:
     ...

      - name: Post a status check
        uses: actions/github-script@v7
        with:
          script: |
            github.rest.repos.createCommitStatus({
              owner: context.repo.owner,
              repo: context.repo.repo,
              sha: context.sha,
              state: "success",
              context: "<check name>"
            });

Another thing I've observed is that Github Actions Workflows can't post status updates with the default token when triggered using on: pull_request. It does work with a on: push though :confused:

I think this project should switch to the Github provided auto-merge PR feature. It would also mean not having to give this app read-only admin permissions.

However, that's only available via GraphQL API AFAIK: https://docs.github.com/en/graphql/reference/mutations#enablepullrequestautomerge

vbehar commented 11 months ago

ok thanks for the details. I'll have a look at enabling auto-merge, and also using the checks API in addition to the status API.