Skip to content

Commit 0de3ab4

Browse files
authored
feat: Plumbs content hash to post-submit LUCI builds (#4795)
This change plumbs the content-aware hash from the scheduler to the LUCI build service for post-submit builds. This will allow LUCI/recipes to use the content hash in post-submit builds (windows arm, ddm, etc). The following changes are included: - The `ContentAwareHashService` is updated to include a `getHashByCommitSha` method. - The `Scheduler` now retrieves the content hash on addCommit() and passes it to `LuciBuildService`. - `LuciBuildService` plumbs the `contentHash` through to buildbucket properties. - Tests have been added to verify that the content hash is correctly passed to the `LuciBuildService`. Extension of flutter/flutter#167779
1 parent 2d6f9cd commit 0de3ab4

File tree

9 files changed

+175
-7
lines changed

9 files changed

+175
-7
lines changed

app_dart/lib/src/service/content_aware_hash_service.dart

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,31 @@ interface class ContentAwareHashService {
336336
);
337337
return contentHash.waitingShas;
338338
}
339+
340+
/// Looks up a content hash via its [commitSha] and returns the hash if
341+
/// the document is found.
342+
///
343+
/// Note: This only returns the content hash if the commitSha was used to
344+
/// build the engine artifacts. It is not a generic "what is the hash" of
345+
/// this git commitSha.
346+
Future<String?> getHashByCommitSha(String commitSha) async {
347+
final docs = await _firestore.query(
348+
ContentAwareHashBuilds.metadata.collectionId,
349+
{'${ContentAwareHashBuilds.fieldCommitSha} =': commitSha},
350+
orderMap: {
351+
ContentAwareHashBuilds.fieldCreateTimestamp: kQueryOrderDescending,
352+
},
353+
);
354+
if (docs.isEmpty) {
355+
return null;
356+
}
357+
if (docs.length > 1) {
358+
log.warn(
359+
'CAHS(commitSha: $commitSha): getHashByCommitSha - multiple hashes found; using latest',
360+
);
361+
}
362+
return ContentAwareHashBuilds.fromDocument(docs.first).contentHash;
363+
}
339364
}
340365

341366
enum MergeQueueHashStatus { wait, build, complete, ignoreJob, error }

