Infer flavor from locally built image

Bug: 117554732
Test: m acloud && atest acloud_test && acloud create --local-image

Change-Id: Ide7112456838c608ef190e1640d749e41d990584
diff --git a/create/avd_spec.py b/create/avd_spec.py
index 62900ed..83f1c81 100644
--- a/create/avd_spec.py
+++ b/create/avd_spec.py
@@ -43,6 +43,7 @@
 _DEFAULT_BUILD_TYPE = "userdebug"
 _ENV_ANDROID_PRODUCT_OUT = "ANDROID_PRODUCT_OUT"
 _ENV_ANDROID_BUILD_TOP = "ANDROID_BUILD_TOP"
+_RE_FLAVOR = re.compile(r"^.+_(?P<flavor>.+)-img.+")
 _RE_GBSIZE = re.compile(r"^(?P<gb_size>\d+)g$", re.IGNORECASE)
 _RE_INT = re.compile(r"^\d+$")
 _RE_RES = re.compile(r"^(?P<x_res>\d+)x(?P<y_res>\d+)$")
@@ -83,6 +84,7 @@
         self._instance_type = None
         self._kernel_build_id = None
         self._local_image_dir = None
+        self._local_image_artifact = None
         self._image_download_dir = None
         self._num_of_instances = None
         self._remote_image = None
@@ -225,7 +227,7 @@
         self._autoconnect = args.autoconnect
         self._report_internal_ip = args.report_internal_ip
         self._avd_type = args.avd_type
-        self._flavor = args.flavor
+        self._flavor = args.flavor or constants.FLAVOR_PHONE
         self._instance_type = (constants.INSTANCE_TYPE_LOCAL
                                if args.local_instance else
                                constants.INSTANCE_TYPE_REMOTE)
@@ -234,6 +236,33 @@
         self._serial_log_file = args.serial_log_file
         self._logcat_file = args.logcat_file
 
+    @staticmethod
+    def _GetFlavorFromLocalImage(image_path):
+        """Get flavor name from local image file name.
+
+        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.
+
+        Args:
+            image_path: String of image path.
+
+        Returns:
+            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)
+
+        return None
+
     def _ProcessLocalImageArgs(self, args):
         """Get local image path.
 
@@ -258,6 +287,15 @@
                     % _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)
+
     def _ProcessRemoteBuildArgs(self, args):
         """Get the remote build args.
 
@@ -369,6 +407,11 @@
         return self._local_image_dir
 
     @property
+    def local_image_artifact(self):
+        """Return local image artifact."""
+        return self._local_image_artifact
+
+    @property
     def avd_type(self):
         """Return the avd type."""
         return self._avd_type
diff --git a/create/avd_spec_test.py b/create/avd_spec_test.py
index 647bca3..bd9ae4c 100644
--- a/create/avd_spec_test.py
+++ b/create/avd_spec_test.py
@@ -18,6 +18,7 @@
 
 from acloud import errors
 from acloud.create import avd_spec
+from acloud.create import create_common
 from acloud.internal import constants
 
 
@@ -33,9 +34,11 @@
         self.AvdSpec = avd_spec.AVDSpec(self.args)
 
     # pylint: disable=protected-access
-    def testProcessLocalImageArgs(self):
+    @mock.patch.object(create_common, "VerifyLocalImageArtifactsExist")
+    def testProcessLocalImageArgs(self, mock_image):
         """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.AvdSpec._ProcessLocalImageArgs(self.args)
         self.assertEqual(self.AvdSpec._local_image_dir, "test_path")
@@ -46,7 +49,8 @@
             self.AvdSpec._ProcessLocalImageArgs(self.args)
             self.assertEqual(self.AvdSpec._local_image_dir, "test_environ")
 
-    def testProcessImageArgs(self):
+    @mock.patch.object(create_common, "VerifyLocalImageArtifactsExist")
+    def testProcessImageArgs(self, mock_image):
         """Test process image source."""
         # No specified local_image, image source is from remote
         self.args.local_image = ""
@@ -55,11 +59,11 @@
         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"
         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.AvdSpec = avd_spec.AVDSpec(self.args)
 
     @mock.patch.object(avd_spec.AVDSpec, "_GetGitRemote")
     @mock.patch("subprocess.check_output")
@@ -139,6 +143,15 @@
         result_dict = self.AvdSpec._ParseHWPropertyStr(args_str)
         self.assertTrue(expected_dict == result_dict)
 
