Merge pull request #10 from scottwittenburg/document-experiments

Update with description of recent experiments
spack · Jan 12, 2023 · a0d580f · a0d580f
2 parents b8f3ce9 + 0fa0e3c
commit a0d580f
Show file tree

Hide file tree

Showing 3 changed files with 169 additions and 1 deletion.
diff --git a/CI.md b/CI.md
@@ -1,3 +1,12 @@
 # Using merge queue in spack CI
 
-This document describes how we might use the merge queue feature in spack CI.
+This document describes how we might use the merge queue feature in spack CI.  There are two ways we can consider taking advantage of the merge queue functionality for testing in spack, using the existing `gh-gl-sync` script, or moving to a fully event-driven system using spackbot or some other event handling system.
+
+## Using the `gh-gl-sync` cron job
+
+
+
+## Using an event-based approach
+
+The events we receive from github are documented in the [NOTES.md](./NOTES.md), with links to actual example events received.
+
diff --git a/NOTES.md b/NOTES.md
@@ -138,3 +138,88 @@ Date:   Wed Jan 11 18:40:37 2023 +0000
 
     move notes to another file
 ```
+
+### With repo public and specific check required
+
+Things behaved differently once the repo was made public (github would enforce the branch protections) and once spackbot-test had posted status on a commit.  With the posted status, we can choose it in the branch protection settings as a required status, and then github waits for that status (using yellow color to indicate pending status, "merge when ready" button is black, not green, though it seems I could still click it).
+
+When these two things are set, we started getting a new event from github (`merge_group / checks_requested`, see [example](./events/merge_group_checks_requested.json)).
+
+Also, at this point, we enabled a new PR comment message:
+
+```
+@spackbot-test check <success|failure> <sha>
+```
+
+which allows to pass or fail any commit at a time of our choosing.  This is convenient to make sure we had time to inspect the state of the repo by fetching branches and looking at logs, examing the github ui, etc.  We can also choose to pass/fail items in the queue in a specific order to see how github responds.  Here is an experiment we conducted once we had the capabilities outlined in this section.
+
+With everything above in place, I posted a success status on the latest commit of an open PR, which turned the "merge when ready" button green, so I pushed it and confirmed.
+
+- This triggered a new type of event from github, as mentioned above, a `merge_group` event with action set to `checks_requested`.
+    _ the event object had a key `merge_group` with fields including `base_sha` as described in this [doc](https://github.blog/changelog/2022-08-18-merge-group-webhook-event-and-github-actions-workflow-trigger).  This `base_sha` was `fe863b8ab7a820b363bfdccf002ae1dc7925a598`.
+
+- Fetching from the repo at this point resulted in a new branch `gh-readonly-queue/main/pr-6-1b1aaaec8784d524335dc225a4e21030941424a5`:
+
+    $ git log -n 5 upstream/gh-readonly-queue/main/pr-6-1b1aaaec8784d524335dc225a4e21030941424a5
+    commit fe863b8ab7a820b363bfdccf002ae1dc7925a598 (upstream/gh-readonly-queue/main/pr-6-1b1aaaec8784d524335dc225a4e21030941424a5)
+    Merge: 1b1aaae 00cc0e8
+    Author: Scott Wittenburg <[email protected]>
+    Date:   Thu Jan 12 16:45:20 2023 +0000
+
+        Merge pull request #6 from scottwittenburg/add-ci-proposal-doc
+
+        Add a CI document describing how we can use merge queue
+
+- Here is the order of events received after clicking "Merge when ready":
+
+    - merge_queue_entry / created
+    - pull_request / enqueued
+    - merge_group / checks_requested
+
+- Once I asked @spackbot-test to post succes on `fe863b8ab7a820b363bfdccf002ae1dc7925a598`, the PR was automatically merged.  This corresponded with receiving the following events:
+
+    - pull_request / dequeued
+    - pull_request / closed
+    - merge_queue_entry / deleted
+
+### Experimenting with several PRs in the queue
+
+For this experiment, we create three PRs (#7, #8, and #9) with passed tests (thanks to @spackbot-test check success <sha>) ready to add to merge queue.  Because we had set the merge group size to `small` (corresponding to 2 PRs per group), we didn't expect to be able to add all three PRs to the merge queue, but it was allowed anyway.
+
+The three PRs in were in the queue in the following order, from highest/first to lowest/last:
+
+1. PR #7 (GH branch included only #7 merged into base)
+2. PR #8 (GH branch included the merge commit from (1), above, as its base)
+3. PR #9 (GH branch included the merge commit from (2), above, as its base, so it included changes from #7 and #8)
+
+First test was to ask @spackbot-test to fail the middle one in the queue (2), and after 5-10 min, nothing had been removed from the queue.  While that is not what this [doc](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue) says will happend, it may just be the doc ommitted to mention action is only taken when status is reported on the first item in the queue.
+
+For the next test, we posted success on the third PR in the queue, and github seemed to do nothing with that information.  But some details to note:
+
+    - the success status is not even reflected on the merge queue page like the failure status is for the middle one
+    - if you go the branches UI, however, you can see the green check on that one
+
+So next we posted failure on the first PR in the queue, and this caused github to take the following actions:
+
+    - PR 7 was indeed removed from the merge queue
+    - the merge queue contains only PRs 8 and 9 (8 is ahead, so 9 includes 8, as expected)
+    - the old branches for 7,8 and 7,8,9 are still in the branches page with their status
+    - the old branch for just 7 is deleted
+    - there are new branches listed for 8 and 8,9
+        - note these are not the same branch names, just force-pushed, as the branch name includes the base sha
+        - e.g. before, the branch for 7 had base sha "sha1" and the branch for 7,8 assumed a base sha w/ 7 having been merged, so when 7 is remove, the new branch for just 8 has base sha "sha1"
+        - this means just pushing the new branches to gitlab won't cancel and previous pipelines
+    - received three events from github:
+        - `merge_queue_entry` / `deleted`
+        - `merge_group` / `checks_requested` (`merge_group` -> `head_sha` = `ed904b3e30246aeca6c3745607632c6272e94bd2`)
+            - corresponds to the new branch for just 8
+        - `merge_group` / `checks_requested` (`merge_group` -> `head_sha` = `a34c222dbcfff335ebfe4845300221b517853d72`)
+            - corresponds to the new branch for 8,9
+
+To complete this experiment we had @spackbot-test pass both of the new merge_group checks.  First we passed the check for PR 9 (which included the changes from both 8,9).  This didn't cause github to react in any way, and after nothing happened for awhile, we passed the check for just 8 (the first PR in the merge queue).  This caused the following things to happen:
+
+    - the merge queue became empty, with both PRs (8 and 9) getting merged
+    - both of the of the (new) readonly merge queue branches had been deleted
+    - received the expected github events associated with merging a PR (mentioned above)
+
+One thing to note is that after this experiment, the branches associated with PRs that had to be re-added to the queue when something ahead of them failed seem to just be orphaned.  They're still sitting on github right now.
diff --git a/events/installation_new_permissions_accepted.json b/events/installation_new_permissions_accepted.json
@@ -0,0 +1,74 @@
+{
+    "action": "new_permissions_accepted",
+    "installation": {
+        "id": 31802092,
+        "account": {
+            "login": "spack",
+            "id": 25539161,
+            "node_id": "MDEyOk9yZ2FuaXphdGlvbjI1NTM5MTYx",
+            "avatar_url": "https://avatars.githubusercontent.com/u/25539161?v=4",
+            "gravatar_id": "",
+            "url": "https://api.github.com/users/spack",
+            "html_url": "https://github.com/spack",
+            "followers_url": "https://api.github.com/users/spack/followers",
+            "following_url": "https://api.github.com/users/spack/following{/other_user}",
+            "gists_url": "https://api.github.com/users/spack/gists{/gist_id}",
+            "starred_url": "https://api.github.com/users/spack/starred{/owner}{/repo}",
+            "subscriptions_url": "https://api.github.com/users/spack/subscriptions",
+            "organizations_url": "https://api.github.com/users/spack/orgs",
+            "repos_url": "https://api.github.com/users/spack/repos",
+            "events_url": "https://api.github.com/users/spack/events{/privacy}",
+            "received_events_url": "https://api.github.com/users/spack/received_events",
+            "type": "Organization",
+            "site_admin": "False"
+        },
+        "repository_selection": "selected",
+        "access_tokens_url": "https://api.github.com/app/installations/31802092/access_tokens",
+        "repositories_url": "https://api.github.com/installation/repositories",
+        "html_url": "https://github.com/organizations/spack/settings/installations/31802092",
+        "app_id": 126480,
+        "app_slug": "spack-bot-test",
+        "target_id": 25539161,
+        "target_type": "Organization",
+        "permissions": {
+            "checks": "read",
+            "issues": "read",
+            "actions": "read",
+            "members": "write",
+            "contents": "write",
+            "metadata": "read",
+            "statuses": "write",
+            "merge_queues": "read",
+            "pull_requests": "write",
+            "administration": "write"
+        },
+        "events": ["check_run", "issue_comment", "merge_group", "merge_queue_entry", "pull_request", "workflow_job", "workflow_run"],
+        "created_at": "2022-12-02T10:46:10.000-08:00",
+        "updated_at": "2023-01-12T08:38:18.000-08:00",
+        "single_file_name": "None",
+        "has_multiple_single_files": "False",
+        "single_file_paths": [],
+        "suspended_by": "None",
+        "suspended_at": "None"
+    },
+    "sender": {
+        "login": "tgamblin",
+        "id": 299842,
+        "node_id": "MDQ6VXNlcjI5OTg0Mg==",
+        "avatar_url": "https://avatars.githubusercontent.com/u/299842?v=4",
+        "gravatar_id": "",
+        "url": "https://api.github.com/users/tgamblin",
+        "html_url": "https://github.com/tgamblin",
+        "followers_url": "https://api.github.com/users/tgamblin/followers",
+        "following_url": "https://api.github.com/users/tgamblin/following{/other_user}",
+        "gists_url": "https://api.github.com/users/tgamblin/gists{/gist_id}",
+        "starred_url": "https://api.github.com/users/tgamblin/starred{/owner}{/repo}",
+        "subscriptions_url": "https://api.github.com/users/tgamblin/subscriptions",
+        "organizations_url": "https://api.github.com/users/tgamblin/orgs",
+        "repos_url": "https://api.github.com/users/tgamblin/repos",
+        "events_url": "https://api.github.com/users/tgamblin/events{/privacy}",
+        "received_events_url": "https://api.github.com/users/tgamblin/received_events",
+        "type": "User",
+        "site_admin": "False"
+    }
+}