Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/clusterfuzz/_internal/base/feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class FeatureFlags(Enum):

ENABLE_FUZZ_FOR_BOTS = 'enable_fuzz_for_bots'
STORAGE_THREADED_OPS_FUZZ_TARGETS = 'storage_threaded_ops_fuzz_targets'
CALL_ANDROID_API = 'call_android_api'

@property
def flag(self):
Expand Down
28 changes: 25 additions & 3 deletions src/clusterfuzz/_internal/platforms/android/fetch_artifact.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,14 @@
from google.oauth2 import service_account
from oauth2client.service_account import ServiceAccountCredentials

from clusterfuzz._internal.base import feature_flags
from clusterfuzz._internal.config import db_config
from clusterfuzz._internal.google_cloud_utils import storage
from clusterfuzz._internal.metrics import logs
from clusterfuzz._internal.platforms.android.android_build_v4_api import \
AndroidBuildV4Api
from clusterfuzz._internal.system import environment

from . import adb

# 20 MB default chunk size.
DEFAULT_CHUNK_SIZE = 20 * 1024 * 1024

Expand Down Expand Up @@ -72,6 +71,20 @@ def _use_v4():
return False


def _call_android_api_enabled():
"""Return True if we should call the Android Build API, enabled by default,
Disabled always if invoked in a uworker
"""
if environment.is_uworker():
logs.info('AndroidBuildAPI access disabled for uworker.')
return False

flag = feature_flags.FeatureFlags.CALL_ANDROID_API.flag
if flag is None:
return True
return flag.enabled
Comment on lines +83 to +85

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.

Suggested change
if flag is None:
return True
return flag.enabled
return flag.enabled if flag else True



