Skip to content

Commit

Permalink
cmd/cueckoo: do not over-run trybots
Browse files Browse the repository at this point in the history
In case we already have a trybot result for the requested change, there
is no real point requesting them again, unless in exceptional
circumstances.

Similarly, if we flip to drive the running of trybots from a label
(whether using webhooks or not) we only want to trigger the trybots if
someone hasn't already requested them.

We can use labels in both instances to conditionally trigger the
trybots.

This short-circuit can be overridden by passing the --force (or -f for
short) flag to runtrybot.

Otherwise, the trybots are only triggered if neither the Run-TryBot ==
+1 nor TryBot-Result == +1. Note this creates something of an edge case
if someone is triggering the trybots against a specific commit, because
as far as I can tell a label is connected to a change (implicitly the
latest revision) and not a specific commit (potentially not the latest
revision).

Signed-off-by: Paul Jolly <[email protected]>
Change-Id: Ia8ad5b61b6e3754918b73cd0aad39b5818c4fab6
Dispatch-Trailer: {"type":"trybot","CL":1169892,"patchset":1,"ref":"refs/changes/92/1169892/1","targetBranch":"master"}
  • Loading branch information
myitcv authored and cueckoo committed Sep 25, 2023
1 parent caf725e commit a0bbad1
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
18 changes: 17 additions & 1 deletion cmd/cueckoo/cmd/cltrigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,14 +244,30 @@ func (c *cltrigger) triggerBuilds(revs []revision) error {

func (c *cltrigger) triggerBuild(rev revision) error {
in, _, err := c.cfg.gerritClient.Changes.GetChange(rev.changeID, &gerrit.ChangeOptions{
AdditionalFields: []string{"ALL_REVISIONS"},
AdditionalFields: []string{"ALL_REVISIONS", "LABELS"},
})
if err != nil {
// Note that this may be a "change not found" error when the changeID is
// an ambiguous identifier. See [revision.changeID].
return fmt.Errorf("failed to get current revision information: %v", err)
}

// Do not trigger trybots when we already have a request or result for the
// trybots. But only do so we do not have the force flag.
if !flagForce.Bool(c.cmd) {
// Order is significant here; check the request for a trybot first
for _, label := range []string{"Run-TryBot", "TryBot-Result"} {
if tbResult, ok := in.Labels[label]; ok {
for _, approval := range tbResult.All {
// We are looking for a score of 1, regardless of from who
if approval.Value == 1 {
return nil
}
}
}
}
}

commit := rev.revision
if commit == "" {
// fall back to the current/latest revision, also a commit hash
Expand Down
2 changes: 2 additions & 0 deletions cmd/cueckoo/cmd/runtrybot.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
const (
flagChange flagName = "change"
flagRunTrybotNoUnity flagName = "nounity"
flagForce flagName = "force"
)

// newRuntrybotCmd creates a new runtrybot command
Expand Down Expand Up @@ -60,6 +61,7 @@ If the --nounity flag is provided, only a trybot run is triggered.
}
cmd.Flags().Bool(string(flagChange), false, "interpret arguments as change numbers or IDs")
cmd.Flags().Bool(string(flagRunTrybotNoUnity), false, "do not simultaenously trigger unity build")
cmd.Flags().BoolP(string(flagForce), string(flagForce[0]), false, "force the trybots to run, ignoring any results")
return cmd
}

Expand Down

0 comments on commit a0bbad1

Please sign in to comment.