From e76d53a2a891ea2ca4c21ffaa415b75f633b0e95 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Tue, 11 May 2021 10:44:02 +0300 Subject: [PATCH 1/4] BlackBox test fixes: improved the mechanism of locating gcp keys and improved error handling if tests can't connect to gcp --- envs/monkey_zoo/blackbox/test_blackbox.py | 8 +++- .../blackbox/utils/gcp_machine_handlers.py | 42 ++++++++++++------- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/envs/monkey_zoo/blackbox/test_blackbox.py b/envs/monkey_zoo/blackbox/test_blackbox.py index 4de60ef55..70b133468 100644 --- a/envs/monkey_zoo/blackbox/test_blackbox.py +++ b/envs/monkey_zoo/blackbox/test_blackbox.py @@ -72,8 +72,12 @@ LOGGER = logging.getLogger(__name__) @pytest.fixture(autouse=True, scope="session") def GCPHandler(request, no_gcp): if not no_gcp: - GCPHandler = gcp_machine_handlers.GCPHandler() - GCPHandler.start_machines(" ".join(GCP_TEST_MACHINE_LIST)) + try: + GCPHandler = gcp_machine_handlers.GCPHandler() + GCPHandler.start_machines(" ".join(GCP_TEST_MACHINE_LIST)) + except Exception as e: + LOGGER.error("GCP Handler failed to initialize: %s." % e) + pytest.exit("Encountered an error while starting GCP machines. Stopping the tests.") wait_machine_bootup() def fin(): diff --git a/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py b/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py index 147958fe2..95c9d1855 100644 --- a/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py +++ b/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py @@ -1,35 +1,47 @@ import logging +import os import subprocess LOGGER = logging.getLogger(__name__) class GCPHandler(object): - + # gcloud commands AUTHENTICATION_COMMAND = "gcloud auth activate-service-account --key-file=%s" SET_PROPERTY_PROJECT = "gcloud config set project %s" MACHINE_STARTING_COMMAND = "gcloud compute instances start %s --zone=%s" MACHINE_STOPPING_COMMAND = "gcloud compute instances stop %s --zone=%s" + # Default configuration parameters + DEFAULT_RELATIVE_KEY_PATH = "../../gcp_keys/gcp_key.json" + DEFAULT_ZONE = "europe-west3-a" + DEFAULT_PROJECT = "guardicore-22050661" + def __init__( self, - key_path="../gcp_keys/gcp_key.json", - zone="europe-west3-a", - project_id="guardicore-22050661", + relative_key_path=DEFAULT_RELATIVE_KEY_PATH, + zone=DEFAULT_ZONE, + project_id=DEFAULT_PROJECT, ): self.zone = zone - try: - # pass the key file to gcp - subprocess.call(GCPHandler.get_auth_command(key_path), shell=True) # noqa: DUO116 - LOGGER.info("GCP Handler passed key") - # set project - subprocess.call( # noqa: DUO116 - GCPHandler.get_set_project_command(project_id), shell=True + abs_key_path = GCPHandler.get_absolute_key_path(relative_key_path) + # pass the key file to gcp + subprocess.call(GCPHandler.get_auth_command(abs_key_path), shell=True) # noqa: DUO116 + LOGGER.info("GCP Handler passed key") + # set project + subprocess.call(GCPHandler.get_set_project_command(project_id), shell=True) # noqa: DUO116 + LOGGER.info("GCP Handler set project") + LOGGER.info("GCP Handler initialized successfully") + + @staticmethod + def get_absolute_key_path(relative_key_path: str) -> str: + file_dir = os.path.dirname(os.path.realpath(__file__)) + absolute_key_path = os.path.join(file_dir, relative_key_path) + if not os.path.isfile(absolute_key_path): + raise FileNotFoundError( + "GCP key not found. " "Add a service key to envs/monkey_zoo/gcp_keys/gcp_key.json" ) - LOGGER.info("GCP Handler set project") - LOGGER.info("GCP Handler initialized successfully") - except Exception as e: - LOGGER.error("GCP Handler failed to initialize: %s." % e) + return os.path.join(file_dir, relative_key_path) def start_machines(self, machine_list): """ From c45de9dae7840282a23670135edd6d3a1872f29e Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Wed, 12 May 2021 10:41:52 +0300 Subject: [PATCH 2/4] Improved readability of gcp_machine_handlers.py --- envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py b/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py index 95c9d1855..89018bde1 100644 --- a/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py +++ b/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py @@ -6,13 +6,12 @@ LOGGER = logging.getLogger(__name__) class GCPHandler(object): - # gcloud commands AUTHENTICATION_COMMAND = "gcloud auth activate-service-account --key-file=%s" SET_PROPERTY_PROJECT = "gcloud config set project %s" MACHINE_STARTING_COMMAND = "gcloud compute instances start %s --zone=%s" MACHINE_STOPPING_COMMAND = "gcloud compute instances stop %s --zone=%s" - # Default configuration parameters + # Key path location relative to this file DEFAULT_RELATIVE_KEY_PATH = "../../gcp_keys/gcp_key.json" DEFAULT_ZONE = "europe-west3-a" DEFAULT_PROJECT = "guardicore-22050661" @@ -25,10 +24,10 @@ class GCPHandler(object): ): self.zone = zone abs_key_path = GCPHandler.get_absolute_key_path(relative_key_path) - # pass the key file to gcp + subprocess.call(GCPHandler.get_auth_command(abs_key_path), shell=True) # noqa: DUO116 LOGGER.info("GCP Handler passed key") - # set project + subprocess.call(GCPHandler.get_set_project_command(project_id), shell=True) # noqa: DUO116 LOGGER.info("GCP Handler set project") LOGGER.info("GCP Handler initialized successfully") @@ -37,11 +36,12 @@ class GCPHandler(object): def get_absolute_key_path(relative_key_path: str) -> str: file_dir = os.path.dirname(os.path.realpath(__file__)) absolute_key_path = os.path.join(file_dir, relative_key_path) + if not os.path.isfile(absolute_key_path): raise FileNotFoundError( "GCP key not found. " "Add a service key to envs/monkey_zoo/gcp_keys/gcp_key.json" ) - return os.path.join(file_dir, relative_key_path) + return os.path.normpath(absolute_key_path) def start_machines(self, machine_list): """ From 7a03a9504dc3cf42a8e942c51ba234a674423d08 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Wed, 12 May 2021 16:33:52 +0300 Subject: [PATCH 3/4] Removed the `relative_key_path` parameter from GCPHandler class because it's unused and has a misleading name. --- .../blackbox/utils/gcp_machine_handlers.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py b/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py index 89018bde1..dd72112ba 100644 --- a/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py +++ b/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py @@ -12,18 +12,17 @@ class GCPHandler(object): MACHINE_STOPPING_COMMAND = "gcloud compute instances stop %s --zone=%s" # Key path location relative to this file - DEFAULT_RELATIVE_KEY_PATH = "../../gcp_keys/gcp_key.json" + RELATIVE_KEY_PATH = "../../gcp_keys/gcp_key.json" DEFAULT_ZONE = "europe-west3-a" DEFAULT_PROJECT = "guardicore-22050661" def __init__( self, - relative_key_path=DEFAULT_RELATIVE_KEY_PATH, zone=DEFAULT_ZONE, project_id=DEFAULT_PROJECT, ): self.zone = zone - abs_key_path = GCPHandler.get_absolute_key_path(relative_key_path) + abs_key_path = GCPHandler.get_absolute_key_path() subprocess.call(GCPHandler.get_auth_command(abs_key_path), shell=True) # noqa: DUO116 LOGGER.info("GCP Handler passed key") @@ -33,15 +32,15 @@ class GCPHandler(object): LOGGER.info("GCP Handler initialized successfully") @staticmethod - def get_absolute_key_path(relative_key_path: str) -> str: - file_dir = os.path.dirname(os.path.realpath(__file__)) - absolute_key_path = os.path.join(file_dir, relative_key_path) + def get_absolute_key_path() -> str: + absolute_key_path = os.path.join(__file__, GCPHandler.RELATIVE_KEY_PATH) + absolute_key_path = os.path.realpath(absolute_key_path) if not os.path.isfile(absolute_key_path): raise FileNotFoundError( "GCP key not found. " "Add a service key to envs/monkey_zoo/gcp_keys/gcp_key.json" ) - return os.path.normpath(absolute_key_path) + return absolute_key_path def start_machines(self, machine_list): """ From 45f2702403dba91b463b72e3ea984eaccb145f91 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Wed, 12 May 2021 16:58:46 +0300 Subject: [PATCH 4/4] Reverted back to fetching file directory first when resolving GCP keys. This is to make gcp key file relative to utils directory, not the current file. This will make it less confusing, because people usually navigate directories, not files. --- envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py b/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py index dd72112ba..c438e92f5 100644 --- a/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py +++ b/envs/monkey_zoo/blackbox/utils/gcp_machine_handlers.py @@ -11,7 +11,7 @@ class GCPHandler(object): MACHINE_STARTING_COMMAND = "gcloud compute instances start %s --zone=%s" MACHINE_STOPPING_COMMAND = "gcloud compute instances stop %s --zone=%s" - # Key path location relative to this file + # Key path location relative to this file's directory RELATIVE_KEY_PATH = "../../gcp_keys/gcp_key.json" DEFAULT_ZONE = "europe-west3-a" DEFAULT_PROJECT = "guardicore-22050661" @@ -33,7 +33,8 @@ class GCPHandler(object): @staticmethod def get_absolute_key_path() -> str: - absolute_key_path = os.path.join(__file__, GCPHandler.RELATIVE_KEY_PATH) + file_dir = os.path.dirname(os.path.realpath(__file__)) + absolute_key_path = os.path.join(file_dir, GCPHandler.RELATIVE_KEY_PATH) absolute_key_path = os.path.realpath(absolute_key_path) if not os.path.isfile(absolute_key_path):