Skip to content

Commit

Permalink
Update Firestore when reseting a prod task (flutter#3505)
Browse files Browse the repository at this point in the history 
Part of flutter/flutter#142951

This PR:
1) inserts new task document to Firestore when retried manually
2) fixes `trigger_type` tag: instead of overriding manual case as `trigger_type: auto`, we now distinguish them with
2.1) `trigger_type: manual_retry` for manual retry
2.2) `trigger_type: auto_retry` for auto retry
  • Loading branch information
@keyonghan
keyonghan authored Feb 21, 2024
1 parent 673a991 commit e85150d
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 2 deletions.
19 changes: 18 additions & 1 deletion app_dart/lib/src/request_handlers/reset_prod_task.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'package:meta/meta.dart';
import '../model/appengine/commit.dart';
import '../model/appengine/key_helper.dart';
import '../model/appengine/task.dart';
import '../model/firestore/task.dart' as firestore;
import '../model/ci_yaml/ci_yaml.dart';
import '../model/ci_yaml/target.dart';
import '../model/google/token_info.dart';
Expand All @@ -19,6 +20,8 @@ import '../request_handling/body.dart';
import '../request_handling/exceptions.dart';
import '../service/config.dart';
import '../service/datastore.dart';
import '../service/firestore.dart';
import '../service/logging.dart';
import '../service/luci_build_service.dart';
import '../service/scheduler.dart';

