-
Notifications
You must be signed in to change notification settings - Fork 587
feat(tasks): Implement OS-based task routing logic #4995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- Modify add_task to publish base_os_version attribute. - Modify task consumers to select subscription based on BASE_OS_VERSION env. - Add unit tests for new publishing and consuming logic.
queue_name = POSTPROCESS_QUEUE | ||
base_os_version = environment.get_value('BASE_OS_VERSION') | ||
if base_os_version: | ||
queue_name = f'{queue_name}-{base_os_version}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it could be an static configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. In this case, the dynamic approach is needed so the bot can be aware of its own OS environment. The BASE_OS_VERSION is set at the infrastructure level (in the Docker image), allowing each worker to correctly identify which filtered queue it should pull from.
queue_name = PREPROCESS_QUEUE | ||
base_os_version = environment.get_value('BASE_OS_VERSION') | ||
if base_os_version: | ||
queue_name = f'{queue_name}-{base_os_version}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
|
||
# Determine base_os_version. | ||
base_os_version = job.base_os_version | ||
if environment.get_value('PROJECT_NAME') == 'oss-fuzz': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could move this oss-fuzz
to a constant to avoid this magic value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I've updated the code to use the existing job.is_external()
method, which removes the magic value.
- Revert generic change in get_regular_task. - Modify default_queue_suffix to be OS-aware for Linux platforms. - Use job.is_external() instead of magic string for OSS-Fuzz check. - Update and fix tests to reflect all refactoring.
This will be moved to a separate PR to accelerate the image build.
This change implements the core application logic to support routing ClusterFuzz tasks to different worker pools based on the target operating system.
This is achieved via two main changes:
Task Publishing (
tasks.add_task
):The
add_task
function now determines the requiredbase_os_version
for a task by checking theJob
andOssFuzzProject
entities. This OS version is then added as abase_os_version
attribute to the message published on Pub/Sub, effectively "tagging" the task for consumption by the correct worker fleet.Task Consuming (
get_*_task
functions):The consumer functions (
get_preprocess_task
,get_postprocess_task
,get_utask_mains
) have been updated to be OS-aware. They now read aBASE_OS_VERSION
environment variable (expected to be set in the Docker image). If this variable is present, they dynamically construct the name of the Pub/Sub subscription to pull from (e.g.,preprocess
becomespreprocess-ubuntu-24-04
).Unit tests have been added to cover both the publishing and consuming logic.
Dependencies:
This change depends on corresponding infrastructure changes in the
clusterfuzz-config
repository, where the filtered Pub/Sub subscriptions (e.g.,preprocess-ubuntu-24-04
) are created. The code will fail to connect if the subscriptions do not exist.Fixes: b/441792657