Merge "acloud: Mkcert install failed"
diff --git a/create/base_avd_create.py b/create/base_avd_create.py
index f7f8c70..fdc8256 100644
--- a/create/base_avd_create.py
+++ b/create/base_avd_create.py
@@ -22,7 +22,7 @@
 from acloud.internal.lib import utils
 
 
-class BaseAVDCreate(object):
+class BaseAVDCreate():
     """Base class for all AVD intance creation classes."""
 
     def _CreateAVD(self, avd_spec, no_prompts):
diff --git a/create/cheeps_remote_image_remote_instance.py b/create/cheeps_remote_image_remote_instance.py
index 5b1b70e..7d2ddcb 100644
--- a/create/cheeps_remote_image_remote_instance.py
+++ b/create/cheeps_remote_image_remote_instance.py
@@ -89,7 +89,7 @@
 
         compute_client = cheeps_compute_client.CheepsComputeClient(
             cfg, self.credentials)
-        super(CheepsDeviceFactory, self).__init__(compute_client)
+        super().__init__(compute_client)
 
         self._cfg = cfg
         self._avd_spec = avd_spec
diff --git a/create/remote_image_remote_instance.py b/create/remote_image_remote_instance.py
index 71ff944..9319414 100644
--- a/create/remote_image_remote_instance.py
+++ b/create/remote_image_remote_instance.py
@@ -20,11 +20,12 @@
 """
 
 import logging
+import re
 import time
 
 from acloud.create import base_avd_create
 from acloud.internal import constants
-from acloud.internal.lib import engprod_client
+from acloud.internal.lib import oxygen_client
 from acloud.internal.lib import utils
 from acloud.public.actions import common_operations
 from acloud.public.actions import remote_instance_cf_device_factory
@@ -36,6 +37,8 @@
 _DEVICES = "devices"
 _DEVICE_KEY_MAPPING = {"serverUrl": "ip", "sessionId": "instance_name"}
 _LAUNCH_CVD_TIME = "launch_cvd_time"
+_RE_SESSION_ID = re.compile(r".*session_id:\"(?P<session_id>[^\"]+)")
+_RE_SERVER_URL = re.compile(r".*server_url:\"(?P<server_url>[^\"]+)")
 
 
 class RemoteImageRemoteInstance(base_avd_create.BaseAVDCreate):
@@ -85,27 +88,54 @@
             A Report instance.
         """
         timestart = time.time()
-        response = engprod_client.EngProdClient.LeaseDevice(
+        response = oxygen_client.OxygenClient.LeaseDevice(
             avd_spec.remote_image[constants.BUILD_TARGET],
             avd_spec.remote_image[constants.BUILD_ID],
-            avd_spec.cfg.api_key,
-            avd_spec.cfg.api_url)
+            avd_spec.cfg.oxygen_client)
+        session_id, server_url = self._GetDeviceInfoFromResponse(response)
         execution_time = round(time.time() - timestart, 2)
         reporter = report.Report(command="create_cf")
-        if _DEVICE in response:
+        if session_id and server_url:
             reporter.SetStatus(report.Status.SUCCESS)
-            device_data = response[_DEVICE]
+            device_data = {"instance_name": session_id,
+                           "ip": server_url}
             device_data[_LAUNCH_CVD_TIME] = execution_time
-            self._ReplaceDeviceDataKeys(device_data)
             dict_devices = {_DEVICES: [device_data]}
             reporter.UpdateData(dict_devices)
         else:
             reporter.SetStatus(report.Status.FAIL)
-            reporter.AddError(response.get("errorMessage"))
+            reporter.AddError(response)
 
         return reporter
 
     @staticmethod
