From 04581a3ee0b9db15fa50214e4d0f85ccc820f9e8 Mon Sep 17 00:00:00 2001 From: Tobias Persson Date: Wed, 16 Oct 2024 12:37:34 +0200 Subject: [PATCH] Fix the code so that tox does not complain --- src/environment_provider/environment.py | 3 - .../environment_provider.py | 71 +++++++++++------- src/environment_provider/lib/config.py | 72 +++++++++++-------- src/environment_provider/lib/log_area.py | 3 +- src/environment_provider/lib/releaser.py | 58 ++++++++++----- src/environment_provider/lib/test_suite.py | 10 ++- tests/library/fake_server.py | 2 +- tests/library/graphql_handler.py | 15 +++- tests/splitter/test_splitter.py | 6 +- tox.ini | 1 + 10 files changed, 158 insertions(+), 83 deletions(-) diff --git a/src/environment_provider/environment.py b/src/environment_provider/environment.py index 43a2578..ab7f21f 100644 --- a/src/environment_provider/environment.py +++ b/src/environment_provider/environment.py @@ -29,9 +29,6 @@ from environment_provider.lib.database import ETCDPath from environment_provider.lib.registry import ProviderRegistry from environment_provider.lib.releaser import EnvironmentReleaser -from etos_lib.kubernetes.schemas import Environment as EnvironmentSchema -from etos_lib.kubernetes.schemas import Provider as ProviderSchema -from etos_lib.kubernetes import Kubernetes, Environment, Provider from execution_space_provider import ExecutionSpaceProvider from execution_space_provider.execution_space import ExecutionSpace from iut_provider import IutProvider diff --git a/src/environment_provider/environment_provider.py b/src/environment_provider/environment_provider.py index e32d152..aff58a4 100644 --- a/src/environment_provider/environment_provider.py +++ b/src/environment_provider/environment_provider.py @@ -66,6 +66,10 @@ class EnvironmentProviderNotConfigured(Exception): """Environment provider was not configured prior to request.""" +class EnvironmentProviderError(Exception): + """Environment provider got an error.""" + + class EnvironmentProvider: # pylint:disable=too-many-instance-attributes """Environment provider.""" @@ -75,12 +79,14 @@ class EnvironmentProvider: # pylint:disable=too-many-instance-attributes execution_space_provider = None testrun = None - def __init__(self, suite_runner_ids: Optional[list[str]]=None) -> None: + def __init__(self, suite_runner_ids: Optional[list[str]] = None) -> None: """Initialize ETOS, dataset, provider registry and splitter. :param suite_runner_ids: IDs from the suite runner to correlate sub suites. """ - self.etos = ETOS("ETOS Environment Provider", os.getenv("HOSTNAME", "Unknown"), "Environment Provider") + self.etos = ETOS( + "ETOS Environment Provider", os.getenv("HOSTNAME", "Unknown"), "Environment Provider" + ) self.environment_provider_config = Config(self.etos, suite_runner_ids) FORMAT_CONFIG.identifier = self.environment_provider_config.requests[0].spec.identifier @@ -221,12 +227,15 @@ def upload_sub_suite(self, sub_suite: dict) -> tuple[str, dict]: with NamedTemporaryFile(mode="w", delete=False) as sub_suite_file: json.dump(sub_suite, sub_suite_file) log_area = LogArea(self.etos, sub_suite) - return log_area.upload( - sub_suite_file.name, - f"{sub_suite['name']}.json", - sub_suite["test_suite_started_id"], - sub_suite["sub_suite_id"], - ), sub_suite + return ( + log_area.upload( + sub_suite_file.name, + f"{sub_suite['name']}.json", + sub_suite["test_suite_started_id"], + sub_suite["sub_suite_id"], + ), + sub_suite, + ) finally: os.remove(sub_suite_file.name) @@ -272,7 +281,9 @@ def recipes_from_tests(self, tests: list[Test]) -> list[dict]: ) return recipes - def create_environment_resource(self, request: EnvironmentRequestSchema, sub_suite: dict) -> tuple[str, dict]: + def create_environment_resource( + self, request: EnvironmentRequestSchema, sub_suite: dict + ) -> tuple[str, dict]: """Create an environment resource in Kubernetes. :param sub_suite: Sub suite to add to Environment resource. @@ -302,12 +313,15 @@ def create_environment_resource(self, request: EnvironmentRequestSchema, sub_sui labels=labels, ownerReferences=owners, ), - spec=EnvironmentSpec(**sub_suite.copy()) + spec=EnvironmentSpec(**sub_suite.copy()), ) - environment_client = Environment(self.kubernetes, strict=True) + environment_client = Environment(self.kubernetes) if not environment_client.create(environment): raise RuntimeError("Failed to create the environment for an etos testrun") - return f"{os.getenv('ETOS_API')}/v1alpha/testrun/{environment_id}", environment.spec.model_dump() + return ( + f"{os.getenv('ETOS_API')}/v1alpha/testrun/{environment_id}", + environment.spec.model_dump(), + ) def checkout_an_execution_space(self) -> ExecutionSpace: """Check out a single execution space. @@ -344,26 +358,26 @@ def checkout_timeout(self) -> int: ) return endtime - def checkout(self, request: EnvironmentRequestSchema) -> None: + def checkout( + self, request: EnvironmentRequestSchema + ) -> None: """Checkout an environment for a test suite. A request can have multiple environments due to IUT availability or the amount of unique test runners in the request. """ + # pylint:disable=too-many-statements self.logger.info("Checkout environment for %r", request.spec.name, extra={"user_log": True}) self.new_dataset(request) splitter = Splitter(self.etos, request.spec.splitter) - # TODO: This is a hack to make it work without too many changes to the code. + # TODO: This is a hack to make the controller environment work without too many changes + # to the original code, since we want to run them at the same time. test_runners = {} for test in request.spec.splitter.tests: test_runners.setdefault( test.execution.testRunner, - { - "docker": test.execution.testRunner, - "priority": 1, - "unsplit_recipes": [] - } + {"docker": test.execution.testRunner, "priority": 1, "unsplit_recipes": []}, ) test_runners[test.execution.testRunner]["unsplit_recipes"].append(test) @@ -409,7 +423,8 @@ def checkout(self, request: EnvironmentRequestSchema) -> None: # Check out and assign IUTs to test runners. iuts = self.iut_provider.wait_for_and_checkout_iuts( minimum_amount=request.spec.minimumAmount, - # maximum_amount=request.spec.maximumAmount, # TODO: Total test count changes, must check + # maximum_amount=request.spec.maximumAmount, + # TODO: Total test count changes, must check maximum_amount=self.dataset.get( "maximum_amount", os.getenv( @@ -421,7 +436,7 @@ def checkout(self, request: EnvironmentRequestSchema) -> None: splitter.assign_iuts(test_runners, iuts) span.set_attribute(SemConvAttributes.IUT_DESCRIPTION, str(iuts)) - for test_runner in test_runners.keys(): + for test_runner in test_runners: # pylint:disable=consider-using-dict-items self.dataset.add("test_runner", test_runner) # No IUTs assigned to test runner @@ -456,7 +471,9 @@ def checkout(self, request: EnvironmentRequestSchema) -> None: request, test_runner, iut, suite, test_runners[test_runner]["priority"] ) if self.environment_provider_config.etos_controller: - self.send_environment_events(*self.create_environment_resource(request, sub_suite)) + self.send_environment_events( + *self.create_environment_resource(request, sub_suite) + ) else: self.send_environment_events(*self.upload_sub_suite(sub_suite)) @@ -584,7 +601,8 @@ def configure_environment_provider(self, request: EnvironmentRequestSchema): self._configure_iut(iut) # type: ignore log_area = provider_client.get(request.spec.providers.logArea.id).to_dict() # type: ignore self._configure_log_area(log_area) # type: ignore - execution_space = provider_client.get(request.spec.providers.executionSpace.id).to_dict() # type: ignore + provider_id = request.spec.providers.executionSpace.id # type: ignore + execution_space = provider_client.get(provider_id).to_dict() # type: ignore self._configure_execution_space(execution_space) # type: ignore def run(self) -> dict: @@ -625,8 +643,11 @@ def get_environment(): try: status = EnvironmentProvider().run() if status.get("error") is not None: - raise Exception(status.get("error")) - result = {"conclusion": "Successful", "description": "Successfully provisioned an environment"} + raise EnvironmentProviderError(status.get("error")) + result = { + "conclusion": "Successful", + "description": "Successfully provisioned an environment", + } with open("/dev/termination-log", "w", encoding="utf-8") as termination_log: json.dump(result, termination_log) except: diff --git a/src/environment_provider/lib/config.py b/src/environment_provider/lib/config.py index acf271d..ecf9422 100644 --- a/src/environment_provider/lib/config.py +++ b/src/environment_provider/lib/config.py @@ -30,8 +30,8 @@ ) from etos_lib.kubernetes import Kubernetes, EnvironmentRequest from etos_lib.kubernetes.schemas.common import Metadata -from environment_provider.lib.registry import ProviderRegistry from jsontas.jsontas import JsonTas +from environment_provider.lib.registry import ProviderRegistry from .graphql import request_activity_triggered, request_artifact_created @@ -43,7 +43,7 @@ class Config: # pylint:disable=too-many-instance-attributes __request = None __activity_triggered = None - def __init__(self, etos: ETOS, ids: Optional[list[str]]=None) -> None: + def __init__(self, etos: ETOS, ids: Optional[list[str]] = None) -> None: """Initialize with ETOS library and automatically load the config. :param etos: ETOS library instance. @@ -82,7 +82,10 @@ def load_config(self) -> None: def __wait_for_activity(self) -> Optional[dict]: """Wait for activity triggered event.""" - self.logger.info("Waiting for an activity triggered event %ds", self.etos.config.get("EVENT_DATA_TIMEOUT")) + self.logger.info( + "Waiting for an activity triggered event %ds", + self.etos.config.get("EVENT_DATA_TIMEOUT"), + ) timeout = time.time() + self.etos.config.get("EVENT_DATA_TIMEOUT") # type: ignore while time.time() <= timeout: time.sleep(1) @@ -105,7 +108,11 @@ def __wait_for_activity(self) -> Optional[dict]: self.logger.info("No activity triggered found yet, retrying") continue return edges[0]["node"] - self.logger.info("Activity triggered event not found after %ds", self.etos.config.get("EVENT_DATA_TIMEOUT")) + self.logger.info( + "Activity triggered event not found after %ds", + self.etos.config.get("EVENT_DATA_TIMEOUT"), + ) + return None # TODO: The requests method shall not return a list in the future, this is just to # keep the changes backwards compatible. @@ -114,14 +121,17 @@ def requests(self) -> list[EnvironmentRequestSchema]: """Request returns the environment request, either from Eiffel TERCC or environment.""" if self.__request is None: if self.etos_controller: - request_client = EnvironmentRequest(self.kubernetes, strict=True) + request_client = EnvironmentRequest(self.kubernetes) request_name = os.getenv("REQUEST") assert request_name is not None, "Environment variable REQUEST must be set!" - self.__request = [EnvironmentRequestSchema.model_validate( - request_client.get(request_name).to_dict() # type: ignore - )] + self.__request = [ + EnvironmentRequestSchema.model_validate( + request_client.get(request_name).to_dict() # type: ignore + ) + ] else: - # Whenever the environment provider is run as a part of the suite runner, this variable is set. + # Whenever the environment provider is run as a part of the suite runner, + # this variable is set. tercc = json.loads(os.getenv("TERCC", "{}")) self.__request = self.__request_from_tercc(tercc) return self.__request @@ -134,14 +144,18 @@ def context(self) -> str: """ if self.__activity_triggered is None: self.__activity_triggered = self.__wait_for_activity() - assert self.__activity_triggered is not None, "ActivityTriggered must exist for the environment provider" + assert ( + self.__activity_triggered is not None + ), "ActivityTriggered must exist for the environment provider" try: return self.__activity_triggered["meta"]["id"] except KeyError: return "" def __request_from_tercc(self, tercc: dict) -> list[EnvironmentRequestSchema]: - assert self.ids is not None, "Suite runner IDs must be provided when running outside of controller" + assert ( + self.ids is not None + ), "Suite runner IDs must be provided when running outside of controller" requests = [] response = request_artifact_created(self.etos, tercc["links"][0]["target"]) assert response is not None, "ArtifactCreated must exist for the environment provider" @@ -160,25 +174,25 @@ def __request_from_tercc(self, tercc: dict) -> list[EnvironmentRequestSchema]: datasets = [datasets] * len(test_suites) for suite in test_suites: - requests.append(EnvironmentRequestSchema( - metadata=Metadata(), - spec=EnvironmentRequestSpec( - id=self.ids.pop(0), - name=suite.get("name"), - identifier=tercc["meta"]["id"], - artifact=artifact["meta"]["id"], - identity=artifact["data"]["identity"], - minimumAmount=1, - maximumAmount=10, # TODO: Ignored in environment_provider.py - image="N/A", - imagePullPolicy="N/A", - splitter=Splitter( - tests=Suite.tests_from_recipes(suite.get("recipes", [])) + requests.append( + EnvironmentRequestSchema( + metadata=Metadata(), + spec=EnvironmentRequestSpec( + id=self.ids.pop(0), + name=suite.get("name"), + identifier=tercc["meta"]["id"], + artifact=artifact["meta"]["id"], + identity=artifact["data"]["identity"], + minimumAmount=1, + maximumAmount=10, # TODO: Ignored in environment_provider.py + image="N/A", + imagePullPolicy="N/A", + splitter=Splitter(tests=Suite.tests_from_recipes(suite.get("recipes", []))), + dataset=datasets.pop(0), + providers=EnvironmentProviders(), ), - dataset=datasets.pop(0), - providers=EnvironmentProviders() ) - )) + ) return requests def __test_suite(self, tercc: dict) -> list[dict]: @@ -193,7 +207,7 @@ def __test_suite(self, tercc: dict) -> list[dict]: raise ValueError("Only one of 'batches' or 'batchesUri' shall be set") if batch is not None: return batch - elif batch_uri is not None: + if batch_uri is not None: response = self.etos.http.get( batch_uri, timeout=self.etos.config.get("TEST_SUITE_TIMEOUT"), diff --git a/src/environment_provider/lib/log_area.py b/src/environment_provider/lib/log_area.py index d7670ff..cb3bf64 100644 --- a/src/environment_provider/lib/log_area.py +++ b/src/environment_provider/lib/log_area.py @@ -28,7 +28,8 @@ from requests.exceptions import ConnectionError as RequestsConnectionError from requests.exceptions import HTTPError -# pylint:disable=too-many-arguments,too-many-positional-arguments +# pylint:disable=too-many-arguments +# pylint:disable=too-many-positional-arguments class LogArea: # pylint:disable=too-few-public-methods diff --git a/src/environment_provider/lib/releaser.py b/src/environment_provider/lib/releaser.py index 0eb60b4..560c96b 100644 --- a/src/environment_provider/lib/releaser.py +++ b/src/environment_provider/lib/releaser.py @@ -35,31 +35,35 @@ class Releaser: + """Releaser is a tool for releasing environments that have been checked out by ETOS.""" + logger = logging.getLogger(__name__) def __init__(self, etos: ETOS, environment: EnvironmentSchema): + """Set up releaser.""" self.environment = environment self.etos = etos self.jsontas = JsonTas() self.kubernetes = Kubernetes() - self.provider_client = Provider(self.kubernetes, strict=True) + self.provider_client = Provider(self.kubernetes) def provider(self, provider_id: str) -> dict: - """Provider gets a provider by ID from Kubernetes or a database.""" + """Get a provider by ID from Kubernetes or a database.""" provider = self.provider_client.get(provider_id) assert provider is not None, f"Could not find a provider with ID {provider_id!r}" provider_model = ProviderSchema.model_validate(provider.to_dict()) if provider_model.spec.jsontas: return provider_model.to_jsontas() - else: - return provider_model.to_external() + return provider_model.to_external() def run(self) -> None: """Run a release task for ETOS.""" - raise NotImplemented + raise NotImplementedError() class Iut(Releaser): + """Iut releases IUTs checked out by ETOS.""" + logger = logging.getLogger(__name__) def get_provider(self) -> IutProvider: @@ -103,12 +107,16 @@ def run(self): class Executor(Releaser): + """Executor releases execution spaces checked out by ETOS.""" + logger = logging.getLogger(__name__) def get_provider(self) -> ExecutionSpaceProvider: """Get provider returns an execution space provider using the provider model.""" ruleset = self.environment.spec.executor - assert ruleset is not None, f"There is no executor field in environment {self.environment!r}" + assert ( + ruleset is not None + ), f"There is no executor field in environment {self.environment!r}" self.logger.info("Releasing executor with ruleset: %r", ruleset) provider_id = ruleset.get("provider_id", "") self.logger.info("Provider to use for release: %r", provider_id) @@ -130,13 +138,16 @@ def release(self): provider.checkin(ExecutionSpace(**ruleset)) self.logger.info("Successfully released executor") except ExecutionSpaceCheckinFailed: - self.logger.error("Failed to release executor %r with provider %r", ruleset, provider_id) + self.logger.error( + "Failed to release executor %r with provider %r", ruleset, provider_id + ) raise def run(self): """Run releases executors that ETOS has checked out for an environment.""" - with TRACER.start_as_current_span(name="stop_execution_space", - kind=trace.SpanKind.CLIENT) as span: + with TRACER.start_as_current_span( + name="stop_execution_space", kind=trace.SpanKind.CLIENT + ) as span: try: self.release() except Exception as exception: @@ -146,12 +157,16 @@ def run(self): class LogArea(Releaser): + """Logarea releases log areas checked out by ETOS.""" + logger = logging.getLogger(__name__) def get_provider(self) -> LogAreaProvider: """Get provider returns an log area provider using the provider model.""" ruleset = self.environment.spec.log_area - assert ruleset is not None, f"There is no log area field in environment {self.environment!r}" + assert ( + ruleset is not None + ), f"There is no log area field in environment {self.environment!r}" self.logger.info("Releasing log area with ruleset: %r", ruleset) provider_id = ruleset.get("provider_id", "") self.logger.info("Provider to use for release: %r", provider_id) @@ -173,13 +188,14 @@ def release(self): provider.checkin(LogAreaSpec(**ruleset)) self.logger.info("Successfully released log area") except LogAreaCheckinFailed: - self.logger.error("Failed to release log area %r with provider %r", ruleset, provider_id) + self.logger.error( + "Failed to release log area %r with provider %r", ruleset, provider_id + ) raise def run(self): """Run releases log areas that ETOS has checked out for an environment.""" - with TRACER.start_as_current_span(name="stop_log_area", - kind=trace.SpanKind.CLIENT) as span: + with TRACER.start_as_current_span(name="stop_log_area", kind=trace.SpanKind.CLIENT) as span: try: self.release() except Exception as exception: @@ -191,12 +207,14 @@ def run(self): class EnvironmentReleaser: """Release environments checked out by ETOS.""" + logger = logging.getLogger(__name__) def environment(self, environment_id: str) -> EnvironmentSchema: """Environment gets an environment from kubernetes with environment_id as name.""" - client = Environment(Kubernetes(), strict=True) - return EnvironmentSchema.model_validate(client.get(environment_id).to_dict()) # type: ignore + client = Environment(Kubernetes()) + environment = client.get(environment_id).to_dict() # type: ignore + return EnvironmentSchema.model_validate(environment) def run(self, environment_id: str): """Run the releaser. It will check which type of environment and release it.""" @@ -207,8 +225,11 @@ def run(self, environment_id: str): try: environment = self.environment(environment_id) except AttributeError: - self.logger.exception("Could not find Environment with id %r in Kubernetes. " - "Trying to release something that's already released?", environment_id) + self.logger.exception( + "Could not find Environment with id %r in Kubernetes. " + "Trying to release something that's already released?", + environment_id, + ) return etos.config.set("SUITE_ID", environment.spec.suite_id) tasks = [Iut(etos, environment), LogArea(etos, environment), Executor(etos, environment)] @@ -218,8 +239,9 @@ def run(self, environment_id: str): self.logger.info("Running release task on %r", type(task).__name__) try: task.run() - except Exception as exception: + except Exception as exception: # pylint:disable=broad-exception-caught self.logger.error("Task %r failed", type(task).__name__) exceptions.append(exception) if exceptions: + # pylint:disable=using-exception-groups-in-unsupported-version raise ExceptionGroup("Some or all release tasks failed", exceptions) diff --git a/src/environment_provider/lib/test_suite.py b/src/environment_provider/lib/test_suite.py index b114c6d..e49f94a 100644 --- a/src/environment_provider/lib/test_suite.py +++ b/src/environment_provider/lib/test_suite.py @@ -14,15 +14,17 @@ # See the License for the specific language governing permissions and # limitations under the License. """Test suite module.""" -from iut_provider.iut import Iut from etos_lib.kubernetes.schemas.environment_request import EnvironmentRequest +from iut_provider.iut import Iut from .config import Config # pylint:disable=line-too-long +# pylint:disable=too-many-arguments +# pylint:disable=too-many-positional-arguments -class TestSuite: +class TestSuite: # pylint:disable=too-few-public-methods """Test suite representation. The resulting test suite might look something like this:: @@ -143,7 +145,9 @@ def __init__( self.suite_runner_id = suite_runner_id self.environment_provider_config = environment_provider_config - def add(self, request: EnvironmentRequest, test_runner: str, iut: Iut, suite: dict, priority: int) -> dict: + def add( + self, request: EnvironmentRequest, test_runner: str, iut: Iut, suite: dict, priority: int + ) -> dict: """Add a new sub suite to suite. :param test_runner: The test runner to use for sub suite. diff --git a/tests/library/fake_server.py b/tests/library/fake_server.py index f4a4ff5..4306ffe 100644 --- a/tests/library/fake_server.py +++ b/tests/library/fake_server.py @@ -121,7 +121,7 @@ def __enter__(self): self.port = self.__free_port() self.mock_server = HTTPServer(("localhost", self.port), self.handler) self.thread = Thread(target=self.mock_server.serve_forever) - self.thread.setDaemon(True) + self.thread.daemon = True self.thread.start() self.mock_server.RequestHandlerClass.parent = self self.mock_server.RequestHandlerClass.response_code = self.status_name diff --git a/tests/library/graphql_handler.py b/tests/library/graphql_handler.py index ca3eefd..decbe25 100644 --- a/tests/library/graphql_handler.py +++ b/tests/library/graphql_handler.py @@ -114,7 +114,20 @@ def artifact_created(self): :rtype dict: """ artifact_id = self.tercc["links"][0]["target"] - return {"data": {"artifactCreated": {"edges": [{"node": {"data": {"identity": "pkg:test/environment-provider"}}, "meta": {"id": artifact_id}}]}}} + return { + "data": { + "artifactCreated": { + "edges": [ + { + "node": { + "data": {"identity": "pkg:test/environment-provider"}, + "meta": {"id": artifact_id}, + } + } + ] + } + } + } def test_suite_started(self): """Create a fake test suite started to simulate ESR. diff --git a/tests/splitter/test_splitter.py b/tests/splitter/test_splitter.py index 67594d1..ba43f7c 100644 --- a/tests/splitter/test_splitter.py +++ b/tests/splitter/test_splitter.py @@ -18,8 +18,10 @@ import unittest from etos_lib import ETOS +from etos_lib.kubernetes.schemas.environment_request import Splitter as SplitterSchema from environment_provider.splitter.split import Splitter +from iut_provider.iut import Iut class TestSplitter(unittest.TestCase): @@ -37,7 +39,7 @@ def test_assign_iuts(self) -> None: 1. Assign IUTs to the provided test runners. 2. Verify that no test runner get 0 assigned IUTs. """ - iuts = ["iut1", "iut2"] + iuts = [Iut(name="iut1"), Iut(name="iut2")] test_runners = { "runner1": {"iuts": {}, "unsplit_recipes": [1]}, "runner2": {"iuts": {}, "unsplit_recipes": [2, 3, 4, 5]}, @@ -48,7 +50,7 @@ def test_assign_iuts(self) -> None: etos.config.set("NUMBER_OF_IUTS", len(iuts)) self.logger.info("STEP: Assign IUTs to the provided test runners.") - _ = Splitter(etos, {}).assign_iuts(test_runners, iuts) + _ = Splitter(etos, SplitterSchema(tests=[])).assign_iuts(test_runners, iuts) self.logger.info("STEP: Verify that no test runner get 0 assigned IUTs.") for test_runner in test_runners.values(): diff --git a/tox.ini b/tox.ini index 9eff780..7b74224 100644 --- a/tox.ini +++ b/tox.ini @@ -7,6 +7,7 @@ setenv = ETOS_GRAPHQL_SERVER=http://localhost/no ETOS_API=http://localhost/nah ETOS_ENVIRONMENT_PROVIDER=http://localhost/nuhuh + ETOS_NAMESPACE=something deps = -r{toxinidir}/test-requirements.txt commands =