+    def testGetFlavorFromLocalImage(self):
+        """Test _GetFlavorFromLocalImage."""
+        img_path = "/fack_path/cf_x86_tv-img-eng.user.zip"
+        self.assertEqual(self.AvdSpec._GetFlavorFromLocalImage(img_path), "tv")
+
+        # Flavor is not supported.
+        img_path = "/fack_path/cf_x86_error-img-eng.user.zip"
+        self.assertEqual(self.AvdSpec._GetFlavorFromLocalImage(img_path), None)
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/create/create_args.py b/create/create_args.py
index abe5cb2..487af00 100644
--- a/create/create_args.py
+++ b/create/create_args.py
@@ -131,8 +131,6 @@
         "--flavor",
         type=str,
         dest="flavor",
-        default=constants.FLAVOR_PHONE,
-        choices=constants.ALL_FLAVORS,
         help="The device flavor of the AVD (default %s)." % constants.FLAVOR_PHONE)
     create_parser.add_argument(
         "--build-target",
@@ -204,7 +202,16 @@
 
     Raises:
         errors.CreateError: Path doesn't exist.
+        errors.UnsupportedFlavor: Flavor doesn't support.
     """
+    # Verify that user specified flavor name is in support list.
+    # We don't use argparse's builtin validation because we need to be able to
+    # tell when a user doesn't specify a flavor.
+    if args.flavor and args.flavor not in constants.ALL_FLAVORS:
+        raise errors.UnsupportedFlavor(
+            "Flavor[%s] isn't in support list: %s" % (args.flavor,
+                                                      constants.ALL_FLAVORS))
+
     if args.local_image and not os.path.exists(args.local_image):
         raise errors.CheckPathError(
             "Specified path doesn't exist: %s" % args.local_image)
diff --git a/create/local_image_local_instance.py b/create/local_image_local_instance.py
index 48c790b..a5ebcff 100644
--- a/create/local_image_local_instance.py
+++ b/create/local_image_local_instance.py
@@ -27,7 +27,6 @@
 
 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 utils
 from acloud.public import report
@@ -91,8 +90,8 @@
     def GetImageArtifactsPath(avd_spec):
         """Get image artifacts path.
 
-        This method will check if local image and launch_cvd are exist and
-        return the tuple path where they are located respectively.
+        This method will check if launch_cvd is exist and return the tuple path
+        (image path and host bins path) where they are located respectively.
         For remote image, RemoteImageLocalInstance will override this method and
         return the artifacts path which is extracted and downloaded from remote.
 
@@ -102,17 +101,6 @@
         Returns:
             Tuple of (local image file, host bins package) paths.
         """
-        try:
-            # Check if local image is exist.
-            create_common.VerifyLocalImageArtifactsExist(
-                avd_spec.local_image_dir)
-
-        # TODO(b/117306227): help user to build out images and host package if
-        # anything needed is not found.
-        except errors.GetLocalImageError as imgerror:
-            logger.error(imgerror.message)
-            raise imgerror
-
         # Check if launch_cvd is exist.
         host_bins_path = os.environ.get(_ENV_ANDROID_HOST_OUT)
         launch_cvd_path = os.path.join(host_bins_path, "bin",
diff --git a/create/local_image_remote_instance.py b/create/local_image_remote_instance.py
index 933d7b3..a31e869 100644
--- a/create/local_image_remote_instance.py
+++ b/create/local_image_remote_instance.py
@@ -26,7 +26,6 @@
 import subprocess
 
 from acloud import errors
-from acloud.create import create_common
 from acloud.create import base_avd_create
 from acloud.internal import constants
 from acloud.internal.lib import auth
@@ -221,7 +220,6 @@
 
     def __init__(self):
         """LocalImageRemoteInstance initialize."""
-        self.local_image_artifact = None
         self.cvd_host_package_artifact = None
 
     def VerifyArtifactsExist(self, local_image_dir):
@@ -230,8 +228,6 @@
         Arsg:
             local_image_dir: A string, path to check the artifacts.
         """
-        self.local_image_artifact = create_common.VerifyLocalImageArtifactsExist(
-            local_image_dir)
         self.cvd_host_package_artifact = self.VerifyHostPackageArtifactsExist(
             local_image_dir)
 
@@ -288,7 +284,7 @@
         self.VerifyArtifactsExist(avd_spec.local_image_dir)
         device_factory = RemoteInstanceDeviceFactory(
             avd_spec,
-            self.local_image_artifact,
+            avd_spec.local_image_artifact,
             self.cvd_host_package_artifact)
         report = common_operations.CreateDevices(
             "create_cf", avd_spec.cfg, device_factory, avd_spec.num,
diff --git a/create/local_image_remote_instance_test.py b/create/local_image_remote_instance_test.py
index e54fc19..d29d492 100644
--- a/create/local_image_remote_instance_test.py
+++ b/create/local_image_remote_instance_test.py
@@ -28,6 +28,7 @@
 
 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.lib import auth
 from acloud.internal.lib import cvd_compute_client
@@ -51,17 +52,12 @@
                 self.local_image_remote_instance.VerifyHostPackageArtifactsExist,
                 "/fake_dirs")
 
-    @mock.patch("glob.glob")
-    def testVerifyArtifactsExist(self, mock_glob):
+    def testVerifyArtifactsExist(self):
         """test verify artifacts exist."""
-        mock_glob.return_value = ["/fake_dirs/image1.zip"]
         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.local_image_artifact,
-                "/fake_dirs/image1.zip")
-            self.assertEqual(
                 self.local_image_remote_instance.cvd_host_package_artifact,
                 "/fake_dirs/cvd-host_package.tar.gz")
 
