1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-04 12:42:43 +01:00
Commit graph

113 commits

Author SHA1 Message Date
epriestley
937e88c399 Remove obsolete, no-op implementations of "willRenderTimeline()"
Summary:
Depends on D19918. Ref T11351. In D19918, I removed all calls to this method. Now, remove all implementations.

All of these implementations just `return $timeline`, only the three sites in D19918 did anything interesting.

Test Plan: Used `grep willRenderTimeline` to find callsites, found none.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T11351

Differential Revision: https://secure.phabricator.com/D19919
2018-12-20 15:04:49 -08:00
epriestley
01c7be059d Add support for "harbormaster.target.search"
Summary: Ref T13222. See PHI986. See PHI896. Harbormaster build targets don't currently have a modern "*.search" API, but there's no reason not to provide one (even if some of the use cases are a little bit questionable).

Test Plan: {F6032423}

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13222

Differential Revision: https://secure.phabricator.com/D19841
2018-11-28 13:49:27 -08:00
epriestley
614f9ba1fb Allow unit test results to specify that their details are formatted with remarkup when reporting to "harbormaster.sendmessage"
Summary: Ref T13189. See PHI710. Ref T13088. Fixes T9951. Allow callers to `harbormaster.sendmessage` to specify that the test details are remarkup so they can use rich formatting and include links, files, etc.

Test Plan: {F5840098}

Reviewers: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13189, T13088, T9951

Differential Revision: https://secure.phabricator.com/D19615
2018-08-28 13:26:11 -07:00
epriestley
9cf3b3bbf8 Count lines in build log slices more cheaply
Summary:
See PHI766. Ref T13164. Build log chunk processing does a `preg_split()` on slices, but this isn't terribly efficient.

We can get the same count more cheaply by just using `substr_count()` a few times.

(I also tried `preg_match_all()`, which was between the two in speed.)