Expand Down Expand Up @@ -54,6 +57,7 @@ class ResetProdTask extends ApiRequestHandler<Body> {
@override
Future<Body> post() async {
final DatastoreService datastore = datastoreProvider(config.db);
final FirestoreService firestoreService = await config.createFirestoreService();
final String? encodedKey = requestData![taskKeyParam] as String?;
String? branch = requestData![branchParam] as String?;
final String owner = requestData![ownerParam] as String? ?? 'flutter';
Expand Down Expand Up @@ -87,6 +91,7 @@ class ResetProdTask extends ApiRequestHandler<Body> {
futures.add(
rerun(
datastore: datastore,
firestoreService: firestoreService,
branch: branch,
sha: sha,
taskName: task.name,
Expand All @@ -99,6 +104,7 @@ class ResetProdTask extends ApiRequestHandler<Body> {
} else {
await rerun(
datastore: datastore,
firestoreService: firestoreService,
encodedKey: encodedKey,
branch: branch,
sha: sha,
Expand All @@ -114,6 +120,7 @@ class ResetProdTask extends ApiRequestHandler<Body> {

Future<void> rerun({
required DatastoreService datastore,
required FirestoreService firestoreService,
String? encodedKey,
String? branch,
String? sha,
Expand All @@ -135,9 +142,17 @@ class ResetProdTask extends ApiRequestHandler<Body> {
final CiYaml ciYaml = await scheduler.getCiYaml(commit);
final Target target = ciYaml.postsubmitTargets.singleWhere((Target target) => target.value.name == task.name);

firestore.Task? taskDocument;
final int currentAttempt = task.attempts!;
final String documentName = '$kDatabase/documents/$kTaskCollectionId/${sha}_${taskName}_$currentAttempt';
try {
taskDocument = await firestore.Task.fromFirestore(firestoreService: firestoreService, documentName: documentName);
} catch (error) {
log.warning('Failed to read task $documentName from Firestore: $error');
}
final Map<String, List<String>> tags = <String, List<String>>{
'triggered_by': <String>[email],
'trigger_type': <String>['manual'],
'trigger_type': <String>['manual_retry'],
};
final bool isRerunning = await luciBuildService.checkRerunBuilder(
commit: commit,
Expand All @@ -146,6 +161,8 @@ class ResetProdTask extends ApiRequestHandler<Body> {
datastore: datastore,
tags: tags,
ignoreChecks: ignoreChecks,
firestoreService: firestoreService,
taskDocument: taskDocument,
);

// For human retries from the dashboard, notify if a task failed to rerun.
Expand Down
2 changes: 1 addition & 1 deletion app_dart/lib/src/service/luci_build_service.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ class LuciBuildService {
}
log.info('Rerun builder: ${target.value.name} for commit ${commit.sha}');
tags ??= <String, List<String>>{};
tags['trigger_type'] = <String>['retry'];
tags['trigger_type'] ??= <String>['auto_retry'];

// TODO(keyonghan): remove check when [ResetProdTask] supports firestore update.
if (taskDocument != null) {
Expand Down
24 changes: 24 additions & 0 deletions app_dart/test/request_handlers/reset_prod_task_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import 'package:cocoon_service/cocoon_service.dart';
import 'package:cocoon_service/src/model/appengine/commit.dart';
import 'package:cocoon_service/src/model/appengine/task.dart';
import 'package:cocoon_service/src/model/firestore/task.dart' as firestore;
import 'package:cocoon_service/src/request_handling/exceptions.dart';
import 'package:googleapis/firestore/v1.dart';
import 'package:mockito/mockito.dart';
import 'package:test/test.dart';

Expand All @@ -24,13 +26,16 @@ void main() {
late FakeConfig config;
FakeKeyHelper keyHelper;
late MockLuciBuildService mockLuciBuildService;
late MockFirestoreService mockFirestoreService;
late ApiRequestHandlerTester tester;
late Commit commit;
late Task task;
final firestore.Task firestoreTask = generateFirestoreTask(1, attempts: 1);

setUp(() {
final FakeDatastoreDB datastoreDB = FakeDatastoreDB();
clientContext = FakeClientContext();
mockFirestoreService = MockFirestoreService();
clientContext.isDevelopmentEnvironment = false;
keyHelper = FakeKeyHelper(applicationContext: clientContext.applicationContext);
config = FakeConfig(
Expand All @@ -39,6 +44,7 @@ void main() {
supportedBranchesValue: <String>[
Config.defaultBranch(Config.flutterSlug),
],
firestoreService: mockFirestoreService,
);
final FakeAuthenticatedContext authContext = FakeAuthenticatedContext(clientContext: clientContext);
tester = ApiRequestHandlerTester(context: authContext);
Expand All @@ -63,6 +69,16 @@ void main() {
'Key': config.keyHelper.encode(task.key),
};

when(
mockFirestoreService.getDocument(
captureAny,
),
).thenAnswer((Invocation invocation) {
return Future<Document>.value(
firestoreTask,
);
});

when(
mockLuciBuildService.checkRerunBuilder(
commit: anyNamed('commit'),
Expand All @@ -71,6 +87,8 @@ void main() {
target: anyNamed('target'),
tags: anyNamed('tags'),
ignoreChecks: anyNamed('ignoreChecks'),
firestoreService: mockFirestoreService,
taskDocument: anyNamed('taskDocument'),
),
).thenAnswer((_) async => true);
});
Expand Down Expand Up @@ -103,6 +121,8 @@ void main() {
target: anyNamed('target'),
tags: anyNamed('tags'),
ignoreChecks: true,
firestoreService: mockFirestoreService,
taskDocument: anyNamed('taskDocument'),
),
).called(1);
});
Expand All @@ -127,6 +147,8 @@ void main() {
target: anyNamed('target'),
tags: anyNamed('tags'),
ignoreChecks: false,
firestoreService: mockFirestoreService,
taskDocument: anyNamed('taskDocument'),
),
).called(2);
});
Expand Down Expand Up @@ -173,6 +195,8 @@ void main() {
target: anyNamed('target'),
tags: anyNamed('tags'),
ignoreChecks: true,
firestoreService: mockFirestoreService,
taskDocument: anyNamed('taskDocument'),
),
).thenAnswer((_) async => false);
config.db.values[task.key] = task;
Expand Down
1 change: 1 addition & 0 deletions app_dart/test/service/luci_build_service_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,7 @@ void main() {
}
expect(scheduleBuildRequest.priority, LuciBuildService.kRerunPriority);
expect(scheduleBuildRequest.gitilesCommit?.project, 'mirrors/engine');
expect(scheduleBuildRequest.tags?['trigger_type'], <String>['auto_retry']);
expect(rerunFlag, isTrue);
expect(task.attempts, 2);
expect(task.status, Task.statusInProgress);
Expand Down

0 comments on commit e85150d

Please sign in to comment.