@@ -92,7 +88,8 @@
         self.assertEqual(factory._ShellCmdWithRetry("fake cmd"), True)
 
     # pylint: disable=protected-access
-    def testCreateGceInstanceName(self):
+    @mock.patch.object(create_common, "VerifyLocalImageArtifactsExist")
+    def testCreateGceInstanceName(self, mock_image):
         """test create gce instance."""
         # Mock uuid
         args = mock.MagicMock()
@@ -100,6 +97,7 @@
         args.config_file = ""
         args.avd_type = "cf"
         args.flavor = "phone"
+        mock_image.return_value = "/fake/aosp_cf_x86_phone-img-eng.username.zip"
         fake_avd_spec = avd_spec.AVDSpec(args)
 
         fake_uuid = mock.MagicMock(hex="1234")
diff --git a/errors.py b/errors.py
index 5d08244..08c825c 100644
--- a/errors.py
+++ b/errors.py
@@ -139,6 +139,10 @@
     """Unsupported create action for given instance/image type."""
 
 
+class UnsupportedFlavor(CreateError):
+    """Unsupported create action for given flavor name."""
+
+
 class GetBuildIDError(CreateError):
     """Can't get build id from Android Build."""
 
diff --git a/internal/lib/cvd_compute_client_test.py b/internal/lib/cvd_compute_client_test.py
index e782cff..4b3f8ac 100644
--- a/internal/lib/cvd_compute_client_test.py
+++ b/internal/lib/cvd_compute_client_test.py
@@ -20,6 +20,7 @@
 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
@@ -72,6 +73,7 @@
         self.cvd_compute_client = cvd_compute_client.CvdComputeClient(
             self._GetFakeConfig(), mock.MagicMock())
 
+    @mock.patch.object(create_common, "VerifyLocalImageArtifactsExist")
     @mock.patch.object(gcompute_client.ComputeClient, "CompareMachineSize",
                        return_value=1)
     @mock.patch.object(gcompute_client.ComputeClient, "GetImage",
@@ -81,7 +83,7 @@
                        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):
+                           _get_image, _compare_machine_size, mock_img_path):
         """Test CreateInstance."""
         expected_metadata = {
             "cvd_01_dpi": str(self.DPI),
@@ -120,6 +122,7 @@
 
         #test use local image in the remote instance.
         args = mock.MagicMock()
+        mock_img_path.return_value = "cf_x86_phone-img-eng.user.zip"
         args.local_image = "/tmp/path"
         args.config_file = ""
         args.avd_type = "cf"
diff --git a/public/config.py b/public/config.py
index c4e7ff2..fb7ebcc 100755
--- a/public/config.py
+++ b/public/config.py
@@ -50,6 +50,7 @@
 
 # pylint: disable=no-name-in-module,import-error
 from acloud import errors
+from acloud.internal import constants
 from acloud.internal.proto import internal_config_pb2
 from acloud.internal.proto import user_config_pb2
 from acloud.create import create_args
@@ -214,12 +215,24 @@
         if parsed_args.which == "create_gf" and parsed_args.base_image:
             self.stable_goldfish_host_image_name = parsed_args.base_image
         if parsed_args.which == create_args.CMD_CREATE and not self.hw_property:
-            self.hw_property = self.common_hw_property_map.get(
-                parsed_args.flavor, "")
+            flavor = parsed_args.flavor or constants.FLAVOR_PHONE
+            self.hw_property = self.common_hw_property_map.get(flavor, "")
         if parsed_args.which in [create_args.CMD_CREATE, "create_cf"]:
             if parsed_args.network:
                 self.network = parsed_args.network
 
+    def OverrideHwPropertyWithFlavor(self, flavor):
+        """Override hw configuration values with flavor name.
+
+        HwProperty will be overrided according to the change of flavor.
+        If flavor is None, set hw configuration with phone(default flavor).
+
+        Args:
+            flavor: string of flavor name.
+        """
+        self.hw_property = self.common_hw_property_map.get(
+            flavor, constants.FLAVOR_PHONE)
+
     def Verify(self):
         """Verify configuration fields."""
         missing = [f for f in self.REQUIRED_FIELD if not getattr(self, f)]