Merge "Fix the exception of no cuttlefish runtime config."
diff --git a/create/avd_spec.py b/create/avd_spec.py
index 7be4ece..b1bdc40 100644
--- a/create/avd_spec.py
+++ b/create/avd_spec.py
@@ -106,6 +106,7 @@
self._instance_name_to_reuse = None
self._unlock_screen = None
self._report_internal_ip = None
+ self._disable_external_ip = None
self._avd_type = None
self._flavor = None
self._image_source = None
@@ -304,6 +305,7 @@
self._autoconnect = args.autoconnect
self._unlock_screen = args.unlock_screen
self._report_internal_ip = args.report_internal_ip
+ self._disable_external_ip = args.disable_external_ip
self._avd_type = args.avd_type
self._flavor = args.flavor or constants.FLAVOR_PHONE
if args.remote_host:
@@ -821,6 +823,11 @@
return self._report_internal_ip
@property
+ def disable_external_ip(self):
+ """Return disable_external_ip."""
+ return self._disable_external_ip
+
+ @property
def kernel_build_info(self):
"""Return kernel build info."""
return self._kernel_build_info
diff --git a/create/create_args.py b/create/create_args.py
index f4a7c4f..3a869a3 100644
--- a/create/create_args.py
+++ b/create/create_args.py
@@ -91,6 +91,12 @@
"ip. Using the internal ip is used when connecting from another "
"GCE instance.")
parser.add_argument(
+ "--disable-external-ip",
+ action="store_true",
+ dest="disable_external_ip",
+ required=False,
+ help="Disable the external ip of the created instance.")
+ parser.add_argument(
"--network",
type=str,
dest="network",
diff --git a/internal/constants.py b/internal/constants.py
index 91f47d1..eb304e9 100755
--- a/internal/constants.py
+++ b/internal/constants.py
@@ -177,6 +177,7 @@
ENV_CUTTLEFISH_CONFIG_FILE = "CUTTLEFISH_CONFIG_FILE"
ENV_CUTTLEFISH_INSTANCE = "CUTTLEFISH_INSTANCE"
ENV_CVD_HOME = "HOME"
+ANDROID_INFO_FILE = "android-info.txt"
CUTTLEFISH_CONFIG_FILE = "cuttlefish_config.json"
TEMP_ARTIFACTS_FOLDER = "acloud_image_artifacts"
diff --git a/internal/lib/cvd_compute_client_multi_stage.py b/internal/lib/cvd_compute_client_multi_stage.py
index 5f7f4ab..7d12673 100644
--- a/internal/lib/cvd_compute_client_multi_stage.py
+++ b/internal/lib/cvd_compute_client_multi_stage.py
@@ -37,6 +37,7 @@
import logging
import os
+import re
import subprocess
import tempfile
import time
@@ -75,6 +76,7 @@
_NO_RETRY = 0
# Launch cvd command for acloud report
_LAUNCH_CVD_COMMAND = "launch_cvd_command"
+_CONFIG_RE = re.compile(r"^config=(?P<config>.+)")
class CvdComputeClient(android_compute_client.AndroidComputeClient):
@@ -270,6 +272,22 @@
self._metadata.update({"kernel_build_id": kernel_build_id})
self._metadata.update({"kernel_build_target": kernel_build_target})
+ def _GetConfigFromAndroidInfo(self):
+ """Get config value from android-info.txt.
+
+ The config in android-info.txt would like "config=phone".
+
+ Returns:
+ Strings of config value.
+ """
+ android_info = self._ssh.GetCmdOutput(
+ "cat %s" % constants.ANDROID_INFO_FILE)
+ logger.debug("Android info: %s", android_info)
+ config_match = _CONFIG_RE.match(android_info)
+ if config_match:
+ return config_match.group("config")
+ return None
+
# pylint: disable=too-many-branches
def _GetLaunchCvdArgs(self, avd_spec=None, blank_data_disk_size_gb=None,
decompress_kernel=None, instance=None):
@@ -293,8 +311,10 @@
launch_cvd_args.append(
"-blank_data_image_mb=%d" % (blank_data_disk_size_gb * 1024))
if avd_spec:
- launch_cvd_args.append("-config=%s" % avd_spec.flavor)
- if avd_spec.hw_customize or not self._ArgSupportInLaunchCVD(_CONFIG_ARG):
+ config = self._GetConfigFromAndroidInfo()
+ if config:
+ launch_cvd_args.append("-config=%s" % config)
+ if avd_spec.hw_customize or not config:
launch_cvd_args.append(
"-x_res=" + avd_spec.hw_property[constants.HW_X_RES])
launch_cvd_args.append(
@@ -339,19 +359,6 @@
launch_cvd_args.append(_AGREEMENT_PROMPT_ARG)
return launch_cvd_args
- def _ArgSupportInLaunchCVD(self, arg):
- """Check if the arg is supported in launch_cvd.
-
- Args:
- arg: String of the arg. e.g. "-config".
-
- Returns:
- True if this arg is supported. Otherwise False.
- """
- if arg in self._ssh.GetCmdOutput("./bin/launch_cvd --help"):
- return True
- return False
-
def StopCvd(self):
"""Stop CVD.
@@ -492,6 +499,7 @@
disk_args = self._GetDiskArgs(
instance, image_name, image_project, boot_disk_size_gb)
+ disable_external_ip = avd_spec.disable_external_ip if avd_spec else False
gcompute_client.ComputeClient.CreateInstance(
self,
instance=instance,
@@ -505,7 +513,8 @@
gpu=self._gpu,
extra_scopes=extra_scopes,
tags=["appstreaming"] if (
- avd_spec and avd_spec.connect_webrtc) else None)
+ avd_spec and avd_spec.connect_webrtc) else None,
+ disable_external_ip=disable_external_ip)
ip = gcompute_client.ComputeClient.GetInstanceIP(
self, instance=instance, zone=self._zone)
logger.debug("'instance_ip': %s", ip.internal
diff --git a/internal/lib/cvd_compute_client_multi_stage_test.py b/internal/lib/cvd_compute_client_multi_stage_test.py
index 0b485dd..d0f63d0 100644
--- a/internal/lib/cvd_compute_client_multi_stage_test.py
+++ b/internal/lib/cvd_compute_client_multi_stage_test.py
@@ -31,6 +31,7 @@
from acloud.internal.lib import gcompute_client
from acloud.internal.lib import utils
from acloud.internal.lib.ssh import Ssh
+from acloud.internal.lib.ssh import IP
from acloud.list import list as list_instances
@@ -59,6 +60,7 @@
LAUNCH_ARGS = "--setupwizard_mode=REQUIRED"
EXTRA_SCOPES = ["scope1"]
GPU = "fake-gpu"
+ FAKE_IP = IP(external="1.1.1.1", internal="10.1.1.1")
def _GetFakeConfig(self):
"""Create a fake configuration object.
@@ -79,15 +81,13 @@
fake_cfg.extra_scopes = self.EXTRA_SCOPES
return fake_cfg
+ # pylint: disable=protected-access
def setUp(self):
"""Set up the test."""
super().setUp()
self.Patch(cvd_compute_client_multi_stage.CvdComputeClient, "InitResourceHandle")
self.Patch(cvd_compute_client_multi_stage.CvdComputeClient, "_VerifyZoneByQuota",
return_value=True)
- self.Patch(cvd_compute_client_multi_stage.CvdComputeClient,
- "_ArgSupportInLaunchCVD",
- return_value=True)
self.Patch(android_build_client.AndroidBuildClient, "InitResourceHandle")
self.Patch(android_build_client.AndroidBuildClient, "DownloadArtifact")
self.Patch(list_instances, "GetInstancesFromInstanceNames", return_value=mock.MagicMock())
@@ -97,6 +97,10 @@
self.Patch(Ssh, "GetBaseCmd")
self.cvd_compute_client_multi_stage = cvd_compute_client_multi_stage.CvdComputeClient(
self._GetFakeConfig(), mock.MagicMock(), gpu=self.GPU)
+ self.cvd_compute_client_multi_stage._ssh = Ssh(
+ ip=self.FAKE_IP,
+ user=constants.GCE_USER,
+ ssh_private_key_path="/fake/acloud_rea")
self.args = mock.MagicMock()
self.args.local_image = constants.FIND_IN_BUILD_ENV
self.args.local_system_image = None
@@ -108,6 +112,7 @@
self.args.num_avds_per_instance = 2
self.args.remote_host = False
self.args.launch_args = self.LAUNCH_ARGS
+ self.args.disable_external_ip = False
# pylint: disable=protected-access
@mock.patch.object(utils, "GetBuildEnvironmentVariable", return_value="fake_env_cf_x86")
@@ -115,6 +120,8 @@
def testGetLaunchCvdArgs(self, _mock_check_img, _mock_env):
"""test GetLaunchCvdArgs."""
# test GetLaunchCvdArgs with avd_spec
+ self.Patch(cvd_compute_client_multi_stage.CvdComputeClient,
+ "_GetConfigFromAndroidInfo", return_value="phone")
fake_avd_spec = avd_spec.AVDSpec(self.args)
expeted_args = ["-config=phone", "-x_res=1080", "-y_res=1920", "-dpi=240",
"-data_policy=always_create", "-blank_data_image_mb=10240",
@@ -188,7 +195,8 @@
zone=self.ZONE,
extra_scopes=self.EXTRA_SCOPES,
gpu=self.GPU,
- tags=None)
+ tags=None,
+ disable_external_ip=False)
mock_check_img.return_value = True
#test use local image in the remote instance.
@@ -222,7 +230,8 @@
zone=self.ZONE,
extra_scopes=self.EXTRA_SCOPES,
gpu=self.GPU,
- tags=None)
+ tags=None,
+ disable_external_ip=False)
def testRecordBuildInfo(self):
"""Test RecordBuildInfo"""
@@ -268,12 +277,13 @@
self.assertEqual(self.cvd_compute_client_multi_stage.stage,
device_stage)
- def testArgSupportInLaunchCVD(self):
- """Test ArgSupportInLaunchCVD"""
- self.Patch(Ssh, "GetCmdOutput", return_value="-config (Config)")
- self.assertTrue(
- self.cvd_compute_client_multi_stage._ArgSupportInLaunchCVD(
- "-config"))
+ def testGetConfigFromAndroidInfo(self):
+ """Test GetConfigFromAndroidInfo"""
+ self.Patch(Ssh, "GetCmdOutput", return_value="config=phone")
+ expected = "phone"
+ self.assertEqual(
+ self.cvd_compute_client_multi_stage._GetConfigFromAndroidInfo(),
+ expected)
if __name__ == "__main__":
diff --git a/internal/lib/gcompute_client.py b/internal/lib/gcompute_client.py
index a9bb8a9..c5fe845 100755
--- a/internal/lib/gcompute_client.py
+++ b/internal/lib/gcompute_client.py
@@ -1144,19 +1144,24 @@
api = self.service.regions().list(project=self._project)
return self.Execute(api)
- def _GetNetworkArgs(self, network, zone):
+ def _GetNetworkArgs(self, network, zone, disable_external_ip):
"""Helper to generate network args that is used to create an instance.
+ See: https://cloud.google.com/compute/docs/reference/rest/v1/instances
+ for more information on the specific fields.
+
Args:
network: A string, e.g. "default".
zone: String, representing zone name, e.g. "us-central1-f"
+ disable_external_ip: Boolean, true if the external ip should be
+ disabled.
Returns:
A dictionary representing network args.
"""
network_args = {
"network": self.GetNetworkUrl(network),
- "accessConfigs": [{
+ "accessConfigs": [] if disable_external_ip else [{
"name": "External NAT",
"type": "ONE_TO_ONE_NAT"
}]
@@ -1229,7 +1234,8 @@
gpu=None,
extra_disk_name=None,
extra_scopes=None,
- tags=None):
+ tags=None,
+ disable_external_ip=False):
"""Create a gce instance with a gce image.
Args:
@@ -1252,6 +1258,8 @@
extra_scopes: A list of extra scopes to be provided to the instance.
tags: A list of tags to associate with the instance. e.g.
["http-server", "https-server"]
+ disable_external_ip: Boolean, true if instance external ip should be
+ disabled.
"""
disk_args = (disk_args
or self._GetDiskArgs(instance, image_name, image_project))
@@ -1268,7 +1276,9 @@
body = {
"machineType": self.GetMachineType(machine_type, zone)["selfLink"],
"name": instance,
- "networkInterfaces": [self._GetNetworkArgs(network, zone)],
+ "networkInterfaces": [
+ self._GetNetworkArgs(network, zone, disable_external_ip)
+ ],
"disks": disk_args,
"labels": {constants.LABEL_CREATE_BY: getpass.getuser()},
"serviceAccounts": [{
@@ -1503,8 +1513,8 @@
ip_name_map = dict.fromkeys(ips)
for instance in self.ListInstances():
try:
- ip = instance["networkInterfaces"][0]["accessConfigs"][0][
- "natIP"]
+ instance_ip = GetInstanceIP(instance)
+ ip = instance_ip.external or instance_ip.internal
if ip in ips:
ip_name_map[ip] = instance["name"]
except (IndexError, KeyError) as e:
@@ -1522,9 +1532,7 @@
ssh.IP object, that stores internal and external ip of the instance.
"""
instance = self.GetInstance(instance, zone)
- internal_ip = instance["networkInterfaces"][0]["networkIP"]
- external_ip = instance["networkInterfaces"][0]["accessConfigs"][0]["natIP"]
- return IP(internal=internal_ip, external=external_ip)
+ return GetInstanceIP(instance)
@utils.TimeExecute(function_description="Updating instance metadata: ")
def SetInstanceMetadata(self, zone, instance, body):
@@ -1755,3 +1763,18 @@
rsa = rsa.strip() if rsa else rsa
utils.VerifyRsaPubKey(rsa)
return rsa
+
+def GetInstanceIP(instance):
+ """Get the internal and external IP for a given instance
+
+ Args:
+ instance: A dict, representing a gce instance
+
+ Returns:
+ A populated IP object
+ """
+ network_interface = instance["networkInterfaces"][0]
+ access_configs = network_interface.get("accessConfigs", [{}])[0]
+ external_ip = access_configs.get("natIP", "")
+ internal_ip = network_interface.get("networkIP", "")
+ return IP(internal=internal_ip, external=external_ip)
diff --git a/list/instance.py b/list/instance.py
index 0da880d..afcaa7c 100644
--- a/list/instance.py
+++ b/list/instance.py
@@ -44,6 +44,7 @@
from acloud.internal.lib import utils
from acloud.internal.lib.adb_tools import AdbTools
from acloud.internal.lib.local_instance_lock import LocalInstanceLock
+from acloud.internal.lib.gcompute_client import GetInstanceIP
logger = logging.getLogger(__name__)
@@ -694,10 +695,8 @@
status = gce_instance.get(constants.INS_KEY_STATUS)
zone = self._GetZoneName(gce_instance.get(constants.INS_KEY_ZONE))
- ip = None
- for network_interface in gce_instance.get("networkInterfaces"):
- for access_config in network_interface.get("accessConfigs"):
- ip = access_config.get("natIP")
+ instance_ip = GetInstanceIP(gce_instance)
+ ip = instance_ip.external or instance_ip.internal
# Get metadata
display = None
diff --git a/public/acloud_main.py b/public/acloud_main.py
index b8d89d5..8102136 100644
--- a/public/acloud_main.py
+++ b/public/acloud_main.py
@@ -332,7 +332,11 @@
if args.which == CMD_CREATE_CUTTLEFISH:
missing_fields.extend(cfg.GetMissingFields(_CREATE_CF_REQUIRE_FIELDS))
if missing_fields:
- return "Missing required configuration fields: %s" % missing_fields
+ return (
+ "Config file (%s) missing required fields: %s, please add these "
+ "fields or reset config file. For reset config information: "
+ "go/acloud-googler-setup#reset-configuration" %
+ (config.GetUserConfigPath(args.config_file), missing_fields))
return None
diff --git a/public/actions/remote_instance_cf_device_factory.py b/public/actions/remote_instance_cf_device_factory.py
index c08df62..8daa2e4 100644
--- a/public/actions/remote_instance_cf_device_factory.py
+++ b/public/actions/remote_instance_cf_device_factory.py
@@ -278,6 +278,8 @@
artifact_files.extend(
os.path.basename(image) for image in glob.glob(
os.path.join(images_dir, file_name)))
+ # Upload android-info.txt to parse config value.
+ artifact_files.append(constants.ANDROID_INFO_FILE)
cmd = ("tar -cf - --lzop -S -C {images_dir} {artifact_files} | "
"{ssh_cmd} -- tar -xf - --lzop -S".format(
images_dir=images_dir,
diff --git a/public/actions/remote_instance_cf_device_factory_test.py b/public/actions/remote_instance_cf_device_factory_test.py
index 1e0aa0b..3bbbc15 100644
--- a/public/actions/remote_instance_cf_device_factory_test.py
+++ b/public/actions/remote_instance_cf_device_factory_test.py
@@ -301,7 +301,7 @@
fake_host_package,
fake_local_image_dir)
expected_cmd = (
- "tar -cf - --lzop -S -C %s fake.img bootloader kernel | "
+ "tar -cf - --lzop -S -C %s fake.img bootloader kernel android-info.txt | "
"%s -- tar -xf - --lzop -S" %
(fake_local_image_dir, factory._ssh.GetBaseCmd(constants.SSH_BIN)))
mock_shell.assert_called_once_with(expected_cmd)
@@ -318,8 +318,8 @@
fake_host_package,
fake_local_image_dir)
expected_cmd = (
- "tar -cf - --lzop -S -C %s boot.img cache.img super.img userdata.img bootloader | "
- "%s -- tar -xf - --lzop -S" %
+ "tar -cf - --lzop -S -C %s boot.img cache.img super.img userdata.img "
+ "bootloader android-info.txt | %s -- tar -xf - --lzop -S" %
(fake_local_image_dir, factory._ssh.GetBaseCmd(constants.SSH_BIN)))
mock_shell.assert_called_once_with(expected_cmd)
diff --git a/public/config.py b/public/config.py
index e7eb8e1..2c8b2f8 100755
--- a/public/config.py
+++ b/public/config.py
@@ -96,6 +96,22 @@
return os.path.join(config_path, _DEFAULT_CONFIG_FILE)
+def GetUserConfigPath(config_path):
+ """Get Acloud user config file path.
+
+ If there is no config provided, Acloud would use default config path.
+
+ Args:
+ config_path: String, path of Acloud config file.
+
+ Returns:
+ Path (string) of the Acloud config.
+ """
+ if config_path:
+ return config_path
+ return GetDefaultConfigFile()
+
+
def GetAcloudConfig(args):
"""Helper function to initialize Config object.