+    def _GetDeviceInfoFromResponse(response):
+        """Get session id and server url from response.
+
+        Args:
+            response: String of the response from oxygen proxy client.
+                      e.g. "2021/08/02 11:28:52 session_id: "74b6b835"
+                      server_url: "0.0.0.34" port:{type:WATERFALL ..."
+
+        Returns:
+            The session id and the server url of leased device.
+        """
+        session_id = ""
+        for line in response.splitlines():
+            session_id_match = _RE_SESSION_ID.match(line)
+            if session_id_match:
+                session_id = session_id_match.group("session_id")
+                break
+
+        server_url = ""
+        for line in response.splitlines():
+            server_url_match = _RE_SERVER_URL.match(line)
+            if server_url_match:
+                server_url = server_url_match.group("server_url")
+                break
+        return session_id, server_url
+
+    @staticmethod
     def _ReplaceDeviceDataKeys(device_data):
         """Replace keys of device data from oxygen response.
 
diff --git a/create/remote_image_remote_instance_test.py b/create/remote_image_remote_instance_test.py
index ba8d810..d24e542 100644
--- a/create/remote_image_remote_instance_test.py
+++ b/create/remote_image_remote_instance_test.py
@@ -20,12 +20,21 @@
 from acloud.create import remote_image_remote_instance
 from acloud.internal import constants
 from acloud.internal.lib import driver_test_lib
-from acloud.internal.lib import engprod_client
+from acloud.internal.lib import oxygen_client
 from acloud.public import report
 from acloud.public.actions import common_operations
 from acloud.public.actions import remote_instance_cf_device_factory
 
 
+ONE_LINE_LEASE_RESPONSE = ("2021/08/02 11:28:52 session_id:\"fake_device\" "
+                           "server_url:\"10.1.1.1\" ports:{type:WATERFALL value:0}")
+MULTIPLE_LINES_LEASE_RESPONSE = """
+2021/08/02 11:28:52
+session_id:"fake_device"
+server_url:"10.1.1.1"
+"""
+
+
 class RemoteImageRemoteInstanceTest(driver_test_lib.BaseDriverTest):
     """Test RemoteImageRemoteInstance method."""
 
@@ -60,11 +69,9 @@
         avd_spec.oxygen = True
         avd_spec.remote_image = {constants.BUILD_TARGET: "fake_target",
                                  constants.BUILD_ID: "fake_id"}
-        response_success = {"device": {"sessionId": "fake_device",
-                                       "serverUrl": "10.1.1.1"}}
-        response_fail = {"errorMessage": "Lease device fail."}
-        self.Patch(engprod_client.EngProdClient, "LeaseDevice",
-                   side_effect=[response_success, response_fail])
+        response_fail = "Lease device fail."
+        self.Patch(oxygen_client.OxygenClient, "LeaseDevice",
+                   side_effect=[ONE_LINE_LEASE_RESPONSE, response_fail])
         expected_status = report.Status.SUCCESS
         reporter = self.remote_image_remote_instance._LeaseOxygenAVD(avd_spec)
         self.assertEqual(reporter.status, expected_status)
@@ -73,6 +80,20 @@
         reporter = self.remote_image_remote_instance._LeaseOxygenAVD(avd_spec)
         self.assertEqual(reporter.status, expected_status)
 
+    def testGetDeviceInfoFromResponse(self):
+        """test GetDeviceInfoFromResponse."""
+        expect_session_id = "fake_device"
+        expect_server_url = "10.1.1.1"
+        self.assertEqual(
+            self.remote_image_remote_instance._GetDeviceInfoFromResponse(
+                ONE_LINE_LEASE_RESPONSE),
+            (expect_session_id, expect_server_url))
+
+        self.assertEqual(
+            self.remote_image_remote_instance._GetDeviceInfoFromResponse(
+                MULTIPLE_LINES_LEASE_RESPONSE),
+            (expect_session_id, expect_server_url))
+
 
     def testReplaceDeviceDataKeys(self):
         """test ReplaceDeviceDataKeys."""
diff --git a/delete/delete.py b/delete/delete.py
index 3a7faff..e1822e5 100644
--- a/delete/delete.py
+++ b/delete/delete.py
@@ -27,8 +27,9 @@
 from acloud.internal import constants
 from acloud.internal.lib import auth
 from acloud.internal.lib import cvd_compute_client_multi_stage
-from acloud.internal.lib import utils
+from acloud.internal.lib import oxygen_client
 from acloud.internal.lib import ssh as ssh_object
+from acloud.internal.lib import utils
 from acloud.list import list as list_instances
 from acloud.public import config
 from acloud.public import device_driver
@@ -287,6 +288,37 @@
     return delete_report
 
 
+def _ReleaseOxygenDevice(cfg, instances, ip):
+    """ Release one Oxygen device.
+
+    Args:
+        cfg: AcloudConfig object.
+        instances: List of instance name.
+        ip: String of device ip.
+
+    Returns:
+        A Report instance.
+    """
+    if len(instances) != 1:
+        raise errors.CommandArgError(
+            "The release device function doesn't support multiple instances. "
+            "Please check the specified instance names: %s" % instances)
+    instance_name = instances[0]
+    delete_report = report.Report(command="delete")
+    try:
+        oxygen_client.OxygenClient.ReleaseDevice(instance_name, ip,
+                                                 cfg.oxygen_client)
+        delete_report.SetStatus(report.Status.SUCCESS)
+        device_driver.AddDeletionResultToReport(
+            delete_report, [instance_name], failed=[],
+            error_msgs=[],
+            resource_name="instance")
+    except subprocess.CalledProcessError as e:
+        delete_report.AddError(str(e))
+        delete_report.SetStatus(report.Status.FAIL)
+    return delete_report
+
+
 def Run(args):
     """Run delete.
 