app_dart/lib/src/service/luci_build_service.dart

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,7 @@ class LuciBuildService {
607607
Future<List<PendingTask>> schedulePostsubmitBuilds({
608608
required CommitRef commit,
609609
required List<PendingTask> toBeScheduled,
610+
String? contentHash,
610611
}) async {
611612
if (toBeScheduled.isEmpty) {
612613
log.debug(
@@ -639,12 +640,16 @@ class LuciBuildService {
639640
log.info(
640641
'create postsubmit schedule request for target: ${pending.target} in commit ${commit.sha}',
641642
);
643+
final properties = <String, Object>{
644+
if (contentHash != null) 'content_hash': contentHash,
645+
};
642646
final scheduleBuildRequest = await _createPostsubmitScheduleBuild(
643647
commit: commit,
644648
target: pending.target,
645649
taskName: pending.taskName,
646650
priority: pending.priority,
647651
currentAttempt: pending.currentAttempt,
652+
properties: properties,
648653
);
649654
buildRequests.add(
650655
bbv2.BatchRequest_Request(scheduleBuild: scheduleBuildRequest),
@@ -703,10 +708,13 @@ class LuciBuildService {
703708
'create postsubmit schedule request for target: $target in commit ${commit.sha}',
704709
);
705710

711+
final properties = <String, Object>{
712+
if (contentHash != null) 'content_hash': contentHash,
713+
};
706714
final scheduleBuildRequest = await _createMergeGroupScheduleBuild(
707715
commit: commit,
708716
target: target,
709-
extraProperties: {if (contentHash != null) 'content_hash': contentHash},
717+
properties: properties,
710718
);
711719
buildRequests.add(
712720
bbv2.BatchRequest_Request(scheduleBuild: scheduleBuildRequest),
@@ -850,7 +858,7 @@ class LuciBuildService {
850858
);
851859

852860
final processedProperties = target.getProperties().cast<String, Object?>();
853-
processedProperties.addAll(properties ?? <String, Object?>{});
861+
if (properties != null) processedProperties.addAll(properties);
854862
processedProperties['git_branch'] = commit.branch;
855863
processedProperties['git_repo'] = commit.slug.name;
856864

@@ -915,7 +923,7 @@ class LuciBuildService {
915923
required CommitRef commit,
916924
required Target target,
917925
int priority = kDefaultPriority,
918-
Map<String, String> extraProperties = const {},
926+
Map<String, Object?>? properties,
919927
}) async {
920928
log.info(
921929
'Creating merge group schedule builder for ${target.name} on commit ${commit.sha}',
@@ -938,7 +946,7 @@ class LuciBuildService {
938946
processedProperties['is_fusion'] = 'true';
939947
processedProperties[kMergeQueueKey] = true;
940948
processedProperties['git_repo'] = commit.slug.name;
941-
processedProperties.addAll(extraProperties);
949+
if (properties != null) processedProperties.addAll(properties);
942950

943951
final propertiesStruct =
944952
bbv2.Struct()..mergeFromProto3Json(processedProperties);

app_dart/lib/src/service/scheduler.dart

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ class Scheduler {
149149
log.debug('Skipping ${commit.sha} as repo is not supported');
150150
return;
151151
}
152+
final contentHash = await _contentAwareHash.getHashByCommitSha(commit.sha);
152153

153154
final _TaskCommitScheduling scheduling;
154155
if (Config.defaultBranch(commit.slug) == commit.branch) {
@@ -216,7 +217,11 @@ class Scheduler {
216217
'Immediately scheduled tasks for $commit: '
217218
'${toBeScheduled.map((t) => '"${t.taskName}"').join(', ')}',
218219
);
219-
await _batchScheduleBuilds(commit.toRef(), toBeScheduled);
220+
await _batchScheduleBuilds(
221+
commit.toRef(),
222+
toBeScheduled,
223+
contentHash: contentHash,
224+
);
220225
await _uploadToBigQuery(commit);
221226
}
222227

@@ -234,8 +239,9 @@ class Scheduler {
234239
/// Each batch request contains [Config.batchSize] builds to be scheduled.
235240
Future<void> _batchScheduleBuilds(
236241
CommitRef commit,
237-
List<PendingTask> toBeScheduled,
238-
) async {
242+
List<PendingTask> toBeScheduled, {
243+
String? contentHash,
244+
}) async {
239245
final batchLog = StringBuffer(
240246
'Scheduling ${toBeScheduled.length} tasks in batches for ${commit.sha} as follows:\n',
241247
);
@@ -250,6 +256,7 @@ class Scheduler {
250256
_luciBuildService.schedulePostsubmitBuilds(
251257
commit: commit,
252258
toBeScheduled: batch,
259+
contentHash: contentHash,
253260
),
254261
);
255262
}

app_dart/test/request_handlers/scheduler/batch_backfiller_test.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,7 @@ final class _FakeLuciBuildService extends Fake implements LuciBuildService {
520520
Future<List<PendingTask>> schedulePostsubmitBuilds({
521521
required CommitRef commit,
522522
required List<PendingTask> toBeScheduled,
523+
String? contentHash,
523524
}) async {
524525
scheduledPostsubmitBuilds.addAll(toBeScheduled);
525526
return [];

app_dart/test/service/content_aware_hash_service_test.dart

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,27 @@ void main() {
379379
);
380380
});
381381
});
382+
383+
group('getHashByCommitSha', () {
384+
test('returns the hash when the document is found', () async {
385+
firestoreService.putDocument(
386+
ContentAwareHashBuilds(
387+
createdOn: DateTime.now(),
388+
contentHash: '1' * 40,
389+
commitSha: 'a' * 40,
390+
buildStatus: BuildStatus.inProgress,
391+
waitingShas: [],
392+
),
393+
);
394+
final hash = await cahs.getHashByCommitSha('a' * 40);
395+
expect(hash, '1' * 40);
396+
});
397+
398+
test('returns null when the document is not found', () async {
399+
final hash = await cahs.getHashByCommitSha('a' * 40);
400+
expect(hash, isNull);
401+
});
402+
});
382403
}
383404

384405
extension on String {

app_dart/test/service/luci_build_service/schedule_prod_builds_test.dart

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,65 @@ void main() {
661661
);
662662
});
663663

664+
test('schedules a post-submit build with content hash', () async {
665+
final commit = generateFirestoreCommit(
666+
1,
667+
branch: 'master',
668+
repo: 'flutter',
669+
);
670+
671+
when(mockBuildBucketClient.listBuilders(any)).thenAnswer((_) async {
672+
return bbv2.ListBuildersResponse(
673+
builders: [
674+
bbv2.BuilderItem(
675+
id: bbv2.BuilderID(
676+
bucket: 'prod',
677+
project: 'flutter',
678+
builder: 'Linux 1',
679+
),
680+
),
681+
],
682+
);
683+
});
684+
685+
await expectLater(
686+
luci.schedulePostsubmitBuilds(
687+
commit: commit.toRef(),
688+
toBeScheduled: [
689+
PendingTask(
690+
target: generateTarget(
691+
1,
692+
properties: {
693+
'recipe': 'devicelab/devicelab',
694+
'os': 'debian-10.12',
695+
},
696+
slug: Config.flutterSlug,
697+
),
698+
taskName: generateFirestoreTask(1, commitSha: commit.sha).taskName,
699+
priority: LuciBuildService.kDefaultPriority,
700+
currentAttempt: 1,
701+
),
702+
],
703+
contentHash: 'abc',
704+
),
705+
completion(isEmpty),
706+
);
707+
708+
final bbv2.ScheduleBuildRequest scheduleBuild;
709+
{
710+
final batchRequest = bbv2.BatchRequest().createEmptyInstance();
711+
batchRequest.mergeFromProto3Json(pubSub.messages.single);
712+
713+
expect(batchRequest.requests, hasLength(1));
714+
scheduleBuild = batchRequest.requests.single.scheduleBuild;
715+
}
716+
717+
expect(
718+
scheduleBuild.properties.fields,
719+
containsPair('content_hash', bbv2.Value(stringValue: 'abc')),
720+
);
721+
});
722+
664723
test('does not run a non-existent builder', () async {
665724
final commit = generateFirestoreCommit(1, branch: 'main', repo: 'flutter');
666725

app_dart/test/service/scheduler_test.dart

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,45 @@ void main() {
281281
);
282282
});
283283

284+
test('schedules cocoon based targets with content hash', () async {
285+
final luciBuildService = MockLuciBuildService();
286+
const contentHash = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa';
287+
fakeContentAwareHash.hashByCommit['1'] = contentHash;
288+
when(
289+
luciBuildService.schedulePostsubmitBuilds(
290+
commit: anyNamed('commit'),
291+
toBeScheduled: anyNamed('toBeScheduled'),
292+
contentHash: contentHash,
293+
),
294+
).thenAnswer((_) async => []);
295+
scheduler = Scheduler(
296+
cache: cache,
297+
config: config,
298+
githubChecksService: GithubChecksService(
299+
config,
300+
githubChecksUtil: mockGithubChecksUtil,
301+
),
302+
getFilesChanged: getFilesChanged,
303+
ciYamlFetcher: ciYamlFetcher,
304+
luciBuildService: luciBuildService,
305+
contentAwareHash: fakeContentAwareHash,
306+
firestore: firestore,
307+
bigQuery: bigQuery,
308+
);
309+
310+
// This test is testing `GuaranteedPolicy` get scheduled - there's only one now.
311+
await scheduler.addCommits(
312+
createCommitList(<String>['1'], branch: 'main', repo: 'packages'),
313+
);
314+
verify(
315+
luciBuildService.schedulePostsubmitBuilds(
316+
commit: anyNamed('commit'),
317+
toBeScheduled: anyNamed('toBeScheduled'),
318+
contentHash: contentHash,
319+
),
320+
).called(1);
321+
});
322+
284323
test(
285324
'schedules cocoon based targets - multiple batch requests',
286325
() async {

app_dart/test/src/service/fake_content_aware_hash_service.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,10 @@ class FakeContentAwareHashService implements ContentAwareHashService {
7373
nextCompletedShas = null;
7474
return shas;
7575
}
76+
77+
final hashByCommit = <String, String>{};
78+
@override
79+
Future<String?> getHashByCommitSha(String commitSha) async {
80+
return hashByCommit[commitSha];
81+
}
7682
}

app_dart/test/src/utilities/mocks.mocks.dart

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)