diff options
author | android-build-team Robot <android-build-team-robot@google.com> | 2019-05-29 03:01:13 +0000 |
---|---|---|
committer | android-build-team Robot <android-build-team-robot@google.com> | 2019-05-29 03:01:13 +0000 |
commit | a5a04590baca2daf95ba38b054af6d19331eed30 (patch) | |
tree | 9eaf02eeeff13c2f203ccfcf9fb3e1b65fe07147 | |
parent | 228363bbca18770914bd6360990b0f574a21374d (diff) | |
parent | 1c4a17aba7508c228e0fe5d9be6142e5349e2cfb (diff) | |
download | acloud-android10-s3-release.tar.gz |
Snap for 5612356 from 1c4a17aba7508c228e0fe5d9be6142e5349e2cfb to qt-releaseandroid-vts-10.0_r1android-security-10.0.0_r75android-security-10.0.0_r74android-security-10.0.0_r73android-security-10.0.0_r72android-security-10.0.0_r71android-security-10.0.0_r70android-security-10.0.0_r69android-security-10.0.0_r68android-security-10.0.0_r67android-security-10.0.0_r66android-security-10.0.0_r65android-security-10.0.0_r64android-security-10.0.0_r63android-security-10.0.0_r62android-security-10.0.0_r61android-security-10.0.0_r60android-security-10.0.0_r59android-security-10.0.0_r58android-security-10.0.0_r57android-security-10.0.0_r56android-security-10.0.0_r55android-security-10.0.0_r54android-security-10.0.0_r53android-security-10.0.0_r52android-security-10.0.0_r51android-security-10.0.0_r50android-security-10.0.0_r49android-security-10.0.0_r48android-cts-10.0_r1android-10.0.0_r6android-10.0.0_r5android-10.0.0_r47android-10.0.0_r46android-10.0.0_r4android-10.0.0_r3android-10.0.0_r2android-10.0.0_r17android-10.0.0_r11android-10.0.0_r10android-10.0.0_r1android10-security-releaseandroid10-s3-releaseandroid10-s2-releaseandroid10-s1-releaseandroid10-release
Change-Id: Id26d40e3c4ed19ec235a3769baaa9d74ea9fbc81
60 files changed, 1528 insertions, 689 deletions
@@ -73,6 +73,7 @@ python_test_host { "public/*_test.py", "public/actions/*_test.py", "internal/lib/*_test.py", + "metrics/*.py", ], libs: [ "acloud_create", @@ -83,6 +84,7 @@ python_test_host { "acloud_proto", "acloud_public", "acloud_setup", + "asuite_cc_client", "py-apitools", "py-dateutil", "py-google-api-python-client", @@ -180,6 +182,7 @@ python_library_host{ "metrics/*.py", ], libs: [ + "asuite_cc_client", "asuite_metrics", ], } diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg index 136a7611..e5c5e28e 100644 --- a/PREUPLOAD.cfg +++ b/PREUPLOAD.cfg @@ -2,4 +2,4 @@ pylint = true [Hook Scripts] -acloud_unittests = ${REPO_ROOT}/prebuilts/asuite/atest/${BUILD_OS}/atest acloud_test +acloud_unittests = ${REPO_ROOT}/prebuilts/asuite/atest/${BUILD_OS}/atest acloud_test --host diff --git a/create/avd_spec.py b/create/avd_spec.py index 039b1678..84bcbc47 100644 --- a/create/avd_spec.py +++ b/create/avd_spec.py @@ -20,6 +20,7 @@ get passed into the create classes. The inferring magic will happen within initialization of AVDSpec (like LKGB build id, image branch, etc). """ +import glob import logging import os import re @@ -31,6 +32,7 @@ from acloud.create import create_common from acloud.internal import constants from acloud.internal.lib import android_build_client from acloud.internal.lib import auth +from acloud.internal.lib import utils from acloud.public import config # Default values for build target. @@ -39,10 +41,16 @@ _BUILD_TARGET = "build_target" _BUILD_BRANCH = "build_branch" _BUILD_ID = "build_id" _COMMAND_REPO_INFO = ["repo", "info"] +_CF_ZIP_PATTERN = "*img*.zip" _DEFAULT_BUILD_BITNESS = "x86" _DEFAULT_BUILD_TYPE = "userdebug" _ENV_ANDROID_PRODUCT_OUT = "ANDROID_PRODUCT_OUT" _ENV_ANDROID_BUILD_TOP = "ANDROID_BUILD_TOP" +_GCE_LOCAL_IMAGE_CANDIDATES = ["avd-system.tar.gz", + "android_system_disk_syslinux.img"] +_LOCAL_ZIP_WARNING_MSG = "'adb sync' will take a long time if using images " \ + "built with `m dist`. Building with just `m` will " \ + "enable a faster 'adb sync' process." _RE_ANSI_ESCAPE = re.compile(r"(\x9B|\x1B\[)[0-?]*[ -\/]*[@-~]") _RE_FLAVOR = re.compile(r"^.+_(?P<flavor>.+)-img.+") _RE_GBSIZE = re.compile(r"^(?P<gb_size>\d+)g$", re.IGNORECASE) @@ -107,6 +115,9 @@ class AVDSpec(object): # Reporting args. self._serial_log_file = None self._logcat_file = None + # gpu and emulator_build_id is only used for goldfish avd_type. + self._gpu = None + self._emulator_build_id = None self._ProcessArgs(args) @@ -248,88 +259,128 @@ class AVDSpec(object): self._kernel_build_id = args.kernel_build_id self._serial_log_file = args.serial_log_file self._logcat_file = args.logcat_file + self._emulator_build_id = args.emulator_build_id + self._gpu = args.gpu @staticmethod - def _GetFlavorFromLocalImage(image_path): - """Get flavor name from local image file name. + def _GetFlavorFromString(flavor_string): + """Get flavor name from flavor string. - If the user didn't specify a flavor, we can infer it from the image - name, e.g. cf_x86_tv-img-eng.zip should be created with a flavor of tv. + Flavor string can come from the zipped image name or the lunch target. + e.g. + If flavor_string come from zipped name:aosp_cf_x86_phone-img-5455843.zip + , then "phone" is the flavor. + If flavor_string come from a lunch'd target:aosp_cf_x86_auto-userdebug, + then "auto" is the flavor. Args: - image_path: String of image path. + flavor_string: String which contains flavor.It can be a + build target or filename. Returns: - String of flavor name, None if flavor can't be determined. + String of flavor name. None if flavor can't be determined. """ - local_image_name = os.path.basename(image_path) - match = _RE_FLAVOR.match(local_image_name) - if match: - image_flavor = match.group("flavor") - if image_flavor in constants.ALL_FLAVORS: - logger.debug("Get flavor[%s] from local image name(%s).", - image_flavor, local_image_name) - return image_flavor - else: - logger.debug("Flavor[%s] from image name is not supported.", - image_flavor) + for flavor in constants.ALL_FLAVORS: + if re.match(r"(.*_)?%s" % flavor, flavor_string): + return flavor + logger.debug("Unable to determine flavor from build target: %s", + flavor_string) return None + def _ProcessLocalImageArgs(self, args): + """Get local image path. + + Args: + args: Namespace object from argparse.parse_args. + """ + if self._avd_type == constants.TYPE_CF: + self._ProcessCFLocalImageArgs(args.local_image, args.flavor) + elif self._avd_type == constants.TYPE_GCE: + self._local_image_artifact = self._GetGceLocalImagePath( + args.local_image) + else: + raise errors.CreateError( + "Local image doesn't support the AVD type: %s" % self._avd_type + ) + @staticmethod - def _GetFlavorFromTarget(build_target): - """Get flavor name from build target name. + def _GetGceLocalImagePath(local_image_dir): + """Get gce local image path. - If the user didn't specify a flavor, we can infer it from the target - name, e.g. cf_x86_phone-userdebug should be created with a flavor of - phone. + Choose image file in local_image_dir over $ANDROID_PRODUCT_OUT. + There are various img files so we prioritize returning the one we find + first based in the specified order in _GCE_LOCAL_IMAGE_CANDIDATES. Args: - build_target: String of build target name. + local_image_dir: A string to specify local image dir. Returns: - String of flavor name. None if flavor can't be determined. + String, image file path if exists. + + Raises: + errors.BootImgDoesNotExist if image doesn't exist. """ - for flavor in constants.ALL_FLAVORS: - if re.match(r"(.*_)?%s-" % flavor, build_target): - return flavor + # IF the user specified a file, return it + if local_image_dir and os.path.isfile(local_image_dir): + return local_image_dir - logger.debug("Unable to determine flavor from build target: %s", - build_target) - return None + # If the user didn't specify a dir, assume $ANDROID_PRODUCT_OUT + if not local_image_dir: + local_image_dir = utils.GetBuildEnvironmentVariable( + _ENV_ANDROID_PRODUCT_OUT) - def _ProcessLocalImageArgs(self, args): - """Get local image path. + for img_name in _GCE_LOCAL_IMAGE_CANDIDATES: + full_file_path = os.path.join(local_image_dir, img_name) + if os.path.exists(full_file_path): + return full_file_path + + raise errors.BootImgDoesNotExist("Could not find any GCE images (%s), " + "you can build them via \"m dist\"" % + ", ".join(_GCE_LOCAL_IMAGE_CANDIDATES)) - -Specified local_image with no arg: Set $ANDROID_PRODUCT_OUT. - -Specified local_image with an arg: Set user specified path. + def _ProcessCFLocalImageArgs(self, local_image_arg, flavor_arg): + """Get local built image path for cuttlefish-type AVD. + + Two scenarios of using --local-image: + - Without a following argument + Set flavor string if the required images are in $ANDROID_PRODUCT_OUT, + - With a following filename/dirname + Set flavor string from the specified image/dir name. Args: - args: Namespace object from argparse.parse_args. + local_image_arg: String of local image args. + flavor_arg: String of flavor arg - Raises: - errors.CreateError: Can't get $ANDROID_PRODUCT_OUT. """ - if args.local_image: - self._local_image_dir = args.local_image + flavor_from_build_string = None + local_image_path = local_image_arg or utils.GetBuildEnvironmentVariable( + _ENV_ANDROID_PRODUCT_OUT) + + if os.path.isfile(local_image_path): + self._local_image_artifact = local_image_arg + flavor_from_build_string = self._GetFlavorFromString( + self._local_image_artifact) + # Since file is provided and I assume it's a zip, so print the + # warning message. + utils.PrintColorString(_LOCAL_ZIP_WARNING_MSG, + utils.TextColors.WARNING) else: - try: - self._local_image_dir = os.environ[_ENV_ANDROID_PRODUCT_OUT] - except KeyError: - raise errors.GetAndroidBuildEnvVarError( - "Could not get environment var: %s\n" - "Try to run '#source build/envsetup.sh && lunch <target>'" - % _ENV_ANDROID_PRODUCT_OUT - ) - - self._local_image_artifact = create_common.VerifyLocalImageArtifactsExist( - self.local_image_dir) - # Overwrite flavor by local image name - local_image_flavor = self._GetFlavorFromLocalImage( - self._local_image_artifact) - if local_image_flavor and not args.flavor: - self._flavor = local_image_flavor - self._cfg.OverrideHwPropertyWithFlavor(local_image_flavor) + self._local_image_dir = local_image_path + # Since dir is provided, so checking that any images exist to ensure + # user didn't forget to 'make' before launch AVD. + image_list = glob.glob(os.path.join(self.local_image_dir, "*.img")) + if not image_list: + raise errors.GetLocalImageError( + "No image found(Did you choose a lunch target and run `m`?)" + ": %s.\n " % self.local_image_dir) + + flavor_from_build_string = self._GetFlavorFromString( + utils.GetBuildEnvironmentVariable(constants.ENV_BUILD_TARGET)) + + if flavor_from_build_string and not flavor_arg: + self._flavor = flavor_from_build_string + self._cfg.OverrideHwPropertyWithFlavor(flavor_from_build_string) def _ProcessRemoteBuildArgs(self, args): """Get the remote build args. @@ -351,7 +402,7 @@ class AVDSpec(object): else: # If flavor isn't specified, try to infer it from build target, # if we can't, just default to phone flavor. - self._flavor = args.flavor or self._GetFlavorFromTarget( + self._flavor = args.flavor or self._GetFlavorFromString( self._remote_image[_BUILD_TARGET]) or constants.FLAVOR_PHONE # infer avd_type from build_target. for avd_type, avd_type_abbr in constants.AVD_TYPES_MAPPING.items(): @@ -516,3 +567,13 @@ class AVDSpec(object): def logcat_file(self): """Return logcat file path.""" return self._logcat_file + + @property + def gpu(self): + """Return gpu.""" + return self._gpu + + @property + def emulator_build_id(self): + """Return emulator_build_id.""" + return self._emulator_build_id diff --git a/create/avd_spec_test.py b/create/avd_spec_test.py index 5d5681b3..48b8038e 100644 --- a/create/avd_spec_test.py +++ b/create/avd_spec_test.py @@ -13,21 +13,27 @@ # limitations under the License. """Tests for avd_spec.""" +import glob +import os import unittest import mock + from acloud import errors from acloud.create import avd_spec from acloud.create import create_common from acloud.internal import constants +from acloud.internal.lib import driver_test_lib +from acloud.internal.lib import utils # pylint: disable=invalid-name,protected-access -class AvdSpecTest(unittest.TestCase): +class AvdSpecTest(driver_test_lib.BaseDriverTest): """Test avd_spec methods.""" def setUp(self): """Initialize new avd_spec.AVDSpec.""" + super(AvdSpecTest, self).setUp() self.args = mock.MagicMock() self.args.local_image = "" self.args.config_file = "" @@ -35,36 +41,69 @@ class AvdSpecTest(unittest.TestCase): self.AvdSpec = avd_spec.AVDSpec(self.args) # pylint: disable=protected-access - @mock.patch.object(create_common, "VerifyLocalImageArtifactsExist") - def testProcessLocalImageArgs(self, mock_image): + def testProcessLocalImageArgs(self): """Test process args.local_image.""" - # Specified local_image with an arg - mock_image.return_value = "cf_x86_phone-img-eng.user.zip" - self.args.local_image = "test_path" + self.Patch(create_common, "ZipCFImageFiles", + return_value="/path/cf_x86_phone-img-eng.user.zip") + self.Patch(glob, "glob", return_value=["fake.img"]) + expected_image_artifact = "/path/cf_x86_phone-img-eng.user.zip" + expected_image_dir = "/path-to-image-dir" + + # Specified --local-image to a local zipped image file + self.Patch(os.path, "isfile", return_value=True) + self.args.local_image = "/path/cf_x86_phone-img-eng.user.zip" + self.AvdSpec._avd_type = constants.TYPE_CF + self.AvdSpec._instance_type = constants.INSTANCE_TYPE_REMOTE + self.AvdSpec._ProcessLocalImageArgs(self.args) + self.assertEqual(self.AvdSpec._local_image_artifact, + expected_image_artifact) + + # Specified --local-image to a dir contains images + self.Patch(utils, "GetBuildEnvironmentVariable", + return_value="test_environ") + self.Patch(os.path, "isfile", return_value=False) + self.args.local_image = "/path-to-image-dir" + self.AvdSpec._avd_type = constants.TYPE_CF + self.AvdSpec._instance_type = constants.INSTANCE_TYPE_REMOTE self.AvdSpec._ProcessLocalImageArgs(self.args) - self.assertEqual(self.AvdSpec._local_image_dir, "test_path") + self.assertEqual(self.AvdSpec._local_image_dir, expected_image_dir) - # Specified local_image with no arg + # Specified local_image without arg self.args.local_image = None - with mock.patch.dict("os.environ", {"ANDROID_PRODUCT_OUT": "test_environ"}): - self.AvdSpec._ProcessLocalImageArgs(self.args) - self.assertEqual(self.AvdSpec._local_image_dir, "test_environ") + self.Patch(utils, "GetBuildEnvironmentVariable", + return_value="test_environ") + self.AvdSpec._ProcessLocalImageArgs(self.args) + self.assertEqual(self.AvdSpec._local_image_dir, "test_environ") + self.assertEqual(self.AvdSpec.local_image_artifact, expected_image_artifact) - @mock.patch.object(create_common, "VerifyLocalImageArtifactsExist") - def testProcessImageArgs(self, mock_image): + def testProcessImageArgs(self): """Test process image source.""" + self.Patch(glob, "glob", return_value=["fake.img"]) # No specified local_image, image source is from remote self.args.local_image = "" self.AvdSpec._ProcessImageArgs(self.args) self.assertEqual(self.AvdSpec._image_source, constants.IMAGE_SRC_REMOTE) self.assertEqual(self.AvdSpec._local_image_dir, None) - # Specified local_image with an arg, image source is from local - mock_image.return_value = "cf_x86_phone-img-eng.user.zip" - self.args.local_image = "test_path" + # Specified local_image with an arg for cf type + self.Patch(os.path, "isfile", return_value=True) + self.args.local_image = "/test_path/cf_x86_phone-img-eng.user.zip" + self.AvdSpec._avd_type = constants.TYPE_CF + self.AvdSpec._instance_type = constants.INSTANCE_TYPE_REMOTE + self.AvdSpec._ProcessImageArgs(self.args) + self.assertEqual(self.AvdSpec._image_source, constants.IMAGE_SRC_LOCAL) + self.assertEqual(self.AvdSpec._local_image_artifact, + "/test_path/cf_x86_phone-img-eng.user.zip") + + # Specified local_image with an arg for gce type + self.Patch(os.path, "isfile", return_value=False) + self.Patch(os.path, "exists", return_value=True) + self.args.local_image = "/test_path_to_dir/" + self.AvdSpec._avd_type = constants.TYPE_GCE self.AvdSpec._ProcessImageArgs(self.args) self.assertEqual(self.AvdSpec._image_source, constants.IMAGE_SRC_LOCAL) - self.assertEqual(self.AvdSpec._local_image_dir, "test_path") + self.assertEqual(self.AvdSpec._local_image_artifact, + "/test_path_to_dir/avd-system.tar.gz") @mock.patch.object(avd_spec.AVDSpec, "_GetGitRemote") @mock.patch("subprocess.check_output") @@ -144,23 +183,20 @@ class AvdSpecTest(unittest.TestCase): result_dict = self.AvdSpec._ParseHWPropertyStr(args_str) self.assertTrue(expected_dict == result_dict) - def testGetFlavorFromLocalImage(self): + def testGetFlavorFromBuildTargetString(self): """Test _GetFlavorFromLocalImage.""" img_path = "/fack_path/cf_x86_tv-img-eng.user.zip" - self.assertEqual(self.AvdSpec._GetFlavorFromLocalImage(img_path), "tv") + self.assertEqual(self.AvdSpec._GetFlavorFromString(img_path), + "tv") + + build_target_str = "aosp_cf_x86_auto" + self.assertEqual(self.AvdSpec._GetFlavorFromString( + build_target_str), "auto") # Flavor is not supported. img_path = "/fack_path/cf_x86_error-img-eng.user.zip" - self.assertEqual(self.AvdSpec._GetFlavorFromLocalImage(img_path), None) - - def testGetFlavorFromTarget(self): - """Test _GetFlavorFromTarget.""" - target_name = "cf_x86_auto-userdebug" - self.assertEqual(self.AvdSpec._GetFlavorFromTarget(target_name), "auto") - - # Target is not supported. - target_name = "cf_x86_error-userdebug" - self.assertEqual(self.AvdSpec._GetFlavorFromTarget(target_name), None) + self.assertEqual(self.AvdSpec._GetFlavorFromString(img_path), + None) # pylint: disable=protected-access def testProcessRemoteBuildArgs(self): @@ -212,6 +248,39 @@ class AvdSpecTest(unittest.TestCase): expected_result = " Manifest branch:" self.assertEqual(avd_spec.EscapeAnsi(test_string), expected_result) + def testGetGceLocalImagePath(self): + """Test get gce local image path.""" + self.Patch(os.path, "isfile", return_value=True) + # Verify when specify --local-image ~/XXX.tar.gz. + fake_image_path = "~/gce_local_image_dir/gce_image.tar.gz" + self.Patch(os.path, "exists", return_value=True) + self.assertEqual(self.AvdSpec._GetGceLocalImagePath(fake_image_path), + "~/gce_local_image_dir/gce_image.tar.gz") + + # Verify when specify --local-image ~/XXX.img. + fake_image_path = "~/gce_local_image_dir/gce_image.img" + self.assertEqual(self.AvdSpec._GetGceLocalImagePath(fake_image_path), + "~/gce_local_image_dir/gce_image.img") + + # Verify if exist argument --local-image as a directory. + self.Patch(os.path, "isfile", return_value=False) + self.Patch(os.path, "exists", return_value=True) + fake_image_path = "~/gce_local_image_dir/" + # Default to find */avd-system.tar.gz if exist then return the path. + self.assertEqual(self.AvdSpec._GetGceLocalImagePath(fake_image_path), + "~/gce_local_image_dir/avd-system.tar.gz") + + # Otherwise choose raw file */android_system_disk_syslinux.img if + # exist then return the path. + self.Patch(os.path, "exists", side_effect=[False, True]) + self.assertEqual(self.AvdSpec._GetGceLocalImagePath(fake_image_path), + "~/gce_local_image_dir/android_system_disk_syslinux.img") + + # Both _GCE_LOCAL_IMAGE_CANDIDATE could not be found then raise error. + self.Patch(os.path, "exists", side_effect=[False, False]) + self.assertRaises(errors.BootImgDoesNotExist, + self.AvdSpec._GetGceLocalImagePath, fake_image_path) + if __name__ == "__main__": unittest.main() diff --git a/create/base_avd_create.py b/create/base_avd_create.py index 109f0cfc..1e5341c3 100644 --- a/create/base_avd_create.py +++ b/create/base_avd_create.py @@ -68,7 +68,8 @@ class BaseAVDCreate(object): avd_spec.instance_type) if avd_spec.image_source == constants.IMAGE_SRC_LOCAL: utils.PrintColorString("Image (local):") - utils.PrintColorString(" %s" % avd_spec.local_image_dir) + utils.PrintColorString(" %s" % (avd_spec.local_image_dir or + avd_spec.local_image_artifact)) elif avd_spec.image_source == constants.IMAGE_SRC_REMOTE: utils.PrintColorString("Image:") utils.PrintColorString( diff --git a/create/cheeps_remote_image_remote_instance.py b/create/cheeps_remote_image_remote_instance.py index ef3b2605..1f6602f9 100644 --- a/create/cheeps_remote_image_remote_instance.py +++ b/create/cheeps_remote_image_remote_instance.py @@ -48,16 +48,16 @@ class CheepsRemoteImageRemoteInstance(base_avd_create.BaseAVDCreate): "Creating a cheeps device in project %s, build_id: %s", avd_spec.cfg.project, build_id) - device_factory = CheepsDeviceFactory(avd_spec.cfg, build_id) + device_factory = CheepsDeviceFactory(avd_spec.cfg, build_id, avd_spec) report = common_operations.CreateDevices( command="create_cheeps", cfg=avd_spec.cfg, device_factory=device_factory, num=avd_spec.num, + report_internal_ip=avd_spec.report_internal_ip, autoconnect=avd_spec.autoconnect, - vnc_port=constants.DEFAULT_CHEEPS_TARGET_VNC_PORT, - adb_port=constants.DEFAULT_CHEEPS_TARGET_ADB_PORT) + avd_type=constants.TYPE_CHEEPS) # Launch vnc client if we're auto-connecting. if avd_spec.autoconnect: @@ -76,12 +76,13 @@ class CheepsDeviceFactory(base_device_factory.BaseDeviceFactory): """ LOG_FILES = [] - def __init__(self, cfg, build_id): + def __init__(self, cfg, build_id, avd_spec=None): """Initialize. Args: cfg: An AcloudConfig instance. build_id: String, Build id, e.g. "2263051", "P2804227" + avd_spec: An AVDSpec instance. """ self.credentials = auth.CreateCredentials(cfg) @@ -91,6 +92,7 @@ class CheepsDeviceFactory(base_device_factory.BaseDeviceFactory): self._cfg = cfg self._build_id = build_id + self._avd_spec = avd_spec def CreateInstance(self): """Creates single configured cheeps device. @@ -103,5 +105,6 @@ class CheepsDeviceFactory(base_device_factory.BaseDeviceFactory): instance=instance, image_name=self._cfg.stable_cheeps_host_image_name, image_project=self._cfg.stable_cheeps_host_image_project, - build_id=self._build_id) + build_id=self._build_id, + avd_spec=self._avd_spec) return instance diff --git a/create/cheeps_remote_image_remote_instance_test.py b/create/cheeps_remote_image_remote_instance_test.py index 473838d6..df0e164a 100644 --- a/create/cheeps_remote_image_remote_instance_test.py +++ b/create/cheeps_remote_image_remote_instance_test.py @@ -72,6 +72,7 @@ class CheepsRemoteImageRemoteInstanceTest(driver_test_lib.BaseDriverTest): avd_spec.cfg = self._CreateCfg() avd_spec.remote_image = {constants.BUILD_ID: self.ANDROID_BUILD_ID} avd_spec.autoconnect = False + avd_spec.report_internal_ip = False instance = cheeps_remote_image_remote_instance.CheepsRemoteImageRemoteInstance() report = instance.Create(avd_spec, no_prompts=False) @@ -80,10 +81,12 @@ class CheepsRemoteImageRemoteInstanceTest(driver_test_lib.BaseDriverTest): instance=self.INSTANCE, image_name=self.CHEEPS_HOST_IMAGE_NAME, image_project=self.CHEEPS_HOST_IMAGE_PROJECT, - build_id=self.ANDROID_BUILD_ID) + build_id=self.ANDROID_BUILD_ID, + avd_spec=avd_spec) self.assertEquals(report.data, { "devices": [{ + "build_id": self.ANDROID_BUILD_ID, "instance_name": self.INSTANCE, "ip": self.IP.external, },], diff --git a/create/create.py b/create/create.py index 011eb8bf..9dfba024 100644 --- a/create/create.py +++ b/create/create.py @@ -32,6 +32,7 @@ from acloud.create import avd_spec from acloud.create import cheeps_remote_image_remote_instance from acloud.create import gce_local_image_remote_instance from acloud.create import gce_remote_image_remote_instance +from acloud.create import goldfish_remote_image_remote_instance from acloud.create import local_image_local_instance from acloud.create import local_image_remote_instance from acloud.create import remote_image_remote_instance @@ -63,6 +64,9 @@ _CREATOR_CLASS_DICT = { # Cheeps types (constants.TYPE_CHEEPS, constants.IMAGE_SRC_REMOTE, constants.INSTANCE_TYPE_REMOTE): cheeps_remote_image_remote_instance.CheepsRemoteImageRemoteInstance, + # GF types + (constants.TYPE_GF, constants.IMAGE_SRC_REMOTE, constants.INSTANCE_TYPE_REMOTE): + goldfish_remote_image_remote_instance.GoldfishRemoteImageRemoteInstance, } @@ -179,7 +183,7 @@ def _CheckForSetup(args): setup.Run(args) else: print("Please run '#acloud setup' so we can get your host setup") - sys.exit() + sys.exit(constants.EXIT_BY_USER) def PreRunCheck(args): diff --git a/create/create_args.py b/create/create_args.py index 638e2eab..4ed04f89 100644 --- a/create/create_args.py +++ b/create/create_args.py @@ -177,7 +177,9 @@ def GetCreateArgParser(subparser): default="", required=False, help="Use the locally built image for the AVD. Look for the image " - "artifact in $ANDROID_TARGET_OUT unless a path is specified.") + "artifact in $ANDROID_PRODUCT_OUT if no args value is provided." + "e.g --local-image or --local-image /path/to/dir or --local-image " + "/path/to/file") create_parser.add_argument( "--image-download-dir", type=str, @@ -208,6 +210,24 @@ def GetCreateArgParser(subparser): choices=constants.SPEC_NAMES, help="The name of a pre-configured device spec that we are " "going to use.") + # Arguments for goldfish type. + # TODO(b/118439885): Verify args that are used in wrong avd_type. + # e.g. $acloud create --avd-type cuttlefish --emulator-build-id + create_parser.add_argument( + "--gpu", + type=str, + dest="gpu", + required=False, + default=None, + help="'goldfish only' GPU accelerator to use if any. " + "e.g. nvidia-tesla-k80, omit to use swiftshader") + create_parser.add_argument( + "--emulator-build-id", + type=int, + dest="emulator_build_id", + required=False, + help="'goldfish only' Emulator build used to run the images. " + "e.g. 4669466.") AddCommonCreateArgs(create_parser) return create_parser @@ -220,7 +240,7 @@ def VerifyArgs(args): args: Namespace object from argparse.parse_args. Raises: - errors.CreateError: Path doesn't exist. + errors.CheckPathError: Zipped image path doesn't exist. errors.UnsupportedFlavor: Flavor doesn't support. """ # Verify that user specified flavor name is in support list. diff --git a/create/create_common.py b/create/create_common.py index 6598cf9c..34802289 100644 --- a/create/create_common.py +++ b/create/create_common.py @@ -20,12 +20,18 @@ from __future__ import print_function import glob import logging import os +import tempfile +import time +import zipfile from acloud import errors +from acloud.internal import constants from acloud.internal.lib import utils logger = logging.getLogger(__name__) +_ACLOUD_IMAGE_ZIP_POSTFIX = "-local-img-%s.zip" + def ParseHWPropertyArgs(dict_str, item_separator=",", key_value_separator=":"): """Helper function to initialize a dict object from string. @@ -62,43 +68,40 @@ def ParseHWPropertyArgs(dict_str, item_separator=",", key_value_separator=":"): return hw_dict -def VerifyLocalImageArtifactsExist(local_image_dir): - """Verify local image artifacts exists. - - Look for the image in the local_image_dir and dist dir as backup. +@utils.TimeExecute(function_description="Compressing images") +def ZipCFImageFiles(basedir): + """Zip images from basedir. - The image name follows the pattern: - Remote image: {target product}-img-{build id}.zip, - e.g. aosp_cf_x86_phone-img-5046769.zip - Local built image: {target product}-img-{username}.zip, - e.g. aosp_cf_x86_64_phone-img-eng.droid.zip + TODO(b/129376163):Use lzop for fast sparse image upload when host image + support it. Args: - local_image_dir: A string to specifies local image dir. + basedir: String of local images path. Return: - Strings of local image path. - - Raises: - errors.GetLocalImageError: Can't find local image. + Strings of zipped image path. """ - dirs_to_check = [local_image_dir] - dist_dir = utils.GetDistDir() - if dist_dir: - dirs_to_check.append(dist_dir) - for img_dir in dirs_to_check: - image_pattern = os.path.join(img_dir, "*img*.zip") - images = glob.glob(image_pattern) - if images: - break - if not images: - raise errors.GetLocalImageError("No images found in %s\n" - "(Try building with 'm dist')" % - [local_image_dir, dist_dir]) - if len(images) > 1: - print("Multiple images found, please choose 1.") - image_path = utils.GetAnswerFromList(images)[0] - else: - image_path = images[0] - logger.debug("Local image: %s ", image_path) - return image_path + tmp_folder = os.path.join(tempfile.gettempdir(), + constants.TEMP_ARTIFACTS_FOLDER) + if not os.path.exists(tmp_folder): + os.makedirs(tmp_folder) + archive_name = "%s-local-%d.zip" % (os.environ.get(constants.ENV_BUILD_TARGET), + int(time.time())) + archive_file = os.path.join(tmp_folder, archive_name) + if os.path.exists(archive_file): + raise errors.ZipImageError("This file shouldn't exist, please delete: %s" + % archive_file) + + zip_file = zipfile.ZipFile(archive_file, 'w', zipfile.ZIP_DEFLATED, + allowZip64=True) + required_files = ([os.path.join(basedir, "android-info.txt")] + + glob.glob(os.path.join(basedir, "*.img"))) + logger.debug("archiving images: %s", required_files) + + for f in required_files: + # Pass arcname arg to remove the directory structure. + zip_file.write(f, arcname=os.path.basename(f)) + + zip_file.close() + logger.debug("zip images done:%s", archive_file) + return archive_file diff --git a/create/create_common_test.py b/create/create_common_test.py index 25b193a0..2af3177a 100644 --- a/create/create_common_test.py +++ b/create/create_common_test.py @@ -13,15 +13,35 @@ # limitations under the License. """Tests for create_common.""" +import os +import tempfile +import time import unittest -import mock +import zipfile from acloud import errors from acloud.create import create_common +from acloud.internal import constants +from acloud.internal.lib import driver_test_lib + + + +class FakeZipFile(object): + """Fake implementation of ZipFile()""" + + # pylint: disable=invalid-name,unused-argument,no-self-use + def write(self, filename, arcname=None, compress_type=None): + """Fake write method.""" + return + + # pylint: disable=invalid-name,no-self-use + def close(self): + """Fake close method.""" + return # pylint: disable=invalid-name,protected-access -class CreateCommonTest(unittest.TestCase): +class CreateCommonTest(driver_test_lib.BaseDriverTest): """Test create_common functions.""" # pylint: disable=protected-access @@ -45,21 +65,25 @@ class CreateCommonTest(unittest.TestCase): result_dict = create_common.ParseHWPropertyArgs(args_str) self.assertTrue(expected_dict == result_dict) - @mock.patch("glob.glob") - def testVerifyLocalImageArtifactsExist(self, mock_glob): - """Test VerifyArtifactsPath.""" - #can't find the image - mock_glob.return_value = [] - self.assertRaises(errors.GetLocalImageError, - create_common.VerifyLocalImageArtifactsExist, - "/fake_dirs") - - mock_glob.return_value = [ - "/fake_dirs/aosp_cf_x86_phone-img-5046769.zip" - ] - self.assertEqual( - create_common.VerifyLocalImageArtifactsExist("/fake_dirs"), - "/fake_dirs/aosp_cf_x86_phone-img-5046769.zip") + def testZipCFImageFiles(self): + """Test ZipCFImageFiles.""" + # Should raise error if zip file already exists + fake_image_path = "/fake_image_dir/" + self.Patch(os.path, "exists", return_value=True) + self.Patch(os, "makedirs") + self.assertRaises(errors.ZipImageError, + create_common.ZipCFImageFiles, + fake_image_path) + + # Test should get archive name by timestamp if zip file does not exist. + self.Patch(zipfile, "ZipFile", return_value=FakeZipFile()) + self.Patch(os.path, "exists", return_value=False) + self.Patch(os.environ, "get", return_value="fake_build_target") + self.Patch(time, "time", return_value=12345) + self.Patch(tempfile, "gettempdir", return_value="/fake_temp") + self.assertEqual(create_common.ZipCFImageFiles(fake_image_path), + "/fake_temp/%s/fake_build_target-local-12345.zip" % + constants.TEMP_ARTIFACTS_FOLDER) if __name__ == "__main__": diff --git a/create/gce_local_image_remote_instance.py b/create/gce_local_image_remote_instance.py index 84c00307..05f5a030 100644 --- a/create/gce_local_image_remote_instance.py +++ b/create/gce_local_image_remote_instance.py @@ -18,15 +18,11 @@ local image. """ import logging -import os -from acloud import errors from acloud.create import base_avd_create from acloud.internal.lib import utils from acloud.public import device_driver -_GCE_LOCAL_IMAGE_CANDIDATE = ["avd-system.tar.gz", - "android_system_disk_syslinux.img"] logger = logging.getLogger(__name__) @@ -46,13 +42,12 @@ class GceLocalImageRemoteInstance(base_avd_create.BaseAVDCreate): Returns: A Report instance. """ - local_image_path = self._GetGceLocalImagePath(avd_spec.local_image_dir) - logger.info("GCE local image: %s", local_image_path) + logger.info("GCE local image: %s", avd_spec.local_image_artifact) report = device_driver.CreateAndroidVirtualDevices( avd_spec.cfg, num=avd_spec.num, - local_disk_image=local_image_path, + local_disk_image=avd_spec.local_image_artifact, autoconnect=avd_spec.autoconnect, report_internal_ip=avd_spec.report_internal_ip, avd_spec=avd_spec) @@ -62,32 +57,3 @@ class GceLocalImageRemoteInstance(base_avd_create.BaseAVDCreate): utils.LaunchVNCFromReport(report, avd_spec, no_prompts) return report - - - @staticmethod - def _GetGceLocalImagePath(local_image_dir): - """Get gce local image path. - - If local_image_dir is dir, prioritize find avd-system.tar.gz; - otherwise, find android_system_disk_syslinux.img. - - Args: - local_image_dir: A string to specify local image dir. - - Returns: - String, image file path if exists. - - Raises: - errors.BootImgDoesNotExist if image not exist. - """ - if os.path.isfile(local_image_dir): - return local_image_dir - - for img_name in _GCE_LOCAL_IMAGE_CANDIDATE: - full_file_path = os.path.join(local_image_dir, img_name) - if os.path.exists(full_file_path): - return full_file_path - - raise errors.BootImgDoesNotExist("Could not find any GCE images (%s), " - "you can build them via \"m dist\"" % - ", ".join(_GCE_LOCAL_IMAGE_CANDIDATE)) diff --git a/create/gce_local_image_remote_instance_test.py b/create/gce_local_image_remote_instance_test.py deleted file mode 100644 index 9b5161b2..00000000 --- a/create/gce_local_image_remote_instance_test.py +++ /dev/null @@ -1,75 +0,0 @@ -# Copyright 2019 - The Android Open Source Project -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -"""Tests for gce_local_image_remote_instance.""" - -import unittest -import os -import mock - -from acloud import errors -from acloud.create.gce_local_image_remote_instance import GceLocalImageRemoteInstance -from acloud.internal.lib import driver_test_lib - - -# pylint: disable=invalid-name, protected-access -class GceLocalImageRemoteInstanceTest(driver_test_lib.BaseDriverTest): - """Test gce_local_image_remote_instance methods.""" - - def setUp(self): - """Initialize gce_local_image_remote_instance.""" - super(GceLocalImageRemoteInstanceTest, self).setUp() - self.build_client = mock.MagicMock() - self.GceLocalImageRemoteInstance = GceLocalImageRemoteInstance() - - def testGetGceLocalImagePath(self): - """Test get gce local image path.""" - self.Patch(os.path, "isfile", return_value=True) - # Verify when specify --local-image ~/XXX.tar.gz. - fake_image_path = "~/gce_local_image_dir/gce_image.tar.gz" - self.Patch(os.path, "exists", return_value=True) - self.assertEqual( - self.GceLocalImageRemoteInstance._GetGceLocalImagePath( - fake_image_path), "~/gce_local_image_dir/gce_image.tar.gz") - - # Verify when specify --local-image ~/XXX.img. - fake_image_path = "~/gce_local_image_dir/gce_image.img" - self.assertEqual( - self.GceLocalImageRemoteInstance._GetGceLocalImagePath( - fake_image_path), "~/gce_local_image_dir/gce_image.img") - - # Verify if exist argument --local-image as a directory. - self.Patch(os.path, "isfile", return_value=False) - self.Patch(os.path, "exists", return_value=True) - fake_image_path = "~/gce_local_image_dir/" - # Default to find */avd-system.tar.gz if exist then return the path. - self.assertEqual( - self.GceLocalImageRemoteInstance._GetGceLocalImagePath( - fake_image_path), "~/gce_local_image_dir/avd-system.tar.gz") - - # Otherwise choose raw file */android_system_disk_syslinux.img if - # exist then return the path. - self.Patch(os.path, "exists", side_effect=[False, True]) - self.assertEqual( - self.GceLocalImageRemoteInstance._GetGceLocalImagePath( - fake_image_path), "~/gce_local_image_dir/android_system_disk_syslinux.img") - - # Both _GCE_LOCAL_IMAGE_CANDIDATE could not be found then raise error. - self.Patch(os.path, "exists", side_effect=[False, False]) - self.assertRaises(errors.BootImgDoesNotExist, - self.GceLocalImageRemoteInstance._GetGceLocalImagePath, - fake_image_path) - - -if __name__ == "__main__": - unittest.main() diff --git a/create/goldfish_remote_image_remote_instance.py b/create/goldfish_remote_image_remote_instance.py new file mode 100644 index 00000000..592ffafb --- /dev/null +++ b/create/goldfish_remote_image_remote_instance.py @@ -0,0 +1,45 @@ +# Copyright 2019 - The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +r"""GoldfishRemoteImageRemoteInstance class. + +Create class that is responsible for creating a goldfish remote instance AVD +with a remote image. +""" + +from acloud.create import base_avd_create +from acloud.internal.lib import utils +from acloud.public.actions import create_goldfish_action + + +class GoldfishRemoteImageRemoteInstance(base_avd_create.BaseAVDCreate): + """Create class for a remote image remote instance AVD.""" + + @utils.TimeExecute(function_description="Total time: ", + print_before_call=False, print_status=False) + def _CreateAVD(self, avd_spec, no_prompts): + """Create the AVD. + + Args: + avd_spec: AVDSpec object that tells us what we're going to create. + no_prompts: Boolean, True to skip all prompts. + + Returns: + A Report instance. + """ + report = create_goldfish_action.CreateDevices(avd_spec=avd_spec) + + # Launch vnc client if we're auto-connecting. + if avd_spec.autoconnect: + utils.LaunchVNCFromReport(report, avd_spec, no_prompts) + return report diff --git a/create/local_image_local_instance.py b/create/local_image_local_instance.py index 147cd3ae..d0e67f3b 100644 --- a/create/local_image_local_instance.py +++ b/create/local_image_local_instance.py @@ -82,8 +82,8 @@ class LocalImageLocalInstance(base_avd_create.BaseAVDCreate): result_report.SetStatus(report.Status.SUCCESS) result_report.AddData( key="devices", - value={"adb_port": constants.DEFAULT_ADB_PORT, - constants.VNC_PORT: constants.DEFAULT_VNC_PORT}) + value={constants.ADB_PORT: constants.CF_ADB_PORT, + constants.VNC_PORT: constants.CF_VNC_PORT}) # Launch vnc client if we're auto-connecting. if avd_spec.autoconnect: utils.LaunchVNCFromReport(result_report, avd_spec, no_prompts) @@ -132,7 +132,7 @@ class LocalImageLocalInstance(base_avd_create.BaseAVDCreate): launch_cvd_w_args = launch_cvd_path + _CMD_LAUNCH_CVD_ARGS % ( hw_property["cpu"], hw_property["x_res"], hw_property["y_res"], hw_property["dpi"], hw_property["memory"], hw_property["disk"], - system_image_dir, constants.DEFAULT_VNC_PORT) + system_image_dir, constants.CF_VNC_PORT) launch_cmd = utils.AddUserGroupsToCmd(launch_cvd_w_args, constants.LIST_CF_USER_GROUPS) @@ -165,11 +165,10 @@ class LocalImageLocalInstance(base_avd_create.BaseAVDCreate): stderr=dev_null, stdout=dev_null, shell=True) # Delete ssvnc viewer - delete.CleanupSSVncviewer(constants.CF_TARGET_VNC_PORT) + delete.CleanupSSVncviewer(constants.CF_VNC_PORT) else: - print("Exiting out") - sys.exit() + sys.exit(constants.EXIT_BY_USER) self._LaunchCvd(cmd) @staticmethod diff --git a/create/local_image_remote_instance.py b/create/local_image_remote_instance.py index 4d247acc..e507fb43 100644 --- a/create/local_image_remote_instance.py +++ b/create/local_image_remote_instance.py @@ -27,6 +27,7 @@ import subprocess from acloud import errors from acloud.create import base_avd_create +from acloud.create import create_common from acloud.internal import constants from acloud.internal.lib import auth from acloud.internal.lib import cvd_compute_client @@ -41,7 +42,6 @@ _CVD_USER = getpass.getuser() _CMD_LAUNCH_CVD_ARGS = (" -cpus %s -x_res %s -y_res %s -dpi %s " "-memory_mb %s -blank_data_image_mb %s " "-data_policy always_create ") - #Output to Serial port 1 (console) group in the instance _OUTPUT_CONSOLE_GROUPS = "tty" SSH_BIN = "ssh" @@ -155,7 +155,7 @@ class RemoteInstanceDeviceFactory(base_device_factory.BaseDeviceFactory): else ip.external)} return instance - @utils.TimeExecute(function_description="Setup GCE environment") + @utils.TimeExecute(function_description="Setting up GCE environment") def _SetAVDenv(self, cvd_user): """set the user to run AVD in the instance. @@ -183,7 +183,7 @@ class RemoteInstanceDeviceFactory(base_device_factory.BaseDeviceFactory): local_image_artifact: A string, path to local image. cvd_host_package_artifact: A string, path to cvd host package. """ - # local image + # TODO(b/129376163) Use lzop for fast sparse image upload remote_cmd = ("\"sudo su -c '/usr/bin/install_zip.sh .' - '%s'\" < %s" % (cvd_user, local_image_artifact)) logger.debug("remote_cmd:\n %s", remote_cmd) @@ -222,32 +222,20 @@ class LocalImageRemoteInstance(base_avd_create.BaseAVDCreate): """LocalImageRemoteInstance initialize.""" self.cvd_host_package_artifact = None - def VerifyArtifactsExist(self, local_image_dir): - """Verify required cuttlefish image artifacts exists. - - Arsg: - local_image_dir: A string, path to check the artifacts. - """ - self.cvd_host_package_artifact = self.VerifyHostPackageArtifactsExist( - local_image_dir) - - def VerifyHostPackageArtifactsExist(self, local_image_dir): + def VerifyHostPackageArtifactsExist(self): """Verify the host package exists and return its path. - Look for the host package in local_image_dir (when we download the - image artifacts from Android Build into 1 folder), if we can't find - it, look in the dist dir (if the user built the image locally). - - Args: - local_image_dir: A string, path to check for the host package. + Look for the host package in $ANDROID_HOST_OUT and dist dir. Return: A string, the path to the host package. """ - dirs_to_check = [local_image_dir] + dirs_to_check = filter(None, + [os.environ.get(constants.ENV_ANDROID_HOST_OUT)]) dist_dir = utils.GetDistDir() if dist_dir: dirs_to_check.append(dist_dir) + cvd_host_package_artifact = self.GetCvdHostPackage(dirs_to_check) logger.debug("cvd host package: %s", cvd_host_package_artifact) return cvd_host_package_artifact @@ -270,8 +258,9 @@ class LocalImageRemoteInstance(base_avd_create.BaseAVDCreate): if os.path.exists(cvd_host_package): return cvd_host_package raise errors.GetCvdLocalHostPackageError, ( - "Can't find the cvd host package (Try building with 'm dist'): \n%s" - % '\n'.join(paths)) + "Can't find the cvd host package (Try lunching a cuttlefish target" + " like aosp_cf_x86_phone-userdebug and running 'm'): \n%s" % + '\n'.join(paths)) @utils.TimeExecute(function_description="Total time: ", print_before_call=False, print_status=False) @@ -282,17 +271,23 @@ class LocalImageRemoteInstance(base_avd_create.BaseAVDCreate): avd_spec: AVDSpec object that tells us what we're going to create. no_prompts: Boolean, True to skip all prompts. """ - self.VerifyArtifactsExist(avd_spec.local_image_dir) + self.cvd_host_package_artifact = self.VerifyHostPackageArtifactsExist() + + if avd_spec.local_image_artifact: + local_image_artifact = avd_spec.local_image_artifact + else: + local_image_artifact = create_common.ZipCFImageFiles( + avd_spec.local_image_dir) + device_factory = RemoteInstanceDeviceFactory( avd_spec, - avd_spec.local_image_artifact, + local_image_artifact, self.cvd_host_package_artifact) report = common_operations.CreateDevices( "create_cf", avd_spec.cfg, device_factory, avd_spec.num, report_internal_ip=avd_spec.report_internal_ip, autoconnect=avd_spec.autoconnect, - vnc_port=constants.CF_TARGET_VNC_PORT, - adb_port=constants.CF_TARGET_ADB_PORT) + avd_type=constants.TYPE_CF) # Launch vnc client if we're auto-connecting. if avd_spec.autoconnect: utils.LaunchVNCFromReport(report, avd_spec, no_prompts) diff --git a/create/local_image_remote_instance_test.py b/create/local_image_remote_instance_test.py index d29d4929..1509d9fe 100644 --- a/create/local_image_remote_instance_test.py +++ b/create/local_image_remote_instance_test.py @@ -20,6 +20,8 @@ local image. """ import uuid +import glob +import os import subprocess import time import unittest @@ -30,16 +32,19 @@ from acloud import errors from acloud.create import avd_spec from acloud.create import create_common from acloud.create import local_image_remote_instance +from acloud.internal import constants from acloud.internal.lib import auth from acloud.internal.lib import cvd_compute_client from acloud.internal.lib import driver_test_lib +from acloud.internal.lib import utils -class LocalImageRemoteInstanceTest(unittest.TestCase): +class LocalImageRemoteInstanceTest(driver_test_lib.BaseDriverTest): """Test LocalImageRemoteInstance method.""" def setUp(self): """Initialize new LocalImageRemoteInstance.""" + super(LocalImageRemoteInstanceTest, self).setUp() self.local_image_remote_instance = local_image_remote_instance.LocalImageRemoteInstance() def testVerifyHostPackageArtifactsExist(self): @@ -49,17 +54,27 @@ class LocalImageRemoteInstanceTest(unittest.TestCase): exists.return_value = False self.assertRaises( errors.GetCvdLocalHostPackageError, - self.local_image_remote_instance.VerifyHostPackageArtifactsExist, - "/fake_dirs") - - def testVerifyArtifactsExist(self): - """test verify artifacts exist.""" + self.local_image_remote_instance.VerifyHostPackageArtifactsExist) + + self.Patch(os.environ, "get", return_value="/fake_dir2") + self.Patch(utils, "GetDistDir", return_value="/fake_dir1") + # First path is host out dir, 2nd path is dist dir. + self.Patch(os.path, "exists", + side_effect=[False, True]) + + # Find cvd host in dist dir. + self.assertEqual( + self.local_image_remote_instance.VerifyHostPackageArtifactsExist(), + "/fake_dir1/cvd-host_package.tar.gz") + + # Find cvd host in host out dir. + self.Patch(os.environ, "get", return_value="/fake_dir2") + self.Patch(utils, "GetDistDir", return_value=None) with mock.patch("os.path.exists") as exists: exists.return_value = True - self.local_image_remote_instance.VerifyArtifactsExist("/fake_dirs") self.assertEqual( - self.local_image_remote_instance.cvd_host_package_artifact, - "/fake_dirs/cvd-host_package.tar.gz") + self.local_image_remote_instance.VerifyHostPackageArtifactsExist(), + "/fake_dir2/cvd-host_package.tar.gz") class RemoteInstanceDeviceFactoryTest(driver_test_lib.BaseDriverTest): @@ -88,16 +103,19 @@ class RemoteInstanceDeviceFactoryTest(driver_test_lib.BaseDriverTest): self.assertEqual(factory._ShellCmdWithRetry("fake cmd"), True) # pylint: disable=protected-access - @mock.patch.object(create_common, "VerifyLocalImageArtifactsExist") - def testCreateGceInstanceName(self, mock_image): + def testCreateGceInstanceName(self): """test create gce instance.""" + self.Patch(utils, "GetBuildEnvironmentVariable", + return_value="test_environ") + self.Patch(glob, "glob", return_vale=["fake.img"]) + self.Patch(create_common, "ZipCFImageFiles", + return_value="/fake/aosp_cf_x86_phone-img-eng.username.zip") # Mock uuid args = mock.MagicMock() - args.local_image = "/tmp/path" args.config_file = "" - args.avd_type = "cf" + args.avd_type = constants.TYPE_CF args.flavor = "phone" - mock_image.return_value = "/fake/aosp_cf_x86_phone-img-eng.username.zip" + args.local_image = None fake_avd_spec = avd_spec.AVDSpec(args) fake_uuid = mock.MagicMock(hex="1234") diff --git a/create/remote_image_local_instance.py b/create/remote_image_local_instance.py index 779efdf7..08f02552 100644 --- a/create/remote_image_local_instance.py +++ b/create/remote_image_local_instance.py @@ -19,6 +19,7 @@ Create class that is responsible for creating a local instance AVD with a remote image. """ from __future__ import print_function +import glob import logging import os import subprocess @@ -46,9 +47,8 @@ _CONFIRM_DOWNLOAD_DIR = ("Download dir %(download_dir)s does not have enough " # Let's add an extra buffer (~2G) to make sure user has enough disk space # for the downloaded image artifacts. _REQUIRED_SPACE = 10 -_CF_IMAGES = ["cache.img", "cmdline", "kernel", "ramdisk.img", "system.img", - "userdata.img", "vendor.img"] _BOOT_IMAGE = "boot.img" +# TODO(b/129009852):UnpackBootImage and setfacl are deprecated. UNPACK_BOOTIMG_CMD = "%s -boot_img %s" % ( os.path.join(_CUTTLEFISH_COMMON_BIN_PATH, "unpack_boot_image.py"), "%s -dest %s") @@ -113,7 +113,7 @@ class RemoteImageLocalInstance(local_image_local_instance.LocalImageLocalInstanc extract_path = os.path.join( avd_spec.image_download_dir, - "acloud_image_artifacts", + constants.TEMP_ARTIFACTS_FOLDER, build_id) logger.debug("Extract path: %s", extract_path) @@ -192,18 +192,12 @@ class RemoteImageLocalInstance(local_image_local_instance.LocalImageLocalInstanc Args: extract_path: String, a path include extracted files. - - Raises: - errors.CheckPathError: Path doesn't exist. """ - logger.info("Start to acl files: %s", ",".join(_CF_IMAGES)) - for image in _CF_IMAGES: - image_path = os.path.join(extract_path, image) - if not os.path.exists(image_path): - raise errors.CheckPathError( - "Specified file doesn't exist: %s" % image_path) + image_list = glob.glob(os.path.join(extract_path, "*.img")) + logger.info("Start to set ACLs on files: %s", ",".join(image_list)) + for image_path in image_list: subprocess.check_call(ACL_CMD % image_path, shell=True) - logger.info("ACL files completed!") + logger.info("The ACLs have set completed!") @staticmethod def _ConfirmDownloadRemoteImageDir(download_dir): @@ -228,8 +222,7 @@ class RemoteImageLocalInstance(local_image_local_instance.LocalImageLocalInstanc if answer.lower() == "y": os.makedirs(download_dir) else: - print("Exiting acloud!") - sys.exit() + sys.exit(constants.EXIT_BY_USER) stat = os.statvfs(download_dir) available_space = stat.f_bavail*stat.f_bsize/(1024)**3 @@ -239,7 +232,6 @@ class RemoteImageLocalInstance(local_image_local_instance.LocalImageLocalInstanc "available_space":available_space, "required_space":_REQUIRED_SPACE}) if download_dir.lower() == "q": - print("Exiting acloud!") - sys.exit() + sys.exit(constants.EXIT_BY_USER) else: return download_dir diff --git a/create/remote_image_local_instance_test.py b/create/remote_image_local_instance_test.py index 4f5bd15e..14bb3a1b 100644 --- a/create/remote_image_local_instance_test.py +++ b/create/remote_image_local_instance_test.py @@ -127,18 +127,6 @@ class RemoteImageLocalInstanceTest(driver_test_lib.BaseDriverTest): self.RemoteImageLocalInstance._UnpackBootImage, self._extract_path) - @mock.patch.object(subprocess, "check_call") - def testAclCfImageFiles(self, mock_call): - """Test acl related files.""" - self.Patch(os.path, "exists", - side_effect=[True, True, True, True, False, True, True]) - # Raise error when acl required file does not exist at 5th run cehck_call. - self.assertRaises(errors.CheckPathError, - self.RemoteImageLocalInstance._AclCfImageFiles, - self._extract_path) - # it should be run check_call 4 times before raise error. - self.assertEqual(mock_call.call_count, 4) - def testConfirmDownloadRemoteImageDir(self): """Test confirm download remote image dir""" self.Patch(os.path, "exists", return_value=True) diff --git a/delete/delete.py b/delete/delete.py index 17531320..2d5ec700 100644 --- a/delete/delete.py +++ b/delete/delete.py @@ -173,8 +173,8 @@ def DeleteLocalInstance(): except subprocess.CalledProcessError as e: delete_report.AddError(str(e)) delete_report.SetStatus(report.Status.FAIL) - - CleanupSSVncviewer(constants.DEFAULT_VNC_PORT) + # Only CF supports local instances so assume it's a CF VNC port. + CleanupSSVncviewer(constants.CF_VNC_PORT) return delete_report @@ -202,6 +202,10 @@ def Run(args): print("There is no local instance AVD to delete.") return report.Report(command="delete") + if args.adb_port: + return DeleteInstances( + cfg, list_instances.GetInstanceFromAdbPort(cfg, args.adb_port)) + # Provide instances list to user and let user choose what to delete if user # didn't specific instance name in args. return DeleteInstances(cfg, list_instances.ChooseInstances(cfg, args.all)) diff --git a/delete/delete_args.py b/delete/delete_args.py index b9c9dc67..dc9c1066 100644 --- a/delete/delete_args.py +++ b/delete/delete_args.py @@ -34,29 +34,36 @@ def GetDeleteArgParser(subparser): delete_parser = subparser.add_parser(CMD_DELETE) delete_parser.required = False delete_parser.set_defaults(which=CMD_DELETE) - delete_parser.add_argument( + delete_group = delete_parser.add_mutually_exclusive_group() + delete_group.add_argument( "--instance-names", dest="instance_names", nargs="+", required=False, help="The names of the remote instances that need to delete, " "separated by spaces, e.g. --instance-names instance-1 instance-2") - delete_parser.add_argument( + delete_group.add_argument( "--all", action="store_true", dest="all", required=False, help="If more than 1 AVD instance is found, delete them all.") - delete_parser.add_argument( + delete_group.add_argument( "--local-instance", action="store_true", dest="local_instance", required=False, help="Only delete the local instance.") + delete_group.add_argument( + "--adb-port", "-p", + type=int, + dest="adb_port", + required=False, + help="Delete instance with specified adb-port.") # TODO(b/118439885): Old arg formats to support transition, delete when # transistion is done. - delete_parser.add_argument( + delete_group.add_argument( "--instance_names", dest="instance_names", nargs="+", @@ -205,3 +205,7 @@ class NoInstancesFound(ReconnectError): class FunctionTimeoutError(Exception): """Timeout error of decorator function.""" + + +class ZipImageError(Exception): + """Zip image error.""" diff --git a/internal/constants.py b/internal/constants.py index aaec56ab..8ea18b31 100755 --- a/internal/constants.py +++ b/internal/constants.py @@ -91,31 +91,29 @@ HW_Y_RES = "y_res" USER_ANSWER_YES = {"y", "yes", "Y"} # Cuttlefish groups -LIST_CF_USER_GROUPS = ["kvm", "libvirt", "cvdnetwork"] -#For the cuttlefish remote instances: adb port is 6520 and vnc is 6444. -CF_TARGET_ADB_PORT = 6520 -CF_TARGET_VNC_PORT = 6444 +LIST_CF_USER_GROUPS = ["kvm", "cvdnetwork"] +ADB_PORT = "adb_port" +VNC_PORT = "vnc_port" +# For cuttlefish remote instances +CF_ADB_PORT = 6520 +CF_VNC_PORT = 6444 +# For cheeps remote instances +CHEEPS_ADB_PORT = 9222 +CHEEPS_VNC_PORT = 5900 # For gce_x86_phones remote instances -DEFAULT_GCE_VNC_PORT = 6444 -DEFAULT_GCE_ADB_PORT = 5555 +GCE_ADB_PORT = 5555 +GCE_VNC_PORT = 6444 # For goldfish remote instances -DEFAULT_GOLDFISH_VNC_PORT = 6444 -DEFAULT_GOLDFISH_ADB_PORT = 5555 -# For cuttlefish remote instances -DEFAULT_VNC_PORT = 6444 -DEFAULT_ADB_PORT = 6520 -VNC_PORT = "vnc_port" -ADB_PORT = "adb_port" - -# For cheeps remote instances. -DEFAULT_CHEEPS_TARGET_ADB_PORT = 9222 -DEFAULT_CHEEPS_TARGET_VNC_PORT = 5900 +GF_ADB_PORT = 5555 +GF_VNC_PORT = 6444 COMMAND_PS = ["ps", "aux"] CMD_LAUNCH_CVD = "launch_cvd" CMD_STOP_CVD = "stop_cvd" ENV_ANDROID_BUILD_TOP = "ANDROID_BUILD_TOP" +ENV_ANDROID_HOST_OUT = "ANDROID_HOST_OUT" +ENV_BUILD_TARGET = "TARGET_PRODUCT" LOCALHOST = "127.0.0.1" LOCALHOST_ADB_SERIAL = LOCALHOST + ":%d" @@ -139,3 +137,8 @@ INS_KEY_AVD_FLAVOR = "flavor" INS_KEY_IS_LOCAL = "remote" INS_STATUS_RUNNING = "RUNNING" LOCAL_INS_NAME = "local-instance" + +TEMP_ARTIFACTS_FOLDER = "acloud_image_artifacts" +TOOL_NAME = "acloud" +EXIT_BY_USER = 1 +EXIT_BY_ERROR = -99 diff --git a/internal/lib/android_build_client.py b/internal/lib/android_build_client.py index 32a11791..b2bf3bd0 100644 --- a/internal/lib/android_build_client.py +++ b/internal/lib/android_build_client.py @@ -85,7 +85,7 @@ class AndroidBuildClient(base_cloud_client.BaseCloudApiClient): while not done: _, done = downloader.next_chunk() logger.info("Downloaded artifact: %s", local_dest) - except OSError as e: + except (OSError, apiclient.errors.HttpError) as e: logger.error("Downloading artifact failed: %s", str(e)) raise errors.DriverError(str(e)) @@ -170,6 +170,7 @@ class AndroidBuildClient(base_cloud_client.BaseCloudApiClient): maxResults=self.ONE_RESULT, successful=self.BUILD_SUCCESSFUL) build = self.Execute(api) + logger.info("GetLKGB build API response: %s", build) if build: return str(build.get("builds")[0].get("buildId")) raise errors.GetBuildIDError( diff --git a/internal/lib/android_compute_client.py b/internal/lib/android_compute_client.py index 4d6b239e..2c77cfbf 100755 --- a/internal/lib/android_compute_client.py +++ b/internal/lib/android_compute_client.py @@ -47,7 +47,6 @@ logger = logging.getLogger(__name__) class AndroidComputeClient(gcompute_client.ComputeClient): """Client that manages Anadroid Virtual Device.""" - INSTANCE_NAME_FMT = "ins-{uuid}-{build_id}-{build_target}" IMAGE_NAME_FMT = "img-{uuid}-{build_id}-{build_target}" DATA_DISK_NAME_FMT = "data-{instance}" BOOT_COMPLETED_MSG = "VIRTUAL_DEVICE_BOOT_COMPLETED" @@ -78,6 +77,8 @@ class AndroidComputeClient(gcompute_client.ComputeClient): self._resolution = acloud_config.resolution self._metadata = acloud_config.metadata_variable.copy() self._ssh_public_key_path = acloud_config.ssh_public_key_path + self._launch_args = acloud_config.launch_args + self._instance_name_pattern = acloud_config.instance_name_pattern @classmethod def _FormalizeName(cls, name): @@ -149,8 +150,7 @@ class AndroidComputeClient(gcompute_client.ComputeClient): name = cls.DATA_DISK_NAME_FMT.format(instance=instance) return cls._FormalizeName(name) - @classmethod - def GenerateInstanceName(cls, build_target=None, build_id=None): + def GenerateInstanceName(self, build_target=None, build_id=None): """Generate an instance name given build_target, build_id. Target is not used as instance name has a length limit. @@ -162,13 +162,10 @@ class AndroidComputeClient(gcompute_client.ComputeClient): Returns: A string, representing instance name. """ - if not build_target and not build_id: - return "instance-" + uuid.uuid4().hex - name = cls.INSTANCE_NAME_FMT.format( - build_target=build_target, - build_id=build_id, - uuid=uuid.uuid4().hex[:8]).replace("_", "-") - return cls._FormalizeName(name) + name = self._instance_name_pattern.format(build_target=build_target, + build_id=build_id, + uuid=uuid.uuid4().hex[:8]) + return self._FormalizeName(name) def CreateDisk(self, disk_name, @@ -241,8 +238,10 @@ class AndroidComputeClient(gcompute_client.ComputeClient): gpu=None, extra_disk_name=None, labels=None, - avd_spec=None): + avd_spec=None, + extra_scopes=None): """Create a gce instance with a gce image. + Args: instance: String, instance name. image_name: String, source image used to create this disk. @@ -262,6 +261,8 @@ class AndroidComputeClient(gcompute_client.ComputeClient): extra_disk_name: String,the name of the extra disk to attach. labels: Dict, will be added to the instance's labels. avd_spec: AVDSpec object that tells us what we're going to create. + extra_scopes: List, extra scopes (strings) to be passed to the + instance. """ self._CheckMachineSize() disk_args = self._GetDiskArgs(instance, image_name) @@ -296,7 +297,7 @@ class AndroidComputeClient(gcompute_client.ComputeClient): super(AndroidComputeClient, self).CreateInstance( instance, image_name, self._machine_type, metadata, self._network, self._zone, disk_args, image_project, gpu, extra_disk_name, - labels=labels) + labels=labels, extra_scopes=extra_scopes) def CheckBootFailure(self, serial_out, instance): """Determine if serial output has indicated any boot failure. diff --git a/internal/lib/android_compute_client_test.py b/internal/lib/android_compute_client_test.py index 9b4bdca6..f73913f9 100644 --- a/internal/lib/android_compute_client_test.py +++ b/internal/lib/android_compute_client_test.py @@ -48,6 +48,7 @@ class AndroidComputeClientTest(driver_test_lib.BaseDriverTest): DPI = 160 X_RES = 720 Y_RES = 1280 + EXTRA_SCOPES = None def _GetFakeConfig(self): """Create a fake configuration object. @@ -67,6 +68,7 @@ class AndroidComputeClientTest(driver_test_lib.BaseDriverTest): fake_cfg.orientation = self.ORIENTATION fake_cfg.resolution = self.DEVICE_RESOLUTION fake_cfg.metadata_variable = {self.METADATA[0]: self.METADATA[1]} + fake_cfg.extra_scopes = self.EXTRA_SCOPES return fake_cfg def setUp(self): @@ -137,7 +139,7 @@ class AndroidComputeClientTest(driver_test_lib.BaseDriverTest): instance_name, self.IMAGE, self.MACHINE_TYPE, expected_metadata, self.NETWORK, self.ZONE, expected_disk_args, image_project, gpu, extra_disk_name, - labels=labels) + labels=labels, extra_scopes=self.EXTRA_SCOPES) # pylint: disable=invalid-name def testCheckMachineSizeMeetsRequirement(self): diff --git a/internal/lib/cheeps_compute_client.py b/internal/lib/cheeps_compute_client.py index 5ef344a8..98e249bd 100644 --- a/internal/lib/cheeps_compute_client.py +++ b/internal/lib/cheeps_compute_client.py @@ -38,6 +38,7 @@ import getpass import logging from acloud import errors +from acloud.internal import constants from acloud.internal.lib import android_compute_client from acloud.internal.lib import gcompute_client @@ -49,7 +50,7 @@ class CheepsComputeClient(android_compute_client.AndroidComputeClient): Cheeps is a VM that run Chrome OS which runs on GCE. """ # This is the timeout for betty to start. - BOOT_TIMEOUT_SECS = 10*60 + BOOT_TIMEOUT_SECS = 15*60 # This is printed by betty.sh. BOOT_COMPLETED_MSG = "VM successfully started" # systemd prints this if betty.sh returns nonzero status code. @@ -62,7 +63,7 @@ class CheepsComputeClient(android_compute_client.AndroidComputeClient): # pylint: disable=too-many-locals,arguments-differ def CreateInstance(self, instance, image_name, image_project, - build_id=None): + build_id=None, avd_spec=None): """ Creates a cheeps instance in GCE. Args: @@ -71,11 +72,23 @@ class CheepsComputeClient(android_compute_client.AndroidComputeClient): image_project: project the GCE image is in build_id: (optional) the Android build id to use. To specify a different betty image you should use a different image_name + avd_spec: An AVDSpec instance. """ metadata = self._metadata.copy() + metadata[constants.INS_KEY_AVD_TYPE] = constants.TYPE_CHEEPS if build_id: metadata['android_build_id'] = build_id + # Update metadata by avd_spec + if avd_spec: + metadata["cvd_01_x_res"] = avd_spec.hw_property[constants.HW_X_RES] + metadata["cvd_01_y_res"] = avd_spec.hw_property[constants.HW_Y_RES] + metadata["cvd_01_dpi"] = avd_spec.hw_property[constants.HW_ALIAS_DPI] + metadata[constants.INS_KEY_DISPLAY] = ("%sx%s (%s)" % ( + avd_spec.hw_property[constants.HW_X_RES], + avd_spec.hw_property[constants.HW_Y_RES], + avd_spec.hw_property[constants.HW_ALIAS_DPI])) + # Add per-instance ssh key if self._ssh_public_key_path: rsa = self._LoadSshPublicKey(self._ssh_public_key_path) @@ -87,6 +100,10 @@ class CheepsComputeClient(android_compute_client.AndroidComputeClient): logger.warning("ssh_public_key_path is not specified in config, " "only project-wide key will be effective.") + # Add labels for giving the instances ability to be filter for + # acloud list/delete cmds. + labels = {constants.LABEL_CREATE_BY: getpass.getuser()} + gcompute_client.ComputeClient.CreateInstance( self, instance=instance, @@ -97,4 +114,4 @@ class CheepsComputeClient(android_compute_client.AndroidComputeClient): machine_type=self._machine_type, network=self._network, zone=self._zone, - ) + labels=labels) diff --git a/internal/lib/cheeps_compute_client_test.py b/internal/lib/cheeps_compute_client_test.py index fa163fa0..f80c0c44 100644 --- a/internal/lib/cheeps_compute_client_test.py +++ b/internal/lib/cheeps_compute_client_test.py @@ -18,6 +18,7 @@ import unittest import mock +from acloud.internal import constants from acloud.internal.lib import cheeps_compute_client from acloud.internal.lib import driver_test_lib from acloud.internal.lib import gcompute_client @@ -36,6 +37,9 @@ class CheepsComputeClientTest(driver_test_lib.BaseDriverTest): METADATA = {"metadata_key": "metadata_value"} BOOT_DISK_SIZE_GB = 10 ANDROID_BUILD_ID = 123 + DPI = 320 + X_RES = 720 + Y_RES = 1280 def _GetFakeConfig(self): """Create a fake configuration object. @@ -49,6 +53,8 @@ class CheepsComputeClientTest(driver_test_lib.BaseDriverTest): fake_cfg.machine_type = self.MACHINE_TYPE fake_cfg.network = self.NETWORK fake_cfg.zone = self.ZONE + fake_cfg.resolution = "{x}x{y}x32x{dpi}".format( + x=self.X_RES, y=self.Y_RES, dpi=self.DPI) fake_cfg.metadata_variable = self.METADATA return fake_cfg @@ -69,14 +75,34 @@ class CheepsComputeClientTest(driver_test_lib.BaseDriverTest): return_value={"diskSizeGb": self.BOOT_DISK_SIZE_GB}) self.Patch(gcompute_client.ComputeClient, "CreateInstance") - def testCreateInstance(self): + @mock.patch("getpass.getuser", return_value="fake_user") + def testCreateInstance(self, _mock_user): """Test CreateInstance.""" - expected_metadata = {'android_build_id': self.ANDROID_BUILD_ID} + expected_metadata = { + 'android_build_id': self.ANDROID_BUILD_ID, + 'avd_type': "cheeps", + 'cvd_01_dpi': str(self.DPI), + 'cvd_01_x_res': str(self.X_RES), + 'cvd_01_y_res': str(self.Y_RES), + 'display': "%sx%s (%s)"%( + str(self.X_RES), + str(self.Y_RES), + str(self.DPI))} expected_metadata.update(self.METADATA) + expected_labels = {'created_by': "fake_user"} + + avd_spec = mock.MagicMock() + avd_spec.hw_property = {constants.HW_X_RES: str(self.X_RES), + constants.HW_Y_RES: str(self.Y_RES), + constants.HW_ALIAS_DPI: str(self.DPI)} self.cheeps_compute_client.CreateInstance( - self.INSTANCE, self.IMAGE, self.IMAGE_PROJECT, self.ANDROID_BUILD_ID) + self.INSTANCE, + self.IMAGE, + self.IMAGE_PROJECT, + self.ANDROID_BUILD_ID, + avd_spec) # pylint: disable=no-member gcompute_client.ComputeClient.CreateInstance.assert_called_with( self.cheeps_compute_client, @@ -87,7 +113,8 @@ class CheepsComputeClientTest(driver_test_lib.BaseDriverTest): metadata=expected_metadata, machine_type=self.MACHINE_TYPE, network=self.NETWORK, - zone=self.ZONE) + zone=self.ZONE, + labels=expected_labels) if __name__ == "__main__": unittest.main() diff --git a/internal/lib/cvd_compute_client.py b/internal/lib/cvd_compute_client.py index 76974056..920e807f 100644 --- a/internal/lib/cvd_compute_client.py +++ b/internal/lib/cvd_compute_client.py @@ -61,21 +61,23 @@ class CvdComputeClient(android_compute_client.AndroidComputeClient): def CreateInstance(self, instance, image_name, image_project, build_target=None, branch=None, build_id=None, kernel_branch=None, kernel_build_id=None, - blank_data_disk_size_gb=None, avd_spec=None): + blank_data_disk_size_gb=None, avd_spec=None, + extra_scopes=None): """Create a cuttlefish instance given stable host image and build id. Args: instance: instance name. image_name: A string, the name of the GCE image. - image_project: A string, name of the project where the image belongs. + image_project: A string, name of the project where the image lives. Assume the default project if None. build_target: Target name, e.g. "aosp_cf_x86_phone-userdebug" branch: Branch name, e.g. "aosp-master" build_id: Build id, a string, e.g. "2263051", "P2804227" - kernel_branch: Kernel branch name, e.g. "kernel-android-cf-4.4-x86_64" - kernel_build_id: Kernel build id, a string, e.g. "2263051", "P2804227" + kernel_branch: Kernel branch name, e.g. "kernel-common-android-4.14" + kernel_build_id: Kernel build id, a string, e.g. "223051", "P280427" blank_data_disk_size_gb: Size of the blank data disk in GB. avd_spec: An AVDSpec instance. + extra_scopes: A list of extra scopes to be passed to the instance. """ self._CheckMachineSize() @@ -100,7 +102,8 @@ class CvdComputeClient(android_compute_client.AndroidComputeClient): if kernel_branch and kernel_build_id: metadata["cvd_01_fetch_kernel_bid"] = "{branch}/{build_id}".format( branch=kernel_branch, build_id=kernel_build_id) - metadata["cvd_01_launch"] = "1" + metadata["cvd_01_launch"] = (self._launch_args + if self._launch_args else "1") # The cuttlefish-google tools changed the usage of this cvd_01_launch # variable. For the local image, we remove the cvd_01_launch from @@ -168,4 +171,5 @@ class CvdComputeClient(android_compute_client.AndroidComputeClient): machine_type=self._machine_type, network=self._network, zone=self._zone, - labels=labels) + labels=labels, + extra_scopes=extra_scopes) diff --git a/internal/lib/cvd_compute_client_test.py b/internal/lib/cvd_compute_client_test.py index 93b63b67..aea25798 100644 --- a/internal/lib/cvd_compute_client_test.py +++ b/internal/lib/cvd_compute_client_test.py @@ -16,15 +16,16 @@ """Tests for acloud.internal.lib.cvd_compute_client.""" +import glob import unittest import mock from acloud.create import avd_spec -from acloud.create import create_common from acloud.internal import constants from acloud.internal.lib import cvd_compute_client from acloud.internal.lib import driver_test_lib from acloud.internal.lib import gcompute_client +from acloud.internal.lib import utils class CvdComputeClientTest(driver_test_lib.BaseDriverTest): @@ -48,6 +49,8 @@ class CvdComputeClientTest(driver_test_lib.BaseDriverTest): METADATA = {"metadata_key": "metadata_value"} EXTRA_DATA_DISK_SIZE_GB = 4 BOOT_DISK_SIZE_GB = 10 + LAUNCH_ARGS = "--setupwizard_mode=REQUIRED" + EXTRA_SCOPES = ["scope1"] def _GetFakeConfig(self): """Create a fake configuration object. @@ -64,6 +67,8 @@ class CvdComputeClientTest(driver_test_lib.BaseDriverTest): x=self.X_RES, y=self.Y_RES, dpi=self.DPI) fake_cfg.metadata_variable = self.METADATA fake_cfg.extra_data_disk_size_gb = self.EXTRA_DATA_DISK_SIZE_GB + fake_cfg.launch_args = self.LAUNCH_ARGS + fake_cfg.extra_scopes = self.EXTRA_SCOPES return fake_cfg def setUp(self): @@ -73,7 +78,8 @@ class CvdComputeClientTest(driver_test_lib.BaseDriverTest): self.cvd_compute_client = cvd_compute_client.CvdComputeClient( self._GetFakeConfig(), mock.MagicMock()) - @mock.patch.object(create_common, "VerifyLocalImageArtifactsExist") + @mock.patch.object(utils, "GetBuildEnvironmentVariable", return_value="fake_env") + @mock.patch.object(glob, "glob", return_value=["fake.img"]) @mock.patch.object(gcompute_client.ComputeClient, "CompareMachineSize", return_value=1) @mock.patch.object(gcompute_client.ComputeClient, "GetImage", @@ -83,7 +89,8 @@ class CvdComputeClientTest(driver_test_lib.BaseDriverTest): return_value=[{"fake_arg": "fake_value"}]) @mock.patch("getpass.getuser", return_value="fake_user") def testCreateInstance(self, _get_user, _get_disk_args, mock_create, - _get_image, _compare_machine_size, mock_img_path): + _get_image, _compare_machine_size, mock_check_img, + _mock_env): """Test CreateInstance.""" expected_metadata = { "cvd_01_dpi": str(self.DPI), @@ -101,14 +108,14 @@ class CvdComputeClientTest(driver_test_lib.BaseDriverTest): } expected_metadata.update(self.METADATA) remote_image_metadata = dict(expected_metadata) - remote_image_metadata["cvd_01_launch"] = "1" + remote_image_metadata["cvd_01_launch"] = self.LAUNCH_ARGS expected_disk_args = [{"fake_arg": "fake_value"}] self.cvd_compute_client.CreateInstance( - self.INSTANCE, self.IMAGE, self.IMAGE_PROJECT, self.TARGET, self.BRANCH, - self.BUILD_ID, self.KERNEL_BRANCH, self.KERNEL_BUILD_ID, - self.EXTRA_DATA_DISK_SIZE_GB) - # gcompute_client.ComputeClient.CreateInstance.assert_called_with( + self.INSTANCE, self.IMAGE, self.IMAGE_PROJECT, self.TARGET, + self.BRANCH, self.BUILD_ID, self.KERNEL_BRANCH, + self.KERNEL_BUILD_ID, self.EXTRA_DATA_DISK_SIZE_GB, + extra_scopes=self.EXTRA_SCOPES) mock_create.assert_called_with( self.cvd_compute_client, instance=self.INSTANCE, @@ -119,15 +126,16 @@ class CvdComputeClientTest(driver_test_lib.BaseDriverTest): machine_type=self.MACHINE_TYPE, network=self.NETWORK, zone=self.ZONE, - labels={constants.LABEL_CREATE_BY: "fake_user"}) + labels={constants.LABEL_CREATE_BY: "fake_user"}, + extra_scopes=self.EXTRA_SCOPES) #test use local image in the remote instance. local_image_metadata = dict(expected_metadata) args = mock.MagicMock() - mock_img_path.return_value = "cf_x86_phone-img-eng.user.zip" - args.local_image = "/tmp/path" + mock_check_img.return_value = True + args.local_image = None args.config_file = "" - args.avd_type = "cf" + args.avd_type = constants.TYPE_CF args.flavor = "phone" fake_avd_spec = avd_spec.AVDSpec(args) fake_avd_spec.hw_property[constants.HW_X_RES] = str(self.X_RES) @@ -135,7 +143,7 @@ class CvdComputeClientTest(driver_test_lib.BaseDriverTest): fake_avd_spec.hw_property[constants.HW_ALIAS_DPI] = str(self.DPI) fake_avd_spec.hw_property[constants.HW_ALIAS_DISK] = str( self.EXTRA_DATA_DISK_SIZE_GB * 1024) - local_image_metadata["avd_type"] = "cf" + local_image_metadata["avd_type"] = constants.TYPE_CF local_image_metadata["flavor"] = "phone" local_image_metadata[constants.INS_KEY_DISPLAY] = ("%sx%s (%s)" % ( fake_avd_spec.hw_property[constants.HW_X_RES], @@ -144,7 +152,8 @@ class CvdComputeClientTest(driver_test_lib.BaseDriverTest): self.cvd_compute_client.CreateInstance( self.INSTANCE, self.IMAGE, self.IMAGE_PROJECT, self.TARGET, self.BRANCH, self.BUILD_ID, self.KERNEL_BRANCH, self.KERNEL_BUILD_ID, - self.EXTRA_DATA_DISK_SIZE_GB, fake_avd_spec) + self.EXTRA_DATA_DISK_SIZE_GB, fake_avd_spec, + extra_scopes=self.EXTRA_SCOPES) expected_labels = {constants.LABEL_CREATE_BY: "fake_user"} mock_create.assert_called_with( @@ -157,7 +166,8 @@ class CvdComputeClientTest(driver_test_lib.BaseDriverTest): machine_type=self.MACHINE_TYPE, network=self.NETWORK, zone=self.ZONE, - labels=expected_labels) + labels=expected_labels, + extra_scopes=self.EXTRA_SCOPES) if __name__ == "__main__": diff --git a/internal/lib/gcompute_client.py b/internal/lib/gcompute_client.py index 1f44de05..759fb651 100755 --- a/internal/lib/gcompute_client.py +++ b/internal/lib/gcompute_client.py @@ -39,6 +39,11 @@ from acloud.internal.lib import utils logger = logging.getLogger(__name__) _MAX_RETRIES_ON_FINGERPRINT_CONFLICT = 10 +_METADATA_KEY = "key" +_METADATA_KEY_VALUE = "value" +_SSH_KEYS_NAME = "sshKeys" +_ITEMS = "items" +_METADATA = "metadata" BASE_DISK_ARGS = { "type": "PERSISTENT", @@ -1078,7 +1083,8 @@ class ComputeClient(base_cloud_client.BaseCloudApiClient): image_project=None, gpu=None, extra_disk_name=None, - labels=None): + labels=None, + extra_scopes=None): """Create a gce instance with a gce image. Args: @@ -1099,11 +1105,18 @@ class ComputeClient(base_cloud_client.BaseCloudApiClient): https://cloud.google.com/compute/docs/gpus/add-gpus extra_disk_name: String,the name of the extra disk to attach. labels: Dict, will be added to the instance's labels. + extra_scopes: A list of extra scopes to be provided to the instance. """ disk_args = (disk_args or self._GetDiskArgs(instance, image_name, image_project)) if extra_disk_name: disk_args.extend(self._GetExtraDiskArgs(extra_disk_name, zone)) + + scopes = [] + scopes.extend(self.DEFAULT_INSTANCE_SCOPE) + if extra_scopes: + scopes.extend(extra_scopes) + body = { "machineType": self.GetMachineType(machine_type, zone)["selfLink"], "name": instance, @@ -1111,10 +1124,11 @@ class ComputeClient(base_cloud_client.BaseCloudApiClient): "disks": disk_args, "serviceAccounts": [{ "email": "default", - "scopes": self.DEFAULT_INSTANCE_SCOPE + "scopes": scopes, }], } + if labels is not None: body["labels"] = labels if gpu: @@ -1127,10 +1141,10 @@ class ComputeClient(base_cloud_client.BaseCloudApiClient): body["scheduling"] = {"onHostMaintenance": "terminate"} if metadata: metadata_list = [{ - "key": key, - "value": val + _METADATA_KEY: key, + _METADATA_KEY_VALUE: val } for key, val in metadata.iteritems()] - body["metadata"] = {"items": metadata_list} + body[_METADATA] = {_ITEMS: metadata_list} logger.info("Creating instance: project %s, zone %s, body:%s", self._project, zone, body) api = self.service.instances().insert( @@ -1364,63 +1378,53 @@ class ComputeClient(base_cloud_client.BaseCloudApiClient): external_ip = instance["networkInterfaces"][0]["accessConfigs"][0]["natIP"] return IP(internal=internal_ip, external=external_ip) - def SetCommonInstanceMetadata(self, body): - """Set project-wide metadata. + @utils.TimeExecute(function_description="Updating instance metadata: ") + def SetInstanceMetadata(self, zone, instance, body): + """Set instance metadata. Args: - body: Metadata body. + zone: String, name of zone. + instance: String, representing instance name. + body: Dict, Metadata body. metdata is in the following format. { "kind": "compute#metadata", "fingerprint": "a-23icsyx4E=", "items": [ { - "key": "google-compute-default-region", - "value": "us-central1" + "key": "sshKeys", + "value": "key" }, ... ] } """ - api = self.service.projects().setCommonInstanceMetadata( - project=self._project, body=body) + api = self.service.instances().setMetadata( + project=self._project, zone=zone, instance=instance, body=body) operation = self.Execute(api) - self.WaitOnOperation(operation, operation_scope=OperationScope.GLOBAL) + self.WaitOnOperation( + operation, operation_scope=OperationScope.ZONE, scope_name=zone) - def AddSshRsa(self, user, ssh_rsa_path): - """Add the public rsa key to the project's metadata. + def AddSshRsaInstanceMetadata(self, zone, user, ssh_rsa_path, instance): + """Add the public rsa key to the instance's metadata. - Compute engine instances that are created after will - by default contain the key. + Confirm that the instance has this public key in the instance's + metadata, if not we will add this public key. Args: - user: the name of the user which the key belongs to. - ssh_rsa_path: The absolute path to public rsa key. + zone: String, name of zone. + user: String, name of the user which the key belongs to. + ssh_rsa_path: String, The absolute path to public rsa key. + instance: String, representing instance name. """ - if not os.path.exists(ssh_rsa_path): - raise errors.DriverError( - "RSA file %s does not exist." % ssh_rsa_path) - - logger.info("Adding ssh rsa key from %s to project %s for user: %s", - ssh_rsa_path, self._project, user) - project = self.GetProject() - with open(ssh_rsa_path) as f: - rsa = f.read() - rsa = rsa.strip() if rsa else rsa - utils.VerifyRsaPubKey(rsa) - metadata = project["commonInstanceMetadata"] - for item in metadata.setdefault("items", []): - if item["key"] == "sshKeys": - sshkey_item = item - break - else: - sshkey_item = {"key": "sshKeys", "value": ""} - metadata["items"].append(sshkey_item) - + ssh_rsa_path = os.path.expanduser(ssh_rsa_path) + rsa = GetRsaKey(ssh_rsa_path) entry = "%s:%s" % (user, rsa) logger.debug("New RSA entry: %s", entry) - sshkey_item["value"] = "\n".join([sshkey_item["value"].strip(), - entry]).strip() - self.SetCommonInstanceMetadata(metadata) + + gce_instance = self.GetInstance(instance, zone) + metadata = gce_instance.get(_METADATA) + if RsaNotInMetadata(metadata, entry): + self.UpdateRsaInMetadata(zone, instance, metadata, entry) def CheckAccess(self): """Check if the user has read access to the cloud project. @@ -1443,3 +1447,88 @@ class ComputeClient(base_cloud_client.BaseCloudApiClient): return False raise return True + + def UpdateRsaInMetadata(self, zone, instance, metadata, entry): + """Update ssh public key to sshKeys's value in this metadata. + + Args: + zone: String, name of zone. + instance: String, representing instance name. + metadata: Dict, maps a metadata name to its value. + entry: String, ssh public key. + """ + ssh_key_item = GetSshKeyFromMetadata(metadata) + if ssh_key_item: + # The ssh key exists in the metadata so update the reference to it + # in the metadata. There may not be an actual ssh key value so + # that's why we filter for None to avoid an empty line in front. + ssh_key_item[_METADATA_KEY_VALUE] = "\n".join( + filter(None, [ssh_key_item[_METADATA_KEY_VALUE], entry])) + else: + # Since there is no ssh key item in the metadata, we need to add it in. + ssh_key_item = {_METADATA_KEY: _SSH_KEYS_NAME, + _METADATA_KEY_VALUE: entry} + metadata[_ITEMS].append(ssh_key_item) + utils.PrintColorString( + "Ssh public key doesn't exist in the instance(%s), adding it." + % instance, utils.TextColors.WARNING) + self.SetInstanceMetadata(zone, instance, metadata) + + +def RsaNotInMetadata(metadata, entry): + """Check ssh public key exist in sshKeys's value. + + Args: + metadata: Dict, maps a metadata name to its value. + entry: String, ssh public key. + + Returns: + Boolean. True if ssh public key doesn't exist in metadata. + """ + for item in metadata.setdefault(_ITEMS, []): + if item[_METADATA_KEY] == _SSH_KEYS_NAME: + if entry in item[_METADATA_KEY_VALUE]: + return False + return True + + +def GetSshKeyFromMetadata(metadata): + """Get ssh key item from metadata. + + Args: + metadata: Dict, maps a metadata name to its value. + + Returns: + Dict of ssk_key_item in metadata, None if can't find the ssh key item + in metadata. + """ + for item in metadata.setdefault(_ITEMS, []): + if item.get(_METADATA_KEY, '') == _SSH_KEYS_NAME: + return item + return None + + +def GetRsaKey(ssh_rsa_path): + """Get rsa key from rsa path. + + Args: + ssh_rsa_path: String, The absolute path to public rsa key. + + Returns: + String, rsa key. + + Raises: + errors.DriverError: RSA file does not exist. + """ + ssh_rsa_path = os.path.expanduser(ssh_rsa_path) + if not os.path.exists(ssh_rsa_path): + raise errors.DriverError( + "RSA file %s does not exist." % ssh_rsa_path) + + with open(ssh_rsa_path) as f: + rsa = f.read() + # The space must be removed here for string processing, + # if it is not string, it doesn't have a strip function. + rsa = rsa.strip() if rsa else rsa + utils.VerifyRsaPubKey(rsa) + return rsa diff --git a/internal/lib/gcompute_client_test.py b/internal/lib/gcompute_client_test.py index a34bfa2e..51763067 100644 --- a/internal/lib/gcompute_client_test.py +++ b/internal/lib/gcompute_client_test.py @@ -57,6 +57,15 @@ class ComputeClientTest(driver_test_lib.BaseDriverTest): OPERATION_NAME = "fake-op" IMAGE_FINGERPRINT = "L_NWHuz7wTY=" GPU = "fancy-graphics" + SSHKEY = ( + "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDBkTOTRze9v2VOqkkf7RG" + "jSkg6Z2kb9Q9UHsDGatvend3fmjIw1Tugg0O7nnjlPkskmlgyd4a/j99WOeLL" + "CPk6xPyoVjrPUVBU/pAk09ORTC4Zqk6YjlW7LOfzvqmXhmIZfYu6Q4Yt50pZzhl" + "lllfu26nYjY7Tg12D019nJi/kqPX5+NKgt0LGXTu8T1r2Gav/q4V7QRWQrB8Eiu" + "pxXR7I2YhynqovkEt/OXG4qWgvLEXGsWtSQs0CtCzqEVxz0Y9ECr7er4VdjSQxV" + "AaeLAsQsK9ROae8hMBFZ3//8zLVapBwpuffCu+fUoql9qeV9xagZcc9zj8XOUOW" + "ApiihqNL1111 test@test1.org") + EXTRA_SCOPES = ["scope1"] def setUp(self): """Set up test.""" @@ -64,6 +73,7 @@ class ComputeClientTest(driver_test_lib.BaseDriverTest): self.Patch(gcompute_client.ComputeClient, "InitResourceHandle") fake_cfg = mock.MagicMock() fake_cfg.project = PROJECT + fake_cfg.extra_scopes = self.EXTRA_SCOPES self.compute_client = gcompute_client.ComputeClient( fake_cfg, mock.MagicMock()) self.compute_client._service = mock.MagicMock() @@ -522,6 +532,9 @@ class ComputeClientTest(driver_test_lib.BaseDriverTest): extra_disk_name = "gce-x86-userdebug-2345-abcd-data" expected_disk_args = [self._disk_args] expected_disk_args.extend([{"fake_extra_arg": "fake_extra_value"}]) + expected_scope = [] + expected_scope.extend(self.compute_client.DEFAULT_INSTANCE_SCOPE) + expected_scope.extend(self.EXTRA_SCOPES) expected_body = { "machineType": self.MACHINE_TYPE_URL, @@ -539,7 +552,7 @@ class ComputeClientTest(driver_test_lib.BaseDriverTest): "disks": expected_disk_args, "serviceAccounts": [ {"email": "default", - "scopes": self.compute_client.DEFAULT_INSTANCE_SCOPE} + "scopes": expected_scope} ], "metadata": { "items": [{"key": self.METADATA[0], @@ -554,7 +567,8 @@ class ComputeClientTest(driver_test_lib.BaseDriverTest): metadata={self.METADATA[0]: self.METADATA[1]}, network=self.NETWORK, zone=self.ZONE, - extra_disk_name=extra_disk_name) + extra_disk_name=extra_disk_name, + extra_scopes=self.EXTRA_SCOPES) resource_mock.insert.assert_called_with( project=PROJECT, zone=self.ZONE, body=expected_body) @@ -624,7 +638,8 @@ class ComputeClientTest(driver_test_lib.BaseDriverTest): metadata={self.METADATA[0]: self.METADATA[1]}, network=self.NETWORK, zone=self.ZONE, - gpu=self.GPU) + gpu=self.GPU, + extra_scopes=None) resource_mock.insert.assert_called_with( project=PROJECT, zone=self.ZONE, body=expected_body) @@ -1026,80 +1041,243 @@ class ComputeClientTest(driver_test_lib.BaseDriverTest): self.assertEqual(ip_name_map, {"172.22.22.22": "instance_1", "172.22.22.23": None}) - def testAddSshRsa(self): - """Test AddSshRsa..""" + def testRsaNotInMetadata(self): + """Test rsa not in metadata.""" fake_user = "fake_user" - sshkey = ( - "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDBkTOTRze9v2VOqkkf7RG" - "jSkg6Z2kb9Q9UHsDGatvend3fmjIw1Tugg0O7nnjlPkskmlgyd4a/j99WOeLL" - "CPk6xPyoVjrPUVBU/pAk09ORTC4Zqk6YjlW7LOfzvqmXhmIZfYu6Q4Yt50pZzhl" - "lllfu26nYjY7Tg12D019nJi/kqPX5+NKgt0LGXTu8T1r2Gav/q4V7QRWQrB8Eiu" - "pxXR7I2YhynqovkEt/OXG4qWgvLEXGsWtSQs0CtCzqEVxz0Y9ECr7er4VdjSQxV" - "AaeLAsQsK9ROae8hMBFZ3//8zLVapBwpuffCu+fUoql9qeV9xagZcc9zj8XOUOW" - "ApiihqNL1111 test@test1.org") - project = { - "commonInstanceMetadata": { - "kind": "compute#metadata", - "fingerprint": "a-23icsyx4E=", - "items": [ - { - "key": "sshKeys", - "value": "user:key" - } - ] - } + fake_ssh_key = "fake_ssh" + metadata = { + "kind": "compute#metadata", + "fingerprint": "a-23icsyx4E=", + "items": [ + { + "key": "sshKeys", + "value": "%s:%s" % (fake_user, self.SSHKEY) + } + ] } - expected = { + # Test rsa doesn't exist in metadata. + new_entry = "%s:%s" % (fake_user, fake_ssh_key) + self.assertEqual(True, gcompute_client.RsaNotInMetadata(metadata, new_entry)) + + # Test rsa exists in metadata. + exist_entry = "%s:%s" %(fake_user, self.SSHKEY) + self.assertEqual(False, gcompute_client.RsaNotInMetadata(metadata, exist_entry)) + + def testGetSshKeyFromMetadata(self): + """Test get ssh key from metadata.""" + fake_user = "fake_user" + metadata_key_exist_value_is_empty = { + "kind": "compute#metadata", + "fingerprint": "a-23icsyx4E=", + "items": [ + { + "key": "sshKeys", + "value": "" + } + ] + } + metadata_key_exist = { "kind": "compute#metadata", "fingerprint": "a-23icsyx4E=", "items": [ { "key": "sshKeys", - "value": "user:key\n%s:%s" % (fake_user, sshkey) + "value": "%s:%s" % (fake_user, self.SSHKEY) } ] } + metadata_key_not_exist = { + "kind": "compute#metadata", + "fingerprint": "a-23icsyx4E=", + "items": [ + { + } + ] + } + expected_key_exist_value_is_empty = { + "key": "sshKeys", + "value": "" + } + expected_key_exist = { + "key": "sshKeys", + "value": "%s:%s" % (fake_user, self.SSHKEY) + } + self.assertEqual(expected_key_exist_value_is_empty, + gcompute_client.GetSshKeyFromMetadata(metadata_key_exist_value_is_empty)) + self.assertEqual(expected_key_exist, + gcompute_client.GetSshKeyFromMetadata(metadata_key_exist)) + self.assertEqual(None, + gcompute_client.GetSshKeyFromMetadata(metadata_key_not_exist)) + + + def testGetRsaKeyPathExistsFalse(self): + """Test the rsa key path not exists.""" + fake_ssh_rsa_path = "/path/to/test_rsa.pub" + self.Patch(os.path, "exists", return_value=False) + self.assertRaisesRegexp(errors.DriverError, + "RSA file %s does not exist." % fake_ssh_rsa_path, + gcompute_client.GetRsaKey, + ssh_rsa_path=fake_ssh_rsa_path) + + def testGetRsaKey(self): + """Test get the rsa key.""" + fake_ssh_rsa_path = "/path/to/test_rsa.pub" + self.Patch(os.path, "exists", return_value=True) + m = mock.mock_open(read_data=self.SSHKEY) + with mock.patch("__builtin__.open", m): + result = gcompute_client.GetRsaKey(fake_ssh_rsa_path) + self.assertEqual(self.SSHKEY, result) + def testUpdateRsaInMetadata(self): + """Test update rsa in metadata.""" + fake_ssh_key = "fake_ssh" + fake_metadata_sshkeys_not_exist = { + "kind": "compute#metadata", + "fingerprint": "a-23icsyx4E=", + "items": [ + { + "key": "not_sshKeys", + "value": "" + } + ] + } + new_entry = "new_user:%s" % fake_ssh_key + expected = { + "kind": "compute#metadata", + "fingerprint": "a-23icsyx4E=", + "items": [ + { + "key": "not_sshKeys", + "value": "" + }, + { + "key": "sshKeys", + "value": new_entry + } + ] + } self.Patch(os.path, "exists", return_value=True) - m = mock.mock_open(read_data=sshkey) self.Patch(gcompute_client.ComputeClient, "WaitOnOperation") - self.Patch( - gcompute_client.ComputeClient, "GetProject", return_value=project) resource_mock = mock.MagicMock() - self.compute_client._service.projects = mock.MagicMock( + self.compute_client.SetInstanceMetadata = mock.MagicMock( return_value=resource_mock) - resource_mock.setCommonInstanceMetadata = mock.MagicMock() - - with mock.patch("__builtin__.open", m): - self.compute_client.AddSshRsa(fake_user, "/path/to/test_rsa.pub") - resource_mock.setCommonInstanceMetadata.assert_called_with( - project=PROJECT, body=expected) + # Test the key item not exists in the metadata. + self.compute_client.UpdateRsaInMetadata( + "fake_zone", + "fake_instance", + fake_metadata_sshkeys_not_exist, + new_entry) + self.compute_client.SetInstanceMetadata.assert_called_with( + "fake_zone", + "fake_instance", + expected) + + # Test the key item exists in the metadata. + fake_metadata_ssh_keys_exists = { + "kind": "compute#metadata", + "fingerprint": "a-23icsyx4E=", + "items": [ + { + "key": "sshKeys", + "value": "old_user:%s" % self.SSHKEY + } + ] + } + expected_ssh_keys_exists = { + "kind": "compute#metadata", + "fingerprint": "a-23icsyx4E=", + "items": [ + { + "key": "sshKeys", + "value": "old_user:%s\n%s" % (self.SSHKEY, new_entry) + } + ] + } - def testAddSshRsaInvalidKey(self): - """Test AddSshRsa..""" + self.compute_client.UpdateRsaInMetadata( + "fake_zone", + "fake_instance", + fake_metadata_ssh_keys_exists, + new_entry) + self.compute_client.SetInstanceMetadata.assert_called_with( + "fake_zone", + "fake_instance", + expected_ssh_keys_exists) + + def testAddSshRsaToInstance(self): + """Test add ssh rsa key to instance.""" fake_user = "fake_user" - sshkey = "ssh-rsa v2VOqkkf7RGL1111 test@test1.org" - project = { - "commonInstanceMetadata": { + instance_metadata_key_not_exist = { + "metadata": { "kind": "compute#metadata", "fingerprint": "a-23icsyx4E=", "items": [ { "key": "sshKeys", - "value": "user:key" + "value": "" } ] } } + instance_metadata_key_exist = { + "metadata": { + "kind": "compute#metadata", + "fingerprint": "a-23icsyx4E=", + "items": [ + { + "key": "sshKeys", + "value": "%s:%s" % (fake_user, self.SSHKEY) + } + ] + } + } + expected = { + "kind": "compute#metadata", + "fingerprint": "a-23icsyx4E=", + "items": [ + { + "key": "sshKeys", + "value": "%s:%s" % (fake_user, self.SSHKEY) + } + ] + } + self.Patch(os.path, "exists", return_value=True) - m = mock.mock_open(read_data=sshkey) + m = mock.mock_open(read_data=self.SSHKEY) self.Patch(gcompute_client.ComputeClient, "WaitOnOperation") + resource_mock = mock.MagicMock() + self.compute_client._service.instances = mock.MagicMock( + return_value=resource_mock) + resource_mock.setMetadata = mock.MagicMock() + + # Test the key not exists in the metadata. + self.Patch( + gcompute_client.ComputeClient, "GetInstance", + return_value=instance_metadata_key_not_exist) + with mock.patch("__builtin__.open", m): + self.compute_client.AddSshRsaInstanceMetadata( + "fake_zone", + fake_user, + "/path/to/test_rsa.pub", + "fake_instance") + resource_mock.setMetadata.assert_called_with( + project=PROJECT, + zone="fake_zone", + instance="fake_instance", + body=expected) + + # Test the key already exists in the metadata. + resource_mock.setMetadata.call_count = 0 self.Patch( - gcompute_client.ComputeClient, "GetProject", return_value=project) + gcompute_client.ComputeClient, "GetInstance", + return_value=instance_metadata_key_exist) with mock.patch("__builtin__.open", m): - self.assertRaisesRegexp(errors.DriverError, "rsa key is invalid:*", - self.compute_client.AddSshRsa, fake_user, - "/path/to/test_rsa.pub") + self.compute_client.AddSshRsaInstanceMetadata( + "fake_zone", + fake_user, + "/path/to/test_rsa.pub", + "fake_instance") + resource_mock.setMetadata.assert_not_called() @mock.patch.object(gcompute_client.ComputeClient, "WaitOnOperation") def testDeleteDisks(self, mock_wait): diff --git a/internal/lib/goldfish_compute_client.py b/internal/lib/goldfish_compute_client.py index 4377d6db..877eb3d9 100644 --- a/internal/lib/goldfish_compute_client.py +++ b/internal/lib/goldfish_compute_client.py @@ -43,6 +43,7 @@ import getpass import logging from acloud import errors +from acloud.internal import constants from acloud.internal.lib import android_compute_client from acloud.internal.lib import gcompute_client @@ -132,7 +133,9 @@ class GoldfishComputeClient(android_compute_client.AndroidComputeClient): emulator_branch=None, emulator_build_id=None, blank_data_disk_size_gb=None, - gpu=None): + gpu=None, + avd_spec=None, + extra_scopes=None): """Create a goldfish instance given a stable host image and a build id. Args: @@ -148,6 +151,8 @@ class GoldfishComputeClient(android_compute_client.AndroidComputeClient): blank_data_disk_size_gb: Integer, size of the blank data disk in GB. gpu: String, GPU that should be attached to the instance, or None of no acceleration is needed. e.g. "nvidia-tesla-k80" + avd_spec: An AVDSpec instance. + extra_scopes: A list of extra scopes to be passed to the instance. """ self._CheckMachineSize() @@ -161,10 +166,10 @@ class GoldfishComputeClient(android_compute_client.AndroidComputeClient): # Goldfish instances are metadata compatible with cuttlefish devices. # See details goto/goldfish-deployment metadata = self._metadata.copy() - resolution = self._resolution.split("x") + metadata["user"] = getpass.getuser() + metadata[constants.INS_KEY_AVD_TYPE] = constants.TYPE_GF # Note that we use the same metadata naming conventions as cuttlefish - metadata["cvd_01_dpi"] = resolution[3] metadata["cvd_01_fetch_android_build_target"] = build_target metadata["cvd_01_fetch_android_bid"] = "{branch}/{build_id}".format( branch=branch, build_id=build_id) @@ -173,8 +178,28 @@ class GoldfishComputeClient(android_compute_client.AndroidComputeClient): "cvd_01_fetch_emulator_bid"] = "{branch}/{build_id}".format( branch=emulator_branch, build_id=emulator_build_id) metadata["cvd_01_launch"] = "1" - metadata["cvd_01_x_res"] = resolution[0] - metadata["cvd_01_y_res"] = resolution[1] + + # Update metadata by avd_spec + # for legacy create_gf cmd, we will keep using resolution. + # And always use avd_spec for acloud create cmd. + if avd_spec: + metadata[constants.INS_KEY_AVD_FLAVOR] = avd_spec.flavor + metadata["cvd_01_x_res"] = avd_spec.hw_property[constants.HW_X_RES] + metadata["cvd_01_y_res"] = avd_spec.hw_property[constants.HW_Y_RES] + metadata["cvd_01_dpi"] = avd_spec.hw_property[constants.HW_ALIAS_DPI] + metadata[constants.INS_KEY_DISPLAY] = ("%sx%s (%s)" % ( + avd_spec.hw_property[constants.HW_X_RES], + avd_spec.hw_property[constants.HW_Y_RES], + avd_spec.hw_property[constants.HW_ALIAS_DPI])) + else: + resolution = self._resolution.split("x") + metadata["cvd_01_x_res"] = resolution[0] + metadata["cvd_01_y_res"] = resolution[1] + metadata["cvd_01_dpi"] = resolution[3] + + # Add labels for giving the instances ability to be filter for + # acloud list/delete cmds. + labels = {constants.LABEL_CREATE_BY: getpass.getuser()} # Add per-instance ssh key if self._ssh_public_key_path: @@ -197,4 +222,6 @@ class GoldfishComputeClient(android_compute_client.AndroidComputeClient): machine_type=self._machine_type, network=self._network, zone=self._zone, - gpu=gpu) + gpu=gpu, + labels=labels, + extra_scopes=extra_scopes) diff --git a/internal/lib/goldfish_compute_client_test.py b/internal/lib/goldfish_compute_client_test.py index 461fa48e..fcdf0852 100644 --- a/internal/lib/goldfish_compute_client_test.py +++ b/internal/lib/goldfish_compute_client_test.py @@ -44,6 +44,7 @@ class GoldfishComputeClientTest(driver_test_lib.BaseDriverTest): EXTRA_DATA_DISK_SIZE_GB = 4 BOOT_DISK_SIZE_GB = 10 GPU = "nvidia-tesla-k80" + EXTRA_SCOPES = "scope1" def _GetFakeConfig(self): """Create a fake configuration object. @@ -60,6 +61,7 @@ class GoldfishComputeClientTest(driver_test_lib.BaseDriverTest): x=self.X_RES, y=self.Y_RES, dpi=self.DPI) fake_cfg.metadata_variable = self.METADATA fake_cfg.extra_data_disk_size_gb = self.EXTRA_DATA_DISK_SIZE_GB + fake_cfg.extra_scopes = self.EXTRA_SCOPES return fake_cfg def setUp(self): @@ -86,34 +88,34 @@ class GoldfishComputeClientTest(driver_test_lib.BaseDriverTest): "fake_arg": "fake_value" }]) - def testCreateInstance(self): + @mock.patch("getpass.getuser", return_value="fake_user") + def testCreateInstance(self, _mock_user): """Test CreateInstance.""" expected_metadata = { - "cvd_01_dpi": - str(self.DPI), - "cvd_01_fetch_android_build_target": - self.TARGET, + "user": "fake_user", + "avd_type": "goldfish", + "cvd_01_fetch_android_build_target": self.TARGET, "cvd_01_fetch_android_bid": "{branch}/{build_id}".format( branch=self.BRANCH, build_id=self.BUILD_ID), "cvd_01_fetch_emulator_bid": "{branch}/{build_id}".format( branch=self.EMULATOR_BRANCH, build_id=self.EMULATOR_BUILD_ID), - "cvd_01_launch": - "1", - "cvd_01_x_res": - str(self.X_RES), - "cvd_01_y_res": - str(self.Y_RES), + "cvd_01_launch": "1", + "cvd_01_dpi": str(self.DPI), + "cvd_01_x_res": str(self.X_RES), + "cvd_01_y_res": str(self.Y_RES), } expected_metadata.update(self.METADATA) expected_disk_args = [{"fake_arg": "fake_value"}] + expected_labels = {'created_by': "fake_user"} self.goldfish_compute_client.CreateInstance( self.INSTANCE, self.IMAGE, self.IMAGE_PROJECT, self.TARGET, self.BRANCH, self.BUILD_ID, self.EMULATOR_BRANCH, - self.EMULATOR_BUILD_ID, self.EXTRA_DATA_DISK_SIZE_GB, self.GPU) + self.EMULATOR_BUILD_ID, self.EXTRA_DATA_DISK_SIZE_GB, self.GPU, + extra_scopes=self.EXTRA_SCOPES) # pylint: disable=no-member gcompute_client.ComputeClient.CreateInstance.assert_called_with( @@ -126,7 +128,9 @@ class GoldfishComputeClientTest(driver_test_lib.BaseDriverTest): machine_type=self.MACHINE_TYPE, network=self.NETWORK, zone=self.ZONE, - gpu=self.GPU) + gpu=self.GPU, + labels=expected_labels, + extra_scopes=self.EXTRA_SCOPES) if __name__ == "__main__": diff --git a/internal/lib/utils.py b/internal/lib/utils.py index 0f19078b..e1a22390 100755 --- a/internal/lib/utils.py +++ b/internal/lib/utils.py @@ -61,6 +61,17 @@ _ADB_CONNECT_ARGS = "connect 127.0.0.1:%(adb_port)d" # Store the ports that vnc/adb are forwarded to, both are integers. ForwardedPorts = collections.namedtuple("ForwardedPorts", [constants.VNC_PORT, constants.ADB_PORT]) +AVD_PORT_DICT = { + constants.TYPE_GCE: ForwardedPorts(constants.GCE_VNC_PORT, + constants.GCE_ADB_PORT), + constants.TYPE_CF: ForwardedPorts(constants.CF_VNC_PORT, + constants.CF_ADB_PORT), + constants.TYPE_GF: ForwardedPorts(constants.GF_VNC_PORT, + constants.GF_ADB_PORT), + constants.TYPE_CHEEPS: ForwardedPorts(constants.CHEEPS_VNC_PORT, + constants.CHEEPS_ADB_PORT) +} + _VNC_BIN = "ssvnc" _CMD_KILL = ["pkill", "-9", "-f"] _CMD_PGREP = "pgrep" @@ -68,7 +79,7 @@ _CMD_SG = "sg " _CMD_START_VNC = "%(bin)s vnc://127.0.0.1:%(port)d" _CMD_INSTALL_SSVNC = "sudo apt-get --assume-yes install ssvnc" _ENV_DISPLAY = "DISPLAY" -_SSVNC_ENV_VARS = {"SSVNC_NO_ENC_WARN": "1", "SSVNC_SCALE": "auto"} +_SSVNC_ENV_VARS = {"SSVNC_NO_ENC_WARN": "1", "SSVNC_SCALE": "auto", "VNCVIEWER_X11CURSOR": "1"} _DEFAULT_DISPLAY_SCALE = 1.0 _DIST_DIR = "DIST_DIR" @@ -82,6 +93,7 @@ _EvaluatedResult = collections.namedtuple("EvaluatedResult", _SUPPORTED_SYSTEMS_AND_DISTS = {"Linux": ["Ubuntu", "Debian"]} _DEFAULT_TIMEOUT_ERR = "Function did not complete within %d secs." + class TempDir(object): """A context manager that ceates a temporary directory. @@ -436,6 +448,7 @@ def VerifyRsaPubKey(rsa): raise errors.DriverError( "rsa key is invalid: %s, error: %s" % (rsa, str(e))) + def Decompress(sourcefile, dest=None): """Decompress .zip or .tar.gz. @@ -459,6 +472,7 @@ def Decompress(sourcefile, dest=None): "Sorry, we could only support compression file type " "for zip or tar.gz.") + # pylint: disable=old-style-class,no-init class TextColors: """A class that defines common color ANSI code.""" @@ -845,6 +859,7 @@ def GetAnswerFromList(answer_list, enable_choose_all=False): Args: answer_list: list of the answers to choose from. + enable_choose_all: True to choose all items from answer list. Return: List holding the answer(s). @@ -870,8 +885,7 @@ def GetAnswerFromList(answer_list, enable_choose_all=False): continue # Filter out choices if choice == 0: - print("Exiting acloud.") - sys.exit() + sys.exit(constants.EXIT_BY_USER) if enable_choose_all and choice == max_choice: return answer_list if choice < 0 or choice > max_choice: @@ -898,8 +912,8 @@ def LaunchVNCFromReport(report, avd_spec, no_prompts=False): PrintColorString("No VNC port specified, skipping VNC startup.", TextColors.FAIL) -def LaunchVncClient(port=constants.DEFAULT_VNC_PORT, avd_width=None, - avd_height=None, no_prompts=False): + +def LaunchVncClient(port, avd_width=None, avd_height=None, no_prompts=False): """Launch ssvnc. Args: @@ -1169,3 +1183,25 @@ def TimeoutException(timeout_secs, timeout_error=_DEFAULT_TIMEOUT_ERR): return _FunctionWrapper return _Wrapper + + +def GetBuildEnvironmentVariable(variable_name): + """Get build environment variable. + + Args: + variable_name: String of variable name. + + Returns: + String, the value of the variable. + + Raises: + errors.GetAndroidBuildEnvVarError: No environment variable found. + """ + try: + return os.environ[variable_name] + except KeyError: + raise errors.GetAndroidBuildEnvVarError( + "Could not get environment var: %s\n" + "Try to run 'source build/envsetup.sh && lunch <target>'" + % variable_name + ) diff --git a/internal/proto/internal_config.proto b/internal/proto/internal_config.proto index a9d539b5..cbb1db77 100755 --- a/internal/proto/internal_config.proto +++ b/internal/proto/internal_config.proto @@ -38,6 +38,10 @@ message DefaultUserConfig { optional string stable_cheeps_host_image_name = 9; // [CHEEPS only] The project where stable host image is optional string stable_cheeps_host_image_project = 10; + // The pattern of the instance name, e.g. ins-{uuid}-{build_id}-{build_target} + // the parts in {} will be automatically replaced with the actual value if + // you specify them in the pattern, uuid will be automatically generated. + optional string instance_name_pattern = 11; } // Internal configuration diff --git a/internal/proto/user_config.proto b/internal/proto/user_config.proto index d8620997..131b42ee 100755 --- a/internal/proto/user_config.proto +++ b/internal/proto/user_config.proto @@ -85,4 +85,17 @@ message UserConfig { optional string stable_cheeps_host_image_name = 22; // [CHEEPS only] The project that the stable host image is released to optional string stable_cheeps_host_image_project = 23; + + // [CVD only] It will get passed into the launch_cvd command if not empty. + // In version 0.7.2 and later. + optional string launch_args = 24; + + // The pattern of the instance name, e.g. ins-{uuid}-{build_id}-{build_target} + // the parts in {} will be automatically replaced with the actual value if + // you specify them in the pattern, uuid will be automatically generated. + optional string instance_name_pattern = 25; + + // List of scopes that will be given to the instance + // https://cloud.google.com/compute/docs/access/create-enable-service-accounts-for-instances#changeserviceaccountandscopes + repeated string extra_scopes = 26; } diff --git a/list/instance.py b/list/instance.py index 4cc64a7d..b347785d 100644 --- a/list/instance.py +++ b/list/instance.py @@ -26,7 +26,6 @@ The details include: - and more! """ -import collections import datetime import logging import re @@ -61,9 +60,6 @@ _RE_LAUNCH_CVD = re.compile(r"(?P<date_str>^[^/]+)(.*launch_cvd --daemon )+" _FULL_NAME_STRING = ("device serial: %(device_serial)s (%(instance_name)s) " "elapsed time: %(elapsed_time)s") -ForwardedPorts = collections.namedtuple("ForwardedPorts", - [constants.VNC_PORT, constants.ADB_PORT]) - def _GetElapsedTime(start_time): """Calculate the elapsed time from start_time till now. @@ -225,14 +221,14 @@ class LocalInstance(Instance): local_instance._elapsed_time = _GetElapsedTime(date_str) local_instance._fullname = (_FULL_NAME_STRING % {"device_serial": "127.0.0.1:%d" % - constants.DEFAULT_ADB_PORT, + constants.CF_ADB_PORT, "instance_name": local_instance._name, "elapsed_time": local_instance._elapsed_time}) local_instance._avd_type = constants.TYPE_CF local_instance._ip = "127.0.0.1" local_instance._status = constants.INS_STATUS_RUNNING - local_instance._adb_port = constants.DEFAULT_ADB_PORT - local_instance._vnc_port = constants.DEFAULT_VNC_PORT + local_instance._adb_port = constants.CF_ADB_PORT + local_instance._vnc_port = constants.CF_VNC_PORT local_instance._display = ("%sx%s (%s)" % (x_res, y_res, dpi)) local_instance._is_local = True local_instance._ssh_tunnel_is_connected = True @@ -336,12 +332,8 @@ class RemoteInstance(Instance): used in the ssh forwarded call. Both fields are integers. """ process_output = subprocess.check_output(constants.COMMAND_PS) - default_vnc_port = (constants.DEFAULT_GCE_VNC_PORT - if avd_type == constants.TYPE_GCE - else constants.DEFAULT_VNC_PORT) - default_adb_port = (constants.DEFAULT_GCE_ADB_PORT - if avd_type == constants.TYPE_GCE - else constants.DEFAULT_ADB_PORT) + default_vnc_port = utils.AVD_PORT_DICT[avd_type].vnc_port + default_adb_port = utils.AVD_PORT_DICT[avd_type].adb_port re_pattern = re.compile(_RE_SSH_TUNNEL_PATTERN % (_RE_GROUP_VNC, default_vnc_port, _RE_GROUP_ADB, default_adb_port, ip)) @@ -359,4 +351,4 @@ class RemoteInstance(Instance): "IP:%s, forwarding (adb:%d, vnc:%d)"), ip, adb_port, vnc_port) - return ForwardedPorts(vnc_port=vnc_port, adb_port=adb_port) + return utils.ForwardedPorts(vnc_port=vnc_port, adb_port=adb_port) diff --git a/list/instance_test.py b/list/instance_test.py index e0f4ffd8..e29994b7 100644 --- a/list/instance_test.py +++ b/list/instance_test.py @@ -57,7 +57,7 @@ class InstanceTest(driver_test_lib.BaseDriverTest): self.assertEqual("1080x1920 (480)", local_instance.display) self.assertEqual("Sat Nov 10 21:55:10 2018", local_instance.createtime) expected_full_name = "device serial: 127.0.0.1:%s (%s) elapsed time: %s" % ( - constants.DEFAULT_ADB_PORT, constants.LOCAL_INS_NAME, "fake_time") + constants.CF_ADB_PORT, constants.LOCAL_INS_NAME, "fake_time") self.assertEqual(expected_full_name, local_instance.fullname) # test return None if no launch_cvd process found @@ -95,7 +95,7 @@ class InstanceTest(driver_test_lib.BaseDriverTest): self.Patch(instance, "_GetElapsedTime", return_value="fake_time") forwarded_ports = instance.RemoteInstance( mock.MagicMock()).GetAdbVncPortFromSSHTunnel( - "fake_ip", "fake_avd_type") + "fake_ip", constants.TYPE_CF) self.assertEqual(54321, forwarded_ports.adb_port) self.assertEqual(12345, forwarded_ports.vnc_port) diff --git a/list/list.py b/list/list.py index 9354a99d..6c188813 100644 --- a/list/list.py +++ b/list/list.py @@ -142,10 +142,10 @@ def ChooseInstances(cfg, select_all_instances=False): """ instances_list = GetInstances(cfg) if (len(instances_list) > 1) and not select_all_instances: - print("Multiple instance detected, choose 1 to proceed:") - instances_to_delete = utils.GetAnswerFromList(instances_list, - enable_choose_all=True) - return instances_to_delete + print("Multiple instances detected, choose any one to proceed:") + instances = utils.GetAnswerFromList(instances_list, + enable_choose_all=True) + return instances return instances_list @@ -184,6 +184,34 @@ def GetInstancesFromInstanceNames(cfg, instance_names): return instance_list +def GetInstanceFromAdbPort(cfg, adb_port): + """Get instance from adb port. + + Args: + cfg: AcloudConfig object. + adb_port: int, adb port of instance. + + Returns: + List of list.Instance() object. + + Raises: + errors.NoInstancesFound: No instances found. + """ + all_instance_info = [] + for instance_object in GetInstances(cfg): + if instance_object.forwarding_adb_port == adb_port: + return [instance_object] + all_instance_info.append(instance_object.fullname) + + # Show devices information to user when user provides wrong adb port. + if all_instance_info: + hint_message = ("No instance with adb port %d, available instances:\n%s" + % (adb_port, "\n".join(all_instance_info))) + else: + hint_message = "No instances to delete." + raise errors.NoInstancesFound(hint_message) + + def Run(args): """Run list. diff --git a/list/list_test.py b/list/list_test.py index 5cbe02a4..d42fa379 100644 --- a/list/list_test.py +++ b/list/list_test.py @@ -61,6 +61,22 @@ class ListTest(driver_test_lib.BaseDriverTest): cfg=cfg, instance_names=instance_names) + # pylint: disable=attribute-defined-outside-init + def testGetInstanceFromAdbPort(self): + """test GetInstanceFromAdbPort.""" + cfg = mock.MagicMock() + alive_instance1 = InstanceObject("alive_instance1") + alive_instance1.forwarding_adb_port = 1111 + alive_instance1.fullname = "device serial: 127.0.0.1:1111 alive_instance1" + expected_instance = [alive_instance1] + self.Patch(list_instance, "GetInstances", return_value=[alive_instance1]) + # Test to find instance by adb port number. + self.assertEqual(expected_instance, + list_instance.GetInstanceFromAdbPort(cfg, 1111)) + # Test for instance can't be found by adb port number. + with self.assertRaises(errors.NoInstancesFound): + list_instance.GetInstanceFromAdbPort(cfg, 2222) + if __name__ == "__main__": unittest.main() diff --git a/metrics/metrics.py b/metrics/metrics.py index 5ec7027a..1d8b1a3e 100644 --- a/metrics/metrics.py +++ b/metrics/metrics.py @@ -26,9 +26,24 @@ _COMMAND_GIT_CONFIG = ["git", "config", "--get", "user.email"] logger = logging.getLogger(__name__) +# pylint: disable=broad-except +def LogUsage(argv): + """Log acloud start event. -def LogUsage(): - """Log acloud run.""" + Log acloud start event and the usage, following are the data we log: + - tool_name: All asuite tools are storing event in the same database. This + property is provided to distinguish different tools. + - command_line: Log all command arguments. + - test_references: Should be a list, we record the acloud sub-command. + e.g. create/delete/reconnect/..etc. We could use this property as filter + criteria to speed up query time. + - cwd: User's current working directory. + - os: The platform that users are working at. + + Args: + argv: A list of system arguments. + """ + # TODO(b131867764): We could remove this metics tool after we apply clearcut. try: from asuite import asuite_metrics asuite_metrics.log_event(_METRICS_URL, dummy_key_fallback=False, @@ -36,8 +51,19 @@ def LogUsage(): except ImportError: logger.debug("No metrics recorder available, not sending metrics.") + #Log start event via clearcut tool. + try: + from asuite import atest_utils + from asuite.metrics import metrics_utils + atest_utils.print_data_collection_notice() + metrics_utils.send_start_event(tool_name=constants.TOOL_NAME, + command_line=' '.join(argv), + test_references=[argv[0]]) + except Exception as e: + logger.debug("Failed to send start event:%s", str(e)) +#TODO(b131867764): We could remove this metics tool after we apply clearcut. def _GetLdap(): """Return string email username for valid domains only, None otherwise.""" try: @@ -52,3 +78,23 @@ def _GetLdap(): except Exception as e: logger.debug("error retrieving email: %s", e) return None + +# pylint: disable=broad-except +def LogExitEvent(exit_code, stacktrace="", logs=""): + """Log acloud exit event. + + A start event should followed by an exit event to calculate the consuming + time. This function will be run at the end of acloud main process or + at the init of the error object. + + Args: + exit_code: Integer, the exit code of acloud main process. + stacktrace: A string of stacktrace. + logs: A string of logs. + """ + try: + from asuite.metrics import metrics_utils + metrics_utils.send_exit_event(exit_code, stacktrace=stacktrace, + logs=logs) + except Exception as e: + logger.debug("Failed to send exit event:%s", str(e)) diff --git a/public/acloud_main.py b/public/acloud_main.py index bb4da6c9..c9602764 100644 --- a/public/acloud_main.py +++ b/public/acloud_main.py @@ -48,10 +48,10 @@ Try $acloud [cmd] --help for further details. from __future__ import print_function import argparse -import getpass import logging import platform import sys +import traceback # TODO: Remove this once we switch over to embedded launcher. # Exit out if python version is < 2.7.13 due to b/120883119. @@ -73,13 +73,14 @@ if (sys.version_info.major == 2 print(" POSIXLY_CORRECT=1 port -N install python27") sys.exit(1) -# Needed to silence oauth2client. -# This is a workaround to get rid of below warning message: +# By Default silence root logger's stream handler since 3p lib may initial +# root logger no matter what level we're using. The acloud logger behavior will +# be defined in _SetupLogging(). This also could workaround to get rid of below +# oauth2client warning: # 'No handlers could be found for logger "oauth2client.contrib.multistore_file' -# TODO(b/112803893): Remove this code once bug is fixed. -OAUTH2_LOGGER = logging.getLogger('oauth2client.contrib.multistore_file') -OAUTH2_LOGGER.setLevel(logging.CRITICAL) -OAUTH2_LOGGER.addHandler(logging.FileHandler("/dev/null")) +DEFAULT_STREAM_HANDLER = logging.StreamHandler() +DEFAULT_STREAM_HANDLER.setLevel(logging.CRITICAL) +logging.getLogger().addHandler(DEFAULT_STREAM_HANDLER) # pylint: disable=wrong-import-position from acloud import errors @@ -87,6 +88,7 @@ from acloud.create import create from acloud.create import create_args from acloud.delete import delete from acloud.delete import delete_args +from acloud.internal import constants from acloud.reconnect import reconnect from acloud.reconnect import reconnect_args from acloud.list import list as list_instances @@ -107,7 +109,6 @@ ACLOUD_LOGGER = "acloud" CMD_CREATE_CUTTLEFISH = "create_cf" CMD_CREATE_GOLDFISH = "create_gf" CMD_CLEANUP = "cleanup" -CMD_SSHKEY = "project_sshkey" # pylint: disable=too-many-statements @@ -145,13 +146,26 @@ def _ParseArgs(args): help="Android branch, e.g. git_master") create_cf_parser.add_argument( "--kernel_build_id", + "--kernel-build-id", type=str, dest="kernel_build_id", required=False, help="Android kernel build id, e.g. 4586590. This is to test a new" - " kernel build with a particular Android build (--build_id). If not" - " specified, the kernel that's bundled with the Android build would" - " be used.") + " kernel build with a particular Android build (--build_id). If neither" + " kernel_branch nor kernel_build_id are specified, the kernel that's" + " bundled with the Android build would be used.") + create_cf_parser.add_argument( + "--kernel_branch", + "--kernel-branch", + type=str, + dest="kernel_branch", + required=False, + help="Android kernel build branch name, e.g." + " kernel-common-android-4.14. This is to test a new kernel build with a" + " particular Android build (--build-id). If specified without" + " specifying kernel_build_id, the last green build in the branch will" + " be used. If neither kernel_branch nor kernel_build_id are specified," + " the kernel that's bundled with the Android build would be used.") create_args.AddCommonCreateArgs(create_cf_parser) subparser_list.append(create_cf_parser) @@ -209,26 +223,6 @@ def _ParseArgs(args): "images that are older than |expiration_mins|.") subparser_list.append(cleanup_parser) - # Command "project_sshkey" - sshkey_parser = subparsers.add_parser(CMD_SSHKEY) - sshkey_parser.required = False - sshkey_parser.set_defaults(which=CMD_SSHKEY) - sshkey_parser.add_argument( - "--user", - type=str, - dest="user", - default=getpass.getuser(), - help="The user name which the sshkey belongs to, default to: %s." % - getpass.getuser()) - sshkey_parser.add_argument( - "--ssh_rsa_path", - type=str, - dest="ssh_rsa_path", - required=True, - help="Absolute path to the file that contains the public rsa key " - "that will be added as project-wide ssh key.") - subparser_list.append(sshkey_parser) - # Command "create" subparser_list.append(create_args.GetCreateArgParser(subparsers)) @@ -363,7 +357,6 @@ def main(argv=None): # Check access. # device_driver.CheckAccess(cfg) - metrics.LogUsage() report = None if args.which == create_args.CMD_CREATE: create.Run(args) @@ -373,6 +366,7 @@ def main(argv=None): build_target=args.build_target, build_id=args.build_id, kernel_build_id=args.kernel_build_id, + kernel_branch=args.kernel_branch, num=args.num, serial_log_file=args.serial_log_file, logcat_file=args.logcat_file, @@ -399,8 +393,6 @@ def main(argv=None): list_instances.Run(args) elif args.which == reconnect_args.CMD_RECONNECT: reconnect.Run(args) - elif args.which == CMD_SSHKEY: - report = device_driver.AddSshRsa(cfg, args.user, args.ssh_rsa_path) elif args.which == setup_args.CMD_SETUP: setup.Run(args) else: @@ -417,4 +409,19 @@ def main(argv=None): if __name__ == "__main__": - main(sys.argv[1:]) + EXIT_CODE = None + EXCEPTION_STACKTRACE = None + EXCEPTION_LOG = None + metrics.LogUsage(sys.argv[1:]) + try: + EXIT_CODE = main(sys.argv[1:]) + except Exception as e: + EXIT_CODE = constants.EXIT_BY_ERROR + EXCEPTION_STACKTRACE = traceback.format_exc() + EXCEPTION_LOG = str(e) + raise + finally: + # Log Exit event here to calculate the consuming time. + metrics.LogExitEvent(EXIT_CODE, + stacktrace=EXCEPTION_STACKTRACE, + logs=EXCEPTION_LOG) diff --git a/public/actions/base_device_factory.py b/public/actions/base_device_factory.py index 4cfdf65e..bd70aad2 100644 --- a/public/actions/base_device_factory.py +++ b/public/actions/base_device_factory.py @@ -24,6 +24,8 @@ BaseDeviceFactory provides basic interface to create a device factory. class BaseDeviceFactory(object): """A class that provides basic interface to create a device factory.""" + LATEST = "latest" + def __init__(self, compute_client): self._compute_client = compute_client diff --git a/public/actions/common_operations.py b/public/actions/common_operations.py index ff862176..79e5ef91 100644 --- a/public/actions/common_operations.py +++ b/public/actions/common_operations.py @@ -34,6 +34,7 @@ from acloud.internal.lib import utils logger = logging.getLogger(__name__) + def CreateSshKeyPairIfNecessary(cfg): """Create ssh key pair if necessary. @@ -237,9 +238,8 @@ class DevicePool(object): # TODO: Delete unused-argument when b/119614469 is resolved. # pylint: disable=unused-argument # pylint: disable=too-many-locals -def CreateDevices(command, cfg, device_factory, num, +def CreateDevices(command, cfg, device_factory, num, avd_type, report_internal_ip=False, autoconnect=False, - vnc_port=None, adb_port=None, serial_log_file=None, logcat_file=None): """Create a set of devices using the given factory. @@ -252,10 +252,9 @@ def CreateDevices(command, cfg, device_factory, num, cfg: An AcloudConfig instance. device_factory: A factory capable of producing a single device. num: The number of devices to create. + avd_type: String, the AVD type(cuttlefish, goldfish...). report_internal_ip: Boolean to report the internal ip instead of external ip. - vnc_port: (int) The VNC port to use for a remote instance. - adb_port: (int) The ADB port to use for a remote instance. serial_log_file: String, the file path to tar the serial logs. logcat_file: String, the file path to tar the logcats. autoconnect: Boolean, whether to auto connect to device. @@ -294,15 +293,18 @@ def CreateDevices(command, cfg, device_factory, num, "ip": ip, "instance_name": device.instance_name } + for attr in ("branch", "build_target", "build_id", "kernel_branch", + "kernel_build_target", "kernel_build_id", + "emulator_branch", "emulator_build_target", + "emulator_build_id"): + if getattr(device_factory, "_%s" % attr, None): + device_dict[attr] = getattr(device_factory, "_%s" % attr) if autoconnect: - if (not vnc_port) or (not adb_port): - logger.error("vnc_port and adb_port must be specified to" - " use autoconnect") - forwarded_ports = utils.AutoConnect(ip, - cfg.ssh_private_key_path, - vnc_port, - adb_port, - getpass.getuser()) + forwarded_ports = utils.AutoConnect( + ip, cfg.ssh_private_key_path, + utils.AVD_PORT_DICT[avd_type].vnc_port, + utils.AVD_PORT_DICT[avd_type].adb_port, + getpass.getuser()) device_dict[constants.VNC_PORT] = forwarded_ports.vnc_port device_dict[constants.ADB_PORT] = forwarded_ports.adb_port if device.instance_name in failures: diff --git a/public/actions/common_operations_test.py b/public/actions/common_operations_test.py index 205a321f..030a9583 100644 --- a/public/actions/common_operations_test.py +++ b/public/actions/common_operations_test.py @@ -36,12 +36,26 @@ class CommonOperationsTest(driver_test_lib.BaseDriverTest): IP = gcompute_client.IP(external="127.0.0.1", internal="10.0.0.1") INSTANCE = "fake-instance" CMD = "test-cmd" + AVD_TYPE = "fake-type" + BRANCH = "fake-branch" + BUILD_TARGET = "fake-target" + BUILD_ID = "fake-build-id" + # pylint: disable=protected-access def setUp(self): """Set up the test.""" super(CommonOperationsTest, self).setUp() self.build_client = mock.MagicMock() self.device_factory = mock.MagicMock() + self.device_factory._branch = self.BRANCH + self.device_factory._build_target = self.BUILD_TARGET + self.device_factory._build_id = self.BUILD_ID + self.device_factory._kernel_branch = None + self.device_factory._kernel_build_id = None + self.device_factory._kernel_build_target = None + self.device_factory._emulator_branch = None + self.device_factory._emulator_build_id = None + self.device_factory._emulator_build_target = None self.Patch( android_build_client, "AndroidBuildClient", @@ -84,14 +98,18 @@ class CommonOperationsTest(driver_test_lib.BaseDriverTest): """Test Create Devices.""" cfg = self._CreateCfg() _report = common_operations.CreateDevices(self.CMD, cfg, - self.device_factory, 1) + self.device_factory, 1, + self.AVD_TYPE) self.assertEqual(_report.command, self.CMD) self.assertEqual(_report.status, report.Status.SUCCESS) self.assertEqual( _report.data, {"devices": [{ "ip": self.IP.external, - "instance_name": self.INSTANCE + "instance_name": self.INSTANCE, + "branch": self.BRANCH, + "build_id": self.BUILD_ID, + "build_target": self.BUILD_TARGET, }]}) def testCreateDevicesInternalIP(self): @@ -99,6 +117,7 @@ class CommonOperationsTest(driver_test_lib.BaseDriverTest): cfg = self._CreateCfg() _report = common_operations.CreateDevices(self.CMD, cfg, self.device_factory, 1, + self.AVD_TYPE, report_internal_ip=True) self.assertEqual(_report.command, self.CMD) self.assertEqual(_report.status, report.Status.SUCCESS) @@ -106,7 +125,10 @@ class CommonOperationsTest(driver_test_lib.BaseDriverTest): _report.data, {"devices": [{ "ip": self.IP.internal, - "instance_name": self.INSTANCE + "instance_name": self.INSTANCE, + "branch": self.BRANCH, + "build_id": self.BUILD_ID, + "build_target": self.BUILD_TARGET, }]}) if __name__ == "__main__": diff --git a/public/actions/create_cuttlefish_action.py b/public/actions/create_cuttlefish_action.py index 288d67f4..bcaef949 100644 --- a/public/actions/create_cuttlefish_action.py +++ b/public/actions/create_cuttlefish_action.py @@ -52,7 +52,7 @@ class CuttlefishDeviceFactory(base_device_factory.BaseDeviceFactory): "/home/vsoc-01/cuttlefish_runtime/cuttlefish_config.json"] def __init__(self, cfg, build_target, build_id, kernel_build_id=None, - avd_spec=None): + avd_spec=None, kernel_branch=None): self.credentials = auth.CreateCredentials(cfg) @@ -67,6 +67,9 @@ class CuttlefishDeviceFactory(base_device_factory.BaseDeviceFactory): self._kernel_build_id = kernel_build_id self._blank_data_disk_size_gb = cfg.extra_data_disk_size_gb self._avd_spec = avd_spec + self._kernel_branch = kernel_branch + self._kernel_build_target = cfg.kernel_build_target + self._extra_scopes = cfg.extra_scopes # Configure clients for interaction with GCE/Build servers self._build_client = android_build_client.AndroidBuildClient( @@ -75,9 +78,18 @@ class CuttlefishDeviceFactory(base_device_factory.BaseDeviceFactory): # Discover branches self._branch = self._build_client.GetBranch(build_target, build_id) self._kernel_branch = None - if kernel_build_id: + if kernel_branch: + # If no kernel_build_id is given or kernel_build_id is latest, + # use the last green build id in the given kernel_branch + if not kernel_build_id or kernel_build_id == self.LATEST: + self._kernel_build_id = ( + self._build_client.GetLKGB(self._kernel_build_target, + kernel_branch)) + elif kernel_build_id: self._kernel_branch = self._build_client.GetBranch( - cfg.kernel_build_target, kernel_build_id) + self._kernel_build_target, kernel_build_id) + else: + self._kernel_build_target = None def CreateInstance(self): """Creates singe configured cuttlefish device. @@ -110,7 +122,8 @@ class CuttlefishDeviceFactory(base_device_factory.BaseDeviceFactory): kernel_branch=self._kernel_branch, kernel_build_id=self._kernel_build_id, blank_data_disk_size_gb=self._blank_data_disk_size_gb, - avd_spec=self._avd_spec) + avd_spec=self._avd_spec, + extra_scopes=self._extra_scopes) return instance @@ -120,6 +133,7 @@ def CreateDevices(avd_spec=None, build_target=None, build_id=None, kernel_build_id=None, + kernel_branch=None, num=1, serial_log_file=None, logcat_file=None, @@ -133,6 +147,7 @@ def CreateDevices(avd_spec=None, build_target: String, Target name. build_id: String, Build id, e.g. "2263051", "P2804227" kernel_build_id: String, Kernel build id. + kernel_branch: String, Kernel branch name. num: Integer, Number of devices to create. serial_log_file: String, A path to a tar file where serial output should be saved to. @@ -161,15 +176,10 @@ def CreateDevices(avd_spec=None, build_id, num, serial_log_file, logcat_file, autoconnect, report_internal_ip) device_factory = CuttlefishDeviceFactory(cfg, build_target, build_id, - kernel_build_id, avd_spec) - return common_operations.CreateDevices( - command="create_cf", - cfg=cfg, - device_factory=device_factory, - num=num, - report_internal_ip=report_internal_ip, - autoconnect=autoconnect, - vnc_port=constants.CF_TARGET_VNC_PORT, - adb_port=constants.CF_TARGET_ADB_PORT, - serial_log_file=serial_log_file, - logcat_file=logcat_file) + avd_spec=avd_spec, + kernel_build_id=kernel_build_id, + kernel_branch=kernel_branch) + return common_operations.CreateDevices("create_cf", cfg, device_factory, + num, constants.TYPE_CF, + report_internal_ip, autoconnect, + serial_log_file, logcat_file) diff --git a/public/actions/create_cuttlefish_action_test.py b/public/actions/create_cuttlefish_action_test.py index 5ec2bd39..1e61412c 100644 --- a/public/actions/create_cuttlefish_action_test.py +++ b/public/actions/create_cuttlefish_action_test.py @@ -40,11 +40,14 @@ class CreateCuttlefishActionTest(driver_test_lib.BaseDriverTest): IMAGE = "fake-image" BUILD_TARGET = "fake-build-target" BUILD_ID = "12345" + KERNEL_BRANCH = "fake-kernel-branch" KERNEL_BUILD_ID = "54321" + KERNEL_BUILD_TARGET = "kernel" BRANCH = "fake-branch" STABLE_HOST_IMAGE_NAME = "fake-stable-host-image-name" STABLE_HOST_IMAGE_PROJECT = "fake-stable-host-image-project" EXTRA_DATA_DISK_GB = 4 + EXTRA_SCOPES = ["scope1", "scope2"] def setUp(self): """Set up the test.""" @@ -78,6 +81,8 @@ class CreateCuttlefishActionTest(driver_test_lib.BaseDriverTest): cfg.stable_host_image_name = self.STABLE_HOST_IMAGE_NAME cfg.stable_host_image_project = self.STABLE_HOST_IMAGE_PROJECT cfg.extra_data_disk_size_gb = self.EXTRA_DATA_DISK_GB + cfg.kernel_build_target = self.KERNEL_BUILD_TARGET + cfg.extra_scopes = self.EXTRA_SCOPES return cfg def testCreateDevices(self): @@ -94,7 +99,8 @@ class CreateCuttlefishActionTest(driver_test_lib.BaseDriverTest): self.compute_client.GenerateInstanceName.return_value = self.INSTANCE # Mock build client method - self.build_client.GetBranch.return_value = self.BRANCH + self.build_client.GetBranch.side_effect = [self.BRANCH, + self.KERNEL_BRANCH] # Setup avd_spec as None to use cfg to create devices none_avd_spec = None @@ -111,14 +117,21 @@ class CreateCuttlefishActionTest(driver_test_lib.BaseDriverTest): build_target=self.BUILD_TARGET, branch=self.BRANCH, build_id=self.BUILD_ID, - kernel_branch=self.BRANCH, + kernel_branch=self.KERNEL_BRANCH, kernel_build_id=self.KERNEL_BUILD_ID, blank_data_disk_size_gb=self.EXTRA_DATA_DISK_GB, - avd_spec=none_avd_spec) + avd_spec=none_avd_spec, + extra_scopes=self.EXTRA_SCOPES) self.assertEquals(report.data, { "devices": [ { + "branch": self.BRANCH, + "build_id": self.BUILD_ID, + "build_target": self.BUILD_TARGET, + "kernel_branch": self.KERNEL_BRANCH, + "kernel_build_id": self.KERNEL_BUILD_ID, + "kernel_build_target": self.KERNEL_BUILD_TARGET, "instance_name": self.INSTANCE, "ip": self.IP.external, }, diff --git a/public/actions/create_goldfish_action.py b/public/actions/create_goldfish_action.py index 42e3f0fe..c67c2559 100644 --- a/public/actions/create_goldfish_action.py +++ b/public/actions/create_goldfish_action.py @@ -22,8 +22,8 @@ import logging import os from acloud import errors -from acloud.public.actions import common_operations from acloud.public.actions import base_device_factory +from acloud.public.actions import common_operations from acloud.internal import constants from acloud.internal.lib import android_build_client from acloud.internal.lib import auth @@ -64,7 +64,8 @@ class GoldfishDeviceFactory(base_device_factory.BaseDeviceFactory): build_id, emulator_build_target, emulator_build_id, - gpu=None): + gpu=None, + avd_spec=None): """Initialize. @@ -75,6 +76,7 @@ class GoldfishDeviceFactory(base_device_factory.BaseDeviceFactory): emulator_build_target: String, the emulator build target, e.g. aosp_x86-eng. emulator_build_id: String, emulator build id. gpu: String, GPU to attach to the device or None. e.g. "nvidia-tesla-k80" + avd_spec: An AVDSpec instance. """ self.credentials = auth.CreateCredentials(cfg) @@ -90,7 +92,9 @@ class GoldfishDeviceFactory(base_device_factory.BaseDeviceFactory): self._emulator_build_id = emulator_build_id self._emulator_build_target = emulator_build_target self._gpu = gpu + self._avd_spec = avd_spec self._blank_data_disk_size_gb = cfg.extra_data_disk_size_gb + self._extra_scopes = cfg.extra_scopes # Configure clients self._build_client = android_build_client.AndroidBuildClient( @@ -109,7 +113,8 @@ class GoldfishDeviceFactory(base_device_factory.BaseDeviceFactory): Returns: String, the name of the created instance. """ - instance = self._compute_client.GenerateInstanceName(self._build_id) + instance = self._compute_client.GenerateInstanceName( + build_id=self._build_id, build_target=self._build_target) self._compute_client.CreateInstance( instance=instance, @@ -121,7 +126,9 @@ class GoldfishDeviceFactory(base_device_factory.BaseDeviceFactory): emulator_branch=self._emulator_branch, emulator_build_id=self._emulator_build_id, gpu=self._gpu, - blank_data_disk_size_gb=self._blank_data_disk_size_gb) + blank_data_disk_size_gb=self._blank_data_disk_size_gb, + avd_spec=self._avd_spec, + extra_scopes=self._extra_scopes) return instance @@ -180,7 +187,8 @@ def _FetchBuildIdFromFile(cfg, build_target, build_id, pattern, filename): return ParseBuildInfo(temp_filename, pattern) -def CreateDevices(cfg, +def CreateDevices(avd_spec=None, + cfg=None, build_target=None, build_id=None, emulator_build_id=None, @@ -194,6 +202,7 @@ def CreateDevices(cfg, """Create one or multiple Goldfish devices. Args: + avd_spec: An AVDSpec instance. cfg: An AcloudConfig instance. build_target: String, the build target, e.g. aosp_x86-eng. build_id: String, Build id, e.g. "2263051", "P2804227" @@ -211,7 +220,23 @@ def CreateDevices(cfg, Returns: A Report instance. """ + if avd_spec: + cfg = avd_spec.cfg + build_target = avd_spec.remote_image[constants.BUILD_TARGET] + build_id = avd_spec.remote_image[constants.BUILD_ID] + branch = avd_spec.remote_image[constants.BUILD_BRANCH] + num = avd_spec.num + emulator_build_id = avd_spec.emulator_build_id + gpu = avd_spec.gpu + serial_log_file = avd_spec.serial_log_file + logcat_file = avd_spec.logcat_file + autoconnect = avd_spec.autoconnect + report_internal_ip = avd_spec.report_internal_ip + if emulator_build_id is None: + logger.info("emulator_build_id not provided. " + "Try to get %s from build %s/%s.", _EMULATOR_INFO_FILENAME, + build_id, build_target) emulator_build_id = _FetchBuildIdFromFile(cfg, build_target, build_id, @@ -242,15 +267,9 @@ def CreateDevices(cfg, device_factory = GoldfishDeviceFactory(cfg, build_target, build_id, cfg.emulator_build_target, - emulator_build_id, gpu) - - return common_operations.CreateDevices( - command="create_gf", - cfg=cfg, - device_factory=device_factory, - num=num, - report_internal_ip=report_internal_ip, - vnc_port=constants.DEFAULT_GOLDFISH_VNC_PORT, - adb_port=constants.DEFAULT_GOLDFISH_ADB_PORT, - serial_log_file=serial_log_file, - logcat_file=logcat_file) + emulator_build_id, gpu, avd_spec) + + return common_operations.CreateDevices("create_gf", cfg, device_factory, + num, constants.TYPE_GF, + report_internal_ip, autoconnect, + serial_log_file, logcat_file) diff --git a/public/actions/create_goldfish_action_test.py b/public/actions/create_goldfish_action_test.py index 3d841b1f..12320f2b 100644 --- a/public/actions/create_goldfish_action_test.py +++ b/public/actions/create_goldfish_action_test.py @@ -15,9 +15,10 @@ # limitations under the License. """Tests for acloud.public.actions.create_goldfish_actions.""" import uuid - import unittest import mock + +from acloud.internal import constants from acloud.internal.lib import android_build_client from acloud.internal.lib import android_compute_client from acloud.internal.lib import auth @@ -43,6 +44,7 @@ class CreateGoldfishActionTest(driver_test_lib.BaseDriverTest): GOLDFISH_HOST_IMAGE_NAME = "fake-stable-host-image-name" GOLDFISH_HOST_IMAGE_PROJECT = "fake-stable-host-image-project" EXTRA_DATA_DISK_GB = 4 + EXTRA_SCOPES = None def setUp(self): """Sets up the test.""" @@ -62,6 +64,16 @@ class CreateGoldfishActionTest(driver_test_lib.BaseDriverTest): "AndroidComputeClient", return_value=self.compute_client) self.Patch(auth, "CreateCredentials", return_value=mock.MagicMock()) + #Initialize new avd_spec + self.avd_spec = mock.MagicMock() + self.avd_spec.cfg = self._CreateCfg() + self.avd_spec.remote_image = {constants.BUILD_ID: self.BUILD_ID, + constants.BUILD_BRANCH: self.BRANCH, + constants.BUILD_TARGET: self.BUILD_TARGET} + self.avd_spec.emulator_build_id = self.EMULATOR_BUILD_ID + self.avd_spec.gpu = self.GPU + self.avd_spec.serial_log_file = None + self.avd_spec.autoconnect = False def _CreateCfg(self): """A helper method that creates a mock configuration object.""" @@ -75,6 +87,7 @@ class CreateGoldfishActionTest(driver_test_lib.BaseDriverTest): cfg.stable_goldfish_host_image_project = self.GOLDFISH_HOST_IMAGE_PROJECT cfg.emulator_build_target = self.EMULATOR_TARGET cfg.extra_data_disk_size_gb = self.EXTRA_DATA_DISK_GB + cfg.extra_scopes = self.EXTRA_SCOPES return cfg def testCreateDevices(self): @@ -95,10 +108,12 @@ class CreateGoldfishActionTest(driver_test_lib.BaseDriverTest): self.BRANCH, self.EMULATOR_BRANCH ] - # Call CreateDevices + none_avd_spec = None + + # Call CreateDevices with avd_spec is None report = create_goldfish_action.CreateDevices( - cfg, self.BUILD_TARGET, self.BUILD_ID, self.EMULATOR_BUILD_ID, - self.GPU) + none_avd_spec, cfg, self.BUILD_TARGET, self.BUILD_ID, + self.EMULATOR_BUILD_ID, self.GPU) # Verify self.compute_client.CreateInstance.assert_called_with( @@ -111,19 +126,47 @@ class CreateGoldfishActionTest(driver_test_lib.BaseDriverTest): build_id=self.BUILD_ID, emulator_branch=self.EMULATOR_BRANCH, emulator_build_id=self.EMULATOR_BUILD_ID, - gpu=self.GPU) + gpu=self.GPU, + avd_spec=none_avd_spec, + extra_scopes=self.EXTRA_SCOPES) self.assertEquals(report.data, { "devices": [ { "instance_name": self.INSTANCE, "ip": self.IP.external, + "branch": self.BRANCH, + "build_id": self.BUILD_ID, + "build_target": self.BUILD_TARGET, + "emulator_branch": self.EMULATOR_BRANCH, + "emulator_build_id": self.EMULATOR_BUILD_ID, + "emulator_build_target": self.EMULATOR_TARGET, }, ], }) self.assertEquals(report.command, "create_gf") self.assertEquals(report.status, "SUCCESS") + # Call CreateDevices with avd_spec + self.build_client.GetBranch.side_effect = [ + self.BRANCH, self.EMULATOR_BRANCH + ] + report = create_goldfish_action.CreateDevices(avd_spec=self.avd_spec) + # Verify + self.compute_client.CreateInstance.assert_called_with( + instance=self.INSTANCE, + blank_data_disk_size_gb=self.EXTRA_DATA_DISK_GB, + image_name=self.GOLDFISH_HOST_IMAGE_NAME, + image_project=self.GOLDFISH_HOST_IMAGE_PROJECT, + build_target=self.BUILD_TARGET, + branch=self.BRANCH, + build_id=self.BUILD_ID, + emulator_branch=self.EMULATOR_BRANCH, + emulator_build_id=self.EMULATOR_BUILD_ID, + gpu=self.GPU, + avd_spec=self.avd_spec, + extra_scopes=self.EXTRA_SCOPES) + def testCreateDevicesWithoutBuildId(self): """Test CreateDevices when emulator sys image build id is not provided.""" cfg = self._CreateCfg() @@ -148,8 +191,10 @@ class CreateGoldfishActionTest(driver_test_lib.BaseDriverTest): "_FetchBuildIdFromFile", return_value=self.BUILD_ID) - # Call CreateDevices + none_avd_spec = None + # Call CreateDevices with no avd_spec report = create_goldfish_action.CreateDevices( + none_avd_spec, cfg, self.BUILD_TARGET, None, @@ -168,17 +213,45 @@ class CreateGoldfishActionTest(driver_test_lib.BaseDriverTest): build_id=self.BUILD_ID, emulator_branch=self.EMULATOR_BRANCH, emulator_build_id=self.EMULATOR_BUILD_ID, - gpu=self.GPU) + gpu=self.GPU, + avd_spec=none_avd_spec, + extra_scopes=self.EXTRA_SCOPES) self.assertEquals(report.data, { "devices": [{ "instance_name": self.INSTANCE, "ip": self.IP.external, + "branch": self.BRANCH, + "build_id": self.BUILD_ID, + "build_target": self.BUILD_TARGET, + "emulator_branch": self.EMULATOR_BRANCH, + "emulator_build_id": self.EMULATOR_BUILD_ID, + "emulator_build_target": self.EMULATOR_TARGET, },], }) self.assertEquals(report.command, "create_gf") self.assertEquals(report.status, "SUCCESS") + # Call CreateDevices with avd_spec + self.build_client.GetBranch.side_effect = [ + self.BRANCH, self.EMULATOR_BRANCH + ] + report = create_goldfish_action.CreateDevices(avd_spec=self.avd_spec) + # Verify + self.compute_client.CreateInstance.assert_called_with( + instance=self.INSTANCE, + blank_data_disk_size_gb=self.EXTRA_DATA_DISK_GB, + image_name=self.GOLDFISH_HOST_IMAGE_NAME, + image_project=self.GOLDFISH_HOST_IMAGE_PROJECT, + build_target=self.BUILD_TARGET, + branch=self.BRANCH, + build_id=self.BUILD_ID, + emulator_branch=self.EMULATOR_BRANCH, + emulator_build_id=self.EMULATOR_BUILD_ID, + gpu=self.GPU, + avd_spec=self.avd_spec, + extra_scopes=self.EXTRA_SCOPES) + #pylint: disable=invalid-name def testCreateDevicesWithoutEmulatorBuildId(self): """Test CreateDevices when emulator build id is not provided.""" @@ -204,9 +277,11 @@ class CreateGoldfishActionTest(driver_test_lib.BaseDriverTest): "_FetchBuildIdFromFile", return_value=self.EMULATOR_BUILD_ID) + none_avd_spec = None # Call CreateDevices report = create_goldfish_action.CreateDevices( - cfg, self.BUILD_TARGET, self.BUILD_ID, None, self.GPU) + none_avd_spec, cfg, self.BUILD_TARGET, self.BUILD_ID, None, + self.GPU) # Verify self.compute_client.CreateInstance.assert_called_with( @@ -219,17 +294,45 @@ class CreateGoldfishActionTest(driver_test_lib.BaseDriverTest): build_id=self.BUILD_ID, emulator_branch=self.EMULATOR_BRANCH, emulator_build_id=self.EMULATOR_BUILD_ID, - gpu=self.GPU) + gpu=self.GPU, + avd_spec=none_avd_spec, + extra_scopes=self.EXTRA_SCOPES) self.assertEquals(report.data, { "devices": [{ "instance_name": self.INSTANCE, "ip": self.IP.external, + "branch": self.BRANCH, + "build_id": self.BUILD_ID, + "build_target": self.BUILD_TARGET, + "emulator_branch": self.EMULATOR_BRANCH, + "emulator_build_id": self.EMULATOR_BUILD_ID, + "emulator_build_target": self.EMULATOR_TARGET, },], }) self.assertEquals(report.command, "create_gf") self.assertEquals(report.status, "SUCCESS") + # Call CreateDevices with avd_spec + self.build_client.GetBranch.side_effect = [ + self.BRANCH, self.EMULATOR_BRANCH + ] + report = create_goldfish_action.CreateDevices(avd_spec=self.avd_spec) + # Verify + self.compute_client.CreateInstance.assert_called_with( + instance=self.INSTANCE, + blank_data_disk_size_gb=self.EXTRA_DATA_DISK_GB, + image_name=self.GOLDFISH_HOST_IMAGE_NAME, + image_project=self.GOLDFISH_HOST_IMAGE_PROJECT, + build_target=self.BUILD_TARGET, + branch=self.BRANCH, + build_id=self.BUILD_ID, + emulator_branch=self.EMULATOR_BRANCH, + emulator_build_id=self.EMULATOR_BUILD_ID, + gpu=self.GPU, + avd_spec=self.avd_spec, + extra_scopes=self.EXTRA_SCOPES) + if __name__ == "__main__": unittest.main() diff --git a/public/config.py b/public/config.py index 176a44d3..888f1855 100755 --- a/public/config.py +++ b/public/config.py @@ -94,6 +94,7 @@ class AcloudConfig(object): "disk_image_name", "disk_image_mime_type" ] + # pylint: disable=too-many-statements def __init__(self, usr_cfg, internal_cfg): """Initialize. @@ -168,6 +169,9 @@ class AcloudConfig(object): if "cfg_sta_ephemeral_data_size_mb" in self.metadata_variable: del self.metadata_variable["cfg_sta_ephemeral_data_size_mb"] + # Additional scopes to be passed to the created instance + self.extra_scopes = usr_cfg.extra_scopes + # Fields that can be overriden by args self.orientation = usr_cfg.orientation self.resolution = usr_cfg.resolution @@ -198,6 +202,12 @@ class AcloudConfig(object): self.common_hw_property_map = internal_cfg.common_hw_property_map self.hw_property = usr_cfg.hw_property + self.launch_args = usr_cfg.launch_args + self.instance_name_pattern = ( + usr_cfg.instance_name_pattern or + internal_cfg.default_usr_cfg.instance_name_pattern) + + # Verify validity of configurations. self.Verify() diff --git a/public/config_test.py b/public/config_test.py index 15e612f1..272f6fcb 100644 --- a/public/config_test.py +++ b/public/config_test.py @@ -49,6 +49,8 @@ metadata_variable { value: "metadata_value_1" } hw_property: "cpu:3,resolution:1080x1920,dpi:480,memory:4g,disk:10g" +extra_scopes: "scope1" +extra_scopes: "scope2" """ INTERNAL_CONFIG = """ @@ -70,6 +72,7 @@ default_usr_cfg { stable_host_image_project: "fake_stable_host_image_project" stable_goldfish_host_image_name: "fake_stable_goldfish_host_image_name" stable_goldfish_host_image_project: "fake_stable_goldfish_host_image_project" + instance_name_pattern: "fake_instance_name_pattern" stable_cheeps_host_image_name: "fake_stable_cheeps_host_image_name" stable_cheeps_host_image_project: "fake_stable_cheeps_host_image_project" metadata_variable { @@ -135,6 +138,7 @@ common_hw_property_map { self.assertEqual(cfg.hw_property, "cpu:3,resolution:1080x1920,dpi:480,memory:4g," "disk:10g") + self.assertEqual(cfg.extra_scopes, ["scope1", "scope2"]) # pylint: disable=protected-access @mock.patch("os.makedirs") @@ -231,6 +235,8 @@ common_hw_property_map { self.assertEqual(cfg.default_usr_cfg.stable_goldfish_host_image_project, "fake_stable_goldfish_host_image_project") self.assertEqual(cfg.emulator_build_target, "sdk_tools_linux") + self.assertEqual(cfg.default_usr_cfg.instance_name_pattern, + "fake_instance_name_pattern") # Cheeps related self.assertEqual(cfg.default_usr_cfg.stable_cheeps_host_image_name, diff --git a/public/data/default.config b/public/data/default.config index bf3e1a76..44ed3ad9 100644 --- a/public/data/default.config +++ b/public/data/default.config @@ -8,10 +8,16 @@ default_extra_data_disk_device: "/dev/block/sdb" creds_cache_file: ".acloud_oauth2.dat" user_agent: "acloud" +# [GOLDFISH only] The emulator build target: "sdk_tools_linux". +# We use it to get build id if build id is not provided and It's very unlikely +# that this will ever change. +emulator_build_target: "sdk_tools_linux" + default_usr_cfg { machine_type: "n1-standard-4" network: "default" extra_data_disk_size_gb: 0 + instance_name_pattern: "ins-{uuid}-{build_id}-{build_target}" metadata_variable { key: "camera_front" @@ -69,7 +75,7 @@ common_hw_property_map { common_hw_property_map { key: "tv" - value: "cpu:2,resolution:1080x1920,dpi:320,memory:4g,disk:4g" + value: "cpu:2,resolution:1280x720,dpi:213,memory:2g,disk:4g" } # Device resolution diff --git a/public/device_driver.py b/public/device_driver.py index 3fabc0c5..a678c02c 100755 --- a/public/device_driver.py +++ b/public/device_driver.py @@ -167,7 +167,8 @@ class AndroidVirtualDevicePool(object): cleanup=True, extra_data_disk_size_gb=None, precreated_data_image=None, - avd_spec=None): + avd_spec=None, + extra_scopes=None): """Creates |num| devices for given build_target and build_id. - If gce_image is provided, will use it to create an instance. @@ -192,6 +193,7 @@ class AndroidVirtualDevicePool(object): extra_data_disk_size_gb: Integer, size of extra disk, or None. precreated_data_image: A string, the image to use for the extra disk. avd_spec: AVDSpec object for pass hw_property. + extra_scopes: A list of extra scopes given to the new instance. Raises: errors.DriverError: If no source is specified for image creation. @@ -227,7 +229,8 @@ class AndroidVirtualDevicePool(object): instance=instance, image_name=image_name, extra_disk_name=extra_disk_name, - avd_spec=avd_spec) + avd_spec=avd_spec, + extra_scopes=extra_scopes) ip = self._compute_client.GetInstanceIP(instance) self.devices.append(avd.AndroidVirtualDevice( ip=ip, instance_name=instance)) @@ -387,7 +390,8 @@ def CreateAndroidVirtualDevices(cfg, extra_data_disk_size_gb=cfg.extra_data_disk_size_gb, precreated_data_image=cfg.precreated_data_image_map.get( cfg.extra_data_disk_size_gb), - avd_spec=avd_spec) + avd_spec=avd_spec, + extra_scopes=cfg.extra_scopes) failures = device_pool.WaitForBoot() # Write result to report. for device in device_pool.devices: @@ -401,8 +405,8 @@ def CreateAndroidVirtualDevices(cfg, forwarded_ports = utils.AutoConnect( ip, cfg.ssh_private_key_path, - constants.DEFAULT_GCE_VNC_PORT, - constants.DEFAULT_GCE_ADB_PORT, + constants.GCE_VNC_PORT, + constants.GCE_ADB_PORT, _SSH_USER) device_dict[constants.VNC_PORT] = forwarded_ports.vnc_port device_dict[constants.ADB_PORT] = forwarded_ports.adb_port @@ -575,30 +579,6 @@ def Cleanup(cfg, expiration_mins): return r -def AddSshRsa(cfg, user, ssh_rsa_path): - """Add public ssh rsa key to the project. - - Args: - cfg: An AcloudConfig instance. - user: the name of the user which the key belongs to. - ssh_rsa_path: The absolute path to public rsa key. - - Returns: - A Report instance. - """ - r = report.Report(command="sshkey") - try: - credentials = auth.CreateCredentials(cfg) - compute_client = android_compute_client.AndroidComputeClient( - cfg, credentials) - compute_client.AddSshRsa(user, ssh_rsa_path) - r.SetStatus(report.Status.SUCCESS) - except errors.DriverError as e: - r.AddError(str(e)) - r.SetStatus(report.Status.FAIL) - return r - - def CheckAccess(cfg): """Check if user has access. diff --git a/public/device_driver_test.py b/public/device_driver_test.py index 9d4f8cb6..60b0fb1d 100644 --- a/public/device_driver_test.py +++ b/public/device_driver_test.py @@ -48,6 +48,7 @@ def _CreateCfg(): 4: "extradisk-image-4gb", 10: "extradisk-image-10gb" } + cfg.extra_scopes = None cfg.ssh_private_key_path = "" cfg.ssh_public_key_path = "" @@ -109,7 +110,8 @@ class DeviceDriverTest(driver_test_lib.BaseDriverTest): instance=fake_instance, image_name=fake_image, extra_disk_name=disk_name, - avd_spec=None) + avd_spec=None, + extra_scopes=None) self.compute_client.DeleteImage.assert_called_with(fake_image) self.storage_client.Delete(cfg.storage_bucket_name, fake_gs_object) diff --git a/public/report.py b/public/report.py index 2ca11da3..061cfe60 100755 --- a/public/report.py +++ b/public/report.py @@ -159,12 +159,12 @@ class Report(object): status=self.status, errors=self.errors, data=self.data) - logger.info("Report: %s", json.dumps(result, indent=2)) + logger.info("Report: %s", json.dumps(result, indent=2, sort_keys=True)) if not report_file: return try: with open(report_file, "w") as f: - json.dump(result, f, indent=2) + json.dump(result, f, indent=2, sort_keys=True) logger.info("Report file generated at %s", os.path.abspath(report_file)) except OSError as e: diff --git a/reconnect/reconnect.py b/reconnect/reconnect.py index 413b1463..b4dc0275 100644 --- a/reconnect/reconnect.py +++ b/reconnect/reconnect.py @@ -21,12 +21,12 @@ Reconnect will: from __future__ import print_function -from collections import namedtuple import getpass import re from acloud.delete import delete -from acloud.internal import constants +from acloud.internal.lib import auth +from acloud.internal.lib import android_compute_client from acloud.internal.lib import utils from acloud.internal.lib.adb_tools import AdbTools from acloud.list import list as list_instance @@ -34,15 +34,6 @@ from acloud.public import config _RE_DISPLAY = re.compile(r"([\d]+)x([\d]+)\s.*") _VNC_STARTED_PATTERN = "ssvnc vnc://127.0.0.1:%(vnc_port)d" -# TODO(b/122929848): merge all definition of ForwardedPorts into one spot. -ForwardedPorts = namedtuple("ForwardedPorts", - [constants.VNC_PORT, constants.ADB_PORT]) -_AVD_PORT_CLASS_DICT = { - constants.TYPE_GCE: ForwardedPorts(constants.DEFAULT_GCE_VNC_PORT, - constants.DEFAULT_GCE_ADB_PORT), - constants.TYPE_CF: ForwardedPorts(constants.CF_TARGET_VNC_PORT, - constants.CF_TARGET_ADB_PORT) -} def StartVnc(vnc_port, display): @@ -68,6 +59,26 @@ def StartVnc(vnc_port, display): utils.LaunchVncClient(vnc_port) +def AddPublicSshRsaToInstance(cfg, user, instance_name): + """Add the public rsa key to the instance's metadata. + + When the public key doesn't exist in the metadata, it will add it. + + Args: + cfg: An AcloudConfig instance. + user: String, the ssh username to access instance. + instance_name: String, instance name. + """ + credentials = auth.CreateCredentials(cfg) + compute_client = android_compute_client.AndroidComputeClient( + cfg, credentials) + compute_client.AddSshRsaInstanceMetadata( + cfg.zone, + user, + cfg.ssh_public_key_path, + instance_name) + + def ReconnectInstance(ssh_private_key_path, instance): """Reconnect adb/vnc/ssh to the specified instance. @@ -88,8 +99,8 @@ def ReconnectInstance(ssh_private_key_path, instance): forwarded_ports = utils.AutoConnect( instance.ip, ssh_private_key_path, - _AVD_PORT_CLASS_DICT.get(instance.avd_type).vnc_port, - _AVD_PORT_CLASS_DICT.get(instance.avd_type).adb_port, + utils.AVD_PORT_DICT[instance.avd_type].vnc_port, + utils.AVD_PORT_DICT[instance.avd_type].adb_port, getpass.getuser()) vnc_port = forwarded_ports.vnc_port @@ -112,4 +123,5 @@ def Run(args): if not instances_to_reconnect: instances_to_reconnect = list_instance.ChooseInstances(cfg, args.all) for instance in instances_to_reconnect: + AddPublicSshRsaToInstance(cfg, getpass.getuser(), instance.name) ReconnectInstance(cfg.ssh_private_key_path, instance) diff --git a/reconnect/reconnect_test.py b/reconnect/reconnect_test.py index b00c1319..82e4b6c9 100644 --- a/reconnect/reconnect_test.py +++ b/reconnect/reconnect_test.py @@ -74,8 +74,8 @@ class ReconnectTest(driver_test_lib.BaseDriverTest): reconnect.ReconnectInstance(ssh_private_key_path, instance_object) utils.AutoConnect.assert_called_with(instance_object.ip, ssh_private_key_path, - constants.CF_TARGET_VNC_PORT, - constants.CF_TARGET_ADB_PORT, + constants.CF_VNC_PORT, + constants.CF_ADB_PORT, "fake_user") utils.LaunchVncClient.assert_called_with(11111) @@ -84,8 +84,8 @@ class ReconnectTest(driver_test_lib.BaseDriverTest): reconnect.ReconnectInstance(ssh_private_key_path, instance_object) utils.AutoConnect.assert_called_with(instance_object.ip, ssh_private_key_path, - constants.CF_TARGET_VNC_PORT, - constants.CF_TARGET_ADB_PORT, + constants.CF_VNC_PORT, + constants.CF_ADB_PORT, "fake_user") utils.LaunchVncClient.assert_called_with(11111, "999", "777") @@ -117,8 +117,8 @@ class ReconnectTest(driver_test_lib.BaseDriverTest): reconnect.ReconnectInstance(ssh_private_key_path, instance_object) utils.AutoConnect.assert_called_with(instance_object.ip, ssh_private_key_path, - constants.DEFAULT_GCE_VNC_PORT, - constants.DEFAULT_GCE_ADB_PORT, + constants.GCE_VNC_PORT, + constants.GCE_ADB_PORT, "fake_user") #test reconnect remote instance when avd_type as cuttlefish. @@ -126,8 +126,8 @@ class ReconnectTest(driver_test_lib.BaseDriverTest): reconnect.ReconnectInstance(ssh_private_key_path, instance_object) utils.AutoConnect.assert_called_with(instance_object.ip, ssh_private_key_path, - constants.CF_TARGET_VNC_PORT, - constants.CF_TARGET_ADB_PORT, + constants.CF_VNC_PORT, + constants.CF_ADB_PORT, "fake_user") def testStartVnc(self): diff --git a/setup/gcp_setup_runner.py b/setup/gcp_setup_runner.py index fa0f542c..0a1b2813 100644 --- a/setup/gcp_setup_runner.py +++ b/setup/gcp_setup_runner.py @@ -192,17 +192,21 @@ class GcpTaskRunner(base_task_runner.BaseTaskRunner): Args: config_path: String, acloud config path. """ + # pylint: disable=invalid-name config_mgr = config.AcloudConfigManager(config_path) cfg = config_mgr.Load() self.config_path = config_mgr.user_config_path - self.client_id = cfg.client_id - self.client_secret = cfg.client_secret self.project = cfg.project self.zone = cfg.zone self.storage_bucket_name = cfg.storage_bucket_name self.ssh_private_key_path = cfg.ssh_private_key_path self.ssh_public_key_path = cfg.ssh_public_key_path self.stable_host_image_name = cfg.stable_host_image_name + self.client_id = cfg.client_id + self.client_secret = cfg.client_secret + self.service_account_name = cfg.service_account_name + self.service_account_private_key_path = cfg.service_account_private_key_path + self.service_account_json_private_key_path = cfg.service_account_json_private_key_path def ShouldRun(self): """Check if we actually need to run GCP setup. @@ -212,9 +216,18 @@ class GcpTaskRunner(base_task_runner.BaseTaskRunner): Returns: True if reqired config fields are empty, False otherwise. """ - return (not self.client_id - or not self.client_secret - or not self.project) + # We need to ensure the config has the proper auth-related fields set, + # so config requires just 1 of the following: + # 1. client id/secret + # 2. service account name/private key path + # 3. service account json private key path + if ((not self.client_id or not self.client_secret) + and (not self.service_account_name or not self.service_account_private_key_path) + and not self.service_account_json_private_key_path): + return True + + # If a project isn't set, then we need to run setup. + return not self.project def _Run(self): """Run GCP setup task.""" diff --git a/setup/setup.py b/setup/setup.py index 8b5eadbb..ad498f41 100644 --- a/setup/setup.py +++ b/setup/setup.py @@ -95,7 +95,7 @@ def _RunPreSetup(): if constants.ENV_ANDROID_BUILD_TOP not in os.environ: print("Can't find $%s." % constants.ENV_ANDROID_BUILD_TOP) print("Please run '#source build/envsetup.sh && lunch <target>' first.") - sys.exit(1) + sys.exit(constants.EXIT_BY_USER) pre_setup_sh = os.path.join(os.environ.get(constants.ENV_ANDROID_BUILD_TOP), "tools", |