Repository navigation
Conversation
| logs.error(f'Unknown command "{task_name}"') | ||
| return None | ||
|
|
||
| if environment.is_tworker() and isinstance(task, task_types.TrustedTask): |
There was a problem hiding this comment.
It is not actually retried. I made it clearer with the new log and a comment. But I could also add the exception when that merges if you think that would be better.
It does sound like a good option for consistency
| if not COMMAND_TYPES[command].is_execution_remote(command): | ||
| return False | ||
| return batch_service.is_remote_task(command, job) | ||
| return batch_service.is_remote_task(command, |
There was a problem hiding this comment.
Please mention why we make this change in the PR description. Also curious if we've decided that it's safe to run untrusted fuzzers on swarming or if there's any context around that decision.
There was a problem hiding this comment.
Good catch, I didn't consider that I was allowing that. And no I think we are not ready to handle that. So I removed this change better
- Make the TrustedTask drop on tworkers explicit: returning normally acks the Pub/Sub message, so the task is dropped and not retried. - Add may_run_task_locally to explain why tworkers skip host platform, queue and CPU arch checks. - Revert the is_no_privilege_workload change: Swarming bots have no isolation between tasks in a pool, so they can't run untrusted workloads.
Swarming can't run untrusted workloads (#5581 no longer treats Swarming jobs as no-privilege workloads). Invert the variant test and add a trusted counterpart.
|
|
||
| if environment.is_tworker() and isinstance(task, task_types.TrustedTask): | ||
| # Returning (not raising) acks the message: dropped, not retried. | ||
| logs.error(f'Dropping TrustedTask "{task_name}" on tworker.') |
There was a problem hiding this comment.
IIUC, your changes are making it so TrustedTasks no longer get processed by tworkers, why is the reasoning behind this and how does this relates to the OS/CPU check that we aim to bypass here
There was a problem hiding this comment.
I think this relates to the comment i posted here: #5583 (comment)
There was a problem hiding this comment.
TrustedTask.execute() already asserts not environment.is_tworker(). Previously, a TrustedTask for a non-Linux/arm64 Swarming job bailed out earlier at the OS/CPU checks in process_command_impl(); bypassing those checks lets it reach TrustedTask.execute() and raise AssertionError (causing Pub/Sub retries), so we return None here to ack and drop it cleanly. Supporting TrustedTasks (blame, impact, unpack) for Swarming jobs is tracked in https://crbug.com/582825425.
There was a problem hiding this comment.
Ah ok makes sense, leaving the comment as unresolved for other reviewers to know
IvanBM18
left a comment
There was a problem hiding this comment.
LGTM with Gio's comments
Bug: 574630499
Stack: #5581 (this) → #5582 → #5583
Context
Today only fuzz tasks of Swarming jobs (e.g. Android emulator) run on Swarming. This stack does the same for the other utasks (minimize, regression, progression, analyze, variant, symbolize, corpus_pruning): a dedicated tworker serves the job's platform queue (e.g.
jobs-android-emulator), runsutask_preprocessand sendsutask_mainto Swarming. Everything is gated bySWARMING_REMOTE_EXECUTION.Description
Tworkers re-queued these tasks because they compared the testcase platform and job CPU arch against their own host. That match only matters for the bot running
utask_main, which Swarming selects, so tworkers now skip those checks.Serving a
jobs-*queue can also hand tworkersTrustedTasks (blame,impact); they're now dropped instead of crashing. Returning normally acks the message, so they aren't retried. #5583 stops creating them.Testing
utask_main-swarming; a strayimpacttask on a tworker is dropped without running.jobs-android-emulatorby the dedicated tworker, ranutask_mainon Swarming to completion and were postprocessed; symbolize was preprocessed and scheduled on Swarming. Details in the bug.