From 30882629ba154c35915d0fada33e26a4b4a01513 Mon Sep 17 00:00:00 2001 From: Francois Kritzinger Date: Thu, 19 Dec 2024 08:55:11 +0200 Subject: ci-github: Address issues found during testing - Update CRs if rebuilding CR of archived tenant Was re-creating them before despite both node ids being available. - handle_check_run_rerequest(): get IAT earlier - Rearrange order of functions --- mod/mod-ci-github.cxx | 1583 +++++++++++++++++++++++++------------------------ mod/mod-ci-github.hxx | 22 +- 2 files changed, 806 insertions(+), 799 deletions(-) diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index f8cc147..25c238e 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -546,64 +546,62 @@ namespace brep // static string conclusion_check_run_name ("CONCLUSION"); - // Return the colored circle corresponding to a result_status. - // - static string - circle (result_status rs) + bool ci_github:: + handle_branch_push (gh_push_event ps, bool warning_success) { - switch (rs) + HANDLER_DIAG; + + l3 ([&]{trace << "push event { " << ps << " }";}); + + // Cancel the CI tenant associated with the overwritten/deleted previous + // head commit if this is a forced push or a branch deletion. + // + if (ps.forced || ps.deleted) { - case result_status::success: return "\U0001F7E2"; // Green circle. - case result_status::warning: return "\U0001F7E0"; // Orange circle. - case result_status::error: - case result_status::abort: - case result_status::abnormal: return "\U0001F534"; // Red circle. + // Service id that will uniquely identify the CI tenant. + // + string sid (ps.repository.node_id + ':' + ps.before); - // Valid values we should never encounter. + // Note that it's possible this commit still exists in another branch so + // we do refcount-aware cancel. // - case result_status::skip: - case result_status::interrupt: - throw invalid_argument ("unexpected result_status value: " + - to_string (rs)); + if (optional ts = cancel (error, warn, + verb_ ? &trace : nullptr, + *build_db_, retry_, + "ci-github", sid, + true /* ref_count */)) + { + l3 ([&]{trace << (ps.forced ? "forced push " + ps.after + " to " + : "deletion of ") + << ps.ref << ": attempted to cancel CI of previous" + << " head commit with tenant_service id " << sid + << " (ref_count: " << ts->ref_count << ')';}); + } + else + { + // It's possible that there was no CI for the previous commit for + // various reasons (e.g., CI was not enabled). + // + l3 ([&]{trace << (ps.forced ? "forced push " + ps.after + " to " + : "deletion of ") + << ps.ref << ": failed to cancel CI of previous" + << " head commit with tenant_service id " << sid;}); + } } - return ""; // Should never reach. - } - - // Make a check run summary from a CI start_result. - // - static string - to_check_run_summary (const optional& r) - { - string s; - - s = "```\n"; - if (r) s += r->message; - else s += "Internal service error"; - s += "\n```"; - - return s; - } - - bool ci_github:: - handle_check_suite_rerequest (gh_check_suite_event cs, bool warning_success) - { - HANDLER_DIAG; - - l3 ([&]{trace << "check_suite event { " << cs << " }";}); - - assert (cs.action == "rerequested"); + if (ps.deleted) + return true; // Do nothing further if this was a branch deletion. // While we don't need the installation access token in this request, // let's obtain it to flush out any permission issues early. Also, it is // valid for an hour so we will most likely make use of it. // - optional jwt (generate_jwt (cs.check_suite.app_id, trace, error)); + optional jwt (generate_jwt (ps.app_id, trace, error)); if (!jwt) throw server_error (); optional iat ( - obtain_installation_access_token (cs.installation.id, + obtain_installation_access_token (ps.installation.id, move (*jwt), error)); if (!iat) @@ -611,84 +609,36 @@ namespace brep l3 ([&]{trace << "installation_access_token { " << *iat << " }";}); - // Service id that uniquely identifies the CI tenant. - // - string sid (cs.repository.node_id + ':' + cs.check_suite.head_sha); + // While it would have been nice to cancel CIs of PRs with this branch as + // base not to waste resources, there are complications: Firstly, we can + // only do this for remote PRs (since local PRs will most likely share the + // result with branch push). Secondly, we try to do our best even if the + // branch protection rule for head behind is not enabled. In this case, it + // would be good to complete the CI. So maybe/later. See also the head + // case in handle_pull_request(), where we do cancel remote PRs that are + // not shared. - // If the user requests a rebuild of the (entire) PR, then this manifests - // as the check_suite rather than pull_request event. Specifically: - // - // - For a local PR, this event is shared with the branch push and all we - // need to do is restart the CI for the head commit. - // - // - For a remote PR, this event will have no gh_check_suite::head_branch. - // In this case we need to load the existing service data for this head - // commit, extract the test merge commit, and restart the CI for that. - // - // Note that it's possible the base branch has moved in the meantime and - // ideally we would want to re-request the test merge commit, etc. - // However, this will only be necessary if the user does not follow our - // recommendation of enabling the head-behind-base protection. And it - // seems all this extra complexity would not be warranted. + // Service id that uniquely identifies the CI tenant. // - string check_sha; - service_data::kind_type kind; - - if (!cs.check_suite.head_branch) - { - // Rebuild of remote PR. - // - kind = service_data::remote; - - if (optional d = find (*build_db_, "ci-github", sid)) - { - tenant_service& ts (d->service); - - try - { - service_data sd (*ts.data); - check_sha = move (sd.check_sha); // Test merge commit. - } - catch (const invalid_argument& e) - { - fail << "failed to parse service data: " << e; - } - } - else - { - error << "check suite " << cs.check_suite.node_id - << " for remote pull request:" - << " re-requested but tenant_service with id " << sid - << " did not exist"; - return true; - } - } - else - { - // Rebuild of branch push or local PR. - // - kind = service_data::local; - check_sha = cs.check_suite.head_sha; - } + string sid (ps.repository.node_id + ':' + ps.after); service_data sd (warning_success, iat->token, iat->expires_at, - cs.check_suite.app_id, - cs.installation.id, - move (cs.repository.node_id), - move (cs.repository.clone_url), - kind, false /* pre_check */, true /* re_requested */, - move (check_sha), - move (cs.check_suite.head_sha) /* report_sha */); - - // Replace the existing CI tenant if it exists. - // - // Note that GitHub UI does not allow re-running the entire check suite - // until all the check runs are completed. - // + ps.app_id, + ps.installation.id, + move (ps.repository.node_id), + move (ps.repository.clone_url), + service_data::local, + false /* pre_check */, + false /* re_requested */, + ps.after /* check_sha */, + ps.after /* report_sha */); - // Create an unloaded CI tenant. + // Create an unloaded CI tenant, doing nothing if one already exists + // (which could've been created by handle_pull_request() or by us as a + // result of a push to another branch). Note that the tenant's reference + // count is incremented in all cases. // // Note: use no delay since we need to (re)create the synthetic conclusion // check run as soon as possible. @@ -700,542 +650,874 @@ namespace brep // notifications until (1) we load the tenant, (2) we cancel it, or (3) // it gets archived after some timeout. // - auto pr (create (error, - warn, - verb_ ? &trace : nullptr, - *build_db_, retry_, - tenant_service (sid, "ci-github", sd.json ()), - chrono::seconds (30) /* interval */, - chrono::seconds (0) /* delay */, - duplicate_tenant_mode::replace)); - - if (!pr) + if (!create (error, warn, verb_ ? &trace : nullptr, + *build_db_, retry_, + tenant_service (sid, "ci-github", sd.json ()), + chrono::seconds (30) /* interval */, + chrono::seconds (0) /* delay */, + duplicate_tenant_mode::ignore)) { - fail << "check suite " << cs.check_suite.node_id + fail << "push " + ps.after + " to " + ps.ref << ": unable to create unloaded CI tenant"; } - if (pr->second == duplicate_tenant_result::created) - { - error << "check suite " << cs.check_suite.node_id - << ": re-requested but tenant_service with id " << sid - << " did not exist"; - return true; - } - return true; } + // Miscellaneous pull request facts + // + // - Although some of the GitHub documentation makes it sound like they + // expect check runs to be added to both the PR head commit and the merge + // commit, the PR UI does not react to the merge commit's check runs + // consistently. It actually seems to be quite broken. The only thing it + // does seem to do reliably is blocking the PR merge if the merge commit's + // check runs are not successful (i.e, overriding the PR head commit's + // check runs). But the UI looks quite messed up generally in this state. + // + // - When new commits are added to a PR base branch, pull_request.base.sha + // does not change, but the test merge commit will be updated to include + // the new commits to the base branch. + // + // - When new commits are added to a PR head branch, pull_request.head.sha + // gets updated with the head commit's SHA and check_suite.pull_requests[] + // will contain all PRs with this branch as head. + // bool ci_github:: - handle_check_suite_completed (gh_check_suite_event cs, bool warning_success) + handle_pull_request (gh_pull_request_event pr, bool warning_success) { - // The plans is as follows: - // - // 1. Load the service data. - // - // 2. Verify it is completed. - // - // 3. Verify the check run counts match. - // - // 4. Verify (like in build_built()) that all the check runs are - // completed. - // - // 5. Verify the result matches what GitHub thinks it is. - HANDLER_DIAG; - l3 ([&]{trace << "check_suite event { " << cs << " }";}); + l3 ([&]{trace << "pull_request event { " << pr << " }";}); - // Service id that uniquely identifies the CI tenant. + // While we don't need the installation access token in this request, + // let's obtain it to flush out any permission issues early. Also, it is + // valid for an hour so we will most likely make use of it. // - string sid (cs.repository.node_id + ':' + cs.check_suite.head_sha); + optional jwt (generate_jwt (pr.pull_request.app_id, trace, error)); + if (!jwt) + throw server_error (); - // The common log entry subject. - // - string sub ("check suite " + cs.check_suite.node_id + '/' + sid); + optional iat ( + obtain_installation_access_token (pr.installation.id, + move (*jwt), + error)); + if (!iat) + throw server_error (); - // Load the service data. + l3 ([&]{trace << "installation_access_token { " << *iat << " }";}); + + // Distinguish between local and remote PRs by comparing the head and base + // repositories' paths. // - service_data sd; + service_data::kind_type kind ( + pr.pull_request.head_path == pr.pull_request.base_path + ? service_data::local + : service_data::remote); - if (optional d = find (*build_db_, "ci-github", sid)) + // Note that similar to the branch push case above, while it would have + // been nice to cancel the previous CI job once the PR head moves (the + // "synchronize" event), due to the head sharing problem the previous CI + // job might actually still be relevant (in both local and remote PR + // cases). So we only do it for the remote PRs and only if the head is not + // shared (via tenant reference counting). + // + if (kind == service_data::remote && pr.action == "synchronize") { - try + if (pr.before) { - sd = service_data (*d->service.data); + // Service id that will uniquely identify the CI tenant. + // + string sid (pr.repository.node_id + ':' + *pr.before); + + if (optional ts = cancel (error, warn, + verb_ ? &trace : nullptr, + *build_db_, retry_, + "ci-github", sid, + true /* ref_count */)) + { + l3 ([&]{trace << "pull request " << pr.pull_request.node_id + << ": attempted to cancel CI of previous head commit" + << " (ref_count: " << ts->ref_count << ')';}); + } + else + { + // It's possible that there was no CI for the previous commit for + // various reasons (e.g., CI was not enabled). + // + l3 ([&]{trace << "pull request " << pr.pull_request.node_id + << ": failed to cancel CI of previous head commit " + << "with tenant_service id " << sid;}); + } } - catch (const invalid_argument& e) + else { - fail << "failed to parse service data: " << e; + error << "pull request " << pr.pull_request.node_id + << ": before commit is missing in synchronize event"; } } - else - { - error << sub << ": tenant_service does not exist"; - return true; - } - // Verify the completed flag and the number of check runs. + // Note: for remote PRs the check_sha will be set later, in + // build_unloaded_pre_check(), to test merge commit id. // - if (!sd.completed) - { - error << sub << " service data complete flag is false"; - return true; - } + string check_sha (kind == service_data::local + ? pr.pull_request.head_sha + : ""); - // Received count will be one higher because we don't store the conclusion - // check run. + // Note that PR rebuilds (re-requested) are handled by + // handle_check_suite_rerequest(). // - size_t check_runs_count (sd.check_runs.size () + 1); - - if (check_runs_count == 1) - { - error << sub << ": no check runs in service data"; - return true; - } - - if (cs.check_suite.check_runs_count != check_runs_count) - { - error << sub << ": check runs count " << cs.check_suite.check_runs_count - << " does not match service data count " << check_runs_count; - return true; - } - - // Verify that all the check runs are built and compute the summary - // conclusion. + // Note that, in the case of a remote PR, GitHub will copy the PR head + // commit from the head (forked) repository into the base repository. So + // the check runs must always be added to the base repository, whether the + // PR is local or remote. The head commit refs are located at + // refs/pull//head. // - result_status conclusion (result_status::success); - - for (const check_run& cr: sd.check_runs) - { - if (cr.state == build_state::built) - { - assert (cr.status.has_value ()); - conclusion |= *cr.status; - } - else - { - error << sub << ": unbuilt check run in service data"; - return true; - } - } + service_data sd (warning_success, + move (iat->token), + iat->expires_at, + pr.pull_request.app_id, + pr.installation.id, + move (pr.repository.node_id), + move (pr.repository.clone_url), + kind, true /* pre_check */, false /* re_request */, + move (check_sha), + move (pr.pull_request.head_sha) /* report_sha */, + pr.pull_request.node_id, + pr.pull_request.number); - // Verify the conclusion. + // Create an unloaded CI tenant for the pre-check phase (during which we + // wait for the PR's merge commit and behindness to become available). // - if (!cs.check_suite.conclusion) - { - error << sub << ": absent conclusion in completed check suite"; - return true; - } - - // Note that the case mismatch is due to GraphQL (gh_conclusion()) - // requiring uppercase conclusion values while the received webhook values - // are lower case. + // Create with an empty service id so that the generated tenant id is used + // instead during the pre-check phase (so as not to clash with a proper + // service id for this head commit, potentially created in + // handle_branch_push() or as another PR). // - string gh_conclusion (gh_to_conclusion (conclusion, warning_success)); + tenant_service ts ("", "ci-github", sd.json ()); - if (icasecmp (*cs.check_suite.conclusion, gh_conclusion) != 0) + // Note: use no delay since we need to start the actual CI (which in turn + // (re)creates the synthetic conclusion check run) as soon as possible. + // + // After this call we will start getting the build_unloaded() + // notifications -- which will be routed to build_unloaded_pre_check() -- + // until we cancel the tenant or it gets archived after some timeout. + // (Note that we never actually load this request, we always cancel it; + // see build_unloaded_pre_check() for details.) + // + if (!create (error, + warn, + verb_ ? &trace : nullptr, + *build_db_, retry_, + move (ts), + chrono::seconds (30) /* interval */, + chrono::seconds (0) /* delay */)) { - error << sub << ": conclusion " << *cs.check_suite.conclusion - << " does not match service data conclusion " << gh_conclusion; - return true; + fail << "pull request " << pr.pull_request.node_id + << ": unable to create unloaded pre-check tenant"; } return true; } - // Create a gq_built_result. - // - // Throw invalid_argument in case of invalid result_status. - // - static gq_built_result - make_built_result (result_status rs, bool warning_success, string message) - { - return {gh_to_conclusion (rs, warning_success), - circle (rs) + ' ' + ucase (to_string (rs)), - move (message)}; - } - - // Parse a check run details URL into a build_id. - // - // Return nullopt if the URL is invalid. - // - static optional - parse_details_url (const string& details_url); - - // Note that GitHub always posts a message to their GUI saying "You have - // successfully requested be rerun", regardless of what - // HTTP status code we respond with. However we do return error status codes - // when there is no better option (like failing the conclusion) in case they - // start handling them someday. - // bool ci_github:: - handle_check_run_rerequest (const gh_check_run_event& cr, - bool warning_success) + handle_check_suite_rerequest (gh_check_suite_event cs, bool warning_success) { HANDLER_DIAG; - l3 ([&]{trace << "check_run event { " << cr << " }";}); + l3 ([&]{trace << "check_suite event { " << cs << " }";}); - // The overall plan is as follows: - // - // 1. Load service data. - // - // 2. If the tenant is archived, then fail (re-create) both the check run - // and the conclusion with appropriate diagnostics. - // - // 3. If the check run is in the queued state, then do nothing. - // - // 4. Re-create the check run in the queued state and the conclusion in - // the building state. Note: do in a single request to make sure we - // either "win" or "loose" the potential race for both (important - // for #7). - // - // 5. Call the rebuild() function to attempt to schedule a rebuild. Pass - // the update function that does the following (if called): - // - // a. Save new node ids. - // - // b. Update the check run state (may also not exist). - // - // c. Clear the completed flag if true. - // - // 6. If the result of rebuild() indicates the tenant is archived, then - // fail (update) both the check run and conclusion with appropriate - // diagnostics. - // - // 7. If original state is queued (no rebuild was scheduled), then fail - // (update) both the check run and the conclusion. - // - // Note that while conceptually we are updating existing check runs, in - // practice we have to re-create as new check runs in order to replace the - // existing ones because GitHub does not allow transitioning out of the - // built state. + assert (cs.action == "rerequested"); - // Get a new installation access token. + // While we don't need the installation access token in this request, + // let's obtain it to flush out any permission issues early. Also, it is + // valid for an hour so we will most likely make use of it. // - auto get_iat = [this, &trace, &error, &cr] () - -> optional - { - optional jwt (generate_jwt (cr.check_run.app_id, trace, error)); - if (!jwt) - return nullopt; + optional jwt (generate_jwt (cs.check_suite.app_id, trace, error)); + if (!jwt) + throw server_error (); - optional iat ( - obtain_installation_access_token (cr.installation.id, + optional iat ( + obtain_installation_access_token (cs.installation.id, move (*jwt), error)); + if (!iat) + throw server_error (); - if (iat) - l3 ([&]{trace << "installation_access_token { " << *iat << " }";}); - - return iat; - }; - - const string& repo_node_id (cr.repository.node_id); - const string& head_sha (cr.check_run.check_suite.head_sha); + l3 ([&]{trace << "installation_access_token { " << *iat << " }";}); - // Prepare the build and conclusion check runs. They are sent to GitHub in - // a single request (unless something goes wrong) so store them together - // from the outset. + // Service id that uniquely identifies the CI tenant. // - vector check_runs (2); - check_run& bcr (check_runs[0]); // Build check run - check_run& ccr (check_runs[1]); // Conclusion check run - - ccr.name = conclusion_check_run_name; + string sid (cs.repository.node_id + ':' + cs.check_suite.head_sha); - // Load the service data, failing the check runs if the tenant has been - // archived. + // If the user requests a rebuild of the (entire) PR, then this manifests + // as the check_suite rather than pull_request event. Specifically: // - service_data sd; - string tenant_id; + // - For a local PR, this event is shared with the branch push and all we + // need to do is restart the CI for the head commit. + // + // - For a remote PR, this event will have no gh_check_suite::head_branch. + // In this case we need to load the existing service data for this head + // commit, extract the test merge commit, and restart the CI for that. + // + // Note that it's possible the base branch has moved in the meantime and + // ideally we would want to re-request the test merge commit, etc. + // However, this will only be necessary if the user does not follow our + // recommendation of enabling the head-behind-base protection. And it + // seems all this extra complexity would not be warranted. + // + string check_sha; + service_data::kind_type kind; + + if (!cs.check_suite.head_branch) { - // Service id that uniquely identifies the CI tenant. + // Rebuild of remote PR. // - string sid (repo_node_id + ':' + head_sha); + kind = service_data::remote; if (optional d = find (*build_db_, "ci-github", sid)) { - if (d->archived) // Tenant is archived - { - // Fail (re-create) the check runs. - // - optional iat (get_iat ()); - if (!iat) - throw server_error (); - - gq_built_result br ( - make_built_result ( - result_status::error, warning_success, - "Unable to rebuild individual configuration: build has " - "been archived")); - - // Try to update the conclusion check run even if the first update - // fails. - // - bool f (false); // Failed. - - if (gq_create_check_run (error, bcr, iat->token, - repo_node_id, head_sha, - cr.check_run.details_url, - build_state::built, br)) - { - l3 ([&]{trace << "created check_run { " << bcr << " }";}); - } - else - { - error << "check_run " << cr.check_run.node_id - << ": unable to re-create check run"; - f = true; - } - - if (gq_create_check_run (error, ccr, iat->token, - repo_node_id, head_sha, - nullopt /* details_url */, - build_state::built, move (br))) - { - l3 ([&]{trace << "created conclusion check_run { " << ccr << " }";}); - } - else - { - error << "check_run " << cr.check_run.node_id - << ": unable to re-create conclusion check run"; - f = true; - } - - // Fail the handler if either of the check runs could not be - // updated. - // - if (f) - throw server_error (); - - return true; - } - tenant_service& ts (d->service); try { - sd = service_data (*ts.data); + service_data sd (*ts.data); + check_sha = move (sd.check_sha); // Test merge commit. } catch (const invalid_argument& e) { fail << "failed to parse service data: " << e; } - - tenant_id = d->tenant_id; } else { - // No such tenant. - // - fail << "check run " << cr.check_run.node_id - << " re-requested but tenant_service with id " << sid - << " does not exist"; + error << "check suite " << cs.check_suite.node_id + << " for remote pull request:" + << " re-requested but tenant_service with id " << sid + << " did not exist"; + return true; } } + else + { + // Rebuild of branch push or local PR. + // + kind = service_data::local; + check_sha = cs.check_suite.head_sha; + } + + service_data sd (warning_success, + iat->token, + iat->expires_at, + cs.check_suite.app_id, + cs.installation.id, + move (cs.repository.node_id), + move (cs.repository.clone_url), + kind, false /* pre_check */, true /* re_requested */, + move (check_sha), + move (cs.check_suite.head_sha) /* report_sha */); - // Get a new IAT if the one from the service data has expired. + // Replace the existing CI tenant if it exists. + // + // Note that GitHub UI does not allow re-running the entire check suite + // until all the check runs are completed. // - const gh_installation_access_token* iat (nullptr); - optional new_iat; - if (system_clock::now () > sd.installation_access.expires_at) + // Create an unloaded CI tenant. + // + // Note: use no delay since we need to (re)create the synthetic conclusion + // check run as soon as possible. + // + // Note that we use the create() API instead of start() since duplicate + // management is not available in start(). + // + // After this call we will start getting the build_unloaded() + // notifications until (1) we load the tenant, (2) we cancel it, or (3) + // it gets archived after some timeout. + // + auto pr (create (error, + warn, + verb_ ? &trace : nullptr, + *build_db_, retry_, + tenant_service (sid, "ci-github", sd.json ()), + chrono::seconds (30) /* interval */, + chrono::seconds (0) /* delay */, + duplicate_tenant_mode::replace)); + + if (!pr) { - if ((new_iat = get_iat ())) - iat = &*new_iat; - else - throw server_error (); + fail << "check suite " << cs.check_suite.node_id + << ": unable to create unloaded CI tenant"; } - else - iat = &sd.installation_access; - // Fail if it's the conclusion check run that is being re-requested. - // - if (cr.check_run.name == conclusion_check_run_name) + if (pr->second == duplicate_tenant_result::created) { - l3 ([&]{trace << "re-requested conclusion check_run";}); + error << "check suite " << cs.check_suite.node_id + << ": re-requested but tenant_service with id " << sid + << " did not exist"; + return true; + } - if (!sd.conclusion_node_id) - fail << "no conclusion node id for check run " << cr.check_run.node_id; + return true; + } - gq_built_result br ( - make_built_result (result_status::error, warning_success, - "Conclusion check run cannot be rebuilt")); + bool ci_github:: + handle_check_suite_completed (gh_check_suite_event cs, bool warning_success) + { + // The plans is as follows: + // + // 1. Load the service data. + // + // 2. Verify it is completed. + // + // 3. Verify the check run counts match. + // + // 4. Verify (like in build_built()) that all the check runs are + // completed. + // + // 5. Verify the result matches what GitHub thinks it is. - // Fail (update) the conclusion check run. - // - if (gq_update_check_run (error, ccr, iat->token, - repo_node_id, *sd.conclusion_node_id, - nullopt /* details_url */, - build_state::built, move (br))) + HANDLER_DIAG; + + l3 ([&]{trace << "check_suite event { " << cs << " }";}); + + // Service id that uniquely identifies the CI tenant. + // + string sid (cs.repository.node_id + ':' + cs.check_suite.head_sha); + + // The common log entry subject. + // + string sub ("check suite " + cs.check_suite.node_id + '/' + sid); + + // Load the service data. + // + service_data sd; + + if (optional d = find (*build_db_, "ci-github", sid)) + { + try { - l3 ([&]{trace << "updated conclusion check_run { " << ccr << " }";}); + sd = service_data (*d->service.data); } - else + catch (const invalid_argument& e) { - fail << "check run " << cr.check_run.node_id - << ": unable to update conclusion check run " - << *sd.conclusion_node_id; + fail << "failed to parse service data: " << e; } - + } + else + { + error << sub << ": tenant_service does not exist"; return true; } - // Parse the check_run's details_url to extract build id. - // - // While this is a bit hackish, there doesn't seem to be a better way - // (like associating custom data with a check run). Note that the GitHub - // UI only allows rebuilding completed check runs, so the details URL - // should be there. + // Verify the completed flag and the number of check runs. // - optional bid (parse_details_url (cr.check_run.details_url)); - if (!bid) + if (!sd.completed) { - fail << "check run " << cr.check_run.node_id - << ": failed to extract build id from details_url"; + error << sub << " service data complete flag is false"; + return true; } - // Initialize the check run (`bcr`) with state from the service data. + // Received count will be one higher because we don't store the conclusion + // check run. // + size_t check_runs_count (sd.check_runs.size () + 1); + + if (check_runs_count == 1) { - // Search for the check run in the service data. - // - // Note that we look by name in case node id got replaced by a racing - // re-request (in which case we ignore this request). - // - auto i (find_if (sd.check_runs.begin (), sd.check_runs.end (), - [&cr] (const check_run& scr) - { - return scr.name == cr.check_run.name; - })); + error << sub << ": no check runs in service data"; + return true; + } - if (i == sd.check_runs.end ()) - fail << "check_run " << cr.check_run.node_id - << " (" << cr.check_run.name << "): " - << "re-requested but does not exist in service data"; + if (cs.check_suite.check_runs_count != check_runs_count) + { + error << sub << ": check runs count " << cs.check_suite.check_runs_count + << " does not match service data count " << check_runs_count; + return true; + } - // Do nothing if node ids don't match. - // - if (i->node_id && *i->node_id != cr.check_run.node_id) + // Verify that all the check runs are built and compute the summary + // conclusion. + // + result_status conclusion (result_status::success); + + for (const check_run& cr: sd.check_runs) + { + if (cr.state == build_state::built) { - l3 ([&]{trace << "check_run " << cr.check_run.node_id - << " (" << cr.check_run.name << "): " - << "node id has changed in service data";}); + assert (cr.status.has_value ()); + conclusion |= *cr.status; + } + else + { + error << sub << ": unbuilt check run in service data"; return true; } + } - // Do nothing if the build is already queued. - // - if (i->state == build_state::queued) - { - l3 ([&]{trace << "ignoring already-queued check run";}); - return true; - } - - bcr.name = i->name; - bcr.build_id = i->build_id; - bcr.state = i->state; + // Verify the conclusion. + // + if (!cs.check_suite.conclusion) + { + error << sub << ": absent conclusion in completed check suite"; + return true; } - // Transition the build and conclusion check runs out of the built state - // (or any other state) by re-creating them. + // Note that the case mismatch is due to GraphQL (gh_conclusion()) + // requiring uppercase conclusion values while the received webhook values + // are lower case. // - bcr.state = build_state::queued; - bcr.state_synced = false; - bcr.details_url = cr.check_run.details_url; - - ccr.state = build_state::building; - ccr.state_synced = false; + string gh_conclusion (gh_to_conclusion (conclusion, warning_success)); - if (gq_create_check_runs (error, check_runs, iat->token, - repo_node_id, head_sha)) + if (icasecmp (*cs.check_suite.conclusion, gh_conclusion) != 0) { - assert (bcr.state == build_state::queued); - assert (ccr.state == build_state::building); - - l3 ([&]{trace << "created check_run { " << bcr << " }";}); - l3 ([&]{trace << "created conclusion check_run { " << ccr << " }";}); + error << sub << ": conclusion " << *cs.check_suite.conclusion + << " does not match service data conclusion " << gh_conclusion; + return true; } - else + + return true; + } + + // Return the colored circle corresponding to a result_status. + // + static string + circle (result_status rs) + { + switch (rs) { - fail << "check run " << cr.check_run.node_id - << ": unable to re-create build and conclusion check runs"; + case result_status::success: return "\U0001F7E2"; // Green circle. + case result_status::warning: return "\U0001F7E0"; // Orange circle. + case result_status::error: + case result_status::abort: + case result_status::abnormal: return "\U0001F534"; // Red circle. + + // Valid values we should never encounter. + // + case result_status::skip: + case result_status::interrupt: + throw invalid_argument ("unexpected result_status value: " + + to_string (rs)); } - // Request the rebuild and update service data. + return ""; // Should never reach. + } + + // Make a check run summary from a CI start_result. + // + static string + to_check_run_summary (const optional& r) + { + string s; + + s = "```\n"; + if (r) s += r->message; + else s += "Internal service error"; + s += "\n```"; + + return s; + } + + // Create a gq_built_result. + // + // Throw invalid_argument in case of invalid result_status. + // + static gq_built_result + make_built_result (result_status rs, bool warning_success, string message) + { + return {gh_to_conclusion (rs, warning_success), + circle (rs) + ' ' + ucase (to_string (rs)), + move (message)}; + } + + // Parse a check run details URL into a build_id. + // + // Return nullopt if the URL is invalid. + // + static optional + parse_details_url (const string& details_url); + + // Note that GitHub always posts a message to their GUI saying "You have + // successfully requested be rerun", regardless of what + // HTTP status code we respond with. However we do return error status codes + // when there is no better option (like failing the conclusion) in case they + // start handling them someday. + // + bool ci_github:: + handle_check_run_rerequest (const gh_check_run_event& cr, + bool warning_success) + { + HANDLER_DIAG; + + l3 ([&]{trace << "check_run event { " << cr << " }";}); + + // The overall plan is as follows: // - bool race (false); + // 1. Load service data. + // + // 2. If the tenant is archived, then fail (re-create) both the check run + // and the conclusion with appropriate diagnostics. + // + // 3. If the check run is in the queued state, then do nothing. + // + // 4. Re-create the check run in the queued state and the conclusion in + // the building state. Note: do in a single request to make sure we + // either "win" or "loose" the potential race for both (important + // for #7). + // + // 5. Call the rebuild() function to attempt to schedule a rebuild. Pass + // the update function that does the following (if called): + // + // a. Save new node ids. + // + // b. Update the check run state (may also not exist). + // + // c. Clear the completed flag if true. + // + // 6. If the result of rebuild() indicates the tenant is archived, then + // fail (update) both the check run and conclusion with appropriate + // diagnostics. + // + // 7. If original state is queued (no rebuild was scheduled), then fail + // (update) both the check run and the conclusion. + // + // Note that while conceptually we are updating existing check runs, in + // practice we have to re-create as new check runs in order to replace the + // existing ones because GitHub does not allow transitioning out of the + // built state. - // Callback function called by rebuild() to update the service data (but - // only if the build is actually restarted). + // Get a new installation access token. // - auto update_sd = [&error, &new_iat, &race, - tenant_id = move (tenant_id), - &cr, &bcr, &ccr] (const string& ti, - const tenant_service& ts, - build_state) -> optional + auto get_iat = [this, &trace, &error, &cr] () + -> optional { - // NOTE: this lambda may be called repeatedly (e.g., due to transaction - // being aborted) and so should not move out of its captures. + optional jwt (generate_jwt (cr.check_run.app_id, trace, error)); + if (!jwt) + return nullopt; - race = false; // Reset. + optional iat ( + obtain_installation_access_token (cr.installation.id, + move (*jwt), + error)); - if (tenant_id != ti) + if (iat) + l3 ([&]{trace << "installation_access_token { " << *iat << " }";}); + + return iat; + }; + + const string& repo_node_id (cr.repository.node_id); + const string& head_sha (cr.check_run.check_suite.head_sha); + + // Prepare the build and conclusion check runs. They are sent to GitHub in + // a single request (unless something goes wrong) so store them together + // from the outset. + // + vector check_runs (2); + check_run& bcr (check_runs[0]); // Build check run + check_run& ccr (check_runs[1]); // Conclusion check run + + ccr.name = conclusion_check_run_name; + + const gh_installation_access_token* iat (nullptr); + optional new_iat; + + // Load the service data, failing the check runs if the tenant has been + // archived. + // + service_data sd; + string tenant_id; + { + // Service id that uniquely identifies the CI tenant. + // + string sid (repo_node_id + ':' + head_sha); + + optional d (find (*build_db_, "ci-github", sid)); + if (!d) { - // The tenant got replaced since we loaded it but we managed to - // trigger a rebuild in the new tenant. Who knows whose check runs are - // visible, so let's fail ours similar to the cases below. + // No such tenant. // - race = true; - return nullopt; + fail << "check run " << cr.check_run.node_id + << " re-requested but tenant_service with id " << sid + << " does not exist"; } - service_data sd; + tenant_service& ts (d->service); + try { sd = service_data (*ts.data); } catch (const invalid_argument& e) { - error << "failed to parse service data: " << e; - return nullopt; + fail << "failed to parse service data: " << e; } - // Note that we again look by name in case node id got replaced by a - // racing re-request. In this case, however, it's impossible to decide - // who won that race, so let's fail the check suite to be on the safe - // side (in a sense, similar to the rebuild() returning queued below). - // - auto i (find_if ( - sd.check_runs.begin (), sd.check_runs.end (), - [&cr] (const check_run& scr) - { - return scr.name == cr.check_run.name; - })); + if (!sd.conclusion_node_id) + fail << "no conclusion node id for check run " << cr.check_run.node_id; - if (i == sd.check_runs.end ()) + tenant_id = d->tenant_id; + + // Get a new IAT if the one from the service data has expired. + // + if (system_clock::now () > sd.installation_access.expires_at) { - error << "check_run " << cr.check_run.node_id - << " (" << cr.check_run.name << "): " - << "re-requested but does not exist in service data"; - return nullopt; + if ((new_iat = get_iat ())) + iat = &*new_iat; + else + throw server_error (); } + else + iat = &sd.installation_access; - if (i->node_id && *i->node_id != cr.check_run.node_id) + if (d->archived) // Tenant is archived { - // Keep the old conclusion node id to make sure any further state - // transitions are ignored. A bit of a hack. + // Fail (update) the check runs. // - race = true; - return nullopt; - } + gq_built_result br ( + make_built_result ( + result_status::error, warning_success, + "Unable to rebuild individual configuration: build has " + "been archived")); + + // Try to update the conclusion check run even if the first update + // fails. + // + bool f (false); // Failed. - *i = bcr; // Update with new node_id, state, state_synced. + if (gq_update_check_run (error, bcr, iat->token, + repo_node_id, cr.check_run.node_id, + nullopt /* details_url */, + build_state::built, br)) + { + l3 ([&]{trace << "updated check_run { " << bcr << " }";}); + } + else + { + error << "check_run " << cr.check_run.node_id + << ": unable to update check run"; + f = true; + } - sd.conclusion_node_id = ccr.node_id; - sd.completed = false; + if (gq_update_check_run (error, ccr, iat->token, + repo_node_id, *sd.conclusion_node_id, + nullopt /* details_url */, + build_state::built, move (br))) + { + l3 ([&]{trace << "updated conclusion check_run { " << ccr << " }";}); + } + else + { + error << "check_run " << cr.check_run.node_id + << ": unable to update conclusion check run"; + f = true; + } + + // Fail the handler if either of the check runs could not be + // updated. + // + if (f) + throw server_error (); + + return true; + } + } + + // Fail if it's the conclusion check run that is being re-requested. + // + // Expect that if the user selects re-run all failed checks we will + // receive multiple check runs, one of which will be the conclusion. And + // if we fail it while it happens to arrive last, then we will end up in + // the wrong overall state (real check run is building while conclusion is + // failed). It seems the best we can do is to ignore it: if the user did + // request a rebuild of the conclusion check run explicitly, there will be + // no change, which is not ideal but is still an indication that this + // operation is not supported. + // + if (cr.check_run.name == conclusion_check_run_name) + { + l3 ([&]{trace << "re-requested conclusion check_run";}); + +#if 0 + if (!sd.conclusion_node_id) + fail << "no conclusion node id for check run " << cr.check_run.node_id; + + gq_built_result br ( + make_built_result (result_status::error, warning_success, + "Conclusion check run cannot be rebuilt")); + + // Fail (update) the conclusion check run. + // + if (gq_update_check_run (error, ccr, iat->token, + repo_node_id, *sd.conclusion_node_id, + nullopt /* details_url */, + build_state::built, move (br))) + { + l3 ([&]{trace << "updated conclusion check_run { " << ccr << " }";}); + } + else + { + fail << "check run " << cr.check_run.node_id + << ": unable to update conclusion check run " + << *sd.conclusion_node_id; + } +#endif + + return true; + } + + // Parse the check_run's details_url to extract build id. + // + // While this is a bit hackish, there doesn't seem to be a better way + // (like associating custom data with a check run). Note that the GitHub + // UI only allows rebuilding completed check runs, so the details URL + // should be there. + // + optional bid (parse_details_url (cr.check_run.details_url)); + if (!bid) + { + fail << "check run " << cr.check_run.node_id + << ": failed to extract build id from details_url"; + } + + // Initialize the check run (`bcr`) with state from the service data. + // + { + // Search for the check run in the service data. + // + // Note that we look by name in case node id got replaced by a racing + // re-request (in which case we ignore this request). + // + auto i (find_if (sd.check_runs.begin (), sd.check_runs.end (), + [&cr] (const check_run& scr) + { + return scr.name == cr.check_run.name; + })); + + if (i == sd.check_runs.end ()) + fail << "check_run " << cr.check_run.node_id + << " (" << cr.check_run.name << "): " + << "re-requested but does not exist in service data"; + + // Do nothing if node ids don't match. + // + if (i->node_id && *i->node_id != cr.check_run.node_id) + { + l3 ([&]{trace << "check_run " << cr.check_run.node_id + << " (" << cr.check_run.name << "): " + << "node id has changed in service data";}); + return true; + } + + // Do nothing if the build is already queued. + // + if (i->state == build_state::queued) + { + l3 ([&]{trace << "ignoring already-queued check run";}); + return true; + } + + bcr.name = i->name; + bcr.build_id = i->build_id; + bcr.state = i->state; + } + + // Transition the build and conclusion check runs out of the built state + // (or any other state) by re-creating them. + // + bcr.state = build_state::queued; + bcr.state_synced = false; + bcr.details_url = cr.check_run.details_url; + + ccr.state = build_state::building; + ccr.state_synced = false; + + if (gq_create_check_runs (error, check_runs, iat->token, + repo_node_id, head_sha)) + { + assert (bcr.state == build_state::queued); + assert (ccr.state == build_state::building); + + l3 ([&]{trace << "created check_run { " << bcr << " }";}); + l3 ([&]{trace << "created conclusion check_run { " << ccr << " }";}); + } + else + { + fail << "check run " << cr.check_run.node_id + << ": unable to re-create build and conclusion check runs"; + } + + // Request the rebuild and update service data. + // + bool race (false); + + // Callback function called by rebuild() to update the service data (but + // only if the build is actually restarted). + // + auto update_sd = [&error, &new_iat, &race, + tenant_id = move (tenant_id), + &cr, &bcr, &ccr] (const string& ti, + const tenant_service& ts, + build_state) -> optional + { + // NOTE: this lambda may be called repeatedly (e.g., due to transaction + // being aborted) and so should not move out of its captures. + + race = false; // Reset. + + if (tenant_id != ti) + { + // The tenant got replaced since we loaded it but we managed to + // trigger a rebuild in the new tenant. Who knows whose check runs are + // visible, so let's fail ours similar to the cases below. + // + race = true; + return nullopt; + } + + service_data sd; + try + { + sd = service_data (*ts.data); + } + catch (const invalid_argument& e) + { + error << "failed to parse service data: " << e; + return nullopt; + } + + // Note that we again look by name in case node id got replaced by a + // racing re-request. In this case, however, it's impossible to decide + // who won that race, so let's fail the check suite to be on the safe + // side (in a sense, similar to the rebuild() returning queued below). + // + auto i (find_if ( + sd.check_runs.begin (), sd.check_runs.end (), + [&cr] (const check_run& scr) + { + return scr.name == cr.check_run.name; + })); + + if (i == sd.check_runs.end ()) + { + error << "check_run " << cr.check_run.node_id + << " (" << cr.check_run.name << "): " + << "re-requested but does not exist in service data"; + return nullopt; + } + + if (i->node_id && *i->node_id != cr.check_run.node_id) + { + // Keep the old conclusion node id to make sure any further state + // transitions are ignored. A bit of a hack. + // + race = true; + return nullopt; + } + + *i = bcr; // Update with new node_id, state, state_synced. + + sd.conclusion_node_id = ccr.node_id; + sd.completed = false; // Save the IAT if we created a new one. // @@ -1335,279 +1617,6 @@ namespace brep return true; } - // Miscellaneous pull request facts - // - // - Although some of the GitHub documentation makes it sound like they - // expect check runs to be added to both the PR head commit and the merge - // commit, the PR UI does not react to the merge commit's check runs - // consistently. It actually seems to be quite broken. The only thing it - // does seem to do reliably is blocking the PR merge if the merge commit's - // check runs are not successful (i.e, overriding the PR head commit's - // check runs). But the UI looks quite messed up generally in this state. - // - // - When new commits are added to a PR base branch, pull_request.base.sha - // does not change, but the test merge commit will be updated to include - // the new commits to the base branch. - // - // - When new commits are added to a PR head branch, pull_request.head.sha - // gets updated with the head commit's SHA and check_suite.pull_requests[] - // will contain all PRs with this branch as head. - // - bool ci_github:: - handle_pull_request (gh_pull_request_event pr, bool warning_success) - { - HANDLER_DIAG; - - l3 ([&]{trace << "pull_request event { " << pr << " }";}); - - // While we don't need the installation access token in this request, - // let's obtain it to flush out any permission issues early. Also, it is - // valid for an hour so we will most likely make use of it. - // - optional jwt (generate_jwt (pr.pull_request.app_id, trace, error)); - if (!jwt) - throw server_error (); - - optional iat ( - obtain_installation_access_token (pr.installation.id, - move (*jwt), - error)); - if (!iat) - throw server_error (); - - l3 ([&]{trace << "installation_access_token { " << *iat << " }";}); - - // Distinguish between local and remote PRs by comparing the head and base - // repositories' paths. - // - service_data::kind_type kind ( - pr.pull_request.head_path == pr.pull_request.base_path - ? service_data::local - : service_data::remote); - - // Note that similar to the branch push case above, while it would have - // been nice to cancel the previous CI job once the PR head moves (the - // "synchronize" event), due to the head sharing problem the previous CI - // job might actually still be relevant (in both local and remote PR - // cases). So we only do it for the remote PRs and only if the head is not - // shared (via tenant reference counting). - // - if (kind == service_data::remote && pr.action == "synchronize") - { - if (pr.before) - { - // Service id that will uniquely identify the CI tenant. - // - string sid (pr.repository.node_id + ':' + *pr.before); - - if (optional ts = cancel (error, warn, - verb_ ? &trace : nullptr, - *build_db_, retry_, - "ci-github", sid, - true /* ref_count */)) - { - l3 ([&]{trace << "pull request " << pr.pull_request.node_id - << ": attempted to cancel CI of previous head commit" - << " (ref_count: " << ts->ref_count << ')';}); - } - else - { - // It's possible that there was no CI for the previous commit for - // various reasons (e.g., CI was not enabled). - // - l3 ([&]{trace << "pull request " << pr.pull_request.node_id - << ": failed to cancel CI of previous head commit " - << "with tenant_service id " << sid;}); - } - } - else - { - error << "pull request " << pr.pull_request.node_id - << ": before commit is missing in synchronize event"; - } - } - - // Note: for remote PRs the check_sha will be set later, in - // build_unloaded_pre_check(), to test merge commit id. - // - string check_sha (kind == service_data::local - ? pr.pull_request.head_sha - : ""); - - // Note that PR rebuilds (re-requested) are handled by - // handle_check_suite_rerequest(). - // - // Note that, in the case of a remote PR, GitHub will copy the PR head - // commit from the head (forked) repository into the base repository. So - // the check runs must always be added to the base repository, whether the - // PR is local or remote. The head commit refs are located at - // refs/pull//head. - // - service_data sd (warning_success, - move (iat->token), - iat->expires_at, - pr.pull_request.app_id, - pr.installation.id, - move (pr.repository.node_id), - move (pr.repository.clone_url), - kind, true /* pre_check */, false /* re_request */, - move (check_sha), - move (pr.pull_request.head_sha) /* report_sha */, - pr.pull_request.node_id, - pr.pull_request.number); - - // Create an unloaded CI tenant for the pre-check phase (during which we - // wait for the PR's merge commit and behindness to become available). - // - // Create with an empty service id so that the generated tenant id is used - // instead during the pre-check phase (so as not to clash with a proper - // service id for this head commit, potentially created in - // handle_branch_push() or as another PR). - // - tenant_service ts ("", "ci-github", sd.json ()); - - // Note: use no delay since we need to start the actual CI (which in turn - // (re)creates the synthetic conclusion check run) as soon as possible. - // - // After this call we will start getting the build_unloaded() - // notifications -- which will be routed to build_unloaded_pre_check() -- - // until we cancel the tenant or it gets archived after some timeout. - // (Note that we never actually load this request, we always cancel it; - // see build_unloaded_pre_check() for details.) - // - if (!create (error, - warn, - verb_ ? &trace : nullptr, - *build_db_, retry_, - move (ts), - chrono::seconds (30) /* interval */, - chrono::seconds (0) /* delay */)) - { - fail << "pull request " << pr.pull_request.node_id - << ": unable to create unloaded pre-check tenant"; - } - - return true; - } - - bool ci_github:: - handle_branch_push (gh_push_event ps, bool warning_success) - { - HANDLER_DIAG; - - l3 ([&]{trace << "push event { " << ps << " }";}); - - // Cancel the CI tenant associated with the overwritten/deleted previous - // head commit if this is a forced push or a branch deletion. - // - if (ps.forced || ps.deleted) - { - // Service id that will uniquely identify the CI tenant. - // - string sid (ps.repository.node_id + ':' + ps.before); - - // Note that it's possible this commit still exists in another branch so - // we do refcount-aware cancel. - // - if (optional ts = cancel (error, warn, - verb_ ? &trace : nullptr, - *build_db_, retry_, - "ci-github", sid, - true /* ref_count */)) - { - l3 ([&]{trace << (ps.forced ? "forced push " + ps.after + " to " - : "deletion of ") - << ps.ref << ": attempted to cancel CI of previous" - << " head commit with tenant_service id " << sid - << " (ref_count: " << ts->ref_count << ')';}); - } - else - { - // It's possible that there was no CI for the previous commit for - // various reasons (e.g., CI was not enabled). - // - l3 ([&]{trace << (ps.forced ? "forced push " + ps.after + " to " - : "deletion of ") - << ps.ref << ": failed to cancel CI of previous" - << " head commit with tenant_service id " << sid;}); - } - } - - if (ps.deleted) - return true; // Do nothing further if this was a branch deletion. - - // While we don't need the installation access token in this request, - // let's obtain it to flush out any permission issues early. Also, it is - // valid for an hour so we will most likely make use of it. - // - optional jwt (generate_jwt (ps.app_id, trace, error)); - if (!jwt) - throw server_error (); - - optional iat ( - obtain_installation_access_token (ps.installation.id, - move (*jwt), - error)); - if (!iat) - throw server_error (); - - l3 ([&]{trace << "installation_access_token { " << *iat << " }";}); - - // While it would have been nice to cancel CIs of PRs with this branch as - // base not to waste resources, there are complications: Firstly, we can - // only do this for remote PRs (since local PRs will most likely share the - // result with branch push). Secondly, we try to do our best even if the - // branch protection rule for head behind is not enabled. In this case, it - // would be good to complete the CI. So maybe/later. See also the head - // case in handle_pull_request(), where we do cancel remote PRs that are - // not shared. - - // Service id that uniquely identifies the CI tenant. - // - string sid (ps.repository.node_id + ':' + ps.after); - - service_data sd (warning_success, - iat->token, - iat->expires_at, - ps.app_id, - ps.installation.id, - move (ps.repository.node_id), - move (ps.repository.clone_url), - service_data::local, - false /* pre_check */, - false /* re_requested */, - ps.after /* check_sha */, - ps.after /* report_sha */); - - // Create an unloaded CI tenant, doing nothing if one already exists - // (which could've been created by handle_pull_request() or by us as a - // result of a push to another branch). Note that the tenant's reference - // count is incremented in all cases. - // - // Note: use no delay since we need to (re)create the synthetic conclusion - // check run as soon as possible. - // - // Note that we use the create() API instead of start() since duplicate - // management is not available in start(). - // - // After this call we will start getting the build_unloaded() - // notifications until (1) we load the tenant, (2) we cancel it, or (3) - // it gets archived after some timeout. - // - if (!create (error, warn, verb_ ? &trace : nullptr, - *build_db_, retry_, - tenant_service (sid, "ci-github", sd.json ()), - chrono::seconds (30) /* interval */, - chrono::seconds (0) /* delay */, - duplicate_tenant_mode::ignore)) - { - fail << "push " + ps.after + " to " + ps.ref - << ": unable to create unloaded CI tenant"; - } - - return true; - } - function (const string&, const tenant_service&)> ci_github:: build_unloaded (const string& ti, tenant_service&& ts, diff --git a/mod/mod-ci-github.hxx b/mod/mod-ci-github.hxx index 4f69f5c..4fcfa7e 100644 --- a/mod/mod-ci-github.hxx +++ b/mod/mod-ci-github.hxx @@ -82,47 +82,45 @@ namespace brep virtual void init (cli::scanner&); - // @@ TODO Reorder handlers: push, pull_request, then check_suite. - - // Handle the check_suite event `rerequested` action. + // Handle push events (branch push). // // If warning_success is true, then map result_status::warning to SUCCESS // and to FAILURE otherwise. // bool - handle_check_suite_rerequest (gh_check_suite_event, bool warning_success); + handle_branch_push (gh_push_event, bool warning_success); - // Handle the check_suite event `completed` action. + // Handle the pull_request event `opened` and `synchronize` actions. // // If warning_success is true, then map result_status::warning to SUCCESS // and to FAILURE otherwise. // bool - handle_check_suite_completed (gh_check_suite_event, bool warning_success); + handle_pull_request (gh_pull_request_event, bool warning_success); - // Handle the check_run event `rerequested` action. + // Handle the check_suite event `rerequested` action. // // If warning_success is true, then map result_status::warning to SUCCESS // and to FAILURE otherwise. // bool - handle_check_run_rerequest (const gh_check_run_event&, bool warning_success); + handle_check_suite_rerequest (gh_check_suite_event, bool warning_success); - // Handle the pull_request event `opened` and `synchronize` actions. + // Handle the check_suite event `completed` action. // // If warning_success is true, then map result_status::warning to SUCCESS // and to FAILURE otherwise. // bool - handle_pull_request (gh_pull_request_event, bool warning_success); + handle_check_suite_completed (gh_check_suite_event, bool warning_success); - // Handle push events (branch push). + // Handle the check_run event `rerequested` action. // // If warning_success is true, then map result_status::warning to SUCCESS // and to FAILURE otherwise. // bool - handle_branch_push (gh_push_event, bool warning_success); + handle_check_run_rerequest (const gh_check_run_event&, bool warning_success); // Build a check run details_url for a build. // -- cgit v1.1