Test Plan:
- Used `bin/harbormaster rebuild-log --id X --force` to rebuild logs. Verified that the linemap is identical before/after this change.
- Saw local time for the 18MB log in PHI766 drop from ~1.7s to ~900ms, and `preg_split()` drop out of the profiler (we're now spending the biggest chunk of time on `gzdeflate()`).

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13164

Differential Revision: https://secure.phabricator.com/D19545
2018-07-30 08:25:17 -07:00
epriestley
a817aa6c71 Add an "Abort Older Builds" build step to Harbormaster
Summary:
Ref T13124. See PHI531. When a revision is updated, builds against the older diff tend to stop being relevant. Add an option to abort outstanding older builds automatically.

At least for now, I'm adding this as a build step instead of some kind of special checkbox. An alternate implementation would be some kind of "Edit Options" action on plans with a checkbox like `[X] When this build starts, abort older builds.`

I think adding it as a build step is a bit simpler, and likely to lead to greater consistency and flexibility down the road, make it easier to add options, etc., and since we don't really have any other current use cases for "a bunch of checkboxes". This might change eventually if we add a bunch of checkboxes for some other reason.

The actual step activates //before// the build queues, so it doesn't need to wait in queue before it can actually act. T13088 discusses some plans here if this sticks.

Test Plan:
  - Created a "Sleep for 120 seconds" build plan and triggered it with Herald.
  - Added an "Abort Older Builds" step.
  - Updated a revision several times in a row, saw older builds abort.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13124

Differential Revision: https://secure.phabricator.com/D19376
2018-04-17 14:59:47 -07:00
epriestley
7f9a9bc800 Make Harbormaster objects destructible
Summary:
Ref T13114. See PHI511. Ref T13072. This makes Buildables, Builds, Targets and Artifacts destructible with `bin/remove destroy`.

This might not be totally exhaustive. In particular:

  - File artifacts won't destroy the file. This is sort of okay because file artifacts are currently just a file reference, but probably shouldn't be how things work in the long term.
  - `BuildCommand` doesn't get cleaned up, but `BuildMessage` does on `Build`. See T13072 for more.

Test Plan: Used `bin/remove destroy` to nuke a bunch of builds, buildables, etc. Loaded stuff in the web UI and it all looked like it got nuked properly.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13114, T13072

Differential Revision: https://secure.phabricator.com/D19269
2018-03-29 13:01:14 -07:00
epriestley
49af4165bc Support rendering arbitrary sections in the middle of a Harbormaster build log so links to line 3500 work
Summary:
Depends on D19162. Ref T13088. When a user links to `$1234`, we need to render a default view of the log with a piece at the head, a piece at the end, and a piece in the middle.

We also need to figure out the offset for line 1234, or multiple offsets for "1234-2345".

Since the logic views/reads mostly anticipated this it isn't too much of a mess, although there are a couple of bugs this exposes with view specifications that use combinations of parameters which were previously impossible.

Test Plan: Viewed a large log with no line marker. Viewed `$1`. Viewed `$end`. Viewed `$35-40`, etc. Expanded context around logs.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19163
2018-03-01 11:18:21 -08:00
epriestley
21ddfe442e Add a "--rate" flag to bin/harbormaster write-log to support testing live log streaming
Summary: Depends on D19151. Ref T13088. While dramatically less exciting than using `lolcat` and less general than `pv`, this should do the job adequately.

Test Plan: Piped a sizable log into `bin/harbormaster write-log` with `--rate 2048`, saw a progress bar. Loaded the log in the web UI and saw it grow as the page reloaded.

Reviewers: yelirekim

Reviewed By: yelirekim

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19152
2018-02-28 12:37:04 -08:00
epriestley
5a2213ef82 Provide API read access to Harbormaster build logs
Summary:
Depends on D19150. Ref T13088. Allow clients to retrieve information about build logs, including log data, over the API.

(To fetch log data, take the `filePHID` to `file.search`, then issue a normal GET against the URI. Use a `Content-Range` header to get part of the log.)

Test Plan: Ran `harbormaster.log.search`, got sensible-looking results.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19151
2018-02-28 12:36:03 -08:00
epriestley
143033dc1f When showing a small piece of a Harbormaster build log, load a small piece of data instead of the entire log
Summary: Depends on D19148. Ref T13088. The new rendering always executes range requests for data it needs, and we can satisfy these requests by loading the smallest number of chunks which span that range.

Test Plan: Piped 50,000 lines of Apache log into Harbormaster, viewed it in the new UI, got sensible rendering times and a reasonable amount of data actually going over the wire.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19149
2018-02-28 12:32:26 -08:00
epriestley
11d1dc484b Sort of make Harbormaster build logs page properly
Summary: Depends on D19139. Ref T13088. This doesn't actually work, but is close enough that a skilled attacker might be able to briefly deceive a small child.

Test Plan:
  - Viewed some very small logs under very controlled conditions, saw content.
  - Larger logs vaguely do something resembling working correctly.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19141
2018-02-26 17:58:33 -08:00
epriestley
6dc341be87 As Harbormaster logs are processed, build a sparse map of byte offsets to line numbers
Summary:
Depends on D19138. Ref T13088. When we want to read the last part of a logfile //and show accurate line numbers//, we need to be able to get from byte offsets to line numbers somehow.

Our fundamental unit must remain byte offsets, because a test can emit an arbitrarily long line, and we should accommodate it cleanly if a test emits 2GB of the letter "A".

To support going from byte offsets to line numbers, compute a map with periodic line markers throughout the offsets of the file. From here, we can figure out the line numbers for arbitrary positions in the file with only a constant amount of work.

Test Plan: Added unit tests; ran unit tests.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19139
2018-02-26 17:56:52 -08:00
epriestley
d6311044bb Store the Harbormaster log chunk format on the log record
Summary: Depends on D19137. Ref T13088. This allows `rebuild-log` to skip work if the chunks are already compressed. It also prepares for a future GC which is looking for "text" or "gzip" chunks to throw away in favor of archival into Files; such a GC can use this column to find collectable logs and then write "file" to it, meaning "chunks are gone, this data is only available in Files".

Test Plan: Ran migration, saw logs populate as "text". Ran `rebuild-log`, saw logs rebuild as "gzip".

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19138
2018-02-26 17:56:14 -08:00
epriestley
57e3d607f5 In Harbormaster, record byte length on the build logs
Summary: Depends on D19135. Ref T13088. Denormalize the total log size onto the log itself. This makes reasoning about the log at display time easier, and we don't need to fish around in the database as much to figure out what we're dealing with.

Test Plan: Ran `bin/harbormaster rebuild-log`, saw an existing log populate. Ran `bin/harbormaster write-log`, saw new log write with proper length information.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19136
2018-02-26 17:54:47 -08:00
epriestley
d152bd5836 Manage log locks on the Log object to prepare for multiple writers
Summary:
Depends on D19134. Ref T13088. Future changes will support API writers, so push the log lock into the Log object.

Allow open/close ("this process is writing to this log") to be separate from live/final ("this log is still generating more data").

Test Plan: Wrote logs with `bin/harbormater write-log` and updated logs with `bin/harbormaster rebuild-log`.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19135
2018-02-26 17:54:17 -08:00
epriestley
e920e2b143 Implement DestructibleInterface on BuildLog
Summary: Depends on D19133. Ref T13088. Allows build logs to be formally destroyed, cleaning up their chunks and file data.

Test Plan:
  - Used `bin/remove destroy` to destroy a log, verified chunks and files were removed.
  - Used `bin/harbormaster rebuild-log` to force a log to rebuild, verified files were destroyed and regenerated.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19134
2018-02-26 17:53:38 -08:00
epriestley
9b4295ed60 Add a very basic standalone view for build logs with a "Download Log" button
Summary: Depends on D19132. Ref T13088. This implements an extremely skeletal dedicated log page with a more-or-less functional "Download Log" button.

Test Plan: Downloaded a recent log. Tried to download an old (un-finalized) log, couldn't. Used `bin/harbormaster write-log` to get a convenient standalone link to a log.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19133
2018-02-26 17:53:10 -08:00
epriestley
8a2604cf06 Add a "filePHID" to HarbormasterBuildLog and copy logs into Files during finalization
Summary: Depends on D19131. Ref T13088. During log finalization, stream the log into Files to support "Download Log", archive to Files, and API access.

Test Plan: Ran `write-log` and `rebuild-log`, saw Files objects generate with log content and appropriate permissions.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19132
2018-02-26 17:52:39 -08:00
epriestley
32c6b649dd Move Harbormaster log compression to the worker task queue
Summary: Depends on D19130. Ref T13088. Currently, when a build log is closed we compress it in the same process. Separate this out into a dedicated worker since the plan is to do a lot more work during finalization, none of which needs to happen inline during builds (or, particuarly, inline during a Conduit call for API writes in the future).

Test Plan: Ran `bin/harbormaster write-log --trace`, saw compression run inline.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13088

Differential Revision: https://secure.phabricator.com/D19131
2018-02-26 17:51:58 -08:00
epriestley
a2d02aed22 When a build is aborted, fail the buildable
Summary:
Ref T13054. Fixes T10746. Fixes T11154. This is really a one-line fix (include `ABORTED` in `BuildEngine->updateBuildable()`) but try to structure the code a little more clearly too and reduce (at least slightly) the number of random lists of status attributes spread throughout the codebase.

Also add a header tag for buildable status.

Test Plan: Aborted a build, saw buildable fail properly.

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13054, T11154, T10746

Differential Revision: https://secure.phabricator.com/D19055
2018-02-10 16:08:41 -08:00
epriestley
66b74073be Provide ANSI color information for Harbormaster build status via API
Summary:
Ref PHI261. This moves Harbormaster build status to work more similarly to other modern status types, like Differential revision status, where we try to specify as much behavior on the server as possible so that the client and server can vary independently.

(I don't have any specific plans to make Harbormaster build status configurable on the server side, but it isn't out of the question.)

Test Plan: Ran `harbormaster.querybuilds` (saw same data as before) and `harbormaster.build.search` (saw same data as before, plus new ANSI color data).

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D18838
2017-12-23 11:39:05 -08:00
epriestley
a03da0c2af Add a missing artifactIndex key to HarbormasterBuildArtifact
Summary: See PHI176. We issue a query with only `artifactIndex` from `BuildTarget`, but don't have an applicable key.

Test Plan: This isn't on the normal Harbormaster execution path so I'm not 100% sure I have a local repro, but will confirm with customer.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D18732
2017-10-26 18:18:24 -07:00
Josh Cox
dda06c6bdc Added a 'name' field to the results for harbormaster.build.search endpoint
Summary: Fixes T11642. Added a 'name' field to the results from harbormaster.build.search.

Test Plan: Went to `/conduit/method/harbormaster.build.search/` and ran a search that would yield results (because otherwise there will be nothing there). Noted that there was, in fact, a name in the results.

Reviewers: yelirekim, #blessed_reviewers, epriestley

Reviewed By: yelirekim, #blessed_reviewers, epriestley

Subscribers: epriestley, yelirekim

Maniphest Tasks: T11642

Differential Revision: https://secure.phabricator.com/D16569
2016-09-19 13:15:52 -04:00
Mike Riley
98492765d3 Subsume 'harbormaster.querybuilds' with a modern search API method
Summary: We deprecate the existing API method used to access build information from the API, but preserve its response structure after calling through to the new method.  I've cordoned off the fields I needed to define in order to meet the output structure by putting those fields in a search attachment.

Test Plan:
Used the API console and looked at the list view controller for builds.

Old output structure:

```lang=json
{
  "data": [
    {
      "id": "16823",
      "phid": "PHID-HMBD-xghrwfz6luoye5rgc2hq",
      "uri": "https://secure.phabricator.com/harbormaster/build/16823/",
      "name": "Run Core Tests",
      "buildablePHID": "PHID-HMBB-s6ykzm2jzxz4ymduztq3",
      "buildPlanPHID": "PHID-HMCP-pcfxcgyoif67l3buc4zt",
      "buildStatus": "passed",
      "buildStatusName": "Passed"
    }
  ],
  "cursor": {
    "limit": 100,
    "after": "16823",
    "before": null
  }
}
```

New output structure:

```lang=json
{
  "data": [
    {
      "id": 1,
      "type": "HMBD",
      "phid": "PHID-HMBD-qpgcmv67tzaauzayzit5",
      "uri": "http://ec2-54-165-244-168.compute-1.amazonaws.com/harbormaster/build/1/",
      "name": "arc lint + arc unit",
      "buildStatusName": "Passed",
      "buildablePHID": "PHID-HMBB-qdefith5uakkepqpjr2g",
      "buildPlanPHID": "PHID-HMCP-zswbhazb7ipmaf4plygg",
      "buildStatus": "passed",
      "initiatorPHID": "PHID-USER-rihx4366f3aczsvc2wtb",
      "dateCreated": 1450295643,
      "dateModified": 1450295644,
      "policy": {
        "view": "users",
        "edit": "users"
      }
    }
  ],
  "maps": {},
  "query": {
    "queryKey": null
  },
  "cursor": {
    "limit": 100,
    "after": null,
    "before": null,
    "order": null
  }
}
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D16356
2016-07-31 21:44:22 +00:00
Mike Riley
4865dbdff1 Search builds based on who kicked them off
Summary:
It's only natural for users to be interested their own builds. We are also building in support for other sources of builds, the only formally supported way to run a build right now is via Herald.

In our third party codebase, we designate an application as the "thing" that started builds which are scheduled and managed automatically by phabricator. I believe this is a common practice elsewhere in the codebase when you're at a loss for a real human identity and you need to apply some transactions.

Test Plan: Ran some builds manually and saw them show up under the list of things I've run.  Looking up builds based on those that had been started by a herald rule.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D16353
2016-07-31 20:54:44 +00:00
Mike Riley
33fca12816 Pick some preset build statuses
Summary:
We're picking three useful groups of build statuses to provide as default queries:

 - Stuff not yet building
 - Stuff building
 - Stuff which has finished building

These are reasonable buckets for builds since (unlike most objects in phabricatorland) users are generally waiting impatiently for the machine to do something for them, rather than being responsible for doing something with the machine.

Test Plan: clicked around the search engine and enjoyed my defaults

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D16349
2016-07-31 15:35:18 +00:00
Mike Riley
42b81a8090 Move build statuses to a constants class
Summary: No functional changes here, just lifting this out to make room for activities, heeding lint warnings along the way.

Test Plan:
before:
```lang=bash
$ grep -Rn "HarbormasterBuild::" *
src/applications/harbormaster/storage/HarbormasterBuildable.php:169:    $build = HarbormasterBuild::initializeNewBuild($viewer)
src/applications/harbormaster/storage/HarbormasterBuildable.php:173:      ->setBuildStatus(HarbormasterBuild::STATUS_PENDING);
src/applications/harbormaster/controller/HarbormasterStepEditController.php:242:    $variables = HarbormasterBuild::getAvailableBuildVariables();
src/applications/harbormaster/controller/HarbormasterBuildableViewController.php:200:        'fa-dot-circle-o '.HarbormasterBuild::getBuildStatusColor($status),
src/applications/harbormaster/controller/HarbormasterBuildableViewController.php:201:        HarbormasterBuild::getBuildStatusName($status));
src/applications/harbormaster/controller/HarbormasterBuildableViewController.php:203:      $item->addAttribute(HarbormasterBuild::getBuildStatusName($status));
src/applications/harbormaster/controller/HarbormasterBuildViewController.php:584:        HarbormasterBuild::getBuildStatusName($status);
src/applications/harbormaster/controller/HarbormasterBuildViewController.php:585:      $icon = HarbormasterBuild::getBuildStatusIcon($status);
src/applications/harbormaster/controller/HarbormasterBuildViewController.php:586:      $color = HarbormasterBuild::getBuildStatusColor($status);
src/applications/harbormaster/event/HarbormasterUIEventListener.php:135:      $status_name = HarbormasterBuild::getBuildStatusName($status);
src/applications/harbormaster/event/HarbormasterUIEventListener.php:136:      $icon = HarbormasterBuild::getBuildStatusIcon($status);
src/applications/harbormaster/event/HarbormasterUIEventListener.php:137:      $color = HarbormasterBuild::getBuildStatusColor($status);
src/applications/harbormaster/conduit/HarbormasterQueryBuildsConduitAPIMethod.php:78:        'buildStatusName' => HarbormasterBuild::getBuildStatusName($status),
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:66:      $build->setBuildStatus(HarbormasterBuild::STATUS_ERROR);
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:109:      $build->setBuildStatus(HarbormasterBuild::STATUS_ABORTED);
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:113:    if (($build->getBuildStatus() == HarbormasterBuild::STATUS_PENDING) ||
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:116:      $build->setBuildStatus(HarbormasterBuild::STATUS_BUILDING);
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:121:      $build->setBuildStatus(HarbormasterBuild::STATUS_BUILDING);
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:126:      $build->setBuildStatus(HarbormasterBuild::STATUS_PAUSED);
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:132:    if ($build->getBuildStatus() == HarbormasterBuild::STATUS_BUILDING) {
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:246:      $build->setBuildStatus(HarbormasterBuild::STATUS_FAILED);
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:254:      $build->setBuildStatus(HarbormasterBuild::STATUS_PASSED);
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:290:      $build->setBuildStatus(HarbormasterBuild::STATUS_DEADLOCKED);
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:446:      if ($build->getBuildStatus() != HarbormasterBuild::STATUS_PASSED) {
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:449:      if ($build->getBuildStatus() == HarbormasterBuild::STATUS_FAILED ||
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:450:          $build->getBuildStatus() == HarbormasterBuild::STATUS_ERROR ||
src/applications/harbormaster/engine/HarbormasterBuildEngine.php:451:          $build->getBuildStatus() == HarbormasterBuild::STATUS_DEADLOCKED) {
```

after:

```lang=bash
$ grep -Rn "HarbormasterBuild::" *
src/applications/harbormaster/storage/HarbormasterBuildable.php:169:    $build = HarbormasterBuild::initializeNewBuild($viewer)
src/applications/harbormaster/controller/HarbormasterStepEditController.php:242:    $variables = HarbormasterBuild::getAvailableBuildVariables();
```

ran a manual build as a sanity check

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D16348
2016-07-31 14:56:31 +00:00
Mike Riley
2c55a4ad72 Provide a basic search engine for builds
Summary:
This supports a few basic use cases that aren't served by the buildable search engine:

 - I'm trying to discover when the last time that this particular build plan failed was.
 - I want to know if any builds have deadlocked.
 - At a glance, I'm more interested in what build plans are running, not which buildables are being built. This is more often than not the case.

Test Plan: {F1744003}

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D16347
2016-07-31 13:35:31 +00:00
epriestley
96c51028e5 In Harbormaster, release artifacts as soon as no waiting/running build steps will use them
Summary:
Ref T11153. If you have a build plan like this:

  - Lease machine A.
  - Lease machine B.
  - Run client-tests on machine A.
  - Run server-tests on machine B.

...and we get machine A quickly, then finish the tests, we currently do not release machine A until the whole plan finishes.

In the best case, this wastes resources (something else could be using that machine for a while).

In a worse case, this wastes a lot of resources (if machine B is slow to acquire, or the server tests are much slower than the client tests, machine A will get tied up for a really long time).

In the absolute worst case, this might deadlock things.

Instead, release artifacts as soon as no waiting/running steps take them as inputs. In this case, we'd release machine A as soon as we finished running the client tests.

In the case where machines A and B are resources of the same type, this should prevent deadlocks. In all cases, this should improve build throughput at least somewhat.

Test Plan:
I wrote this build plan which runs a "fast" step (10 seconds) and a "slow" step (120 seconds):

{F1691190}

Before the patch, running this build plan held the lease on the "fast" machine for the full 120 seconds, then released both leases at the same time at the very end.

After this patch, I ran this plan and observed the "fast" lease get released after 10 seconds, while the "slow" lease was held for the full 120.

(Also added some `var_dump()` into things to sanity check the logic; it appeared correct.)

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11153

Differential Revision: https://secure.phabricator.com/D16145
2016-06-17 16:13:56 -07:00
epriestley
2ddd78647b Improve "Land Revision" errors for issues related to staging areas
Summary: Ref T10093. Changes must be pushed to staging before they can be landed from the web.

Test Plan:
Changes must be pushed to staging before they can be landed from the web.

{F1161909}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10093

Differential Revision: https://secure.phabricator.com/D15427
2016-03-07 12:46:07 -08:00
epriestley
078bf59f59 Compress Harbormaster build logs inline
Summary:
Ref T5822.

  - After a log is closed, compress it if possible.
  - Provide `bin/harbormaster archive-logs` to make it easier to change the storage format of logs.

Test Plan:
  - Ran `bin/harbormaster archive-logs` on a bunch of logs, compressing and decompressing them without issues (same hashes, same decompressed size across multiple iterations).
  - Ran new builds, verified logs were compressed after they closed.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5822

Differential Revision: https://secure.phabricator.com/D15380
2016-03-01 15:26:12 -08:00
epriestley
6514237c0e Implement an iterator for build log chunks
Summary: Ref T5822. This will make it easier to compress and archive chunks without needing to hold them in memory.

Test Plan: Ran a build, looked at some logs.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5822

Differential Revision: https://secure.phabricator.com/D15378
2016-03-01 15:25:51 -08:00
epriestley
e174cac1b4 Give HarbormasterBuildLogChunk a real table
Summary:
Ref T10457. Currently, this table is an ad-hoc table, but can easily be turned into a normal table.

This will make iterating over log chunks to compress and archive them easier.

Test Plan: Viewed logs, ran `bin/storage adjust` with no issues.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10457

Differential Revision: https://secure.phabricator.com/D15376
2016-03-01 15:25:34 -08:00
epriestley
0daa9ad987 Use PhutilRope as a buffer in Harbormaster BuildLogs
Summary:
Ref T10457. Currently, every `append()` call necessarily generates queries, and these queries are slightly inefficient if a large block of data is appended to a partial log (they do about twice as much work as they technically need to).

Use `PhutilRope` to buffer `append()` input so the logic is a little cleaner and we could add a rule like "flush logs no more than once every 500ms" later.

Test Plan:
  - Ran a build, saw logs.
  - Set chunk size very small, ran build, saw logs, verified small logs in database.

{F1137115}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10457

Differential Revision: https://secure.phabricator.com/D15375
2016-03-01 15:25:19 -08:00
epriestley
cf0957451e Slightly simplify the Harbormaster build log API
Summary:
Ref T5822. This prepares for inline compression and garbage collection of build logs.

This reduces the API surface area and removes a log from the "wait" step that would just log a message every 15 seconds. (If this is actually useful, I think we find a better way to communicate it.)

Test Plan:
Ran a build, saw a log:

{F1136691}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5822

Differential Revision: https://secure.phabricator.com/D15371
2016-03-01 15:25:04 -08:00
epriestley
d436fecdab Show additional details for failed builds in Harbormaster
Summary:
Ref T10457. When tests fail, it currently takes several clicks to figure out //why// they failed.

In this project, map rebuilds and `liberate` are fairly common failure conditions, but verifying that they were the root issue requires jumping into a build, then scrolling through a log.

Instead, display details if they're available.

Test Plan:
Before:

{F1135453}

After:

{F1135454}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9951, T10457

Differential Revision: https://secure.phabricator.com/D15363
2016-02-29 14:26:45 -08:00
epriestley
3c19b72ca0 Begin making Harbormaster unit test results a little easier to read
Summary: Ref T10457. These lack color and iconography and are difficult to parse. Make them easier to read.

Test Plan:
Before:

{F1135396}

After:

{F1135399}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10457

Differential Revision: https://secure.phabricator.com/D15362
2016-02-29 14:24:08 -08:00
epriestley
93b8f803a0 Require "Can Edit" on a build plan to abort or pause associated builds
Summary: Fixes T9614. This is kind of silly, but stop users from fighting turf wars over build resources or showing up on an install and just aborting a bunch of builds for the heck of it.

Test Plan:
  - Restarted / paused / aborted / etc builds.
  - Tried to do the same for builds I didn't have edit permission on the build plan for, got errors.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9614

Differential Revision: https://secure.phabricator.com/D15353
2016-02-26 11:38:43 -08:00
epriestley
6ebe8db380 Add a missing key to HarbormasterBuildArtifact
Summary: Fixes T10192. This key improves some common queries and is not currently present.

Test Plan: See discussion in T10192. Verified current query plan of real queries is garbage and improved by adding this key.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10192

Differential Revision: https://secure.phabricator.com/D15075
2016-01-21 10:21:19 -08:00
Aviv Eyal
1898864b6c add initiator.phid parameter to HM builds
Summary:
Fix T9662.

Record who initiated the build, and allow this information as a parameter.

In this implementation, a 're-run' keeps the original initiator, which we maybe not desired?

Test Plan:
Make a HTTP step with initiator.phid, trigger manually, via HM, via ./bin/harbormaster build.
Look at requests made.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T9662

Differential Revision: https://secure.phabricator.com/D14380
2015-11-04 18:32:18 +00:00
epriestley
2728a9f964 Allow builds to have parameters
Summary: Ref T9352. See D13635. Build targets can have variables already, but let builds have them too. This mostly enables future use cases (sub-builds, more sophisticated build triggers).

Test Plan: With a custom Herald rule + action like the one in T9352, updated a revision and saw it generate multiple builds with varying parameters.

Reviewers: chad, hach-que

Reviewed By: hach-que

Maniphest Tasks: T9352

Differential Revision: https://secure.phabricator.com/D14222
2015-10-02 06:32:08 -07:00
epriestley
4ac82be5ed Merge the DrydockLease workers into a single worker
Summary:
Ref T9252. This is the same as D14201, but for lease stuff instead of resource stuff.

This one is a little heavier but still feels pretty reasonable to me at the end of the day (worker is <1K lines and has a ton of comment stuff).

Also fixes a few random bugs I hit in the task queue.

Test Plan:
  - Restarted some Harbormaster builds, saw them go through cleanly.
  - Released pre-activation resources/leases.
  - Probably still kinda buggy but I'll iron the details out over time.

Logs are starting to look somewhat plausible:

{F855747}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9252

Differential Revision: https://secure.phabricator.com/D14202
2015-10-01 08:11:02 -07:00
epriestley
a5c4177160 Fix an issue with BuildLogs and web UI during builds
Summary:
Ref T9123. After recent changes, viewing a live build log from the web UI throws a CSRF exception.

Check `start` ("this object is an active log"), not `live` ("this log is open somewhere").

Test Plan: Will push / etc.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9123

Differential Revision: https://secure.phabricator.com/D14185
2015-09-28 19:11:51 -07:00
epriestley
bfaa93aa9b Allow Harbormaster build plans to request additional working copies
Summary:
Ref T9123. To run upstream builds in Harbormaster/Drydock, we need to be able to check out `libphutil`, `arcanist` and `phabricator` next to one another.

This adds an "Also Clone: ..." field to Harbormaster working copy build steps so I can type all three repos into it and get a proper clone with everything we need.

This is somewhat upstream-centric and a bit narrow, but I don't think it's totally unreasonable, and most of the underlying stuff is relatively general.

This adds some more typechecking and improves data/type handling for custom fields, too. In particular, it prevents users from entering an invalid/restricted value in a field (for example, you can't "Also Clone" a repository you don't have permission to see).

Test Plan: Restarted build, got a Drydock resource with multiple repositories in it.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9123

Differential Revision: https://secure.phabricator.com/D14183
2015-09-28 17:57:41 -07:00
epriestley
d735c7adf2 Allow Harbormaster to run commands on Drydock working copies
Summary: Ref T9252. This mostly cleans up future and log handling, and edges us closer to being able to do useful work with Harbormaster / Drydock.

Test Plan:
  - Added a "Run `ls -alh`" step to my trivial build plan.
  - Ran it a bunch of times.
  - Worked great.
  - Also did an HTTP plan.

{F835227}

{F835228}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9252

Differential Revision: https://secure.phabricator.com/D14161
2015-09-25 10:43:32 -07:00
epriestley
8ce90a7c42 Allow lint codes to be up to 128 bytes long
Summary:
Fixes T9145. We currently restrict lint codes to 32 bytes, but PHPCS generates codes like "PHPCS.E.PEAR.Comments.Messages.Line.TooLong".

These codes seem reasonable as codes, and we don't currently have any key-length problems or other technical concerns with simply raising the size of this column.

Test Plan: Ran `bin/storage upgrade` to pick up adjustments.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9145

Differential Revision: https://secure.phabricator.com/D14166
2015-09-25 10:42:57 -07:00
epriestley
284fe0fe51 Allow Harbormaster to lease working copies from Drydock
Summary: Ref T9252. This is still crude in a few ways but basically works, at least for commits.

Test Plan:
  - Made a build plan with just this build step.
  - Ran `bin/harbormaster build --plan 10 ...` on a commit.
  - It actually built a working copy, leased it, took no action, and released the lease. MAGIC~~~

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9252

Differential Revision: https://secure.phabricator.com/D14160
2015-09-24 17:29:47 -07:00
June Rhodes
d5dc4588fc [harbormaster/abort-builds] Support aborting builds in Harbormaster
Summary: Ref T1049.  This adds support for aborting builds in Harbormaster.

Test Plan: Tested it in production.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: yelirekim, traviscline, joshuaspence, Korvin, epriestley

Projects: #harbormaster

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D11870
2015-09-21 12:07:38 -07:00
epriestley
57b0353034 Add harbormaster.createartifact
Summary:
Ref T8659. In the general case, this eventually allows build processes to do things like:

  - Upload build results (like a ".app" or ".exe" or other binary).
  - Pass complex results between build steps (e.g., build step A does something hard and build step B uses it to do something else).

Today, we're a long way away from having the infrastructure for that. However, it is useful to let third party build processes (like Jenkins) upload URIs that link back to the external build results.

This adds `harbormaster.createartifact` so they can do that. The only useful thing to do with this method today is have your Jenkins build do this:

  params = array(
    "uri": "https://jenkins.mycompany.com/build/23923/details/",
    "name": "View Build Results in Jenkins",
    "ui.external": true,
  );
  harbormaster.createartifact(target, 'uri', params);

Then (after the next diff) we'll show a link in Differential and a prominent link in Harbormaster. I didn't actually do the UI stuff in this diff since it's already pretty big.

This change moves a lot of code around, too:

  - Adds PHIDs to artifacts.
  - It modularizes build artifact types (currently "file", "host" and "URI").
  - It formalizes build artifact parameters and construction:
    - This lets me generate usable documentation about how to create artifacts.
    - This prevents users from doing dangerous or policy-violating things.
  - It does some other general modernization.

Test Plan:
{F715633}

{F715634}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8659

Differential Revision: https://secure.phabricator.com/D13900
2015-08-15 07:28:56 -07:00
Joshua Spence
368f359114 Use PhutilClassMapQuery instead of PhutilSymbolLoader
Summary: Use `PhutilClassMaQuery` instead of `PhutilSymbolLoader`, mostly for consistency. Depends on D13588.

Test Plan: Poked around a bunch of pages.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13589
2015-08-14 07:49:01 +10:00