diff --git a/.pylintrc b/.pylintrc index 2755ce32..72a9285a 100644 --- a/.pylintrc +++ b/.pylintrc @@ -16,5 +16,4 @@ jobs=4 disable=R0902, R0903, R0913, W0221, C0103, C0122, C0301, R0201, C0209, R0801 [TYPECHECK] -# Ignore the googleapiclient module to avoid no-member checks -ignored-modules=googleapiclient.discovery +# Add typechecking rules here \ No newline at end of file diff --git a/VERSION b/VERSION index 14d701a0..0f06a8a9 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1!9.2.0 +1!9.3.0 diff --git a/pycloudlib/gce/cloud.py b/pycloudlib/gce/cloud.py index 7b44089c..9a2ec035 100644 --- a/pycloudlib/gce/cloud.py +++ b/pycloudlib/gce/cloud.py @@ -12,7 +12,9 @@ from itertools import count from typing import Any, MutableMapping, Optional -import googleapiclient.discovery +from google.api_core.exceptions import GoogleAPICallError +from google.api_core.extended_operation import ExtendedOperation +from google.cloud import compute_v1 from pycloudlib.cloud import BaseCloud, ImageType from pycloudlib.config import ConfigFile @@ -25,7 +27,7 @@ from pycloudlib.gce.util import get_credentials, raise_on_error from pycloudlib.util import UBUNTU_RELEASE_VERSION_MAP, subp -logging.getLogger("googleapiclient.discovery").setLevel(logging.WARNING) +logging.getLogger("google.cloud").setLevel(logging.WARNING) class GCE(BaseCloud): @@ -104,13 +106,16 @@ def __init__( raise CloudSetupError(exception_text) project = result.stdout - # disable cache_discovery due to: - # https://github.com/google/google-api-python-client/issues/299 - self.compute = googleapiclient.discovery.build( - "compute", - "v1", - cache_discovery=False, - credentials=credentials, + self._images_client = compute_v1.ImagesClient(credentials=credentials) + self._disks_client = compute_v1.DisksClient(credentials=credentials) + self._instances_client = compute_v1.InstancesClient( + credentials=credentials + ) + self._zone_operations_client = compute_v1.ZoneOperationsClient( + credentials=credentials + ) + self._global_operations_client = compute_v1.GlobalOperationsClient( + credentials=credentials ) region = region or self.config.get("region") or "us-west2" zone = zone or self.config.get("zone") or "a" @@ -214,19 +219,23 @@ def _query_image_list( page_token = "" reqs = 0 while page_token is not None: - image_list_result = ( - self.compute.images() - .list( + try: + image_list_request = compute_v1.ListImagesRequest( project=project, filter=filter_string, - maxResults=500, - pageToken=page_token, + max_results=500, + page_token=page_token, ) - .execute() - ) + image_list_result = self._images_client.list( + image_list_request + ) + except GoogleAPICallError as e: + raise_on_error(e) reqs += 1 - image_list += image_list_result.get("items", []) - page_token = image_list_result.get("nextPageToken", None) + image_list += image_list_result.items + page_token = image_list_result.next_page_token + if page_token == "": + break self._log.debug( ( @@ -284,13 +293,13 @@ def daily_image( image_type=image_type.value, arch=arch, release=release ) - image = sorted(image_list, key=lambda x: x["creationTimestamp"])[-1] + image = sorted(image_list, key=lambda x: x.creation_timestamp)[-1] self._log.debug( 'Found image name "%s" for arch "%s"', - image["name"], + image.name, arch, ) - return "projects/{}/global/images/{}".format(project, image["id"]) + return "projects/{}/global/images/{}".format(project, image.id) def image_serial(self, image_id): """Find the image serial of the latest daily image for a particular release. @@ -311,25 +320,27 @@ def delete_image(self, image_id, **kwargs): image_id: string, id of the image to delete """ try: - api_image_id = ( - self.compute.images() - .get(project=self.project, image=os.path.basename(image_id)) - .execute()["id"] + images_get_request = compute_v1.GetImageRequest( + project=self.project, + image=os.path.basename(image_id), ) - except googleapiclient.errors.HttpError as e: + api_image_id = str(self._images_client.get(images_get_request).id) + except GoogleAPICallError as e: if "was not found" not in str(e): raise return - response = ( - self.compute.images() - .delete( + + try: + delete_image_request = compute_v1.DeleteImageRequest( project=self.project, image=api_image_id, ) - .execute() - ) - - raise_on_error(response) + operation: ExtendedOperation = self._images_client.delete( + delete_image_request + ) + raise_on_error(operation) + except GoogleAPICallError as e: + raise_on_error(e) def get_instance( self, @@ -386,22 +397,22 @@ def launch( instance_name = "i{}-{}".format(next(self.instance_counter), self.tag) config: MutableMapping[str, Any] = { "name": instance_name, - "machineType": "zones/%s/machineTypes/%s" + "machine_type": "zones/%s/machineTypes/%s" % (self.zone, instance_type), "disks": [ { "boot": True, - "autoDelete": True, - "initializeParams": { - "sourceImage": image_id, + "auto_delete": True, + "initialize_params": { + "source_image": image_id, }, } ], - "networkInterfaces": [ + "network_interfaces": [ { "network": "global/networks/default", - "accessConfigs": [ - {"type": "ONE_TO_ONE_NAT", "name": "External NAT"} + "access_configs": [ + {"type_": "ONE_TO_ONE_NAT", "name": "External NAT"} ], } ], @@ -425,26 +436,31 @@ def launch( user_metadata = {"key": "user-data", "value": user_data} config["metadata"]["items"].append(user_metadata) - operation = ( - self.compute.instances() - .insert(project=self.project, zone=self.zone, body=config) - .execute() - ) - raise_on_error(operation) + try: + insert_instance_request = compute_v1.InsertInstanceRequest( + project=self.project, + zone=self.zone, + instance_resource=config, + ) + operation: ExtendedOperation = self._instances_client.insert( + insert_instance_request + ) + raise_on_error(operation) + except GoogleAPICallError as e: + raise_on_error(e) - result = ( - self.compute.instances() - .get( + try: + instance_get_request = compute_v1.GetInstanceRequest( project=self.project, zone=self.zone, instance=instance_name, ) - .execute() - ) - raise_on_error(result) + result = self._instances_client.get(instance_get_request) + except GoogleAPICallError as e: + raise_on_error(e) instance = self.get_instance( - result["id"], name=result["name"], username=username + result.id, name=result.name, username=username ) self.created_instances.append(instance) return instance @@ -459,14 +475,17 @@ def snapshot(self, instance: GceInstance, clean=True, **kwargs): Returns: An image id """ - response = ( - self.compute.disks() - .list(project=self.project, zone=self.zone) - .execute() - ) + try: + list_disks_request = compute_v1.ListDisksRequest( + project=self.project, + zone=self.zone, + ) + response = self._disks_client.list(list_disks_request) + except GoogleAPICallError as e: + raise_on_error(e) instance_disks = [ - disk for disk in response["items"] if disk["name"] == instance.name + disk for disk in response.items if disk.name == instance.name ] if len(instance_disks) > 1: @@ -477,18 +496,21 @@ def snapshot(self, instance: GceInstance, clean=True, **kwargs): instance.shutdown() snapshot_name = "{}-image".format(instance.name) - operation = ( - self.compute.images() - .insert( + try: + image_resource = compute_v1.Image( + name=snapshot_name, + source_disk=instance_disks[0].self_link, + ) + insert_image_request = compute_v1.InsertImageRequest( project=self.project, - body={ - "name": snapshot_name, - "sourceDisk": instance_disks[0]["selfLink"], - }, + image_resource=image_resource, ) - .execute() - ) - raise_on_error(operation) + operation: ExtendedOperation = self._images_client.insert( + insert_image_request + ) + raise_on_error(operation) + except GoogleAPICallError as e: + raise_on_error(e) self._wait_for_operation(operation) image_id = "projects/{}/global/images/{}".format( @@ -500,34 +522,29 @@ def snapshot(self, instance: GceInstance, clean=True, **kwargs): def _wait_for_operation( self, operation, operation_type="global", sleep_seconds=300 ): - response = None - kwargs = {"project": self.project, "operation": operation["name"]} if operation_type == "zone": - kwargs["zone"] = self.zone - api = self.compute.zoneOperations() + api = self._zone_operations_client + request = compute_v1.GetZoneOperationRequest( + project=self.project, zone=self.zone, operation=operation.name + ) else: - api = self.compute.globalOperations() + api = self._global_operations_client + request = compute_v1.GetGlobalOperationRequest( + project=self.project, operation=operation.name + ) for _ in range(sleep_seconds): try: - response = api.get(**kwargs).execute() - except ConnectionResetError: - # This exception is known to be raised by GCE every so often: - # https://github.com/canonical/pycloudlib/issues/101. - response = { - "status": "ConnectionResetError", - "statusMessage": "n/a", - } + response = api.get(request) + except GoogleAPICallError as e: + raise_on_error(e) else: - if response["status"] == "DONE": + if str(response.status) == "Status.DONE": break time.sleep(1) else: raise PycloudlibError( "Expected DONE state, but found {} after waiting {} seconds. " - "Check GCE console for more details. \n" - "Status message: {}".format( - response["status"], - sleep_seconds, - response["statusMessage"], + "Check GCE console for more details. \n".format( + response.status, sleep_seconds ) ) diff --git a/pycloudlib/gce/instance.py b/pycloudlib/gce/instance.py index 58f0eeeb..562ed6d5 100644 --- a/pycloudlib/gce/instance.py +++ b/pycloudlib/gce/instance.py @@ -4,8 +4,10 @@ from time import sleep from typing import List, Optional -import googleapiclient.discovery -from googleapiclient.errors import HttpError +from google.api_core.exceptions import GoogleAPICallError, NotFound +from google.api_core.extended_operation import ExtendedOperation +from google.cloud import compute_v1 +from google.cloud.compute_v1.types import Instance from pycloudlib.errors import PycloudlibTimeoutError from pycloudlib.gce.util import get_credentials, raise_on_error @@ -50,9 +52,9 @@ def __init__( self.zone = zone self._ip = None credentials = get_credentials(credentials_path) - self.instance = googleapiclient.discovery.build( - "compute", "v1", cache_discovery=False, credentials=credentials - ).instances() + self._instances_client = compute_v1.InstancesClient( + credentials=credentials + ) def __repr__(self): """Create string representation of class.""" @@ -70,12 +72,16 @@ def id(self): def name(self): """Return the instance name.""" if not self._name: - result = self.instance.get( - project=self.project, - zone=self.zone, - instance=self.instance_id, - ).execute() - self._name = result["name"] + try: + get_instance_request = compute_v1.GetInstanceRequest( + project=self.project, + zone=self.zone, + instance=str(self.instance_id), + ) + result = self._instances_client.get(get_instance_request) + self._name = result.name + except GoogleAPICallError as e: + raise_on_error(e) return self._name @property @@ -86,12 +92,16 @@ def ip(self): return self._ip def _get_ip(self): - result = self.instance.get( - project=self.project, - zone=self.zone, - instance=self.instance_id, - ).execute() - ip = result["networkInterfaces"][0]["accessConfigs"][0]["natIP"] + try: + get_instance_request = compute_v1.GetInstanceRequest( + project=self.project, + zone=self.zone, + instance=str(self.instance_id), + ) + result = self._instances_client.get(get_instance_request) + ip = result.network_interfaces[0].access_configs[0].nat_i_p + except GoogleAPICallError as e: + raise_on_error(e) return ip # pylint: disable=broad-except @@ -104,9 +114,14 @@ def delete(self, wait=True) -> List[Exception]: if not self.instance_id: return [] try: - response = self.instance.delete( - project=self.project, zone=self.zone, instance=self.instance_id - ).execute() + delete_instance_request = compute_v1.DeleteInstanceRequest( + project=self.project, + zone=self.zone, + instance=str(self.instance_id), + ) + response: ExtendedOperation = self._instances_client.delete( + delete_instance_request + ) raise_on_error(response) if wait: self.wait_for_delete() @@ -127,10 +142,18 @@ def shutdown(self, wait=True, **kwargs): Args: wait: wait for the instance to shutdown """ - response = self.instance.stop( - project=self.project, zone=self.zone, instance=self.instance_id - ).execute() - raise_on_error(response) + try: + stop_instance_request = compute_v1.StopInstanceRequest( + project=self.project, + zone=self.zone, + instance=str(self.instance_id), + ) + operation: ExtendedOperation = self._instances_client.stop( + stop_instance_request + ) + raise_on_error(operation) + except GoogleAPICallError as e: + raise_on_error(e) if wait: self.wait_for_stop() @@ -140,10 +163,18 @@ def start(self, wait=True): Args: wait: wait for the instance to start. """ - response = self.instance.start( - project=self.project, zone=self.zone, instance=self.instance_id - ).execute() - raise_on_error(response) + try: + start_instance_request = compute_v1.StartInstanceRequest( + project=self.project, + zone=self.zone, + instance=str(self.instance_id), + ) + operation: ExtendedOperation = self._instances_client.start( + start_instance_request + ) + raise_on_error(operation) + except GoogleAPICallError as e: + raise_on_error(e) if wait: self.wait() @@ -154,24 +185,26 @@ def _wait_for_instance_start(self, **kwargs): def wait_for_delete(self, sleep_seconds=30, raise_on_fail=False): """Wait for instance to be deleted.""" + get_instance_request = compute_v1.GetInstanceRequest( + project=self.project, + zone=self.zone, + instance=str(self.instance_id), + ) for _ in range(sleep_seconds): try: - result = self.instance.get( - project=self.project, - zone=self.zone, - instance=self.instance_id, - ).execute() - if result["status"] == "TERMINATED": + response = self._instances_client.get(get_instance_request) + if response.status == "TERMINATED": break - except HttpError as e: + except NotFound: # Sometimes URL just 404s once deleted - if e.resp.status == 404: - break - raise e + break + except GoogleAPICallError as e: + raise_on_error(e) + sleep(1) else: msg = ( - f"Instance not terminated after {sleep_seconds} seconds. " - "Check GCE console." + f"Instance {self.instance_id} still exists after waiting " + f"{sleep_seconds} seconds. Check GCE console for more details." ) if raise_on_fail: raise PycloudlibTimeoutError(msg) @@ -182,17 +215,23 @@ def wait_for_stop(self, **kwargs): self._wait_for_status("TERMINATED") def _wait_for_status(self, status, sleep_seconds=300): - response = {"status": None} + response: Instance = Instance(status=None) + get_instance_request = compute_v1.GetInstanceRequest( + project=self.project, + zone=self.zone, + instance=str(self.instance_id), + ) for _ in range(sleep_seconds): - response = self.instance.get( - project=self.project, zone=self.zone, instance=self.instance_id - ).execute() - if response["status"] == status: - break + try: + response = self._instances_client.get(get_instance_request) + if response.status == status: + break + except GoogleAPICallError as e: + raise_on_error(e) sleep(1) else: raise PycloudlibTimeoutError( - f"Expected {status} state, but found {response['status']} " + f"Expected {status} state, but found {response.status} " f"after waiting {sleep_seconds} seconds. " "Check GCE console for more details." ) diff --git a/pycloudlib/gce/util.py b/pycloudlib/gce/util.py index 0fcc3036..a26a3ac3 100644 --- a/pycloudlib/gce/util.py +++ b/pycloudlib/gce/util.py @@ -1,9 +1,10 @@ """Common GCE utils.""" import os -from urllib.error import HTTPError import google.auth +from google.api_core.exceptions import GoogleAPICallError +from google.api_core.extended_operation import ExtendedOperation from google.oauth2 import service_account from pycloudlib.gce.errors import GceException @@ -11,20 +12,17 @@ def raise_on_error(response): """Look for errors in response and raise if found.""" - if "httpErrorStatusCode" in response: - raise HTTPError( - url=response["selfLink"], - code=response["httpErrorStatusCode"], - msg=response["httpErrorMessage"], - hdrs={}, - fp=None, - ) - if "error" in response: + if isinstance(response, GoogleAPICallError): raise GceException( - "Received error(s)!\n" "Errors: {}".format( - response["error"]["errors"] - ) + "Received error(s)!\n" "Errors: {}".format(response.error_message) ) + if isinstance(response, ExtendedOperation): + if response.error_code != 0: + raise GceException( + "Received error(s)!\n" "Errors: {}".format( + response.error_message + ) + ) def get_credentials(credentials_path): diff --git a/pyproject.toml b/pyproject.toml index e7d97161..b357fec2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,7 +15,6 @@ module = [ "boto3", "botocore.*", "google.*", - "googleapiclient.*", "ibm_vpc.*", "ibm_cloud_sdk_core.*", "ibm_platform_services.*", diff --git a/setup.cfg b/setup.cfg index e66d5629..9aae29e0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -30,7 +30,8 @@ install_requires = azure-mgmt-resource >= 15 boto3 >= 1.14.20 botocore >= 1.17.20 - google-api-python-client >= 1.7.7 + google-cloud-compute + googleapis-common-protos == 1.63.1 # Fixes dependency issue with google-cloud-compute and the pinned version of protobuf ibm-cloud-sdk-core == 3.14.0;python_version=='3.6' ibm-cloud-sdk-core >= 3.14.0;python_version>='3.7' ibm-platform-services diff --git a/tests/integration_tests/gce/test_launch.py b/tests/integration_tests/gce/test_launch.py index 13b4841a..49d5512b 100644 --- a/tests/integration_tests/gce/test_launch.py +++ b/tests/integration_tests/gce/test_launch.py @@ -1,5 +1,8 @@ import pytest from pycloudlib.gce.cloud import GCE +from pycloudlib.gce.util import raise_on_error +from google.cloud import compute_v1 +from google.api_core.exceptions import GoogleAPICallError @pytest.fixture @@ -13,13 +16,13 @@ def test_gce_launch_updates_config(gce_instance: GCE): daily = gce_instance.daily_image("noble", arch="x86_64") description = "Test description for kwarg verification." with gce_instance.launch(daily, description=description) as inst: - result = ( - gce_instance.compute.instances() - .get( + try: + instance_get_request = compute_v1.GetInstanceRequest( project=gce_instance.project, zone=gce_instance.zone, instance=inst.name, ) - .execute() - ) - assert result["description"] == description + result = gce_instance._instances_client.get(instance_get_request) + assert result.description == description + except GoogleAPICallError as e: + raise_on_error(e) diff --git a/tests/unit_tests/gce/test_cloud.py b/tests/unit_tests/gce/test_cloud.py index b07b7854..e3ac695b 100644 --- a/tests/unit_tests/gce/test_cloud.py +++ b/tests/unit_tests/gce/test_cloud.py @@ -7,6 +7,9 @@ from pycloudlib.gce.cloud import GCE from pycloudlib.result import Result +from google.cloud import compute_v1 +from google.cloud.compute_v1.services.images.pagers import ListPager + # mock module path MPATH = "pycloudlib.gce.cloud." @@ -26,8 +29,8 @@ def common_mocks(tmpdir): with mock.patch( MPATH + "subp", return_value=Result("my-project", "", 0) ), mock.patch( - MPATH + "googleapiclient.discovery.build", - return_value="fake_google_compute", + MPATH + "compute_v1.ImagesClient", + return_value="fake_images_client", ), mock.patch( MPATH + "get_credentials", return_value=FakeGCECredentials("service-acct@mail.com"), @@ -110,7 +113,7 @@ def test_init_config_parsing_from_environment( pytest.param( "xenial", "arm64", - Exception(), + [Exception()], [], [], id="xenial_no_arm64_support_zero_sdk_list_calls", @@ -118,41 +121,107 @@ def test_init_config_parsing_from_environment( pytest.param( "xenial", "x86_64", - [{"items": [1, 2, 3]}, Exception()], + [ + ListPager( + method=mock.MagicMock(), + request=compute_v1.ListImagesRequest( + project="project-name", + filter="name=name-filter", + max_results=500, + page_token="", + ), + response=compute_v1.ImageList( + items=[ + compute_v1.Image(name="image1"), + compute_v1.Image(name="image2"), + compute_v1.Image(name="image3"), + ] + ), + ), + Exception(), + ], [ mock.call( - project="project-name", - filter="name=name-filter", - maxResults=500, - pageToken="", + compute_v1.ListImagesRequest( + project="project-name", + filter="name=name-filter", + max_results=500, + page_token="", + ) ) ], - [1, 2, 3], + [ + compute_v1.Image(name="image1"), + compute_v1.Image(name="image2"), + compute_v1.Image(name="image3"), + ], id="xenial_x86_64_suppport_one_sdk_list_call_empty_pagetoken", ), pytest.param( "kinetic", "arm64", [ - {"items": [1, 2, 3], "nextPageToken": "something"}, - {"items": [4, 5, 6]}, + ListPager( + method=mock.MagicMock(), + request=compute_v1.ListImagesRequest( + project="project-name", + filter="(name=name-filter) AND (architecture=ARM64)", + max_results=500, + page_token="", + ), + response=compute_v1.ImageList( + items=[ + compute_v1.Image(name="image1"), + compute_v1.Image(name="image2"), + compute_v1.Image(name="image3"), + ], + next_page_token="something", + ), + ), + ListPager( + method=mock.MagicMock(), + request=compute_v1.ListImagesRequest( + project="project-name", + filter="(name=name-filter) AND (architecture=ARM64)", + max_results=500, + page_token="something", + ), + response=compute_v1.ImageList( + items=[ + compute_v1.Image(name="image4"), + compute_v1.Image(name="image5"), + compute_v1.Image(name="image6"), + ] + ), + ), Exception(), ], [ mock.call( - project="project-name", - filter="(name=name-filter) AND (architecture=ARM64)", - maxResults=500, - pageToken="", + compute_v1.ListImagesRequest( + project="project-name", + filter="(name=name-filter) AND (architecture=ARM64)", + max_results=500, + page_token="", + ) ), mock.call( - project="project-name", - filter="(name=name-filter) AND (architecture=ARM64)", - maxResults=500, - pageToken="something", + compute_v1.ListImagesRequest( + project="project-name", + filter="(name=name-filter) AND (architecture=ARM64)", + max_results=500, + page_token="something", + ) ), ], - [1, 2, 3, 4, 5, 6], + [ + compute_v1.Image(name="image1"), + compute_v1.Image(name="image2"), + compute_v1.Image(name="image3"), + compute_v1.Image(name="image4"), + compute_v1.Image(name="image5"), + compute_v1.Image(name="image6"), + ], id="non_xenial_arm64_suppport_one_sdk_list_call_per_page", ), ], @@ -166,48 +235,35 @@ def test_query_image_list( # noqa: D102 expected_image_list, gce, ): - with mock.patch.object(gce, "compute") as m_compute: - m_execute = mock.MagicMock( - name="m_execute", side_effect=api_side_effects - ) - m_executor = mock.MagicMock(name="m_executor") - m_executor.execute = m_execute - m_list = mock.MagicMock(name="m_list", return_value=m_executor) - m_lister = mock.MagicMock(name="m_lister") - m_lister.list = m_list - m_images = mock.MagicMock(name="m_images", return_value=m_lister) - m_compute.images = m_images + with mock.patch.object(gce, "_images_client") as m_images: + m_images.list.side_effect = api_side_effects - assert expected_image_list == gce._query_image_list( + qil = gce._query_image_list( release, "project-name", "name-filter", arch ) - assert m_list.call_args_list == expected_filter_calls + assert expected_image_list == qil + assert m_images.list.call_args_list == expected_filter_calls @pytest.mark.parametrize("gce", [{}], indirect=True) @mock.patch( MPATH + "GCE._query_image_list", - return_value=[ - { - "id": "2", - "name": "2", - "creationTimestamp": "2", - }, - { - "id": "4", - "name": "4", - "creationTimestamp": "4", - }, - { - "id": "1", - "name": "1", - "creationTimestamp": "1", - }, - { - "id": "3", - "name": "3", - "creationTimestamp": "3", - }, - ], + return_value=ListPager( + method=mock.MagicMock(), + request=compute_v1.ListImagesRequest( + project="project-name", + filter="name=name-filter", + max_results=500, + page_token="", + ), + response=compute_v1.ImageList( + items=[ + compute_v1.Image(id="2", name="2", creation_timestamp="2"), + compute_v1.Image(id="4", name="4", creation_timestamp="4"), + compute_v1.Image(id="1", name="1", creation_timestamp="1"), + compute_v1.Image(id="3", name="3", creation_timestamp="3"), + ] + ), + ), ) @mock.patch(MPATH + "GCE._get_name_filter", return_value="name-filter") @mock.patch(MPATH + "GCE._get_project", return_value="project-name")