From 869e11f3d1cc4f627379168df2c559fd4d84cc28 Mon Sep 17 00:00:00 2001 From: Guy Lichtman <1395797+glicht@users.noreply.github.com> Date: Mon, 30 Nov 2020 16:09:53 +0200 Subject: [PATCH] fail on unexpected exception + handle mac os docker (#904) * fail on unexpected exception + handle mac os docker * remove un-necessary pass * handle also other cases that we use containers * fix exception string * improve no-docker mode * fix tests * fail if we are in CI content build and there is no docker * update release notes --- CHANGELOG.md | 2 + demisto_sdk/commands/lint/helpers.py | 7 +- demisto_sdk/commands/lint/lint_manager.py | 5 +- demisto_sdk/commands/lint/linter.py | 143 +++++++++--------- .../lint/tests/test_linter/os_runner_test.py | 10 +- 5 files changed, 91 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebaa20e20e9..d4e5d61864a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ # 1.2.11 * Fixed an issue where the ***generate-docs*** command reset the enumeration of line numbering after an MD table. * Updated the **upload** command to support mappers. +* Fixed an issue with support for Docker Desktop on Mac version 2.5.0+. +* Added support for vulture and mypy linting when running without docker. * Fixed an issue where exceptions were no printed in the **format** while the *--verbose* flag is set. * Fixed an issue where *--assume-yes* flag did not work in the **format** command when running on a playbook without a `fromversion` field. * Fixed an issue where the **format** command would fail in case `conf.json` file was not found instead of skipping the update. diff --git a/demisto_sdk/commands/lint/helpers.py b/demisto_sdk/commands/lint/helpers.py index 80d90d940d3..8464a93f959 100644 --- a/demisto_sdk/commands/lint/helpers.py +++ b/demisto_sdk/commands/lint/helpers.py @@ -95,9 +95,9 @@ def build_skipped_exit_code(no_flake8: bool, no_bandit: bool, no_mypy: bool, no_ skipped_code |= EXIT_CODES["XSOAR_linter"] if no_bandit: skipped_code |= EXIT_CODES["bandit"] - if no_mypy or not docker_engine: + if no_mypy: skipped_code |= EXIT_CODES["mypy"] - if no_vulture or not docker_engine: + if no_vulture: skipped_code |= EXIT_CODES["vulture"] if no_pylint or not docker_engine: skipped_code |= EXIT_CODES["pylint"] @@ -263,7 +263,8 @@ def add_tmp_lint_files(content_repo: git.Repo, pack_path: Path, lint_files: List added_modules.append(cur_path) yield except Exception as e: - logger.error(str(e)) + logger.error(f'add_tmp_lint_files unexpected exception: {str(e)}') + raise finally: # If we want to change handling of files after finishing - do it here pass diff --git a/demisto_sdk/commands/lint/lint_manager.py b/demisto_sdk/commands/lint/lint_manager.py index 110df7844a8..4f15e5e9a59 100644 --- a/demisto_sdk/commands/lint/lint_manager.py +++ b/demisto_sdk/commands/lint/lint_manager.py @@ -133,7 +133,10 @@ def _gather_facts() -> Dict[str, Any]: docker_client: docker.DockerClient = docker.from_env() try: docker_client.ping() - except (requests.exceptions.ConnectionError, urllib3.exceptions.ProtocolError, docker.errors.APIError): + except (requests.exceptions.ConnectionError, urllib3.exceptions.ProtocolError, docker.errors.APIError) as ex: + if os.getenv("CI") and os.getenv("CIRCLE_PROJECT_REPONAME") == "content": + # when running lint in content we fail if docker isn't available for some reason + raise ValueError("Docker engine not available and we are in content CI env. Can not run lint!!") from ex facts["docker_engine"] = False print_warning("Can't communicate with Docker daemon - check your docker Engine is ON - Skipping lint, " "test which require docker!") diff --git a/demisto_sdk/commands/lint/linter.py b/demisto_sdk/commands/lint/linter.py index cb7e9af77a0..aa934f53668 100644 --- a/demisto_sdk/commands/lint/linter.py +++ b/demisto_sdk/commands/lint/linter.py @@ -4,6 +4,8 @@ import json import logging import os +import platform +import traceback from copy import deepcopy from typing import Any, Dict, List, Optional, Tuple @@ -123,30 +125,34 @@ def run_dev_packages(self, no_flake8: bool, no_bandit: bool, no_mypy: bool, no_p # If not python pack - skip pack if skip: return self._pkg_lint_status - - # Locate mandatory files in pack path - for more info checkout the context manager LintFiles - with add_tmp_lint_files(content_repo=self._content_repo, # type: ignore - pack_path=self._pack_abs_dir, - lint_files=self._facts["lint_files"], - modules=modules, - pack_type=self._pkg_lint_status["pack_type"]): - # Run lint check on host - flake8, bandit, mypy - if self._pkg_lint_status["pack_type"] == TYPE_PYTHON: - self._run_lint_in_host(no_flake8=no_flake8, - no_bandit=no_bandit, - no_mypy=no_mypy, - no_vulture=no_vulture, - no_xsoar_linter=no_xsoar_linter) - - # Run lint and test check on pack docker image - if self._facts["docker_engine"]: - self._run_lint_on_docker_image(no_pylint=no_pylint, - no_test=no_test, - no_pwsh_analyze=no_pwsh_analyze, - no_pwsh_test=no_pwsh_test, - keep_container=keep_container, - test_xml=test_xml) - + try: + # Locate mandatory files in pack path - for more info checkout the context manager LintFiles + with add_tmp_lint_files(content_repo=self._content_repo, # type: ignore + pack_path=self._pack_abs_dir, + lint_files=self._facts["lint_files"], + modules=modules, + pack_type=self._pkg_lint_status["pack_type"]): + # Run lint check on host - flake8, bandit, mypy + if self._pkg_lint_status["pack_type"] == TYPE_PYTHON: + self._run_lint_in_host(no_flake8=no_flake8, + no_bandit=no_bandit, + no_mypy=no_mypy, + no_vulture=no_vulture, + no_xsoar_linter=no_xsoar_linter) + + # Run lint and test check on pack docker image + if self._facts["docker_engine"]: + self._run_lint_on_docker_image(no_pylint=no_pylint, + no_test=no_test, + no_pwsh_analyze=no_pwsh_analyze, + no_pwsh_test=no_pwsh_test, + keep_container=keep_container, + test_xml=test_xml) + except Exception as ex: + err = f'{self._pack_abs_dir}: Unexpected fatal exception: {str(ex)}' + logger.error(f"{err}. Traceback: {traceback.format_exc()}") + self._pkg_lint_status["errors"].append(err) + self._pkg_lint_status['exit_code'] += FAIL return self._pkg_lint_status def _gather_facts(self, modules: dict) -> bool: @@ -229,6 +235,11 @@ def _gather_facts(self, modules: dict) -> bool: logger.info(f"{log_prompt} - Additional package Pypi packages found - {additional_req}") except (FileNotFoundError, IOError): self._pkg_lint_status["errors"].append('Unable to parse test-requirements.txt in package') + elif not self._facts["python_version"]: + # get python version from yml + pynum = 3.7 if (script_obj.get('subtype', 'python3') == 'python3') else 2.7 + self._facts["python_version"] = pynum + logger.info(f"{log_prompt} - Using python version from yml: {pynum}") # Get lint files lint_files = set(self._pack_abs_dir.glob(["*.py", "!__init__.py", "!*.tmp"], flags=NEGATE)) @@ -289,17 +300,17 @@ def _run_lint_in_host(self, no_flake8: bool, no_bandit: bool, no_mypy: bool, no_ exit_code = SUCCESS output = "" if lint_check == "flake8" and not no_flake8: - exit_code, output = self._run_flake8(py_num=self._facts["images"][0][1], + exit_code, output = self._run_flake8(py_num=self._facts["python_version"], lint_files=self._facts["lint_files"]) elif lint_check == "XSOAR_linter" and not no_xsoar_linter: - exit_code, output = self._run_xsoar_linter(py_num=self._facts["images"][0][1], + exit_code, output = self._run_xsoar_linter(py_num=self._facts["python_version"], lint_files=self._facts["lint_files"]) elif lint_check == "bandit" and not no_bandit: exit_code, output = self._run_bandit(lint_files=self._facts["lint_files"]) - elif lint_check == "mypy" and not no_mypy and self._facts["docker_engine"]: - exit_code, output = self._run_mypy(py_num=self._facts["images"][0][1], + elif lint_check == "mypy" and not no_mypy: + exit_code, output = self._run_mypy(py_num=self._facts["python_version"], lint_files=self._facts["lint_files"]) - elif lint_check == "vulture" and not no_vulture and self._facts["docker_engine"]: + elif lint_check == "vulture" and not no_vulture: exit_code, output = self._run_vulture(py_num=self._facts["python_version"], lint_files=self._facts["lint_files"]) @@ -323,7 +334,7 @@ def _run_lint_in_host(self, no_flake8: bool, no_bandit: bool, no_mypy: bool, no_ exit_code = SUCCESS output = "" if lint_check == "flake8" and not no_flake8: - exit_code, output = self._run_flake8(py_num=self._facts["images"][0][1], + exit_code, output = self._run_flake8(py_num=self._facts["python_version"], lint_files=self._facts["lint_unittest_files"]) if exit_code: error, warning, other = split_warnings_errors(output) @@ -701,6 +712,17 @@ def _docker_image_create(self, docker_base_image: List[Any]) -> Tuple[str, str]: return test_image_name, errors + def _docker_remove_container(self, container_name: str): + try: + container_obj = self._docker_client.containers.get(container_name) + container_obj.remove(force=True) + except docker.errors.NotFound: + pass + except requests.exceptions.ChunkedEncodingError as err: + # see: https://github.com/docker/docker-py/issues/2696#issuecomment-721322548 + if platform.system() != 'Darwin' or 'Connection broken' not in str(err): + raise + def _docker_run_pylint(self, test_image: str, keep_container: bool) -> Tuple[int, str]: """ Run Pylint in created test image @@ -716,24 +738,19 @@ def _docker_run_pylint(self, test_image: str, keep_container: bool) -> Tuple[int logger.info(f"{log_prompt} - Start") container_name = f"{self._pack_name}-pylint" # Check if previous run left container a live if it do, we remove it - container_obj: docker.models.containers.Container - try: - container_obj = self._docker_client.containers.get(container_name) - container_obj.remove(force=True) - except docker.errors.NotFound: - pass + self._docker_remove_container(container_name) # Run container exit_code = SUCCESS output = "" try: - container_obj = self._docker_client.containers.run(name=container_name, - image=test_image, - command=[ - build_pylint_command(self._facts["lint_files"])], - user=f"{os.getuid()}:4000", - detach=True, - environment=self._facts["env_vars"]) + container_obj: docker.models.containers.Container = self._docker_client.containers.run(name=container_name, + image=test_image, + command=[ + build_pylint_command(self._facts["lint_files"])], + user=f"{os.getuid()}:4000", + detach=True, + environment=self._facts["env_vars"]) stream_docker_container_output(container_obj.logs(stream=True)) # wait for container to finish container_status = container_obj.wait(condition="exited") @@ -791,25 +808,20 @@ def _docker_run_pytest(self, test_image: str, keep_container: bool, test_xml: st logger.info(f"{log_prompt} - Start") container_name = f"{self._pack_name}-pytest" # Check if previous run left container a live if it does, Remove it - container_obj: docker.models.containers.Container - try: - container_obj = self._docker_client.containers.get(container_name) - container_obj.remove(force=True) - except docker.errors.NotFound: - pass + self._docker_remove_container(container_name) # Collect tests exit_code = SUCCESS output = '' test_json = {} try: # Running pytest container - container_obj = self._docker_client.containers.run(name=container_name, - image=test_image, - command=[ - build_pytest_command(test_xml=test_xml, json=True)], - user=f"{os.getuid()}:4000", - detach=True, - environment=self._facts["env_vars"]) + container_obj: docker.models.containers.Container = self._docker_client.containers.run(name=container_name, + image=test_image, + command=[ + build_pytest_command(test_xml=test_xml, json=True)], + user=f"{os.getuid()}:4000", + detach=True, + environment=self._facts["env_vars"]) stream_docker_container_output(container_obj.logs(stream=True)) # Waiting for container to be finished container_status: dict = container_obj.wait(condition="exited") @@ -950,23 +962,18 @@ def _docker_run_pwsh_test(self, test_image: str, keep_container: bool) -> Tuple[ logger.info(f"{log_prompt} - Start") container_name = f"{self._pack_name}-pwsh-test" # Check if previous run left container a live if it do, we remove it - container_obj: docker.models.containers.Container - try: - container_obj = self._docker_client.containers.get(container_name) - container_obj.remove(force=True) - except docker.errors.NotFound: - pass + self._docker_remove_container(container_name) # Run container exit_code = SUCCESS output = "" try: - container_obj = self._docker_client.containers.run(name=container_name, - image=test_image, - command=build_pwsh_test_command(), - user=f"{os.getuid()}:4000", - detach=True, - environment=self._facts["env_vars"]) + container_obj: docker.models.containers.Container = self._docker_client.containers.run(name=container_name, + image=test_image, + command=build_pwsh_test_command(), + user=f"{os.getuid()}:4000", + detach=True, + environment=self._facts["env_vars"]) stream_docker_container_output(container_obj.logs(stream=True)) # wait for container to finish container_status = container_obj.wait(condition="exited") diff --git a/demisto_sdk/commands/lint/tests/test_linter/os_runner_test.py b/demisto_sdk/commands/lint/tests/test_linter/os_runner_test.py index 1d42a85be0d..08d6355bdb5 100644 --- a/demisto_sdk/commands/lint/tests/test_linter/os_runner_test.py +++ b/demisto_sdk/commands/lint/tests/test_linter/os_runner_test.py @@ -339,12 +339,13 @@ def test_fail_lint_on_only_test_file(self, mocker, linter_obj, lint_files): from demisto_sdk.commands.lint.linter import EXIT_CODES unittest_path = lint_files[0].parent / 'intergration_sample_test.py' mocker.patch.dict(linter_obj._facts, { - "images": [["image", "3.7"]], + "images": [["image", 3.7]], "test": False, "version_two": False, "lint_files": [], "lint_unittest_files": [unittest_path], - "additional_requirements": [] + "additional_requirements": [], + "python_version": 3.7, }) mocker.patch.object(linter_obj, '_run_flake8') linter_obj._run_flake8.return_value = (0b1, 'Error') @@ -390,12 +391,13 @@ def test_fail_lint_on_normal_and_test_file(self, mocker, linter_obj, lint_files) from demisto_sdk.commands.lint.linter import EXIT_CODES unittest_path = lint_files[0].parent / 'intergration_sample_test.py' mocker.patch.dict(linter_obj._facts, { - "images": [["image", "3.7"]], + "images": [["image", 3.7]], "test": False, "version_two": False, "lint_files": lint_files, "lint_unittest_files": [unittest_path], - "additional_requirements": [] + "additional_requirements": [], + "python_version": 3.7, }) mocker.patch.object(linter_obj, '_run_flake8') linter_obj._run_flake8.return_value = (0b1, 'Error')