@@ -302,6 +334,8 @@
     # Prioritize delete instances by names without query all instance info from
     # GCP project.
     cfg = config.GetAcloudConfig(args)
+    if args.oxygen:
+        return _ReleaseOxygenDevice(cfg, args.instance_names, args.ip)
     if args.instance_names:
         return DeleteInstanceByNames(cfg,
                                      args.instance_names)
diff --git a/delete/delete_args.py b/delete/delete_args.py
index 867194f..3b34dcb 100644
--- a/delete/delete_args.py
+++ b/delete/delete_args.py
@@ -86,6 +86,19 @@
         help="'remote host only' Provide host ssh private key path for logging "
         "in to the host. For exmaple: '--host-ssh-private-key-path "
         "~/.ssh/acloud_rsa'")
+    delete_parser.add_argument(
+        "--oxygen",
+        action="store_true",
+        dest="oxygen",
+        required=False,
+        help=argparse.SUPPRESS)
+    # The ip means server_url in Oxygen proxy api.
+    delete_parser.add_argument(
+        "--ip",
+        type=str,
+        dest="ip",
+        required=False,
+        help=argparse.SUPPRESS)
 
     # TODO(b/118439885): Old arg formats to support transition, delete when
     # transistion is done.
diff --git a/delete/delete_test.py b/delete/delete_test.py
index 1d7f97f..5d36f27 100644
--- a/delete/delete_test.py
+++ b/delete/delete_test.py
@@ -18,8 +18,10 @@
 
 from unittest import mock
 
+from acloud import errors
 from acloud.delete import delete
 from acloud.internal.lib import driver_test_lib
+from acloud.internal.lib import oxygen_client
 from acloud.list import list as list_instances
 from acloud.public import report
 
@@ -174,6 +176,21 @@
         delete.DeleteInstanceByNames(cfg, instances)
         mock_delete_remote_ins.assert_called()
 
+    @mock.patch.object(oxygen_client.OxygenClient, "ReleaseDevice")
+    def testReleaseOxygenDevice(self, mock_release):
+        """test ReleaseOxygenDevice"""
+        cfg = mock.Mock()
+        cfg.oxygen_client = "oxygen_client"
+        ip = "0.0.0.0"
+        # Raise exception for multiple instances
+        instances = ["local-instance-1", "local-instance-2"]
+        self.assertRaises(errors.CommandArgError, delete._ReleaseOxygenDevice, cfg, instances, ip)
+
+        # Test release device with oxygen client
+        instances = ["local-instance-1"]
+        delete._ReleaseOxygenDevice(cfg, instances, ip)
+        mock_release.assert_called_once()
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/internal/lib/oxygen_client.py b/internal/lib/oxygen_client.py
new file mode 100644
index 0000000..6bb7197
--- /dev/null
+++ b/internal/lib/oxygen_client.py
@@ -0,0 +1,59 @@
+# Copyright 2021 - 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.
+
+"""A client that talks to Oxygen proxy APIs."""
+
+import logging
+import subprocess
+
+
+logger = logging.getLogger(__name__)
+
+
+class OxygenClient():
+    """Client that manages Oxygen proxy api."""
+
+    @staticmethod
+    def LeaseDevice(build_target, build_id, oxygen_client):
+        """Lease one cuttlefish device.
+
+        Args:
+            build_target: Target name, e.g. "aosp_cf_x86_64_phone-userdebug"
+            build_id: Build ID, a string, e.g. "2263051", "P2804227"
+            oxygen_client: String of oxygen client path.
+
+        Returns:
+            The response of calling oxygen proxy client.
+        """
+        response = subprocess.check_output([
+            oxygen_client, "-lease", "-build_id", build_id, "-build_target",
+            build_target
+        ])
+        logger.debug("The response from oxygen client: %s", response)
+        return response
+
+    @staticmethod
+    def ReleaseDevice(session_id, server_url, oxygen_client):
+        """Release one cuttlefish device.
+
+        Args:
+            session_id: String of session id.
+            server_url: String of server url.
+            oxygen_client: String of oxygen client path.
+        """
+        response = subprocess.check_output([
+            oxygen_client, "-release", "-session_id", session_id, "-server_url",
+            server_url
+        ])
+        logger.debug("The response from oxygen client: %s", response)
diff --git a/internal/proto/user_config.proto b/internal/proto/user_config.proto
index 0dec771..1cd80d6 100755
--- a/internal/proto/user_config.proto
+++ b/internal/proto/user_config.proto
@@ -118,4 +118,7 @@
 
   // [Oxygen only] The API service url.
   optional string api_url = 33;
+
+  // [Oxygen only] The client to call oxygen api.
+  optional string oxygen_client = 34;
 }
diff --git a/public/avd.py b/public/avd.py
index 4ac1bb3..ef4712e 100755
--- a/public/avd.py
+++ b/public/avd.py
@@ -35,7 +35,7 @@
 logger = logging.getLogger(__name__)
 
 
-class AndroidVirtualDevice(object):
+class AndroidVirtualDevice():
     """Represent an Android device."""
 
     def __init__(self, instance_name, ip=None, time_info=None, stage=None):
diff --git a/public/config.py b/public/config.py
index 2c8b2f8..cc764cd 100755
--- a/public/config.py
+++ b/public/config.py
@@ -162,38 +162,26 @@
         self.ssh_private_key_path = usr_cfg.ssh_private_key_path
         self.ssh_public_key_path = usr_cfg.ssh_public_key_path
         self.storage_bucket_name = usr_cfg.storage_bucket_name
-        self.metadata_variable = {
-            key: val for key, val in
-            six.iteritems(internal_cfg.default_usr_cfg.metadata_variable)
-        }
+        self.metadata_variable = dict(
+            six.iteritems(internal_cfg.default_usr_cfg.metadata_variable))
         self.metadata_variable.update(usr_cfg.metadata_variable)
 
-        self.device_resolution_map = {
-            device: resolution for device, resolution in
-            six.iteritems(internal_cfg.device_resolution_map)
-        }
-        self.device_default_orientation_map = {
-            device: orientation for device, orientation in
-            six.iteritems(internal_cfg.device_default_orientation_map)
-        }
-        self.no_project_access_msg_map = {
-            project: msg for project, msg in
-            six.iteritems(internal_cfg.no_project_access_msg_map)
-        }
+        self.device_resolution_map = dict(
+            six.iteritems(internal_cfg.device_resolution_map))
+        self.device_default_orientation_map = dict(
+            six.iteritems(internal_cfg.device_default_orientation_map))
+        self.no_project_access_msg_map = dict(
+            six.iteritems(internal_cfg.no_project_access_msg_map))
         self.min_machine_size = internal_cfg.min_machine_size
         self.disk_image_name = internal_cfg.disk_image_name
         self.disk_image_mime_type = internal_cfg.disk_image_mime_type
         self.disk_image_extension = internal_cfg.disk_image_extension
         self.disk_raw_image_name = internal_cfg.disk_raw_image_name
         self.disk_raw_image_extension = internal_cfg.disk_raw_image_extension
-        self.valid_branch_and_min_build_id = {
-            branch: min_build_id for branch, min_build_id in
-            six.iteritems(internal_cfg.valid_branch_and_min_build_id)
-        }
-        self.precreated_data_image_map = {
-            size_gb: image_name for size_gb, image_name in
-            six.iteritems(internal_cfg.precreated_data_image)
-        }
+        self.valid_branch_and_min_build_id = dict(
+            six.iteritems(internal_cfg.valid_branch_and_min_build_id))
+        self.precreated_data_image_map = dict(
+            six.iteritems(internal_cfg.precreated_data_image))
         self.extra_data_disk_size_gb = (
             usr_cfg.extra_data_disk_size_gb or
             internal_cfg.default_usr_cfg.extra_data_disk_size_gb)
@@ -250,6 +238,7 @@
         self.launch_args = usr_cfg.launch_args
         self.api_key = usr_cfg.api_key
         self.api_url = usr_cfg.api_url
+        self.oxygen_client = usr_cfg.oxygen_client
         self.instance_name_pattern = (
             usr_cfg.instance_name_pattern or
             internal_cfg.default_usr_cfg.instance_name_pattern)
diff --git a/public/device_driver.py b/public/device_driver.py
index 192386c..73497dc 100755
--- a/public/device_driver.py
+++ b/public/device_driver.py
@@ -52,7 +52,7 @@
 
 
 # pylint: disable=invalid-name
-class AndroidVirtualDevicePool(object):
+class AndroidVirtualDevicePool():
     """A class that manages a pool of devices."""
 
     def __init__(self, cfg, devices=None):
diff --git a/public/device_driver_test.py b/public/device_driver_test.py
index e3c44f2..04ace4e 100644
--- a/public/device_driver_test.py
+++ b/public/device_driver_test.py
@@ -57,7 +57,7 @@
 
     def setUp(self):
         """Set up the test."""
-        super(DeviceDriverTest, self).setUp()
+        super().setUp()
         self.build_client = mock.MagicMock()
         self.Patch(android_build_client, "AndroidBuildClient",
                    return_value=self.build_client)
diff --git a/setup/base_task_runner.py b/setup/base_task_runner.py
index ded7161..7a2beeb 100644
--- a/setup/base_task_runner.py
+++ b/setup/base_task_runner.py
@@ -31,7 +31,7 @@
 _PARAGRAPH_BREAK = "="
 
 
-class BaseTaskRunner(object):
+class BaseTaskRunner():
     """A basic task runner class for setup cmd."""
 
     # WELCOME_MESSAGE and WELCOME_MESSAGE_TITLE should both be defined as
diff --git a/setup/gcp_setup_runner.py b/setup/gcp_setup_runner.py
index 26c1d06..e63c4f7 100644
--- a/setup/gcp_setup_runner.py
+++ b/setup/gcp_setup_runner.py
@@ -140,7 +140,7 @@
     return False
 
 
-class GoogleSDKBins(object):
+class GoogleSDKBins():
     """Class to run tools in the Google SDK."""
 
     def __init__(self, google_sdk_folder):
@@ -184,7 +184,7 @@
                                  env=self._env, **kwargs)
 
 
-class GoogleAPIService(object):
+class GoogleAPIService():
     """Class to enable api service in the gcp project."""
 
     def __init__(self, service_name, error_msg, required=False):
@@ -360,7 +360,7 @@
         if project_changed:
             logger.info("Your project changed. Start to run setup process.")
             return True
-        elif not self.client_id or not self.client_secret:
+        if not self.client_id or not self.client_secret:
             logger.info("Client ID or client secret is empty. Start to run setup process.")
             return True
         logger.info("Project was unchanged and client ID didn't need to changed.")
diff --git a/setup/google_sdk.py b/setup/google_sdk.py
index 1343a3d..e216228 100644
--- a/setup/google_sdk.py
+++ b/setup/google_sdk.py
@@ -77,16 +77,16 @@
     if is_64bits:
         if os_type == LINUX:
             return LINUX_GCP_SDK_64_URL
-        elif os_type == MAC:
+        if os_type == MAC:
             return MAC_GCP_SDK_64_URL
-        elif os_type == WIN:
+        if os_type == WIN:
             return WIN_GCP_SDK_64_URL
     else:
         if os_type == LINUX:
             return LINUX_GCP_SDK_32_URL
-        elif os_type == MAC:
+        if os_type == MAC:
             return MAC_GCP_SDK_32_URL
-        elif os_type == WIN:
+        if os_type == WIN:
             return WIN_GCP_SDK_32_URL
     raise errors.OSTypeError("no gcloud for os type: %s" % (os_type))
 
@@ -105,7 +105,7 @@
     return False
 
 
-class GoogleSDK(object):
+class GoogleSDK():
     """Google SDK tools installer."""
 
     def __init__(self):
@@ -155,7 +155,7 @@
         builtin_gcloud = utils.FindExecutable(GCLOUD_BIN)
         if builtin_gcloud:
             return os.path.dirname(builtin_gcloud)
-        elif os.path.exists(self._tmp_sdk_path):
+        if os.path.exists(self._tmp_sdk_path):
             return self._tmp_sdk_path
         raise errors.NoGoogleSDKDetected("no sdk path.")
 
diff --git a/setup/host_setup_runner_test.py b/setup/host_setup_runner_test.py
index 9ab1778..3123562 100644
--- a/setup/host_setup_runner_test.py
+++ b/setup/host_setup_runner_test.py
@@ -43,7 +43,7 @@
     # pylint: disable=invalid-name
     def setUp(self):
         """Set up the test."""
-        super(CuttlefishHostSetupTest, self).setUp()
+        super().setUp()
         self.CuttlefishHostSetup = CuttlefishHostSetup()
 
     def testShouldRunFalse(self):
@@ -86,7 +86,7 @@
     # pylint: disable=invalid-name
     def setUp(self):
         """Set up the test."""
-        super(AvdPkgInstallerTest, self).setUp()
+        super().setUp()
         self.AvdPkgInstaller = AvdPkgInstaller()
 
     def testShouldNotRun(self):
@@ -101,7 +101,7 @@
     # pylint: disable=invalid-name
     def setUp(self):
         """Set up the test."""
-        super(CuttlefishCommonPkgInstallerTest, self).setUp()
+        super().setUp()
         self.CuttlefishCommonPkgInstaller = CuttlefishCommonPkgInstaller()
 
     def testShouldRun(self):
@@ -128,7 +128,7 @@
     # pylint: disable=invalid-name
     def setUp(self):
         """Set up the test."""
-        super(MkcertPkgInstallerTest, self).setUp()
+        super().setUp()
         self.MkcertPkgInstaller = MkcertPkgInstaller()
 
     def testShouldRun(self):