def _execute_request_with_retries(request):
"""Executes request and retries on failure."""
result = None
Expand Down Expand Up @@ -282,7 +295,6 @@ def _get_artifacts_for_build(client,
if not artifacts:
logs.error(f'No artifact found for target {target}, build id {bid}.\n'
f'results {results}')
adb.bad_state_reached()

return artifacts

Expand Down Expand Up @@ -343,6 +355,11 @@ def _get_stable_build_info():

def get_latest_artifact_info(branch, target, signed=False, stable_build=False):
"""Return latest artifact for a branch and target."""
if not _call_android_api_enabled():
logs.warning(
'Android build API is disabled by feature flag call_android_api.')
return None

client = _get_client()
if not client:
return None
Expand Down Expand Up @@ -409,6 +426,11 @@ def get_latest_artifact_info(branch, target, signed=False, stable_build=False):

def get(bid, target, regex, output_directory, output_filename=None):
"""Return artifact for a given build id, target and file regex."""
if not _call_android_api_enabled():
logs.warning(
'Android build API is disabled by feature flag call_android_api.')
return None

client = _get_client()
if not client:
return None
Expand Down
7 changes: 5 additions & 2 deletions src/clusterfuzz/_internal/platforms/android/flash.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def download_latest_build(build_info, image_regexes, image_directory):
logs.error('Failed to download artifact %s for '
'branch %s and target %s.' % (image_file_paths,
build_info['branch'], target))
return
adb.bad_state_reached()

for file_path in image_file_paths:
if file_path.endswith('.zip') or file_path.endswith('.tar.gz'):
Expand All @@ -97,7 +97,10 @@ def boot_stable_build_cuttlefish(branch, target, image_directory):
"""Boot cuttlefish instance using stable build id fetched from gcs."""
build_info = fetch_artifact.get_latest_artifact_info(
branch, target, stable_build=True)
download_latest_build(build_info, FLASH_CUTTLEFISH_REGEXES, image_directory)
if not build_info:

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.

what happens if this returns without the build? Does the device get stuck in a flash cycle?

@IvanBM18 IvanBM18 Jul 21, 2026

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.

Trueeeee forgot about what i told you in this doc's appendix.

I performed an extensive investigation with jetski(TMI to digest by myself), and heres what i found out:

  • All non uworker bots can update their codebase themselves trough update_task
  • When a PHYSICAL android bot needs to flash and for some reason, the artifact/build is not found, we reach this bad_state method, which causes the android device to sleep for 1 hour.
  • After that the bot wakes up and since we are still in a loop it updates itself and tries to work again, so if lets say we introduce a defect in the code then the bot will just update itself each hour till we eventually publish the fix
  • If there's a none code issue(like a failing api) or something is inherently wrong with the bot then the bot will just continue to operate till it hits another timeout

So the way i see it here is to ensure that if a bot fails it calls the bad_state function to trigger the whole self-healing process, which we already do(see changes in flash.py)

For emulated uworker devices(like in swarming), we are 100% sure that they wont try to flash themselves, so theres no worry in here. The other place where we could run into an android api v4 issue is at symbols_downloader.py where it tries to get artifacts to process a stack-trace of errors, but i also investigated if this could cause the bot to loop, and the validations that we have in place avoid the uworker bot from crashing and just ignores the stack_trace symbolization.

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.

Thanks! This flag is fine as long as the bot is in the retry loop but isn't actually making a bunch of calls that we know will fail.

logs.error('Unable to fetch stable build info for cuttlefish.')
else:
download_latest_build(build_info, FLASH_CUTTLEFISH_REGEXES, image_directory)
adb.recreate_cuttlefish_device()
adb.connect_to_cuttlefish_device()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,23 @@ def download_trusty_symbols_if_needed(symbols_directory, app_name, bid):

branch = 'polygon-trusty-whitechapel-master'
if not bid:
bid = fetch_artifact.get_latest_artifact_info(branch, ab_target)['bid']
build_info = fetch_artifact.get_latest_artifact_info(branch, ab_target)
if not build_info:
logs.error(f'Unable to fetch build info for branch {branch} '
f'and target {ab_target}.')
return
bid = build_info['bid']

artifact_filename = f'{ab_target}-{bid}.syms.zip'
symbols_archive_path = os.path.join(symbols_directory, artifact_filename)

download_artifact_if_needed(bid, symbols_directory, symbols_archive_path,
[ab_target], artifact_filename, None)

if not os.path.exists(symbols_archive_path):
logs.error(f'Unable to locate symbols archive {symbols_archive_path}.')
return

with zipfile.ZipFile(symbols_archive_path, 'r') as symbols_zipfile:
for filepath in symbols_zipfile.namelist():
if f'{app_name}.syms.elf' in filepath:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import unittest
from unittest import mock

from clusterfuzz._internal.base import feature_flags
from clusterfuzz._internal.platforms.android import fetch_artifact
from clusterfuzz._internal.tests.test_libs import helpers

Expand All @@ -29,10 +30,12 @@ def setUp(self):
helpers.patch(self, [
'clusterfuzz._internal.platforms.android.fetch_artifact._get_client',
'clusterfuzz._internal.platforms.android.fetch_artifact._use_v4',
'clusterfuzz._internal.platforms.android.fetch_artifact._call_android_api_enabled',
'clusterfuzz._internal.platforms.android.fetch_artifact._execute_request_with_retries',
])
self.mock_client = mock.MagicMock()
self.mock._get_client.return_value = self.mock_client
self.mock._call_android_api_enabled.return_value = True

def test_get_latest_artifact_v4_success(self):
"""Tests get_latest_artifact_info (V4). Expects extraction of {bid, branch, target} when list_builds returns data."""
Expand Down Expand Up @@ -116,10 +119,56 @@ def test_get_latest_artifact_client_auth_failure(self):
result = fetch_artifact.get_latest_artifact_info('branch1', 'target1')
self.assertIsNone(result)

def test_get_latest_artifact_disabled_by_feature_flag(self):
"""Tests get_latest_artifact_info exits early and returns None when disabled by feature flag."""
self.mock._call_android_api_enabled.return_value = False

result = fetch_artifact.get_latest_artifact_info('branch1', 'target1')
self.assertIsNone(result)

def test_get_artifacts_for_build_empty_regexp(self):
"""Tests _get_artifacts_for_build returns [] returning early when regexp is empty, bypassing API calls."""
result = fetch_artifact._get_artifacts_for_build(
self.mock_client, 'bid', 'target', regexp='')
self.assertEqual(result, [])
self.mock_client.list_artifacts.assert_not_called()
self.mock_client.buildartifact().list.assert_not_called()


class CallAndroidApiEnabledTest(unittest.TestCase):
"""Tests for _call_android_api_enabled."""

def setUp(self):
helpers.patch(self, [
'clusterfuzz._internal.system.environment.is_uworker',
])
self.mock.is_uworker.return_value = False

def test_is_uworker_returns_false(self):
self.mock.is_uworker.return_value = True
self.assertFalse(fetch_artifact._call_android_api_enabled())

def test_flag_none_returns_true(self):
with mock.patch.object(
feature_flags.FeatureFlags, 'flag',
new_callable=mock.PropertyMock) as mock_flag:
mock_flag.return_value = None
self.assertTrue(fetch_artifact._call_android_api_enabled())

def test_flag_enabled_returns_true(self):
mock_flag_obj = mock.MagicMock()
mock_flag_obj.enabled = True
with mock.patch.object(
feature_flags.FeatureFlags, 'flag',
new_callable=mock.PropertyMock) as mock_flag:
mock_flag.return_value = mock_flag_obj
self.assertTrue(fetch_artifact._call_android_api_enabled())

def test_flag_disabled_returns_false(self):
mock_flag_obj = mock.MagicMock()
mock_flag_obj.enabled = False
with mock.patch.object(
feature_flags.FeatureFlags, 'flag',
new_callable=mock.PropertyMock) as mock_flag:
mock_flag.return_value = mock_flag_obj
self.assertFalse(fetch_artifact._call_android_api_enabled())
Loading