Skip to content

[Swarming] Crash service on startup #5285

Merged
IvanBM18 merged 1 commit into
masterfrom
feature/swarming/crash_service_at_startup
Jun 10, 2026
Merged

[Swarming] Crash service on startup #5285
IvanBM18 merged 1 commit into
masterfrom
feature/swarming/crash_service_at_startup

Conversation

@IvanBM18

@IvanBM18 IvanBM18 commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Overview

SwarmingService and other remote task services will now be instantiated in RemoteTaskGate based on the their feature flag.

Now the SwarmingService will crash horribly at startup, this is done to avoid happing an api instances that does nothing, and a swarming service that just fails when called, with this new logic we make sure that all parts work as expected and if they dont.... well we just wont be able to fully schedule on envs where swarming is enabled.

Background

This PR comes from a discussion & agreement to a required change from this review

Changes

  • Raises an exception at SwarmingService constructor if the swarming config was not found
  • Updated RemoteTaskGate to conditionally instantiate services based on the linked feature flag.
    • This safeguards envs where the swarming feature flag is not enabled.
  • Added unit tests & updated a lot of them for the remote task gate changes.

@IvanBM18 IvanBM18 changed the title Service instantiated based on feature flag, crash swarming service on startup Crash swarming service on startup May 20, 2026
@IvanBM18 IvanBM18 changed the title Crash swarming service on startup [Swarming] Crash service on startup May 20, 2026
@IvanBM18 IvanBM18 self-assigned this May 20, 2026
@IvanBM18 IvanBM18 added the swarming Changes related to the clusterfuzz-swarming integration label May 20, 2026

@letitz letitz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC the only diff from PR #5277 is 5fbc97d. At least that's what I reviewed :)

In that case, this LGTM other than one optional suggestion and one request for more tests.

Comment thread src/clusterfuzz/_internal/tests/core/remote_task/remote_task_test.py Outdated
@IvanBM18 IvanBM18 marked this pull request as ready for review May 26, 2026 22:02
@IvanBM18 IvanBM18 requested a review from a team as a code owner May 26, 2026 22:02
Comment thread src/clusterfuzz/_internal/remote_task/remote_task_gate.py
@IvanBM18 IvanBM18 marked this pull request as draft May 27, 2026 19:55
@IvanBM18 IvanBM18 marked this pull request as ready for review June 10, 2026 06:00
@Xeicker

Xeicker commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

I'd suggest rebase your branch on top of master so we don't merge many commits like Merge branch 'master' into feature/swarming/crash_service_at_startup

Other than that, LGTM

@fernandofloresg fernandofloresg left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@IvanBM18 IvanBM18 force-pushed the feature/swarming/crash_service_at_startup branch from 0d9a46b to f537ad7 Compare June 10, 2026 18:28
@IvanBM18

Copy link
Copy Markdown
Collaborator Author

I'd suggest rebase your branch on top of master so we don't merge many commits like Merge branch 'master' into feature/swarming/crash_service_at_startup

Other than that, LGTM

Done :D

id='swarming',
service=mock.Mock(return_value=self.mock_swarming_service),
feature_flag=None,
feature_flag=mock.Mock(enabled=True),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have a test case for enabled=False?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, i didn't read this before merging(sorry about that) but ill add it in this open pr

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IvanBM18 IvanBM18 merged commit 5d75bd5 into master Jun 10, 2026
14 checks passed
@IvanBM18 IvanBM18 deleted the feature/swarming/crash_service_at_startup branch June 10, 2026 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

swarming Changes related to the clusterfuzz-swarming integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants