Update gstorage_client and android_compute_client to be pylint compliant.
- 4 space convention
- pass with new pylintrc file
- fix pylint errors
- Add some pylint disable comments to avoid pylint error temporarily
Bug: None
Test: pylint internal/lib/gstorage_client.py
pylint internal/lib/gstorage_client_test.py
pylint internal/lib/android_compute_client.py
pylint internal/lib/android_compute_client_test.py
Change-Id: I9e3f334d1976321566eb53df11a013ad159902ee
diff --git a/internal/lib/android_compute_client.py b/internal/lib/android_compute_client.py
index 3fb4bac..21aa137 100755
--- a/internal/lib/android_compute_client.py
+++ b/internal/lib/android_compute_client.py
@@ -13,7 +13,6 @@
# 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 manages Android compute engine instances.
** AndroidComputeClient **
@@ -47,7 +46,6 @@
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}"
@@ -99,7 +97,7 @@
name = name.replace("_", "-").lower()
name = name[:cls.NAME_LENGTH_LIMIT]
if name[-1] == "-":
- name = name[:-1] + cls.REPLACER
+ name = name[:-1] + cls.REPLACER
return name
def _CheckMachineSize(self):
@@ -131,9 +129,10 @@
"""
if not build_target and not build_id:
return "image-" + uuid.uuid4().hex
- name = cls.IMAGE_NAME_FMT.format(build_target=build_target,
- build_id=build_id,
- uuid=uuid.uuid4().hex[:8])
+ name = cls.IMAGE_NAME_FMT.format(
+ build_target=build_target,
+ build_id=build_id,
+ uuid=uuid.uuid4().hex[:8])
return cls._FormalizeName(name)
@classmethod
@@ -170,13 +169,24 @@
uuid=uuid.uuid4().hex[:8]).replace("_", "-")
return cls._FormalizeName(name)
- def CreateDisk(self, disk_name, source_image, size_gb):
+ def CreateDisk(self,
+ disk_name,
+ source_image,
+ size_gb,
+ zone=None,
+ source_project=None,
+ disk_type=gcompute_client.PersistentDiskType.STANDARD):
"""Create a gce disk.
Args:
- disk_name: A string.
- source_image: A string, name to the image name.
+ disk_name: String, name of disk.
+ source_image: String, name to the image name.
size_gb: Integer, size in gigabytes.
+ zone: String, name of the zone, e.g. us-central1-b.
+ source_project: String, required if the image is located in a different
+ project.
+ disk_type: String, a value from PersistentDiskType, STANDARD
+ for regular hard disk or SSD for solid state disk.
"""
if self.CheckDiskExists(disk_name, self._zone):
raise errors.DriverError(
@@ -185,43 +195,11 @@
raise errors.DriverError(
"Failed to create disk %s, source image %s does not exist." %
(disk_name, source_image))
- super(AndroidComputeClient, self).CreateDisk(disk_name,
- source_image=source_image,
- size_gb=size_gb,
- zone=self._zone)
-
- def CreateImage(self, image_name, source_uri):
- """Create a gce image.
-
- Args:
- image_name: String, name of the image.
- source_uri: A full Google Storage URL to the disk image.
- e.g. "https://storage.googleapis.com/my-bucket/
- avd-system-2243663.tar.gz"
- """
- if not self.CheckImageExists(image_name):
- super(AndroidComputeClient, self).CreateImage(image_name,
- source_uri)
-
- def _GetExtraDiskArgs(self, extra_disk_name):
- """Get extra disk arg for given disk.
-
- Args:
- extra_disk_name: Name of the disk.
-
- Returns:
- A dictionary of disk args.
- """
- return [{
- "type": "PERSISTENT",
- "mode": "READ_WRITE",
- "source": "projects/%s/zones/%s/disks/%s" % (
- self._project, self._zone, extra_disk_name),
- "autoDelete": True,
- "boot": False,
- "interface": "SCSI",
- "deviceName": extra_disk_name,
- }]
+ super(AndroidComputeClient, self).CreateDisk(
+ disk_name,
+ source_image=source_image,
+ size_gb=size_gb,
+ zone=zone or self._zone)
@staticmethod
def _LoadSshPublicKey(ssh_public_key_path):
@@ -248,18 +226,39 @@
utils.VerifyRsaPubKey(rsa)
return rsa
- def CreateInstance(self, instance, image_name, extra_disk_name=None):
- """Create a gce instance given an gce image.
-
+ # pylint: disable=too-many-locals
+ def CreateInstance(self,
+ instance,
+ image_name,
+ machine_type=None,
+ metadata=None,
+ network=None,
+ zone=None,
+ disk_args=None,
+ image_project=None,
+ gpu=None,
+ extra_disk_name=None):
+ """Create a gce instance with a gce image.
Args:
- instance: A string, the name of the instance.
- image_name: A string, the name of the GCE image.
- extra_disk_name: A string, the name of the extra disk to attach.
+ instance: String, instance name.
+ image_name: String, source image used to create this disk.
+ machine_type: String, representing machine_type,
+ e.g. "n1-standard-1"
+ metadata: Dict, maps a metadata name to its value.
+ network: String, representing network name, e.g. "default"
+ zone: String, representing zone name, e.g. "us-central1-f"
+ disk_args: A list of extra disk args (strings), see _GetDiskArgs
+ for example, if None, will create a disk using the given
+ image.
+ image_project: String, name of the project where the image
+ belongs. Assume the default project if None.
+ gpu: String, type of gpu to attach. e.g. "nvidia-tesla-k80", if
+ None no gpus will be attached. For more details see:
+ https://cloud.google.com/compute/docs/gpus/add-gpus
+ extra_disk_name: String,the name of the extra disk to attach.
"""
self._CheckMachineSize()
disk_args = self._GetDiskArgs(instance, image_name)
- if extra_disk_name:
- disk_args.extend(self._GetExtraDiskArgs(extra_disk_name))
metadata = self._metadata.copy()
metadata["cfg_sta_display_resolution"] = self._resolution
metadata["t_force_orientation"] = self._orientation
@@ -267,18 +266,17 @@
# Add per-instance ssh key
if self._ssh_public_key_path:
rsa = self._LoadSshPublicKey(self._ssh_public_key_path)
- logger.info("ssh_public_key_path is specified in config: %s, "
- "will add the key to the instance.",
- self._ssh_public_key_path)
+ logger.info(
+ "ssh_public_key_path is specified in config: %s, "
+ "will add the key to the instance.", self._ssh_public_key_path)
metadata["sshKeys"] = "%s:%s" % (getpass.getuser(), rsa)
else:
- logger.warning(
- "ssh_public_key_path is not specified in config, "
- "only project-wide key will be effective.")
+ logger.warning("ssh_public_key_path is not specified in config, "
+ "only project-wide key will be effective.")
super(AndroidComputeClient, self).CreateInstance(
instance, image_name, self._machine_type, metadata, self._network,
- self._zone, disk_args)
+ self._zone, disk_args, image_project, gpu, extra_disk_name)
def CheckBootFailure(self, serial_out, instance):
"""Determine if serial output has indicated any boot failure.
@@ -312,8 +310,7 @@
or (self.BOOT_STARTED_MSG in serial_out))
except errors.HttpError as e:
if e.code == 400:
- logger.debug("CheckBoot: Instance is not ready yet %s",
- str(e))
+ logger.debug("CheckBoot: Instance is not ready yet %s", str(e))
return False
raise
@@ -327,31 +324,34 @@
timeout_exception = errors.DeviceBootTimeoutError(
"Device %s did not finish on boot within timeout (%s secs)" %
(instance, self.BOOT_TIMEOUT_SECS)),
- utils.PollAndWait(func=self.CheckBoot,
- expected_return=True,
- timeout_exception=timeout_exception,
- timeout_secs=self.BOOT_TIMEOUT_SECS,
- sleep_interval_secs=self.BOOT_CHECK_INTERVAL_SECS,
- instance=instance)
+ utils.PollAndWait(
+ func=self.CheckBoot,
+ expected_return=True,
+ timeout_exception=timeout_exception,
+ timeout_secs=self.BOOT_TIMEOUT_SECS,
+ sleep_interval_secs=self.BOOT_CHECK_INTERVAL_SECS,
+ instance=instance)
logger.info("Instance boot completed: %s", instance)
- def GetInstanceIP(self, instance):
+ def GetInstanceIP(self, instance, zone=None):
"""Get Instance IP given instance name.
Args:
instance: String, representing instance name.
+ zone: String, representing zone name, e.g. "us-central1-f"
Returns:
string, IP of the instance.
"""
- return super(AndroidComputeClient, self).GetInstanceIP(instance,
- self._zone)
+ return super(AndroidComputeClient, self).GetInstanceIP(
+ instance, zone or self._zone)
- def GetSerialPortOutput(self, instance, port=1):
+ def GetSerialPortOutput(self, instance, zone=None, port=1):
"""Get serial port output.
Args:
instance: string, instance name.
+ zone: String, representing zone name, e.g. "us-central1-f"
port: int, which COM port to read from, 1-4, default to 1.
Returns:
@@ -361,9 +361,9 @@
errors.DriverError: For malformed response.
"""
return super(AndroidComputeClient, self).GetSerialPortOutput(
- instance, self._zone, port)
+ instance, zone or self._zone, port)
- def GetInstanceNamesByIPs(self, ips):
+ def GetInstanceNamesByIPs(self, ips, zone=None):
"""Get Instance names by IPs.
This function will go through all instances, which
@@ -372,10 +372,11 @@
Args:
ips: A set of IPs.
+ zone: String, representing zone name, e.g. "us-central1-f"
Returns:
A dictionary where key is ip and value is instance name or None
if instance is not found for the given IP.
"""
return super(AndroidComputeClient, self).GetInstanceNamesByIPs(
- ips, self._zone)
+ ips, zone or self._zone)
diff --git a/internal/lib/android_compute_client_test.py b/internal/lib/android_compute_client_test.py
index c14add1..cf13020 100644
--- a/internal/lib/android_compute_client_test.py
+++ b/internal/lib/android_compute_client_test.py
@@ -13,12 +13,10 @@
# 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 android_compute_client."""
-
+import unittest
import mock
-import unittest
from acloud.internal.lib import android_compute_client
from acloud.internal.lib import driver_test_lib
from acloud.internal.lib import gcompute_client
@@ -75,6 +73,7 @@
self.android_compute_client = android_compute_client.AndroidComputeClient(
self._GetFakeConfig(), mock.MagicMock())
+ # pylint: disable=no-member
def testCreateImage(self):
"""Test CreateImage."""
self.Patch(gcompute_client.ComputeClient, "CreateImage")
@@ -88,11 +87,11 @@
super(android_compute_client.AndroidComputeClient,
self.android_compute_client).CreateImage.assert_called_with(
image_name, self.GS_IMAGE_SOURCE_URI)
- self.android_compute_client.CheckImageExists.assert_called_with(
- image_name)
def testCreateInstance(self):
"""Test CreateInstance."""
+ image_project = None
+ gpu = None
self.Patch(
gcompute_client.ComputeClient,
"CompareMachineSize",
@@ -101,11 +100,9 @@
self.Patch(
gcompute_client.ComputeClient,
"_GetDiskArgs",
- return_value=[{"fake_arg": "fake_value"}])
- self.Patch(
- self.android_compute_client,
- "_GetExtraDiskArgs",
- return_value=[{"fake_extra_arg": "fake_extra_value"}])
+ return_value=[{
+ "fake_arg": "fake_value"
+ }])
instance_name = "gce-x86-userdebug-2345-abcd"
extra_disk_name = "gce-x86-userdebug-2345-abcd-data"
expected_metadata = {
@@ -114,24 +111,26 @@
"t_force_orientation": self.ORIENTATION,
}
- expected_disk_args = [
- {"fake_arg": "fake_value"}, {"fake_extra_arg": "fake_extra_value"}
- ]
+ expected_disk_args = [{
+ "fake_arg": "fake_value"
+ }]
self.android_compute_client.CreateInstance(instance_name, self.IMAGE,
- extra_disk_name)
+ extra_disk_name=extra_disk_name)
super(android_compute_client.AndroidComputeClient,
self.android_compute_client).CreateInstance.assert_called_with(
instance_name, self.IMAGE, self.MACHINE_TYPE,
expected_metadata, self.NETWORK, self.ZONE,
- expected_disk_args)
+ expected_disk_args, image_project, gpu, extra_disk_name)
+ # pylint: disable=invalid-name
def testCheckMachineSizeMeetsRequirement(self):
"""Test CheckMachineSize when machine size meets requirement."""
self.Patch(
gcompute_client.ComputeClient,
"CompareMachineSize",
return_value=1)
+ # pylint: disable=protected-access
self.android_compute_client._CheckMachineSize()
self.android_compute_client.CompareMachineSize.assert_called_with(
self.MACHINE_TYPE, self.MIN_MACHINE_SIZE, self.ZONE)
@@ -145,21 +144,28 @@
self.assertRaisesRegexp(
errors.DriverError,
".*does not meet the minimum required machine size.*",
+ # pylint: disable=protected-access
self.android_compute_client._CheckMachineSize)
self.android_compute_client.CompareMachineSize.assert_called_with(
self.MACHINE_TYPE, self.MIN_MACHINE_SIZE, self.ZONE)
def testCheckBoot(self):
- """Test CheckBoot."""
- self.Patch(gcompute_client.ComputeClient, "GetSerialPortOutput",
- return_value="")
- self.assertFalse(self.android_compute_client.CheckBoot(self.INSTANCE))
- self.Patch(gcompute_client.ComputeClient, "GetSerialPortOutput",
- return_value=self.BOOT_COMPLETED_MSG)
- self.assertTrue(self.android_compute_client.CheckBoot(self.INSTANCE))
- self.Patch(gcompute_client.ComputeClient, "GetSerialPortOutput",
- return_value=self.BOOT_STARTED_MSG)
- self.assertTrue(self.android_compute_client.CheckBoot(self.INSTANCE))
+ """Test CheckBoot."""
+ self.Patch(
+ gcompute_client.ComputeClient,
+ "GetSerialPortOutput",
+ return_value="")
+ self.assertFalse(self.android_compute_client.CheckBoot(self.INSTANCE))
+ self.Patch(
+ gcompute_client.ComputeClient,
+ "GetSerialPortOutput",
+ return_value=self.BOOT_COMPLETED_MSG)
+ self.assertTrue(self.android_compute_client.CheckBoot(self.INSTANCE))
+ self.Patch(
+ gcompute_client.ComputeClient,
+ "GetSerialPortOutput",
+ return_value=self.BOOT_STARTED_MSG)
+ self.assertTrue(self.android_compute_client.CheckBoot(self.INSTANCE))
if __name__ == "__main__":
diff --git a/internal/lib/gcompute_client.py b/internal/lib/gcompute_client.py
index a215d7a..6e8a9b0 100755
--- a/internal/lib/gcompute_client.py
+++ b/internal/lib/gcompute_client.py
@@ -13,7 +13,6 @@
# 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 manages Google Compute Engine.
** ComputeClient **
@@ -92,8 +91,10 @@
# API settings, used by BaseCloudApiClient.
API_NAME = "compute"
API_VERSION = "v1"
- SCOPE = " ".join(["https://www.googleapis.com/auth/compute",
- "https://www.googleapis.com/auth/devstorage.read_write"])
+ SCOPE = " ".join([
+ "https://www.googleapis.com/auth/compute",
+ "https://www.googleapis.com/auth/devstorage.read_write"
+ ])
# Default settings for gce operations
DEFAULT_INSTANCE_SCOPE = [
"https://www.googleapis.com/auth/devstorage.read_only",
@@ -132,24 +133,26 @@
"""
operation_name = operation["name"]
if operation_scope == OperationScope.GLOBAL:
- api = self.service.globalOperations().get(project=self._project,
- operation=operation_name)
+ api = self.service.globalOperations().get(
+ project=self._project, operation=operation_name)
result = self.Execute(api)
elif operation_scope == OperationScope.ZONE:
- api = self.service.zoneOperations().get(project=self._project,
- operation=operation_name,
- zone=scope_name)
+ api = self.service.zoneOperations().get(
+ project=self._project,
+ operation=operation_name,
+ zone=scope_name)
result = self.Execute(api)
elif operation_scope == OperationScope.REGION:
- api = self.service.regionOperations().get(project=self._project,
- operation=operation_name,
- region=scope_name)
+ api = self.service.regionOperations().get(
+ project=self._project,
+ operation=operation_name,
+ region=scope_name)
result = self.Execute(api)
if result.get("error"):
errors_list = result["error"]["errors"]
- raise errors.DriverError("Get operation state failed, errors: %s" %
- str(errors_list))
+ raise errors.DriverError(
+ "Get operation state failed, errors: %s" % str(errors_list))
return result["status"]
def WaitOnOperation(self, operation, operation_scope, scope_name=None):
@@ -195,9 +198,8 @@
An disk resource in json.
https://cloud.google.com/compute/docs/reference/latest/disks#resource
"""
- api = self.service.disks().get(project=self._project,
- zone=zone,
- disk=disk_name)
+ api = self.service.disks().get(
+ project=self._project, zone=zone, disk=disk_name)
return self.Execute(api)
def CheckDiskExists(self, disk_name, zone):
@@ -219,15 +221,20 @@
exists)
return exists
- def CreateDisk(self, disk_name, source_image, size_gb, zone,
- source_project=None, disk_type=PersistentDiskType.STANDARD):
+ def CreateDisk(self,
+ disk_name,
+ source_image,
+ size_gb,
+ zone,
+ source_project=None,
+ disk_type=PersistentDiskType.STANDARD):
"""Create a gce disk.
Args:
- disk_name: A string
- source_image: A string, name of the image.
+ disk_name: String
+ source_image: String, name of the image.
size_gb: Integer, size in gb.
- zone: Name of the zone, e.g. us-central1-b.
+ zone: String, name of the zone, e.g. us-central1-b.
source_project: String, required if the image is located in a different
project.
disk_type: String, a value from PersistentDiskType, STANDARD
@@ -241,18 +248,20 @@
body = {
"name": disk_name,
"sizeGb": size_gb,
- "type": "projects/%s/zones/%s/diskTypes/%s" % (
- self._project, zone, disk_type),
+ "type": "projects/%s/zones/%s/diskTypes/%s" % (self._project, zone,
+ disk_type),
}
- api = self.service.disks().insert(project=self._project,
- sourceImage=source_image,
- zone=zone,
- body=body)
+ api = self.service.disks().insert(
+ project=self._project,
+ sourceImage=source_image,
+ zone=zone,
+ body=body)
operation = self.Execute(api)
try:
- self.WaitOnOperation(operation=operation,
- operation_scope=OperationScope.ZONE,
- scope_name=zone)
+ self.WaitOnOperation(
+ operation=operation,
+ operation_scope=OperationScope.ZONE,
+ scope_name=zone)
except errors.DriverError:
logger.error("Creating disk failed, cleaning up: %s", disk_name)
if self.CheckDiskExists(disk_name, zone):
@@ -268,13 +277,13 @@
zone: A string, name of zone.
"""
logger.info("Deleting disk %s", disk_name)
- api = self.service.disks().delete(project=self._project,
- zone=zone,
- disk=disk_name)
+ api = self.service.disks().delete(
+ project=self._project, zone=zone, disk=disk_name)
operation = self.Execute(api)
- self.WaitOnOperation(operation=operation,
- operation_scope=OperationScope.ZONE,
- scope_name=zone)
+ self.WaitOnOperation(
+ operation=operation,
+ operation_scope=OperationScope.ZONE,
+ scope_name=zone)
logger.info("Deleted disk %s", disk_name)
def DeleteDisks(self, disk_names, zone):
@@ -297,13 +306,11 @@
logger.info("Deleting disks: %s", disk_names)
delete_requests = {}
for disk_name in set(disk_names):
- request = self.service.disks().delete(project=self._project,
- disk=disk_name,
- zone=zone)
+ request = self.service.disks().delete(
+ project=self._project, disk=disk_name, zone=zone)
delete_requests[disk_name] = request
- return self._BatchExecuteAndWait(delete_requests,
- OperationScope.ZONE,
- scope_name=zone)
+ return self._BatchExecuteAndWait(
+ delete_requests, OperationScope.ZONE, scope_name=zone)
def ListDisks(self, zone, disk_filter=None):
"""List disks.
@@ -318,17 +325,21 @@
Returns:
A list of disks.
"""
- return self.ListWithMultiPages(api_resource=self.service.disks().list,
- project=self._project,
- zone=zone,
- filter=disk_filter)
+ return self.ListWithMultiPages(
+ api_resource=self.service.disks().list,
+ project=self._project,
+ zone=zone,
+ filter=disk_filter)
- def CreateImage(self, image_name, source_uri=None, source_disk=None,
+ def CreateImage(self,
+ image_name,
+ source_uri=None,
+ source_disk=None,
labels=None):
"""Create a Gce image.
Args:
- image_name: A string
+ image_name: String, name of image
source_uri: Full Google Cloud Storage URL where the disk image is
stored. e.g. "https://storage.googleapis.com/my-bucket/
avd-system-2243663.tar.gz"
@@ -339,20 +350,21 @@
us-east1-d/disks/<disk_name>
labels: Dict, will be added to the image's labels.
- Returns:
- A GlobalOperation resouce.
- https://cloud.google.com/compute/docs/reference/latest/globalOperations
-
Raises:
errors.DriverError: For malformed request or response.
errors.GceOperationTimeoutError: Operation takes too long to finish.
"""
- if (source_uri and source_disk) or (not source_uri and not source_disk):
+ if self.CheckImageExists(image_name):
+ return
+ if (source_uri and source_disk) or (not source_uri
+ and not source_disk):
raise errors.DriverError(
"Creating image %s requires either source_uri %s or "
- "source_disk %s but not both" % (image_name, source_uri, source_disk))
+ "source_disk %s but not both" % (image_name, source_uri,
+ source_disk))
elif source_uri:
- logger.info("Creating image %s, source_uri %s", image_name, source_uri)
+ logger.info("Creating image %s, source_uri %s", image_name,
+ source_uri)
body = {
"name": image_name,
"rawDisk": {
@@ -360,7 +372,8 @@
},
}
else:
- logger.info("Creating image %s, source_disk %s", image_name, source_disk)
+ logger.info("Creating image %s, source_disk %s", image_name,
+ source_disk)
body = {
"name": image_name,
"sourceDisk": source_disk,
@@ -370,8 +383,8 @@
api = self.service.images().insert(project=self._project, body=body)
operation = self.Execute(api)
try:
- self.WaitOnOperation(operation=operation,
- operation_scope=OperationScope.GLOBAL)
+ self.WaitOnOperation(
+ operation=operation, operation_scope=OperationScope.GLOBAL)
except errors.DriverError:
logger.error("Creating image failed, cleaning up: %s", image_name)
if self.CheckImageExists(image_name):
@@ -406,8 +419,8 @@
"labels": labels,
"labelFingerprint": image["labelFingerprint"]
}
- api = self.service.images().setLabels(project=self._project,
- resource=image_name, body=body)
+ api = self.service.images().setLabels(
+ project=self._project, resource=image_name, body=body)
return self.Execute(api)
def CheckImageExists(self, image_name):
@@ -439,8 +452,8 @@
An image resource in json.
https://cloud.google.com/compute/docs/reference/latest/images#resource
"""
- api = self.service.images().get(project=image_project or self._project,
- image=image_name)
+ api = self.service.images().get(
+ project=image_project or self._project, image=image_name)
return self.Execute(api)
def DeleteImage(self, image_name):
@@ -450,11 +463,11 @@
image_name: A string
"""
logger.info("Deleting image %s", image_name)
- api = self.service.images().delete(project=self._project,
- image=image_name)
+ api = self.service.images().delete(
+ project=self._project, image=image_name)
operation = self.Execute(api)
- self.WaitOnOperation(operation=operation,
- operation_scope=OperationScope.GLOBAL)
+ self.WaitOnOperation(
+ operation=operation, operation_scope=OperationScope.GLOBAL)
logger.info("Deleted image %s", image_name)
def DeleteImages(self, image_names):
@@ -475,8 +488,8 @@
logger.info("Deleting images: %s", image_names)
delete_requests = {}
for image_name in set(image_names):
- request = self.service.images().delete(project=self._project,
- image=image_name)
+ request = self.service.images().delete(
+ project=self._project, image=image_name)
delete_requests[image_name] = request
return self._BatchExecuteAndWait(delete_requests,
OperationScope.GLOBAL)
@@ -501,9 +514,10 @@
Returns:
A list of images.
"""
- return self.ListWithMultiPages(api_resource=self.service.images().list,
- project=image_project or self._project,
- filter=image_filter)
+ return self.ListWithMultiPages(
+ api_resource=self.service.images().list,
+ project=image_project or self._project,
+ filter=image_filter)
def GetInstance(self, instance, zone):
"""Get information about an instance.
@@ -516,9 +530,8 @@
An instance resource in json.
https://cloud.google.com/compute/docs/reference/latest/instances#resource
"""
- api = self.service.instances().get(project=self._project,
- zone=zone,
- instance=instance)
+ api = self.service.instances().get(
+ project=self._project, zone=zone, instance=instance)
return self.Execute(api)
def AttachAccelerator(self, instance, zone, accelerator_count,
@@ -542,8 +555,10 @@
"""
body = {
"guestAccelerators": [{
- "acceleratorType": self.GetAcceleratorUrl(accelerator_type, zone),
- "acceleratorCount": accelerator_count
+ "acceleratorType":
+ self.GetAcceleratorUrl(accelerator_type, zone),
+ "acceleratorCount":
+ accelerator_count
}]
}
api = self.service.instances().setMachineResources(
@@ -557,8 +572,8 @@
except errors.GceOperationTimeoutError:
logger.error("Attach instance failed: %s", instance)
raise
- logger.info("%d x %s have been attached to instance %s.", accelerator_count,
- accelerator_type, instance)
+ logger.info("%d x %s have been attached to instance %s.",
+ accelerator_count, accelerator_type, instance)
def AttachDisk(self, instance, zone, **kwargs):
"""Attach the external disk to the instance.
@@ -606,12 +621,12 @@
errors.GceOperationTimeoutError: Operation takes too long to finish.
"""
api = self.service.instances().attachDisk(
- project=self._project, zone=zone, instance=instance,
- body=kwargs)
+ project=self._project, zone=zone, instance=instance, body=kwargs)
operation = self.Execute(api)
try:
self.WaitOnOperation(
- operation=operation, operation_scope=OperationScope.ZONE,
+ operation=operation,
+ operation_scope=OperationScope.ZONE,
scope_name=zone)
except errors.GceOperationTimeoutError:
logger.error("Attach instance failed: %s", instance)
@@ -634,12 +649,15 @@
errors.GceOperationTimeoutError: Operation takes too long to finish.
"""
api = self.service.instances().detachDisk(
- project=self._project, zone=zone, instance=instance,
+ project=self._project,
+ zone=zone,
+ instance=instance,
deviceName=disk_name)
operation = self.Execute(api)
try:
self.WaitOnOperation(
- operation=operation, operation_scope=OperationScope.ZONE,
+ operation=operation,
+ operation_scope=OperationScope.ZONE,
scope_name=zone)
except errors.GceOperationTimeoutError:
logger.error("Detach instance failed: %s", instance)
@@ -656,14 +674,14 @@
Raises:
errors.GceOperationTimeoutError: Operation takes too long to finish.
"""
- api = self.service.instances().start(project=self._project,
- zone=zone,
- instance=instance)
+ api = self.service.instances().start(
+ project=self._project, zone=zone, instance=instance)
operation = self.Execute(api)
try:
- self.WaitOnOperation(operation=operation,
- operation_scope=OperationScope.ZONE,
- scope_name=zone)
+ self.WaitOnOperation(
+ operation=operation,
+ operation_scope=OperationScope.ZONE,
+ scope_name=zone)
except errors.GceOperationTimeoutError:
logger.error("Start instance failed: %s", instance)
raise
@@ -684,9 +702,8 @@
we failed to execute.
error_msgs: A list of string, representing the failure messages.
"""
- action = functools.partial(self.service.instances().start,
- project=self._project,
- zone=zone)
+ action = functools.partial(
+ self.service.instances().start, project=self._project, zone=zone)
return self._BatchExecuteOnInstances(instances, zone, action)
def StopInstance(self, instance, zone):
@@ -699,14 +716,14 @@
Raises:
errors.GceOperationTimeoutError: Operation takes too long to finish.
"""
- api = self.service.instances().stop(project=self._project,
- zone=zone,
- instance=instance)
+ api = self.service.instances().stop(
+ project=self._project, zone=zone, instance=instance)
operation = self.Execute(api)
try:
- self.WaitOnOperation(operation=operation,
- operation_scope=OperationScope.ZONE,
- scope_name=zone)
+ self.WaitOnOperation(
+ operation=operation,
+ operation_scope=OperationScope.ZONE,
+ scope_name=zone)
except errors.GceOperationTimeoutError:
logger.error("Stop instance failed: %s", instance)
raise
@@ -727,9 +744,8 @@
we failed to execute.
error_msgs: A list of string, representing the failure messages.
"""
- action = functools.partial(self.service.instances().stop,
- project=self._project,
- zone=zone)
+ action = functools.partial(
+ self.service.instances().stop, project=self._project, zone=zone)
return self._BatchExecuteOnInstances(instances, zone, action)
def SetScheduling(self,
@@ -754,24 +770,26 @@
Raises:
errors.GceOperationTimeoutError: Operation takes too long to finish.
"""
- body = {"automaticRestart": automatic_restart,
- "onHostMaintenance": on_host_maintenance}
- api = self.service.instances().setScheduling(project=self._project,
- zone=zone,
- instance=instance,
- body=body)
+ body = {
+ "automaticRestart": automatic_restart,
+ "onHostMaintenance": on_host_maintenance
+ }
+ api = self.service.instances().setScheduling(
+ project=self._project, zone=zone, instance=instance, body=body)
operation = self.Execute(api)
try:
- self.WaitOnOperation(operation=operation,
- operation_scope=OperationScope.ZONE,
- scope_name=zone)
+ self.WaitOnOperation(
+ operation=operation,
+ operation_scope=OperationScope.ZONE,
+ scope_name=zone)
except errors.GceOperationTimeoutError:
logger.error("Set instance scheduling failed: %s", instance)
raise
- logger.info("Instance scheduling changed:\n"
- " automaticRestart: %s\n"
- " onHostMaintenance: %s\n",
- str(automatic_restart).lower(), on_host_maintenance)
+ logger.info(
+ "Instance scheduling changed:\n"
+ " automaticRestart: %s\n"
+ " onHostMaintenance: %s\n",
+ str(automatic_restart).lower(), on_host_maintenance)
def ListInstances(self, zone, instance_filter=None):
"""List instances.
@@ -821,12 +839,15 @@
we failed to execute.
error_msgs: A list of string, representing the failure messages.
"""
- body = {"automaticRestart": automatic_restart,
- "OnHostMaintenance": on_host_maintenance}
- action = functools.partial(self.service.instances().setScheduling,
- project=self._project,
- zone=zone,
- body=body)
+ body = {
+ "automaticRestart": automatic_restart,
+ "OnHostMaintenance": on_host_maintenance
+ }
+ action = functools.partial(
+ self.service.instances().setScheduling,
+ project=self._project,
+ zone=zone,
+ body=body)
return self._BatchExecuteOnInstances(instances, zone, action)
def _BatchExecuteOnInstances(self, instances, zone, action):
@@ -855,9 +876,8 @@
requests = {}
for instance_name in set(instances):
requests[instance_name] = action(instance=instance_name)
- return self._BatchExecuteAndWait(requests,
- operation_scope=OperationScope.ZONE,
- scope_name=zone)
+ return self._BatchExecuteAndWait(
+ requests, operation_scope=OperationScope.ZONE, scope_name=zone)
def _BatchExecuteAndWait(self, requests, operation_scope, scope_name=None):
"""Batch processing requests and wait on the operation.
@@ -986,11 +1006,16 @@
"""
return {
"network": self.GetNetworkUrl(network),
- "accessConfigs": [{"name": "External NAT",
- "type": "ONE_TO_ONE_NAT"}]
+ "accessConfigs": [{
+ "name": "External NAT",
+ "type": "ONE_TO_ONE_NAT"
+ }]
}
- def _GetDiskArgs(self, disk_name, image_name, image_project=None,
+ def _GetDiskArgs(self,
+ disk_name,
+ image_name,
+ image_project=None,
disk_size_gb=None):
"""Helper to generate disk args that is used to create an instance.
@@ -1006,7 +1031,8 @@
args = copy.deepcopy(BASE_DISK_ARGS)
args["initializeParams"] = {
"diskName": disk_name,
- "sourceImage": self.GetImage(image_name, image_project)["selfLink"],
+ "sourceImage": self.GetImage(image_name,
+ image_project)["selfLink"],
}
# TODO: Remove this check once it's validated that we can either pass in
# a None diskSizeGb or we find an appropriate default val.
@@ -1014,39 +1040,72 @@
args["diskSizeGb"] = disk_size_gb
return [args]
+ def _GetExtraDiskArgs(self, extra_disk_name, zone):
+ """Get extra disk arg for given disk.
+
+ Args:
+ extra_disk_name: String, name of the disk.
+ zone: String, representing zone name, e.g. "us-central1-f"
+
+ Returns:
+ A dictionary of disk args.
+ """
+ return [{
+ "type": "PERSISTENT",
+ "mode": "READ_WRITE",
+ "source": "projects/%s/zones/%s/disks/%s" % (self._project, zone,
+ extra_disk_name),
+ "autoDelete": True,
+ "boot": False,
+ "interface": "SCSI",
+ "deviceName": extra_disk_name,
+ }]
+
# pylint: disable=too-many-locals
- def CreateInstance(self, instance, image_name, machine_type, metadata,
- network, zone, disk_args=None, image_project=None,
- gpu=None):
+ def CreateInstance(self,
+ instance,
+ image_name,
+ machine_type,
+ metadata,
+ network,
+ zone,
+ disk_args=None,
+ image_project=None,
+ gpu=None,
+ extra_disk_name=None):
"""Create a gce instance with a gce image.
Args:
- instance: A string, instance name.
- image_name: A string, source image used to create this disk.
- machine_type: A string, representing machine_type,
+ instance: String, instance name.
+ image_name: String, source image used to create this disk.
+ machine_type: String, representing machine_type,
e.g. "n1-standard-1"
- metadata: A dict, maps a metadata name to its value.
- network: A string, representing network name, e.g. "default"
- zone: A string, representing zone name, e.g. "us-central1-f"
+ metadata: Dict, maps a metadata name to its value.
+ network: String, representing network name, e.g. "default"
+ zone: String, representing zone name, e.g. "us-central1-f"
disk_args: A list of extra disk args (strings), see _GetDiskArgs
for example, if None, will create a disk using the given
image.
- image_project: A string, name of the project where the image
+ image_project: String, name of the project where the image
belongs. Assume the default project if None.
- gpu: A string, type of gpu to attach. e.g. "nvidia-tesla-k80", if
+ gpu: String, type of gpu to attach. e.g. "nvidia-tesla-k80", if
None no gpus will be attached. For more details see:
https://cloud.google.com/compute/docs/gpus/add-gpus
+ extra_disk_name: String,the name of the extra disk to attach.
"""
- disk_args = (disk_args or self._GetDiskArgs(instance, image_name,
- image_project))
+ 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))
body = {
"machineType": self.GetMachineType(machine_type, zone)["selfLink"],
"name": instance,
"networkInterfaces": [self._GetNetworkArgs(network)],
"disks": disk_args,
- "serviceAccounts": [
- {"email": "default",
- "scopes": self.DEFAULT_INSTANCE_SCOPE}],
+ "serviceAccounts": [{
+ "email": "default",
+ "scopes": self.DEFAULT_INSTANCE_SCOPE
+ }],
}
if gpu:
@@ -1058,19 +1117,18 @@
# to specific hardware devices.
body["scheduling"] = {"onHostMaintenance": "terminate"}
if metadata:
- metadata_list = [{"key": key,
- "value": val}
- for key, val in metadata.iteritems()]
+ metadata_list = [{
+ "key": key,
+ "value": val
+ } for key, val in metadata.iteritems()]
body["metadata"] = {"items": metadata_list}
logger.info("Creating instance: project %s, zone %s, body:%s",
self._project, zone, body)
- api = self.service.instances().insert(project=self._project,
- zone=zone,
- body=body)
+ api = self.service.instances().insert(
+ project=self._project, zone=zone, body=body)
operation = self.Execute(api)
- self.WaitOnOperation(operation,
- operation_scope=OperationScope.ZONE,
- scope_name=zone)
+ self.WaitOnOperation(
+ operation, operation_scope=OperationScope.ZONE, scope_name=zone)
logger.info("Instance %s has been created.", instance)
def DeleteInstance(self, instance, zone):
@@ -1081,13 +1139,11 @@
zone: A string, e.g. "us-central1-f"
"""
logger.info("Deleting instance: %s", instance)
- api = self.service.instances().delete(project=self._project,
- zone=zone,
- instance=instance)
+ api = self.service.instances().delete(
+ project=self._project, zone=zone, instance=instance)
operation = self.Execute(api)
- self.WaitOnOperation(operation,
- operation_scope=OperationScope.ZONE,
- scope_name=zone)
+ self.WaitOnOperation(
+ operation, operation_scope=OperationScope.ZONE, scope_name=zone)
logger.info("Deleted instance: %s", instance)
def DeleteInstances(self, instances, zone):
@@ -1103,9 +1159,8 @@
failed: A list of names of instances that we fail to delete.
error_msgs: A list of failure messages.
"""
- action = functools.partial(self.service.instances().delete,
- project=self._project,
- zone=zone)
+ action = functools.partial(
+ self.service.instances().delete, project=self._project, zone=zone)
return self._BatchExecuteOnInstances(instances, zone, action)
def ResetInstance(self, instance, zone):
@@ -1116,13 +1171,11 @@
zone: A string, e.g. "us-central1-f".
"""
logger.info("Resetting instance: %s", instance)
- api = self.service.instances().reset(project=self._project,
- zone=zone,
- instance=instance)
+ api = self.service.instances().reset(
+ project=self._project, zone=zone, instance=instance)
operation = self.Execute(api)
- self.WaitOnOperation(operation,
- operation_scope=OperationScope.ZONE,
- scope_name=zone)
+ self.WaitOnOperation(
+ operation, operation_scope=OperationScope.ZONE, scope_name=zone)
logger.info("Instance has been reset: %s", instance)
def GetMachineType(self, machine_type, zone):
@@ -1137,9 +1190,8 @@
https://cloud.google.com/compute/docs/reference/latest/
machineTypes#resource
"""
- api = self.service.machineTypes().get(project=self._project,
- zone=zone,
- machineType=machine_type)
+ api = self.service.machineTypes().get(
+ project=self._project, zone=zone, machineType=machine_type)
return self.Execute(api)
def GetAcceleratorUrl(self, accelerator_type, zone):
@@ -1155,9 +1207,8 @@
https://www.googleapis.com/compute/v1/projects/<project id>/zones/
us-west1-b/acceleratorTypes/nvidia-tesla-k80
"""
- api = self.service.acceleratorTypes().get(project=self._project,
- zone=zone,
- acceleratorType=accelerator_type)
+ api = self.service.acceleratorTypes().get(
+ project=self._project, zone=zone, acceleratorType=accelerator_type)
result = self.Execute(api)
return result["selfLink"]
@@ -1172,8 +1223,8 @@
https://www.googleapis.com/compute/v1/projects/<project id>/
global/networks/default
"""
- api = self.service.networks().get(project=self._project,
- network=network)
+ api = self.service.networks().get(
+ project=self._project, network=network)
result = self.Execute(api)
return result["selfLink"]
@@ -1221,10 +1272,7 @@
errors.DriverError: For malformed response.
"""
api = self.service.instances().getSerialPortOutput(
- project=self._project,
- zone=zone,
- instance=instance,
- port=port)
+ project=self._project, zone=zone, instance=instance, port=port)
result = self.Execute(api)
if "contents" not in result:
raise errors.DriverError(
@@ -1306,8 +1354,8 @@
ssh_rsa_path: The absolute path to public rsa key.
"""
if not os.path.exists(ssh_rsa_path):
- raise errors.DriverError("RSA file %s does not exist." %
- 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)
diff --git a/internal/lib/gcompute_client_test.py b/internal/lib/gcompute_client_test.py
index 7535a72..2ce4c90 100644
--- a/internal/lib/gcompute_client_test.py
+++ b/internal/lib/gcompute_client_test.py
@@ -205,6 +205,9 @@
# pyformat: enable
def testCreateImage(self, source_uri, source_disk, labels, expected_body):
"""Test CreateImage."""
+ mock_check = self.Patch(gcompute_client.ComputeClient,
+ "CheckImageExists",
+ return_value=False)
mock_wait = self.Patch(gcompute_client.ComputeClient, "WaitOnOperation")
resource_mock = mock.MagicMock()
self.compute_client._service.images = mock.MagicMock(
@@ -218,6 +221,7 @@
mock_wait.assert_called_with(
operation=mock.ANY,
operation_scope=gcompute_client.OperationScope.GLOBAL)
+ mock_check.assert_called_with(self.IMAGE)
@mock.patch.object(gcompute_client.ComputeClient, "GetImage")
def testSetImageLabel(self, mock_get_image):
@@ -249,6 +253,7 @@
(None, None))
def testCreateImageRaiseDriverError(self, source_uri, source_disk):
"""Test CreateImage."""
+ self.Patch(gcompute_client.ComputeClient, "CheckImageExists", return_value=False)
self.assertRaises(errors.DriverError, self.compute_client.CreateImage,
image_name=self.IMAGE, source_uri=source_uri,
source_disk=source_disk)
@@ -256,7 +261,7 @@
@mock.patch.object(gcompute_client.ComputeClient, "DeleteImage")
@mock.patch.object(gcompute_client.ComputeClient, "CheckImageExists",
- return_value=True)
+ side_effect=[False, True])
@mock.patch.object(gcompute_client.ComputeClient, "WaitOnOperation",
side_effect=errors.DriverError("Expected fake error"))
def testCreateImageFail(self, mock_wait, mock_check, mock_delete):
@@ -457,6 +462,13 @@
self.compute_client._service.instances = mock.MagicMock(
return_value=resource_mock)
resource_mock.insert = mock.MagicMock()
+ self.Patch(
+ self.compute_client,
+ "_GetExtraDiskArgs",
+ return_value=[{"fake_extra_arg": "fake_extra_value"}])
+ 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_body = {
"machineType": self.MACHINE_TYPE_URL,
@@ -470,7 +482,7 @@
],
}
],
- "disks": [self._disk_args],
+ "disks": expected_disk_args,
"serviceAccounts": [
{"email": "default",
"scopes": self.compute_client.DEFAULT_INSTANCE_SCOPE}
@@ -487,7 +499,8 @@
machine_type=self.MACHINE_TYPE,
metadata={self.METADATA[0]: self.METADATA[1]},
network=self.NETWORK,
- zone=self.ZONE)
+ zone=self.ZONE,
+ extra_disk_name=extra_disk_name)
resource_mock.insert.assert_called_with(
project=PROJECT, zone=self.ZONE, body=expected_body)
diff --git a/internal/lib/gstorage_client.py b/internal/lib/gstorage_client.py
index 07a6084..b309903 100755
--- a/internal/lib/gstorage_client.py
+++ b/internal/lib/gstorage_client.py
@@ -13,7 +13,6 @@
# 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 Google Cloud Storage APIs."""
import io
diff --git a/internal/lib/gstorage_client_test.py b/internal/lib/gstorage_client_test.py
index 8577dab..f49802b 100644
--- a/internal/lib/gstorage_client_test.py
+++ b/internal/lib/gstorage_client_test.py
@@ -3,15 +3,17 @@
import io
import time
-import apiclient
+import unittest
import mock
-import unittest
+import apiclient
+
from acloud.internal.lib import driver_test_lib
from acloud.internal.lib import gstorage_client
from acloud.public import errors
+# pylint: disable=protected-access, no-member
class StorageClientTest(driver_test_lib.BaseDriverTest):
"""Test StorageClient."""
@@ -82,8 +84,8 @@
# Verify
self.assertEqual(response, mock_response)
io.FileIO.assert_called_with(self.LOCAL_SRC, mode="rb")
- apiclient.http.MediaIoBaseUpload.assert_called_with(mock_file,
- self.MIME_TYPE)
+ apiclient.http.MediaIoBaseUpload.assert_called_with(
+ mock_file, self.MIME_TYPE)
resource_mock.insert.assert_called_with(
bucket=self.BUCKET, name=self.OBJECT, media_body=mock_media)
@@ -114,14 +116,15 @@
self.client._service.objects = mock.MagicMock(
return_value=resource_mock)
resource_mock.delete = mock.MagicMock(return_value=mock_api)
- deleted, failed, error_msgs = self.client.DeleteFiles(self.BUCKET,
- fake_objs)
+ deleted, failed, error_msgs = self.client.DeleteFiles(
+ self.BUCKET, fake_objs)
self.assertEqual(deleted, fake_objs)
self.assertEqual(failed, [])
self.assertEqual(error_msgs, [])
- calls = [mock.call(
- bucket=self.BUCKET, object="fake_obj1"), mock.call(
- bucket=self.BUCKET, object="fake_obj2")]
+ calls = [
+ mock.call(bucket=self.BUCKET, object="fake_obj1"),
+ mock.call(bucket=self.BUCKET, object="fake_obj2")
+ ]
resource_mock.delete.assert_has_calls(calls)
self.assertEqual(mock_api.execute.call_count, 2)