From d47e26744655a2682f9e0f60f051d54173ddff7f Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Wed, 21 Aug 2019 18:26:21 -0700 Subject: [PATCH 01/18] Pack dependency management Fixes https://github.com/StackStorm/discussions/issues/354 1. `packs.install` workflow changed from `achtion-chain` to `orquesta` for workflow complexity. 2. Download all packs and dependency packs, check for conflict packs before `install pack requirements` and `register pack`. 3. Workflow fails on any time if conflict pack is recognized, downloaded packs will be removed. 4. Register packs in reverse order, it means the last dependency pack will be installed first. 5. New option `--skip` (default value is False) for skipping dependencies check: `st2 install pack --skip` 6. Only case that will check conflict is that if pack was installed and dependency pack has specified pack version. 7. Nested level of dependencies to prevent infinite or long download loops is 3. 8. Support StackStorm Exchange packs, the private repo ULR and local file system for conflict pack checking. --- CHANGELOG.rst | 2 + contrib/packs/actions/delete.yaml | 5 + contrib/packs/actions/download.yaml | 6 + .../packs/actions/get_pack_dependencies.yaml | 20 +++ contrib/packs/actions/install.meta.yaml | 7 +- contrib/packs/actions/pack_mgmt/delete.py | 19 +-- contrib/packs/actions/pack_mgmt/download.py | 33 +++-- .../pack_mgmt/get_pack_dependencies.py | 133 +++++++++++++++++ contrib/packs/actions/pack_mgmt/register.py | 1 + .../pack_mgmt/virtualenv_setup_prerun.py | 13 +- contrib/packs/actions/virtualenv_prerun.yaml | 7 + contrib/packs/actions/workflows/install.yaml | 135 +++++++++++++----- st2api/st2api/controllers/v1/packs.py | 3 + st2client/st2client/commands/pack.py | 12 +- st2client/st2client/models/core.py | 5 +- st2common/st2common/openapi.yaml | 5 + st2common/st2common/openapi.yaml.j2 | 5 + 17 files changed, 351 insertions(+), 60 deletions(-) create mode 100644 contrib/packs/actions/get_pack_dependencies.yaml create mode 100644 contrib/packs/actions/pack_mgmt/get_pack_dependencies.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4b6e5ec92e..cc5bcf7fe3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,6 +10,8 @@ Added * Add support for blacklisting / whitelisting hosts to the HTTP runner by adding new ``url_hosts_blacklist`` and ``url_hosts_whitelist`` runner attribute. (new feature) #4757 +* Add support for Pack dependency management to install dependency packs and find out confilct packs. + #4769 Changed ~~~~~~~ diff --git a/contrib/packs/actions/delete.yaml b/contrib/packs/actions/delete.yaml index cb7e5f33e7..36f5f51023 100644 --- a/contrib/packs/actions/delete.yaml +++ b/contrib/packs/actions/delete.yaml @@ -14,3 +14,8 @@ type: "string" default: "/opt/stackstorm/packs/" immutable: true + delete_env: + type: "boolean" + description: Flag for if need to delete virtual environment. + default: true + required: false diff --git a/contrib/packs/actions/download.yaml b/contrib/packs/actions/download.yaml index 543db4a63c..575d1ff45a 100644 --- a/contrib/packs/actions/download.yaml +++ b/contrib/packs/actions/download.yaml @@ -27,3 +27,9 @@ description: "True to use Python 3 binary for this pack." required: false default: false + dependency_list: + type: "array" + description: "Dependency list that needs to be downloaded." + items: + type: "string" + required: false diff --git a/contrib/packs/actions/get_pack_dependencies.yaml b/contrib/packs/actions/get_pack_dependencies.yaml new file mode 100644 index 0000000000..45c7c01245 --- /dev/null +++ b/contrib/packs/actions/get_pack_dependencies.yaml @@ -0,0 +1,20 @@ + + +--- + name: "get_pack_dependencies" + runner_type: "python-script" + description: "Get pack dependencies specified in pack.yaml" + enabled: true + pack: packs + entry_point: "pack_mgmt/get_pack_dependencies.py" + parameters: + packs_status: + type: object + description: Name of the pack in Exchange or a git repo URL and download status. + required: true + default: null + nested: + type: integer + description: Nested level of dependencies to prevent infinite or really long download loops. + required: true + default: 3 diff --git a/contrib/packs/actions/install.meta.yaml b/contrib/packs/actions/install.meta.yaml index 46632de72a..d1f93782fa 100644 --- a/contrib/packs/actions/install.meta.yaml +++ b/contrib/packs/actions/install.meta.yaml @@ -1,6 +1,6 @@ --- name: "install" - runner_type: "action-chain" + runner_type: "orquesta" description: "Installs or upgrades a pack into local content repository, either by git URL or a short name matching an index entry. Will download pack, load the actions, sensors and rules from the pack. @@ -32,3 +32,8 @@ description: "Use Python 3 binary when creating a virtualenv for this pack." required: false default: false + skip: + type: "boolean" + description: "Set to True to skip pack dependency installations." + required: false + default: false diff --git a/contrib/packs/actions/pack_mgmt/delete.py b/contrib/packs/actions/pack_mgmt/delete.py index e023585b8c..570c16865a 100644 --- a/contrib/packs/actions/pack_mgmt/delete.py +++ b/contrib/packs/actions/pack_mgmt/delete.py @@ -30,7 +30,7 @@ def __init__(self, config=None, action_service=None): self._base_virtualenvs_path = os.path.join(cfg.CONF.system.base_path, 'virtualenvs/') - def run(self, packs, abs_repo_base): + def run(self, packs, abs_repo_base, delete_env=True): intersection = BLOCKED_PACKS & frozenset(packs) if len(intersection) > 0: names = ', '.join(list(intersection)) @@ -43,12 +43,13 @@ def run(self, packs, abs_repo_base): self.logger.debug('Deleting pack directory "%s"' % (abs_fp)) shutil.rmtree(abs_fp) - # 2. Delete pack virtual environment - for pack_name in packs: - pack_name = quote_unix(pack_name) - virtualenv_path = os.path.join(self._base_virtualenvs_path, pack_name) + if delete_env: + # 2. Delete pack virtual environment + for pack_name in packs: + pack_name = quote_unix(pack_name) + virtualenv_path = os.path.join(self._base_virtualenvs_path, pack_name) - if os.path.isdir(virtualenv_path): - self.logger.debug('Deleting virtualenv "%s" for pack "%s"' % - (virtualenv_path, pack_name)) - shutil.rmtree(virtualenv_path) + if os.path.isdir(virtualenv_path): + self.logger.debug('Deleting virtualenv "%s" for pack "%s"' % + (virtualenv_path, pack_name)) + shutil.rmtree(virtualenv_path) diff --git a/contrib/packs/actions/pack_mgmt/download.py b/contrib/packs/actions/pack_mgmt/download.py index 344e3dfa66..f3c877f377 100644 --- a/contrib/packs/actions/pack_mgmt/download.py +++ b/contrib/packs/actions/pack_mgmt/download.py @@ -62,18 +62,29 @@ def __init__(self, config=None, action_service=None): if self.proxy_ca_bundle_path and not os.environ.get('proxy_ca_bundle_path', None): os.environ['no_proxy'] = self.no_proxy - def run(self, packs, abs_repo_base, verifyssl=True, force=False, python3=False): + def run(self, packs, abs_repo_base, verifyssl=True, force=False, python3=False, + dependency_list=None): result = {} - - for pack in packs: - pack_result = download_pack(pack=pack, abs_repo_base=abs_repo_base, - verify_ssl=verifyssl, force=force, - proxy_config=self.proxy_config, - force_permissions=True, - use_python3=python3, - logger=self.logger) - pack_url, pack_ref, pack_result = pack_result - result[pack_ref] = pack_result + pack_url = None + + if dependency_list: + for pack_dependency in dependency_list: + pack_result = download_pack(pack=pack_dependency, abs_repo_base=abs_repo_base, + verify_ssl=verifyssl, force=force, + proxy_config=self.proxy_config, force_permissions=True, + use_python3=python3, logger=self.logger) + pack_url, pack_ref, pack_result = pack_result + result[pack_ref] = pack_result + else: + for pack in packs: + pack_result = download_pack(pack=pack, abs_repo_base=abs_repo_base, + verify_ssl=verifyssl, force=force, + proxy_config=self.proxy_config, + force_permissions=True, + use_python3=python3, + logger=self.logger) + pack_url, pack_ref, pack_result = pack_result + result[pack_ref] = pack_result return self._validate_result(result=result, repo_url=pack_url) diff --git a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py new file mode 100644 index 0000000000..cffbc7f35b --- /dev/null +++ b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py @@ -0,0 +1,133 @@ +# Copyright 2019 Extreme Networks, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +import six + +from st2common.constants.pack import PACK_VERSION_SEPARATOR +from st2common.content.utils import get_pack_base_path +from st2common.runners.base_action import Action +from st2common.util.pack import get_pack_metadata + +STACKSTORM_EXCHANGE_PACK = 'StackStorm-Exchange/stackstorm-' +LOCAL_FILE_PREFIX = 'file://' + + +class GetPackDependencies(Action): + def __init__(self, config=None, action_service=None): + self.dependency_list = [] + self.conflict_list = [] + + def run(self, packs_status, nested): + """ + :param packs_status: Name of the pack in Exchange or a git repo URL and download status. + :type: packs_status: ``dict`` + + :param nested: Nested level of dependencies to prevent infinite or really + long download loops. + :type nested: ``integer`` + """ + result = {} + + if not packs_status or nested == 0: + return result + + for pack, status in six.iteritems(packs_status): + if 'success' not in status.lower(): + continue + + dependency_packs = get_dependency_list(pack) + if not dependency_packs: + continue + + for dependency_pack in dependency_packs: + pack_and_version = dependency_pack.split(PACK_VERSION_SEPARATOR) + name_or_url = pack_and_version[0] + pack_version = pack_and_version[1] if len(pack_and_version) > 1 else None + + if name_or_url.startswith(LOCAL_FILE_PREFIX): + self.get_local_pack_dependencies(name_or_url, pack_version, dependency_pack) + elif (len(name_or_url.split('/')) == 1 and STACKSTORM_EXCHANGE_PACK not in + name_or_url) or STACKSTORM_EXCHANGE_PACK in name_or_url: + self.get_exchange_pack_dependencies(name_or_url, pack_version, dependency_pack) + else: + self.get_none_exchange_pack_dependencies(name_or_url, pack_version, + dependency_pack) + + result['dependency_list'] = self.dependency_list + result['conflict_list'] = self.conflict_list + result['nested'] = nested - 1 + + return result + + # For StackStorm Exchange packs. E.g. + # email + # https://github.com/StackStorm-Exchange/stackstorm-email + # https://github.com/StackStorm-Exchange/stackstorm-email.git + def get_exchange_pack_dependencies(self, name_or_url, pack_version, dependency_pack): + if len(name_or_url.split('/')) == 1: + pack_name = name_or_url + else: + name_or_git = name_or_url.split(STACKSTORM_EXCHANGE_PACK)[-1] + pack_name = name_or_git if '.git' not in name_or_git else name_or_git.split('.')[0] + + existing_pack_version = get_pack_version(pack_name) + self.check_conflict(dependency_pack, pack_version, existing_pack_version) + + # For None StackStorm Exchange packs. E.g. + # https://github.com/EncoreTechnologies/stackstorm-freeipa.git + # https://github.com/EncoreTechnologies/stackstorm-freeipa + # https://github.com/EncoreTechnologies/pack.git + def get_none_exchange_pack_dependencies(self, name_or_url, pack_version, dependency_pack): + name_or_git = name_or_url.split('/')[-1] + name = name_or_git if '.git' not in name_or_git else name_or_git.split('.')[0] + pack_name = name.split('-')[-1] if "stackstorm-" in name else name + + existing_pack_version = get_pack_version(pack_name) + self.check_conflict(dependency_pack, pack_version, existing_pack_version) + + # For local file. E.g + # file:///opt/stackstorm/st2/lib/python3.6/site-packages/st2tests/fixtures/packs/dummy_pack_3 + def get_local_pack_dependencies(self, name_or_url, pack_version, dependency_pack): + pack_name = name_or_url.split("/")[-1] + + existing_pack_version = get_pack_version(pack_name) + self.check_conflict(dependency_pack, pack_version, existing_pack_version) + + def check_conflict(self, pack, version, existing_version): + if existing_version: + existing_version = 'v' + existing_version + if version and existing_version != version and pack not in self.conflict_list: + self.conflict_list.append(pack) + elif pack not in self.dependency_list: + self.dependency_list.append(pack) + + +def get_pack_version(pack=None): + pack_path = get_pack_base_path(pack) + try: + pack_metadata = get_pack_metadata(pack_dir=pack_path) + return pack_metadata.get('version', None) + except Exception: + return None + + +def get_dependency_list(pack=None): + pack_path = get_pack_base_path(pack) + + try: + pack_metadata = get_pack_metadata(pack_dir=pack_path) + return pack_metadata.get('dependencies', None) + except Exception: + print ('Could not open pack.yaml at location %s' % pack_path) + return None diff --git a/contrib/packs/actions/pack_mgmt/register.py b/contrib/packs/actions/pack_mgmt/register.py index d59ba21513..d79324a55d 100644 --- a/contrib/packs/actions/pack_mgmt/register.py +++ b/contrib/packs/actions/pack_mgmt/register.py @@ -72,6 +72,7 @@ def run(self, register, packs=None): 'types': types } + packs.reverse() if packs: method_kwargs['packs'] = packs diff --git a/contrib/packs/actions/pack_mgmt/virtualenv_setup_prerun.py b/contrib/packs/actions/pack_mgmt/virtualenv_setup_prerun.py index 5801c4e83a..91a7731128 100644 --- a/contrib/packs/actions/pack_mgmt/virtualenv_setup_prerun.py +++ b/contrib/packs/actions/pack_mgmt/virtualenv_setup_prerun.py @@ -18,13 +18,22 @@ class PacksTransformationAction(Action): - def run(self, packs_status): + def run(self, packs_status, packs_list=None): """ :param packs_status: Result from packs.download action. :type: packs_status: ``dict`` + + :param packs_list: Names of the pack in Exchange, a git repo URL or local file system. + :type: packs_list: ``list`` """ + if not packs_list: + packs_list = [] + packs = [] for pack_name, status in six.iteritems(packs_status): if 'success' in status.lower(): packs.append(pack_name) - return packs + + packs_list.extend(packs) + + return packs_list diff --git a/contrib/packs/actions/virtualenv_prerun.yaml b/contrib/packs/actions/virtualenv_prerun.yaml index f65bf198cb..74448d2937 100644 --- a/contrib/packs/actions/virtualenv_prerun.yaml +++ b/contrib/packs/actions/virtualenv_prerun.yaml @@ -7,3 +7,10 @@ parameters: packs_status: type: "object" + packs_list: + type: array + items: + type: string + required: false + default: null + description: Names of the pack in Exchange, a git repo URL or local file system. diff --git a/contrib/packs/actions/workflows/install.yaml b/contrib/packs/actions/workflows/install.yaml index e1412fb5ee..7159d77541 100644 --- a/contrib/packs/actions/workflows/install.yaml +++ b/contrib/packs/actions/workflows/install.yaml @@ -1,32 +1,103 @@ ---- - chain: - - - name: "download pack" - ref: "packs.download" - parameters: - packs: "{{packs}}" - force: "{{force}}" - python3: "{{python3}}" - on-success: "make a prerun" - - - name: "make a prerun" - ref: "packs.virtualenv_prerun" - parameters: - packs_status: "{{ __results['download pack'].result }}" - on-success: "install pack dependencies" - - - name: "install pack dependencies" - ref: "packs.setup_virtualenv" - parameters: - packs: "{{ __results['make a prerun'].result }}" - env: "{{env}}" - python3: "{{python3}}" - on-success: "register pack" - - - name: "register pack" - ref: "packs.load" - parameters: - register: "{{register}}" - packs: "{{ __results['make a prerun'].result }}" - - default: "download pack" +version: 1.0 + +description: A orquesta workflow to install packs. + +input: + - packs + - register + - env + - force + - python3 + - skip + +vars: + - packs_list: null + - dependency_list: null + - conflict_list: null + - nested: 3 + +tasks: + init_task: + action: core.noop + next: + - do: download_pack + + download_pack: + action: packs.download + input: + packs: <% ctx().packs %> + force: <% ctx().force %> + python3: <% ctx().python3 %> + dependency_list: <% ctx().dependency_list %> + next: + - when: <% succeeded() %> + do: make_a_prerun + + make_a_prerun: + action: packs.virtualenv_prerun + input: + packs_status: <% task(download_pack).result.result %> + packs_list: <% ctx().packs_list %> + next: + - when: <% succeeded() %> + publish: + - packs_list: <% task(make_a_prerun).result.result %> + do: find_next_action + + find_next_action: + action: core.noop + next: + - when: <% ctx().skip %> + do: install_pack_requirements + - when: <% ctx().nested = 0 and not ctx().skip %> + do: install_pack_requirements + - when: <% ctx().nested > 0 and not ctx().skip %> + do: get_pack_dependencies + + get_pack_dependencies: + action: packs.get_pack_dependencies + input: + packs_status: <% task(download_pack).result.result %> + nested: <% ctx().nested%> + next: + - when: <% succeeded() %> + publish: + - dependency_list: <% task(get_pack_dependencies).result.result.dependency_list %> + - conflict_list: <% task(get_pack_dependencies).result.result.conflict_list %> + - nested: <% task(get_pack_dependencies).result.result.nested %> + do: check_dependency_and_conflict_list + + check_dependency_and_conflict_list: + action: core.noop + next: + - when: <% ctx().conflict_list %> + do: stop_installation_and_cleanup + - when: <% not ctx().conflict_list and ctx().dependency_list %> + do: download_pack + - when: <% not ctx().conflict_list and not ctx().dependency_list %> + do: install_pack_requirements + + stop_installation_and_cleanup: + action: packs.delete + input: + packs: <% ctx().packs_list %> + delete_env: <% false %> + + install_pack_requirements: + action: packs.setup_virtualenv + input: + packs: <% ctx().packs_list %> + env: <% ctx().env %> + python3: <% ctx().python3 %> + next: + - when: <% succeeded() %> + do: register_pack + + register_pack: + action: packs.load + input: + register: <% ctx().register %> + packs: <% ctx().packs_list %> + +output: + - packs_list: <% ctx().packs_list %> diff --git a/st2api/st2api/controllers/v1/packs.py b/st2api/st2api/controllers/v1/packs.py index 9be03b504d..7910ac12b5 100644 --- a/st2api/st2api/controllers/v1/packs.py +++ b/st2api/st2api/controllers/v1/packs.py @@ -103,6 +103,9 @@ def post(self, pack_install_request, requester_user=None): if pack_install_request.force: parameters['force'] = True + if pack_install_request.skip: + parameters['skip'] = True + if not requester_user: requester_user = UserDB(cfg.CONF.system_user.user) diff --git a/st2client/st2client/commands/pack.py b/st2client/st2client/commands/pack.py index 4e7c7cb084..001335704c 100644 --- a/st2client/st2client/commands/pack.py +++ b/st2client/st2client/commands/pack.py @@ -130,7 +130,8 @@ def run_and_print(self, args, **kwargs): if getattr(execution, 'parent', None) == parent_id: status = execution.status - name = execution.context['chain']['name'] + name = execution.context['orquesta']['task_name'] \ + if 'orquesta' in execution.context else execution.context['chain']['name'] if status == LIVEACTION_STATUS_SCHEDULED: indicator.add_stage(status, name) @@ -194,6 +195,10 @@ def __init__(self, resource, *args, **kwargs): action='store_true', default=False, help='Force pack installation.') + self.parser.add_argument('--skip', + action='store_true', + default=False, + help='Skip pack dependency installation.') def run(self, args, **kwargs): is_structured_output = args.json or args.yaml @@ -203,7 +208,8 @@ def run(self, args, **kwargs): if not is_structured_output: self._get_content_counts_for_pack(args, **kwargs) - return self.manager.install(args.packs, python3=args.python3, force=args.force, **kwargs) + return self.manager.install(args.packs, python3=args.python3, force=args.force, + skip=args.skip, **kwargs) def _get_content_counts_for_pack(self, args, **kwargs): # Global content list, excluding "tests" @@ -258,7 +264,7 @@ def _print_pack_content(pack_name, pack_content): def run_and_print(self, args, **kwargs): instance = super(PackInstallCommand, self).run_and_print(args, **kwargs) # Hack to get a list of resolved references of installed packs - packs = instance.result['tasks'][1]['result']['result'] + packs = instance.result['output']['packs_list'] if len(packs) == 1: pack_instance = self.app.client.managers['Pack'].get_by_ref_or_id(packs[0], **kwargs) diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index 42e7fb7b72..d0950c9057 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -494,12 +494,13 @@ class AsyncRequest(Resource): class PackResourceManager(ResourceManager): @add_auth_token_to_kwargs_from_env - def install(self, packs, force=False, python3=False, **kwargs): + def install(self, packs, force=False, python3=False, skip=False, **kwargs): url = '/%s/install' % (self.resource.get_url_path_name()) payload = { 'packs': packs, 'force': force, - 'python3': python3 + 'python3': python3, + 'skip': skip } response = self.client.post(url, payload, **kwargs) if response.status_code != http_client.OK: diff --git a/st2common/st2common/openapi.yaml b/st2common/st2common/openapi.yaml index 535876b1c6..1757dd6156 100644 --- a/st2common/st2common/openapi.yaml +++ b/st2common/st2common/openapi.yaml @@ -5028,6 +5028,11 @@ definitions: description: Force pack installation. type: boolean default: false + skip: + type: boolean + description: Set to True to skip pack dependency installations. + required: false + default: false PacksUninstall: type: object properties: diff --git a/st2common/st2common/openapi.yaml.j2 b/st2common/st2common/openapi.yaml.j2 index d69e73eed4..e6864128a0 100644 --- a/st2common/st2common/openapi.yaml.j2 +++ b/st2common/st2common/openapi.yaml.j2 @@ -5024,6 +5024,11 @@ definitions: description: Force pack installation. type: boolean default: false + skip: + type: boolean + description: Set to True to skip pack dependency installations. + required: false + default: false PacksUninstall: type: object properties: From 754f6321eefba62b21f68441472ff687c8902d04 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Thu, 22 Aug 2019 11:29:22 -0700 Subject: [PATCH 02/18] Add unit test cases for Pack dependency management Two new test files for testing get_pack_dependencies.py and virtualenv_setup_prerun.py New test cases for download dependencies packs. New test cases for `skip` install dependency flag. Modified install workflow for more simplicity. --- contrib/packs/actions/workflows/install.yaml | 4 +- contrib/packs/tests/test_action_download.py | 26 ++ .../packs/tests/test_get_pack_dependencies.py | 238 ++++++++++++++++++ .../tests/test_virtualenv_setup_prerun.py | 47 ++++ .../tests/unit/controllers/v1/test_packs.py | 10 + 5 files changed, 322 insertions(+), 3 deletions(-) create mode 100644 contrib/packs/tests/test_get_pack_dependencies.py create mode 100644 contrib/packs/tests/test_virtualenv_setup_prerun.py diff --git a/contrib/packs/actions/workflows/install.yaml b/contrib/packs/actions/workflows/install.yaml index 7159d77541..a522058fbe 100644 --- a/contrib/packs/actions/workflows/install.yaml +++ b/contrib/packs/actions/workflows/install.yaml @@ -47,9 +47,7 @@ tasks: find_next_action: action: core.noop next: - - when: <% ctx().skip %> - do: install_pack_requirements - - when: <% ctx().nested = 0 and not ctx().skip %> + - when: <% ctx().skip or ctx().nested = 0 %> do: install_pack_requirements - when: <% ctx().nested > 0 and not ctx().skip %> do: get_pack_dependencies diff --git a/contrib/packs/tests/test_action_download.py b/contrib/packs/tests/test_action_download.py index 7d0becc2b3..2b5b3bc828 100644 --- a/contrib/packs/tests/test_action_download.py +++ b/contrib/packs/tests/test_action_download.py @@ -65,6 +65,14 @@ "keywords": ["some", "special", "terms"], "email": "info@stackstorm.com", "description": "another st2 pack to test package management pipeline" + }, + "test4": { + "version": "0.5.0", + "name": "test4", + "repo_url": "https://github.com/StackStorm-Exchange/stackstorm-test4", + "author": "stanley", + "keywords": ["some", "special", "terms"], "email": "info@stackstorm.com", + "description": "another st2 pack to test package management pipeline" } } @@ -148,6 +156,24 @@ def test_run_pack_download(self): self.repo_instance.git.branch.assert_called() self.repo_instance.git.checkout.assert_called() + def test_run_pack_download_dependencies(self): + action = self.get_action_instance() + result = action.run(packs=['test'], dependency_list=['test2', 'test4'], + abs_repo_base=self.repo_base) + temp_dirs = [ + hashlib.md5(PACK_INDEX['test2']['repo_url'].encode()).hexdigest(), + hashlib.md5(PACK_INDEX['test4']['repo_url'].encode()).hexdigest() + ] + + self.assertEqual(result, {'test2': 'Success.', 'test4': 'Success.'}) + self.clone_from.assert_any_call(PACK_INDEX['test2']['repo_url'], + os.path.join(os.path.expanduser('~'), temp_dirs[0])) + self.clone_from.assert_any_call(PACK_INDEX['test4']['repo_url'], + os.path.join(os.path.expanduser('~'), temp_dirs[1])) + self.assertEqual(self.clone_from.call_count, 2) + self.assertTrue(os.path.isfile(os.path.join(self.repo_base, 'test2/pack.yaml'))) + self.assertTrue(os.path.isfile(os.path.join(self.repo_base, 'test4/pack.yaml'))) + def test_run_pack_download_existing_pack(self): action = self.get_action_instance() action.run(packs=['test'], abs_repo_base=self.repo_base) diff --git a/contrib/packs/tests/test_get_pack_dependencies.py b/contrib/packs/tests/test_get_pack_dependencies.py new file mode 100644 index 0000000000..0262a462f2 --- /dev/null +++ b/contrib/packs/tests/test_get_pack_dependencies.py @@ -0,0 +1,238 @@ +#!/usr/bin/env python + +# Copyright 2019 Extreme Networks, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +import mock + +from st2tests.base import BaseActionTestCase + +from pack_mgmt.get_pack_dependencies import GetPackDependencies + +UNINSTALLED_PACK = 'uninstalled_pack' +UNINSTALLED_PACK_URL = 'https://github.com/StackStorm-Exchange/stackstorm-' + UNINSTALLED_PACK\ + + '.git' +UNINSTALLED_PRIVATE_PACK_URL = 'https://github.com/privaterepo/' + UNINSTALLED_PACK + '.git=v4.5.0' +UNINSTALLED_LOCAL_PACK = 'file:///opt/pack_dir/' + UNINSTALLED_PACK + '=v4.5.0' + +DOWNLOADED_OR_INSTALLED_PACK_METAdATA = { + # No dependencies. + "no_dependencies": { + "version": "0.4.0", + "name": "no_dependencies", + "repo_url": "https://github.com/StackStorm-Exchange/stackstorm-no_dependencies", + "author": "st2-dev", + "keywords": ["some", "search", "another", "terms"], + "email": "info@stackstorm.com", + "description": "st2 pack to test package management pipeline", + }, + # One uninstalled and one installed dependency packs. + "test2": { + "version": "0.5.0", + "name": "test2", + "repo_url": "https://github.com/StackStorm-Exchange/stackstorm-test2", + "author": "stanley", + "keywords": ["some", "special", "terms"], + "email": "info@stackstorm.com", + "description": "another st2 pack to test package management pipeline", + "dependencies": [ + UNINSTALLED_PACK, "test3" + ] + }, + # One uninstalled, one installed and one conflict dependency packs. + "test3": { + "version": "0.6.0", + "stackstorm_version": ">=1.6.0, <2.2.0", + "name": "test3", + "repo_url": "https://github.com/StackStorm-Exchange/stackstorm-test3", + "author": "stanley", + "keywords": ["some", "special", "terms"], + "email": "info@stackstorm.com", + "description": "another st2 pack to test package management pipeline", + "dependencies": [ + UNINSTALLED_PACK, "test2=v0.4.0", "test4=v0.7.0" + ] + }, + # One uninstalled, one installed and one conflict with StackStorm Exchange urls. + "test4": { + "version": "0.7.0", + "stackstorm_version": ">=1.6.0, <2.2.0", + "name": "test4", + "repo_url": "https://github.com/StackStorm-Exchange/stackstorm-test4", + "author": "stanley", + "keywords": ["some", "special", "terms"], + "email": "info@stackstorm.com", + "description": "another st2 pack to test package management pipeline", + "dependencies": [ + UNINSTALLED_PACK_URL, + "https://github.com/StackStorm-Exchange/stackstorm-test2=v0.4.0", + "https://github.com/StackStorm-Exchange/stackstorm-test5.git" + ] + }, + # One uninstalled, one installed and one conflict private urls. + "test5": { + "version": "0.8.0", + "stackstorm_version": ">=1.6.0, <2.2.0", + "name": "test3", + "repo_url": "https://github.com/StackStorm-Exchange/stackstorm-test5", + "author": "stanley", + "keywords": ["some", "special", "terms"], + "email": "info@stackstorm.com", + "description": "another st2 pack to test package management pipeline", + "dependencies": [ + UNINSTALLED_PRIVATE_PACK_URL, + "https://github.com/privaterepo/test4.git=v0.5.0", + "https://github.com/privaterepo/test6.git" + ] + }, + # One uninstalled, one installed and one conflict local files. + "test6": { + "version": "0.9.0", + "stackstorm_version": ">=1.6.0, <2.2.0", + "name": "test3", + "repo_url": "https://github.com/StackStorm-Exchange/stackstorm-test6", + "author": "stanley", + "keywords": ["some", "special", "terms"], + "email": "info@stackstorm.com", + "description": "another st2 pack to test package management pipeline", + "dependencies": [ + UNINSTALLED_LOCAL_PACK, + "file:///opt/pack_dir/test2=v0.7.0", + "file:///opt/some_dir/test5=v0.8.0" + ] + } +} + + +def mock_get_dependency_list(pack): + """ + Mock get_dependency_list function which return dependencies list + """ + dependencies = None + + if pack in DOWNLOADED_OR_INSTALLED_PACK_METAdATA: + metadata = DOWNLOADED_OR_INSTALLED_PACK_METAdATA[pack] + dependencies = metadata.get('dependencies', None) + + return dependencies + + +def mock_get_pack_version(pack): + """ + Mock get_pack_version function which return mocked pack version + """ + version = None + + if pack in DOWNLOADED_OR_INSTALLED_PACK_METAdATA: + metadata = DOWNLOADED_OR_INSTALLED_PACK_METAdATA[pack] + version = metadata.get('version', None) + + return version + + +@mock.patch('pack_mgmt.get_pack_dependencies.get_dependency_list', mock_get_dependency_list) +@mock.patch('pack_mgmt.get_pack_dependencies.get_pack_version', mock_get_pack_version) +class GetPackDependenciesTestCase(BaseActionTestCase): + action_cls = GetPackDependencies + + def setUp(self): + super(GetPackDependenciesTestCase, self).setUp() + + def test_run_get_pack_dependencies_with_nested_zero_value(self): + action = self.get_action_instance() + packs_status = {"test": "Success."} + nested = 0 + + result = action.run(packs_status=packs_status, nested=nested) + self.assertEqual(result, {}) + + def test_run_get_pack_dependencies_with_empty_packs_status(self): + action = self.get_action_instance() + packs_status = None + nested = 3 + + result = action.run(packs_status=packs_status, nested=nested) + self.assertEqual(result, {}) + + def test_run_get_pack_dependencies_with_failed_packs_status(self): + action = self.get_action_instance() + packs_status = {"test3": "Failed."} + nested = 2 + + result = action.run(packs_status=packs_status, nested=nested) + self.assertEqual(result['dependency_list'], []) + self.assertEqual(result['conflict_list'], []) + self.assertEqual(result['nested'], nested - 1) + + def test_run_get_pack_dependencies_with_failed_and_succeeded_packs_status(self): + action = self.get_action_instance() + packs_status = {"test3": "Failed.", "test2": "Success."} + nested = 2 + + result = action.run(packs_status=packs_status, nested=nested) + self.assertEqual(result['dependency_list'], [UNINSTALLED_PACK]) + self.assertEqual(result['conflict_list'], []) + self.assertEqual(result['nested'], nested - 1) + + def test_run_get_pack_dependencies_with_no_dependency(self): + action = self.get_action_instance() + packs_status = {"no_dependencies": "Success."} + nested = 3 + + result = action.run(packs_status=packs_status, nested=nested) + self.assertEqual(result['dependency_list'], []) + self.assertEqual(result['conflict_list'], []) + self.assertEqual(result['nested'], nested - 1) + + def test_run_get_pack_dependencies(self): + action = self.get_action_instance() + packs_status = {"test3": "Success.", "test2": "Success."} + nested = 1 + + result = action.run(packs_status=packs_status, nested=nested) + self.assertEqual(result['dependency_list'], [UNINSTALLED_PACK]) + self.assertEqual(result['conflict_list'], ['test2=v0.4.0']) + self.assertEqual(result['nested'], nested - 1) + + def test_run_get_pack_dependencies_with_stackstorm_exchange_url(self): + action = self.get_action_instance() + packs_status = {"test4": "Success."} + nested = 3 + + result = action.run(packs_status=packs_status, nested=nested) + self.assertEqual(result['dependency_list'], [UNINSTALLED_PACK_URL]) + self.assertEqual(result['conflict_list'], + ['https://github.com/StackStorm-Exchange/stackstorm-test2=v0.4.0']) + self.assertEqual(result['nested'], nested - 1) + + def test_run_get_pack_dependencies_with_private_url(self): + action = self.get_action_instance() + packs_status = {"test5": "Success."} + nested = 3 + + result = action.run(packs_status=packs_status, nested=nested) + self.assertEqual(result['dependency_list'], [UNINSTALLED_PRIVATE_PACK_URL]) + self.assertEqual(result['conflict_list'], + ['https://github.com/privaterepo/test4.git=v0.5.0']) + self.assertEqual(result['nested'], nested - 1) + + def test_run_get_pack_dependencies_with_local_file(self): + action = self.get_action_instance() + packs_status = {"test6": "Success."} + nested = 3 + + result = action.run(packs_status=packs_status, nested=nested) + self.assertEqual(result['dependency_list'], [UNINSTALLED_LOCAL_PACK]) + self.assertEqual(result['conflict_list'], ["file:///opt/pack_dir/test2=v0.7.0"]) + self.assertEqual(result['nested'], nested - 1) diff --git a/contrib/packs/tests/test_virtualenv_setup_prerun.py b/contrib/packs/tests/test_virtualenv_setup_prerun.py new file mode 100644 index 0000000000..141aab3026 --- /dev/null +++ b/contrib/packs/tests/test_virtualenv_setup_prerun.py @@ -0,0 +1,47 @@ +#!/usr/bin/env python + +# Copyright 2019 Extreme Networks, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +from st2tests.base import BaseActionTestCase + +from pack_mgmt.virtualenv_setup_prerun import PacksTransformationAction + + +class VirtualenvSetUpPreRunTestCase(BaseActionTestCase): + action_cls = PacksTransformationAction + + def setUp(self): + super(VirtualenvSetUpPreRunTestCase, self).setUp() + + def test_run_with_pack_list(self): + action = self.get_action_instance() + result = action.run(packs_status={'test1': 'Success.', 'test2': 'Success.'}, + packs_list=['test3', 'test4']) + + self.assertEqual(result, ['test3', 'test4', 'test1', 'test2']) + + def test_run_with_none_pack_list(self): + action = self.get_action_instance() + result = action.run(packs_status={'test1': 'Success.', 'test2': 'Success.'}, + packs_list=None) + + self.assertEqual(result, ['test1', 'test2']) + + def test_run_with_failed_status(self): + action = self.get_action_instance() + result = action.run(packs_status={'test1': 'Failed.', 'test2': 'Success.'}, + packs_list=['test3', 'test4']) + + self.assertEqual(result, ['test3', 'test4', 'test2']) diff --git a/st2api/tests/unit/controllers/v1/test_packs.py b/st2api/tests/unit/controllers/v1/test_packs.py index 03d62ed2e7..96e5f292ab 100644 --- a/st2api/tests/unit/controllers/v1/test_packs.py +++ b/st2api/tests/unit/controllers/v1/test_packs.py @@ -177,6 +177,16 @@ def test_install_with_force_parameter(self, _handle_schedule_execution): self.assertEqual(resp.status_int, 202) self.assertEqual(resp.json, {'execution_id': '123'}) + @mock.patch.object(ActionExecutionsControllerMixin, '_handle_schedule_execution') + def test_install_with_skip_parameter(self, _handle_schedule_execution): + _handle_schedule_execution.return_value = Response(json={'id': '123'}) + payload = {'packs': ['some'], 'skip': True} + + resp = self.app.post_json('/v1/packs/install', payload) + + self.assertEqual(resp.status_int, 202) + self.assertEqual(resp.json, {'execution_id': '123'}) + @mock.patch.object(ActionExecutionsControllerMixin, '_handle_schedule_execution') def test_uninstall(self, _handle_schedule_execution): _handle_schedule_execution.return_value = Response(json={'id': '123'}) From 6bb65120e17b02d58ddc8929c756af5b2c22399d Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Thu, 22 Aug 2019 14:33:08 -0700 Subject: [PATCH 03/18] Add pack `ref` in pack install/remove command output When install/remove list of packs, if the pack `ref` is not same as pack `name`, the pack information is not printed. See file: https://github.com/StackStorm/st2/blob/master/st2tests/st2tests/fixtures/packs/dummy_pack_19/pack.yaml For example: `st2 pack install files:///dummy_pack_19 csv` Actually result: +---------------+-------------------------------+---------+--------------+ | name | description | version | author | +---------------+-------------------------------+---------+--------------+ | email | E-Mail Actions/Sensors for | 1.1.3 | James Fryman | | | StackStorm | | | +---------------+-------------------------------+---------+--------------+ Expected result: +-------------------+---------------+-------------------------------+---------+--------------+ | ref | name | description | version | author | +-------------------+---------------+-------------------------------+---------+--------------+ | dummy_pack_19_ref | dummy_pack_19 | dummy pack | 0.1.0 | st2-dev | | email | email | E-Mail Actions/Sensors for | 1.1.3 | James Fryman | | | | StackStorm | | | +-------------------+---------------+-------------------------------+---------+--------------+ Root Cause: Pack install/remove is not query `ref` as print out parameter, and only pack `name` is checked to find out pack. When pack `name` and `ref` is not the same, pack information is not printed. --- st2client/st2client/commands/pack.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/st2client/st2client/commands/pack.py b/st2client/st2client/commands/pack.py index 001335704c..9431eb033d 100644 --- a/st2client/st2client/commands/pack.py +++ b/st2client/st2client/commands/pack.py @@ -96,7 +96,7 @@ def __init__(self, *args, **kwargs): detail_arg_grp = self.parser.add_mutually_exclusive_group() detail_arg_grp.add_argument('--attr', nargs='+', - default=['name', 'description', 'version', 'author'], + default=['ref', 'name', 'description', 'version', 'author'], help=('List of attributes to include in the ' 'output. "all" or unspecified will ' 'return all attributes.')) @@ -276,7 +276,7 @@ def run_and_print(self, args, **kwargs): pack_instances = [] for pack in all_pack_instances: - if pack.name in packs: + if pack.name in packs or pack.ref in packs: pack_instances.append(pack) self.print_output(pack_instances, table.MultiColumnTable, @@ -324,7 +324,7 @@ def run_and_print(self, args, **kwargs): pack_instances = [] for pack in all_pack_instances: - if pack.name in packs: + if pack.name in packs or pack.ref in packs: pack_instances.append(pack) if pack in remaining_pack_instances: raise OperationFailureException('Pack %s has not been removed properly' From 2f4e361fe7c1295b4df036af4366f19b5c38a775 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Fri, 23 Aug 2019 16:47:52 -0700 Subject: [PATCH 04/18] Code improvement and simplify getting dependencies code 1. Some English wording changes. 2. Change "st2 pack install" input from "skip" to "no_deps". 3. Increase Nested level of dependencies to prevent infinite loop from 3 to 10. 4. Simplify getting dependencies pack code, reduce duplication, and remove some assumptions about pack name (But for some case, it still need to check `stackstrom-` for some StackStorm Exchange packs.) 5. Modified test code accordingly. --- CHANGELOG.rst | 2 +- contrib/packs/actions/delete.yaml | 2 +- contrib/packs/actions/install.meta.yaml | 2 +- .../pack_mgmt/get_pack_dependencies.py | 89 +++++--------- contrib/packs/actions/workflows/install.yaml | 12 +- .../packs/tests/test_get_pack_dependencies.py | 109 ++++++------------ st2api/st2api/controllers/v1/packs.py | 4 +- .../tests/unit/controllers/v1/test_packs.py | 4 +- st2client/st2client/commands/pack.py | 4 +- st2client/st2client/models/core.py | 4 +- st2common/st2common/openapi.yaml | 2 +- st2common/st2common/openapi.yaml.j2 | 2 +- 12 files changed, 82 insertions(+), 154 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cc5bcf7fe3..268023c7b3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,7 +10,7 @@ Added * Add support for blacklisting / whitelisting hosts to the HTTP runner by adding new ``url_hosts_blacklist`` and ``url_hosts_whitelist`` runner attribute. (new feature) #4757 -* Add support for Pack dependency management to install dependency packs and find out confilct packs. +* Add support to manage dependency conflicts on pack installation. #4769 Changed diff --git a/contrib/packs/actions/delete.yaml b/contrib/packs/actions/delete.yaml index 36f5f51023..c8eb18f842 100644 --- a/contrib/packs/actions/delete.yaml +++ b/contrib/packs/actions/delete.yaml @@ -16,6 +16,6 @@ immutable: true delete_env: type: "boolean" - description: Flag for if need to delete virtual environment. + description: Delete virtual environment for pack when set to True. default: true required: false diff --git a/contrib/packs/actions/install.meta.yaml b/contrib/packs/actions/install.meta.yaml index d1f93782fa..6ba45e6c42 100644 --- a/contrib/packs/actions/install.meta.yaml +++ b/contrib/packs/actions/install.meta.yaml @@ -32,7 +32,7 @@ description: "Use Python 3 binary when creating a virtualenv for this pack." required: false default: false - skip: + no_deps: type: "boolean" description: "Set to True to skip pack dependency installations." required: false diff --git a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py index cffbc7f35b..04e4ba638a 100644 --- a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py +++ b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py @@ -19,15 +19,8 @@ from st2common.runners.base_action import Action from st2common.util.pack import get_pack_metadata -STACKSTORM_EXCHANGE_PACK = 'StackStorm-Exchange/stackstorm-' -LOCAL_FILE_PREFIX = 'file://' - class GetPackDependencies(Action): - def __init__(self, config=None, action_service=None): - self.dependency_list = [] - self.conflict_list = [] - def run(self, packs_status, nested): """ :param packs_status: Name of the pack in Exchange or a git repo URL and download status. @@ -38,6 +31,8 @@ def run(self, packs_status, nested): :type nested: ``integer`` """ result = {} + dependency_list = [] + conflict_list = [] if not packs_status or nested == 0: return result @@ -50,68 +45,40 @@ def run(self, packs_status, nested): if not dependency_packs: continue - for dependency_pack in dependency_packs: - pack_and_version = dependency_pack.split(PACK_VERSION_SEPARATOR) + for dep_pack in dependency_packs: + pack_and_version = dep_pack.split(PACK_VERSION_SEPARATOR) name_or_url = pack_and_version[0] pack_version = pack_and_version[1] if len(pack_and_version) > 1 else None - if name_or_url.startswith(LOCAL_FILE_PREFIX): - self.get_local_pack_dependencies(name_or_url, pack_version, dependency_pack) - elif (len(name_or_url.split('/')) == 1 and STACKSTORM_EXCHANGE_PACK not in - name_or_url) or STACKSTORM_EXCHANGE_PACK in name_or_url: - self.get_exchange_pack_dependencies(name_or_url, pack_version, dependency_pack) + if len(name_or_url.split('/')) == 1: + pack_name = name_or_url else: - self.get_none_exchange_pack_dependencies(name_or_url, pack_version, - dependency_pack) - - result['dependency_list'] = self.dependency_list - result['conflict_list'] = self.conflict_list + name_or_git = name_or_url.split("/")[-1] + pack_name = name_or_git if '.git' not in name_or_git else \ + name_or_git.split('.')[0] + + # Check existing pack by pack name + existing_pack_version = get_pack_version(pack_name) + + # Try one more time to get existing pack version by name if 'stackstorm-' is in + # pack name + if not existing_pack_version and 'stackstorm-' in pack_name.lower(): + existing_pack_version = get_pack_version(pack_name.split('stackstorm-')[-1]) + + if existing_pack_version: + existing_pack_version = 'v' + existing_pack_version + if pack_version and existing_pack_version != pack_version \ + and dep_pack not in conflict_list: + conflict_list.append(dep_pack) + elif dep_pack not in dependency_list: + dependency_list.append(dep_pack) + + result['dependency_list'] = dependency_list + result['conflict_list'] = conflict_list result['nested'] = nested - 1 return result - # For StackStorm Exchange packs. E.g. - # email - # https://github.com/StackStorm-Exchange/stackstorm-email - # https://github.com/StackStorm-Exchange/stackstorm-email.git - def get_exchange_pack_dependencies(self, name_or_url, pack_version, dependency_pack): - if len(name_or_url.split('/')) == 1: - pack_name = name_or_url - else: - name_or_git = name_or_url.split(STACKSTORM_EXCHANGE_PACK)[-1] - pack_name = name_or_git if '.git' not in name_or_git else name_or_git.split('.')[0] - - existing_pack_version = get_pack_version(pack_name) - self.check_conflict(dependency_pack, pack_version, existing_pack_version) - - # For None StackStorm Exchange packs. E.g. - # https://github.com/EncoreTechnologies/stackstorm-freeipa.git - # https://github.com/EncoreTechnologies/stackstorm-freeipa - # https://github.com/EncoreTechnologies/pack.git - def get_none_exchange_pack_dependencies(self, name_or_url, pack_version, dependency_pack): - name_or_git = name_or_url.split('/')[-1] - name = name_or_git if '.git' not in name_or_git else name_or_git.split('.')[0] - pack_name = name.split('-')[-1] if "stackstorm-" in name else name - - existing_pack_version = get_pack_version(pack_name) - self.check_conflict(dependency_pack, pack_version, existing_pack_version) - - # For local file. E.g - # file:///opt/stackstorm/st2/lib/python3.6/site-packages/st2tests/fixtures/packs/dummy_pack_3 - def get_local_pack_dependencies(self, name_or_url, pack_version, dependency_pack): - pack_name = name_or_url.split("/")[-1] - - existing_pack_version = get_pack_version(pack_name) - self.check_conflict(dependency_pack, pack_version, existing_pack_version) - - def check_conflict(self, pack, version, existing_version): - if existing_version: - existing_version = 'v' + existing_version - if version and existing_version != version and pack not in self.conflict_list: - self.conflict_list.append(pack) - elif pack not in self.dependency_list: - self.dependency_list.append(pack) - def get_pack_version(pack=None): pack_path = get_pack_base_path(pack) diff --git a/contrib/packs/actions/workflows/install.yaml b/contrib/packs/actions/workflows/install.yaml index a522058fbe..5c28f1a76c 100644 --- a/contrib/packs/actions/workflows/install.yaml +++ b/contrib/packs/actions/workflows/install.yaml @@ -8,13 +8,13 @@ input: - env - force - python3 - - skip + - no_deps vars: - packs_list: null - dependency_list: null - conflict_list: null - - nested: 3 + - nested: 10 tasks: init_task: @@ -47,9 +47,9 @@ tasks: find_next_action: action: core.noop next: - - when: <% ctx().skip or ctx().nested = 0 %> + - when: <% ctx().no_deps or ctx().nested = 0 %> do: install_pack_requirements - - when: <% ctx().nested > 0 and not ctx().skip %> + - when: <% ctx().nested > 0 and not ctx().no_deps %> do: get_pack_dependencies get_pack_dependencies: @@ -69,13 +69,13 @@ tasks: action: core.noop next: - when: <% ctx().conflict_list %> - do: stop_installation_and_cleanup + do: stop_installation_and_cleanup_because_conflict - when: <% not ctx().conflict_list and ctx().dependency_list %> do: download_pack - when: <% not ctx().conflict_list and not ctx().dependency_list %> do: install_pack_requirements - stop_installation_and_cleanup: + stop_installation_and_cleanup_because_conflict: action: packs.delete input: packs: <% ctx().packs_list %> diff --git a/contrib/packs/tests/test_get_pack_dependencies.py b/contrib/packs/tests/test_get_pack_dependencies.py index 0262a462f2..2291ece41d 100644 --- a/contrib/packs/tests/test_get_pack_dependencies.py +++ b/contrib/packs/tests/test_get_pack_dependencies.py @@ -21,10 +21,21 @@ from pack_mgmt.get_pack_dependencies import GetPackDependencies UNINSTALLED_PACK = 'uninstalled_pack' -UNINSTALLED_PACK_URL = 'https://github.com/StackStorm-Exchange/stackstorm-' + UNINSTALLED_PACK\ - + '.git' -UNINSTALLED_PRIVATE_PACK_URL = 'https://github.com/privaterepo/' + UNINSTALLED_PACK + '.git=v4.5.0' -UNINSTALLED_LOCAL_PACK = 'file:///opt/pack_dir/' + UNINSTALLED_PACK + '=v4.5.0' +UNINSTALLED_PACKS = [ + UNINSTALLED_PACK, + 'https://github.com/StackStorm-Exchange/stackstorm-pack1', + 'https://github.com/StackStorm-Exchange/stackstorm-pack2.git', + 'https://github.com/StackStorm-Exchange/stackstorm-pack3.git=v2.1.1', + 'StackStorm-Exchange/stackstorm-pack4', + 'git://StackStorm-Exchange/stackstorm-pack5=v2.1.1', + 'git://StackStorm-Exchange/stackstorm-pack6.git', + 'git@github.com:foo/pack7.git' + 'git@github.com:foo/pack8.git=v3.2.1', + 'file:///home/vagrant/stackstorm-pack9', + 'file://localhost/home/vagrant/stackstorm-pack10', + 'ssh:///AutomationStackStorm11', + 'ssh://joe@local/AutomationStackStorm12' +] DOWNLOADED_OR_INSTALLED_PACK_METAdATA = { # No dependencies. @@ -46,11 +57,9 @@ "keywords": ["some", "special", "terms"], "email": "info@stackstorm.com", "description": "another st2 pack to test package management pipeline", - "dependencies": [ - UNINSTALLED_PACK, "test3" - ] + "dependencies": ['uninstalled_pack', 'no_dependencies'] }, - # One uninstalled, one installed and one conflict dependency packs. + # List of uninstalled dependency packs. "test3": { "version": "0.6.0", "stackstorm_version": ">=1.6.0, <2.2.0", @@ -60,11 +69,9 @@ "keywords": ["some", "special", "terms"], "email": "info@stackstorm.com", "description": "another st2 pack to test package management pipeline", - "dependencies": [ - UNINSTALLED_PACK, "test2=v0.4.0", "test4=v0.7.0" - ] + "dependencies": UNINSTALLED_PACKS }, - # One uninstalled, one installed and one conflict with StackStorm Exchange urls. + # One conflict pack. "test4": { "version": "0.7.0", "stackstorm_version": ">=1.6.0, <2.2.0", @@ -75,41 +82,7 @@ "email": "info@stackstorm.com", "description": "another st2 pack to test package management pipeline", "dependencies": [ - UNINSTALLED_PACK_URL, - "https://github.com/StackStorm-Exchange/stackstorm-test2=v0.4.0", - "https://github.com/StackStorm-Exchange/stackstorm-test5.git" - ] - }, - # One uninstalled, one installed and one conflict private urls. - "test5": { - "version": "0.8.0", - "stackstorm_version": ">=1.6.0, <2.2.0", - "name": "test3", - "repo_url": "https://github.com/StackStorm-Exchange/stackstorm-test5", - "author": "stanley", - "keywords": ["some", "special", "terms"], - "email": "info@stackstorm.com", - "description": "another st2 pack to test package management pipeline", - "dependencies": [ - UNINSTALLED_PRIVATE_PACK_URL, - "https://github.com/privaterepo/test4.git=v0.5.0", - "https://github.com/privaterepo/test6.git" - ] - }, - # One uninstalled, one installed and one conflict local files. - "test6": { - "version": "0.9.0", - "stackstorm_version": ">=1.6.0, <2.2.0", - "name": "test3", - "repo_url": "https://github.com/StackStorm-Exchange/stackstorm-test6", - "author": "stanley", - "keywords": ["some", "special", "terms"], - "email": "info@stackstorm.com", - "description": "another st2 pack to test package management pipeline", - "dependencies": [ - UNINSTALLED_LOCAL_PACK, - "file:///opt/pack_dir/test2=v0.7.0", - "file:///opt/some_dir/test5=v0.8.0" + "test2=v0.4.0" ] } } @@ -167,7 +140,7 @@ def test_run_get_pack_dependencies_with_empty_packs_status(self): def test_run_get_pack_dependencies_with_failed_packs_status(self): action = self.get_action_instance() - packs_status = {"test3": "Failed."} + packs_status = {"test": "Failed."} nested = 2 result = action.run(packs_status=packs_status, nested=nested) @@ -177,7 +150,7 @@ def test_run_get_pack_dependencies_with_failed_packs_status(self): def test_run_get_pack_dependencies_with_failed_and_succeeded_packs_status(self): action = self.get_action_instance() - packs_status = {"test3": "Failed.", "test2": "Success."} + packs_status = {"test": "Failed.", "test2": "Success."} nested = 2 result = action.run(packs_status=packs_status, nested=nested) @@ -195,44 +168,32 @@ def test_run_get_pack_dependencies_with_no_dependency(self): self.assertEqual(result['conflict_list'], []) self.assertEqual(result['nested'], nested - 1) - def test_run_get_pack_dependencies(self): + def test_run_get_pack_dependencies_with_dependency(self): action = self.get_action_instance() - packs_status = {"test3": "Success.", "test2": "Success."} + packs_status = {"test2": "Success."} nested = 1 result = action.run(packs_status=packs_status, nested=nested) self.assertEqual(result['dependency_list'], [UNINSTALLED_PACK]) - self.assertEqual(result['conflict_list'], ['test2=v0.4.0']) - self.assertEqual(result['nested'], nested - 1) - - def test_run_get_pack_dependencies_with_stackstorm_exchange_url(self): - action = self.get_action_instance() - packs_status = {"test4": "Success."} - nested = 3 - - result = action.run(packs_status=packs_status, nested=nested) - self.assertEqual(result['dependency_list'], [UNINSTALLED_PACK_URL]) - self.assertEqual(result['conflict_list'], - ['https://github.com/StackStorm-Exchange/stackstorm-test2=v0.4.0']) + self.assertEqual(result['conflict_list'], []) self.assertEqual(result['nested'], nested - 1) - def test_run_get_pack_dependencies_with_private_url(self): + def test_run_get_pack_dependencies_with_dependencies(self): action = self.get_action_instance() - packs_status = {"test5": "Success."} - nested = 3 + packs_status = {"test3": "Success."} + nested = 1 result = action.run(packs_status=packs_status, nested=nested) - self.assertEqual(result['dependency_list'], [UNINSTALLED_PRIVATE_PACK_URL]) - self.assertEqual(result['conflict_list'], - ['https://github.com/privaterepo/test4.git=v0.5.0']) + self.assertEqual(result['dependency_list'], UNINSTALLED_PACKS) + self.assertEqual(result['conflict_list'], []) self.assertEqual(result['nested'], nested - 1) - def test_run_get_pack_dependencies_with_local_file(self): + def test_run_get_pack_dependencies_with_conflict(self): action = self.get_action_instance() - packs_status = {"test6": "Success."} - nested = 3 + packs_status = {"test2": "Success.", "test4": "Success."} + nested = 1 result = action.run(packs_status=packs_status, nested=nested) - self.assertEqual(result['dependency_list'], [UNINSTALLED_LOCAL_PACK]) - self.assertEqual(result['conflict_list'], ["file:///opt/pack_dir/test2=v0.7.0"]) + self.assertEqual(result['dependency_list'], [UNINSTALLED_PACK]) + self.assertEqual(result['conflict_list'], ['test2=v0.4.0']) self.assertEqual(result['nested'], nested - 1) diff --git a/st2api/st2api/controllers/v1/packs.py b/st2api/st2api/controllers/v1/packs.py index 7910ac12b5..c9039f1287 100644 --- a/st2api/st2api/controllers/v1/packs.py +++ b/st2api/st2api/controllers/v1/packs.py @@ -103,8 +103,8 @@ def post(self, pack_install_request, requester_user=None): if pack_install_request.force: parameters['force'] = True - if pack_install_request.skip: - parameters['skip'] = True + if pack_install_request.no_deps: + parameters['no_deps'] = True if not requester_user: requester_user = UserDB(cfg.CONF.system_user.user) diff --git a/st2api/tests/unit/controllers/v1/test_packs.py b/st2api/tests/unit/controllers/v1/test_packs.py index 96e5f292ab..97b4cd8846 100644 --- a/st2api/tests/unit/controllers/v1/test_packs.py +++ b/st2api/tests/unit/controllers/v1/test_packs.py @@ -178,9 +178,9 @@ def test_install_with_force_parameter(self, _handle_schedule_execution): self.assertEqual(resp.json, {'execution_id': '123'}) @mock.patch.object(ActionExecutionsControllerMixin, '_handle_schedule_execution') - def test_install_with_skip_parameter(self, _handle_schedule_execution): + def test_install_with_no_deps_parameter(self, _handle_schedule_execution): _handle_schedule_execution.return_value = Response(json={'id': '123'}) - payload = {'packs': ['some'], 'skip': True} + payload = {'packs': ['some'], 'no_deps': True} resp = self.app.post_json('/v1/packs/install', payload) diff --git a/st2client/st2client/commands/pack.py b/st2client/st2client/commands/pack.py index 9431eb033d..5858c662fa 100644 --- a/st2client/st2client/commands/pack.py +++ b/st2client/st2client/commands/pack.py @@ -195,7 +195,7 @@ def __init__(self, resource, *args, **kwargs): action='store_true', default=False, help='Force pack installation.') - self.parser.add_argument('--skip', + self.parser.add_argument('--no_deps', action='store_true', default=False, help='Skip pack dependency installation.') @@ -209,7 +209,7 @@ def run(self, args, **kwargs): self._get_content_counts_for_pack(args, **kwargs) return self.manager.install(args.packs, python3=args.python3, force=args.force, - skip=args.skip, **kwargs) + no_deps=args.no_deps, **kwargs) def _get_content_counts_for_pack(self, args, **kwargs): # Global content list, excluding "tests" diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index d0950c9057..98f76fcd8a 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -494,13 +494,13 @@ class AsyncRequest(Resource): class PackResourceManager(ResourceManager): @add_auth_token_to_kwargs_from_env - def install(self, packs, force=False, python3=False, skip=False, **kwargs): + def install(self, packs, force=False, python3=False, no_deps=False, **kwargs): url = '/%s/install' % (self.resource.get_url_path_name()) payload = { 'packs': packs, 'force': force, 'python3': python3, - 'skip': skip + 'no_deps': no_deps } response = self.client.post(url, payload, **kwargs) if response.status_code != http_client.OK: diff --git a/st2common/st2common/openapi.yaml b/st2common/st2common/openapi.yaml index 1757dd6156..dc1362da5c 100644 --- a/st2common/st2common/openapi.yaml +++ b/st2common/st2common/openapi.yaml @@ -5028,7 +5028,7 @@ definitions: description: Force pack installation. type: boolean default: false - skip: + no_deps: type: boolean description: Set to True to skip pack dependency installations. required: false diff --git a/st2common/st2common/openapi.yaml.j2 b/st2common/st2common/openapi.yaml.j2 index e6864128a0..43315af6b2 100644 --- a/st2common/st2common/openapi.yaml.j2 +++ b/st2common/st2common/openapi.yaml.j2 @@ -5024,7 +5024,7 @@ definitions: description: Force pack installation. type: boolean default: false - skip: + no_deps: type: boolean description: Set to True to skip pack dependency installations. required: false From 1a9c02683aa74850b1d2142f669836479e588558 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Wed, 28 Aug 2019 16:48:50 -0700 Subject: [PATCH 05/18] Check conflict between dependency packs Check conflict between dependency packs and add unit tests accordingly. --- .../pack_mgmt/get_pack_dependencies.py | 33 +++++++++++-- .../packs/tests/test_get_pack_dependencies.py | 46 ++++++++++++++++++- 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py index 04e4ba638a..ef1503800d 100644 --- a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py +++ b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py @@ -46,9 +46,7 @@ def run(self, packs_status, nested): continue for dep_pack in dependency_packs: - pack_and_version = dep_pack.split(PACK_VERSION_SEPARATOR) - name_or_url = pack_and_version[0] - pack_version = pack_and_version[1] if len(pack_and_version) > 1 else None + name_or_url, pack_version = self.get_name_and_version(dep_pack) if len(name_or_url.split('/')) == 1: pack_name = name_or_url @@ -70,8 +68,13 @@ def run(self, packs_status, nested): if pack_version and existing_pack_version != pack_version \ and dep_pack not in conflict_list: conflict_list.append(dep_pack) - elif dep_pack not in dependency_list: - dependency_list.append(dep_pack) + else: + conflict = self.check_dependency_list_for_conflict(name_or_url, pack_version, + dependency_list) + if conflict: + conflict_list.append(dep_pack) + elif dep_pack not in dependency_list: + dependency_list.append(dep_pack) result['dependency_list'] = dependency_list result['conflict_list'] = conflict_list @@ -79,6 +82,26 @@ def run(self, packs_status, nested): return result + def check_dependency_list_for_conflict(self, name, version, dependency_list): + conflict = False + + for pack in dependency_list: + name_or_url, pack_version = self.get_name_and_version(pack) + if name == name_or_url: + if version != pack_version: + conflict = True + break + + return conflict + + @staticmethod + def get_name_and_version(pack): + pack_and_version = pack.split(PACK_VERSION_SEPARATOR) + name_or_url = pack_and_version[0] + pack_version = pack_and_version[1] if len(pack_and_version) > 1 else None + + return name_or_url, pack_version + def get_pack_version(pack=None): pack_path = get_pack_base_path(pack) diff --git a/contrib/packs/tests/test_get_pack_dependencies.py b/contrib/packs/tests/test_get_pack_dependencies.py index 2291ece41d..6df299bb3e 100644 --- a/contrib/packs/tests/test_get_pack_dependencies.py +++ b/contrib/packs/tests/test_get_pack_dependencies.py @@ -71,7 +71,7 @@ "description": "another st2 pack to test package management pipeline", "dependencies": UNINSTALLED_PACKS }, - # One conflict pack. + # One conflict pack with existing pack. "test4": { "version": "0.7.0", "stackstorm_version": ">=1.6.0, <2.2.0", @@ -84,6 +84,28 @@ "dependencies": [ "test2=v0.4.0" ] + }, + # One uninstalled conflict pack. + "test5": { + "version": "0.7.0", + "stackstorm_version": ">=1.6.0, <2.2.0", + "name": "test4", + "repo_url": "https://github.com/StackStorm-Exchange/stackstorm-test4", + "author": "stanley", + "keywords": ["some", "special", "terms"], "email": "info@stackstorm.com", + "description": "another st2 pack to test package management pipeline", + "dependencies": ["uninstalled_pack=v0.4.0"] + }, + # One dependency pack without version. It is not checked against conflict. + "test6": { + "version": "0.7.0", + "stackstorm_version": ">=1.6.0, <2.2.0", + "name": "test4", + "repo_url": "https://github.com/StackStorm-Exchange/stackstorm-test4", + "author": "stanley", + "keywords": ["some", "special", "terms"], "email": "info@stackstorm.com", + "description": "another st2 pack to test package management pipeline", + "dependencies": ["test2"] } } @@ -188,7 +210,7 @@ def test_run_get_pack_dependencies_with_dependencies(self): self.assertEqual(result['conflict_list'], []) self.assertEqual(result['nested'], nested - 1) - def test_run_get_pack_dependencies_with_conflict(self): + def test_run_get_pack_dependencies_with_existing_pack_conflict(self): action = self.get_action_instance() packs_status = {"test2": "Success.", "test4": "Success."} nested = 1 @@ -197,3 +219,23 @@ def test_run_get_pack_dependencies_with_conflict(self): self.assertEqual(result['dependency_list'], [UNINSTALLED_PACK]) self.assertEqual(result['conflict_list'], ['test2=v0.4.0']) self.assertEqual(result['nested'], nested - 1) + + def test_run_get_pack_dependencies_with_dependency_conflict(self): + action = self.get_action_instance() + packs_status = {"test2": "Success.", "test5": "Success."} + nested = 1 + + result = action.run(packs_status=packs_status, nested=nested) + self.assertEqual(result['dependency_list'], ['uninstalled_pack']) + self.assertEqual(result['conflict_list'], ['uninstalled_pack=v0.4.0']) + self.assertEqual(result['nested'], nested - 1) + + def test_run_get_pack_dependencies_with_no_version(self): + action = self.get_action_instance() + packs_status = {"test2": "Success.", "test6": "Success."} + nested = 1 + + result = action.run(packs_status=packs_status, nested=nested) + self.assertEqual(result['dependency_list'], [UNINSTALLED_PACK]) + self.assertEqual(result['conflict_list'], []) + self.assertEqual(result['nested'], nested - 1) From 925794d55ebca2f5ae1527e5bad214ba580bf96c Mon Sep 17 00:00:00 2001 From: blag Date: Wed, 4 Sep 2019 15:02:17 -0600 Subject: [PATCH 06/18] Rename --no_deps parameter to --skip-dependencies --- contrib/packs/actions/install.meta.yaml | 2 +- contrib/packs/actions/workflows/install.yaml | 6 +++--- st2api/st2api/controllers/v1/packs.py | 4 ++-- st2api/tests/unit/controllers/v1/test_packs.py | 4 ++-- st2client/st2client/commands/pack.py | 4 ++-- st2client/st2client/models/core.py | 4 ++-- st2common/st2common/openapi.yaml | 2 +- st2common/st2common/openapi.yaml.j2 | 2 +- 8 files changed, 14 insertions(+), 14 deletions(-) diff --git a/contrib/packs/actions/install.meta.yaml b/contrib/packs/actions/install.meta.yaml index 6ba45e6c42..1b6401ce4c 100644 --- a/contrib/packs/actions/install.meta.yaml +++ b/contrib/packs/actions/install.meta.yaml @@ -32,7 +32,7 @@ description: "Use Python 3 binary when creating a virtualenv for this pack." required: false default: false - no_deps: + skip_dependencies: type: "boolean" description: "Set to True to skip pack dependency installations." required: false diff --git a/contrib/packs/actions/workflows/install.yaml b/contrib/packs/actions/workflows/install.yaml index 5c28f1a76c..f87e864677 100644 --- a/contrib/packs/actions/workflows/install.yaml +++ b/contrib/packs/actions/workflows/install.yaml @@ -8,7 +8,7 @@ input: - env - force - python3 - - no_deps + - skip_dependencies vars: - packs_list: null @@ -47,9 +47,9 @@ tasks: find_next_action: action: core.noop next: - - when: <% ctx().no_deps or ctx().nested = 0 %> + - when: <% ctx().skip_dependencies or ctx().nested = 0 %> do: install_pack_requirements - - when: <% ctx().nested > 0 and not ctx().no_deps %> + - when: <% ctx().nested > 0 and not ctx().skip_dependencies %> do: get_pack_dependencies get_pack_dependencies: diff --git a/st2api/st2api/controllers/v1/packs.py b/st2api/st2api/controllers/v1/packs.py index c9039f1287..6478037f55 100644 --- a/st2api/st2api/controllers/v1/packs.py +++ b/st2api/st2api/controllers/v1/packs.py @@ -103,8 +103,8 @@ def post(self, pack_install_request, requester_user=None): if pack_install_request.force: parameters['force'] = True - if pack_install_request.no_deps: - parameters['no_deps'] = True + if pack_install_request.skip_dependencies: + parameters['skip_dependencies'] = True if not requester_user: requester_user = UserDB(cfg.CONF.system_user.user) diff --git a/st2api/tests/unit/controllers/v1/test_packs.py b/st2api/tests/unit/controllers/v1/test_packs.py index 97b4cd8846..d6a09f00c1 100644 --- a/st2api/tests/unit/controllers/v1/test_packs.py +++ b/st2api/tests/unit/controllers/v1/test_packs.py @@ -178,9 +178,9 @@ def test_install_with_force_parameter(self, _handle_schedule_execution): self.assertEqual(resp.json, {'execution_id': '123'}) @mock.patch.object(ActionExecutionsControllerMixin, '_handle_schedule_execution') - def test_install_with_no_deps_parameter(self, _handle_schedule_execution): + def test_install_with_skip_dependencies_parameter(self, _handle_schedule_execution): _handle_schedule_execution.return_value = Response(json={'id': '123'}) - payload = {'packs': ['some'], 'no_deps': True} + payload = {'packs': ['some'], 'skip_dependencies': True} resp = self.app.post_json('/v1/packs/install', payload) diff --git a/st2client/st2client/commands/pack.py b/st2client/st2client/commands/pack.py index 5858c662fa..533d78d78a 100644 --- a/st2client/st2client/commands/pack.py +++ b/st2client/st2client/commands/pack.py @@ -195,7 +195,7 @@ def __init__(self, resource, *args, **kwargs): action='store_true', default=False, help='Force pack installation.') - self.parser.add_argument('--no_deps', + self.parser.add_argument('--skip-dependencies', action='store_true', default=False, help='Skip pack dependency installation.') @@ -209,7 +209,7 @@ def run(self, args, **kwargs): self._get_content_counts_for_pack(args, **kwargs) return self.manager.install(args.packs, python3=args.python3, force=args.force, - no_deps=args.no_deps, **kwargs) + skip_dependencies=args.skip_dependencies, **kwargs) def _get_content_counts_for_pack(self, args, **kwargs): # Global content list, excluding "tests" diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index 98f76fcd8a..229d5d7434 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -494,13 +494,13 @@ class AsyncRequest(Resource): class PackResourceManager(ResourceManager): @add_auth_token_to_kwargs_from_env - def install(self, packs, force=False, python3=False, no_deps=False, **kwargs): + def install(self, packs, force=False, python3=False, skip_dependencies=False, **kwargs): url = '/%s/install' % (self.resource.get_url_path_name()) payload = { 'packs': packs, 'force': force, 'python3': python3, - 'no_deps': no_deps + 'skip_dependencies': skip_dependencies } response = self.client.post(url, payload, **kwargs) if response.status_code != http_client.OK: diff --git a/st2common/st2common/openapi.yaml b/st2common/st2common/openapi.yaml index dc1362da5c..91e299ff05 100644 --- a/st2common/st2common/openapi.yaml +++ b/st2common/st2common/openapi.yaml @@ -5028,7 +5028,7 @@ definitions: description: Force pack installation. type: boolean default: false - no_deps: + skip_dependencies: type: boolean description: Set to True to skip pack dependency installations. required: false diff --git a/st2common/st2common/openapi.yaml.j2 b/st2common/st2common/openapi.yaml.j2 index 43315af6b2..253f45cfde 100644 --- a/st2common/st2common/openapi.yaml.j2 +++ b/st2common/st2common/openapi.yaml.j2 @@ -5024,7 +5024,7 @@ definitions: description: Force pack installation. type: boolean default: false - no_deps: + skip_dependencies: type: boolean description: Set to True to skip pack dependency installations. required: false From 1f4ecc6ead99008ebd859546c0978e0a40fc93f4 Mon Sep 17 00:00:00 2001 From: blag Date: Fri, 6 Sep 2019 15:47:10 -0600 Subject: [PATCH 07/18] Use Python's print function --- contrib/packs/actions/pack_mgmt/get_pack_dependencies.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py index ef1503800d..d9b1fc5631 100644 --- a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py +++ b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py @@ -11,6 +11,7 @@ # 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. +from __future__ import print_function import six @@ -119,5 +120,5 @@ def get_dependency_list(pack=None): pack_metadata = get_pack_metadata(pack_dir=pack_path) return pack_metadata.get('dependencies', None) except Exception: - print ('Could not open pack.yaml at location %s' % pack_path) + print('Could not open pack.yaml at location %s' % pack_path) return None From bdb4aa9b4501933a7905454eb050a0990dd1710d Mon Sep 17 00:00:00 2001 From: blag Date: Fri, 6 Sep 2019 15:49:48 -0600 Subject: [PATCH 08/18] Use one return statement in try/except/finally blocks --- .../packs/actions/pack_mgmt/get_pack_dependencies.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py index d9b1fc5631..6521d17f0e 100644 --- a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py +++ b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py @@ -108,9 +108,11 @@ def get_pack_version(pack=None): pack_path = get_pack_base_path(pack) try: pack_metadata = get_pack_metadata(pack_dir=pack_path) - return pack_metadata.get('version', None) + result = pack_metadata.get('version', None) except Exception: - return None + result = None + finally: + return result def get_dependency_list(pack=None): @@ -118,7 +120,9 @@ def get_dependency_list(pack=None): try: pack_metadata = get_pack_metadata(pack_dir=pack_path) - return pack_metadata.get('dependencies', None) + result = pack_metadata.get('dependencies', None) except Exception: print('Could not open pack.yaml at location %s' % pack_path) - return None + result = None + finally: + return result From b5067874c08d15e1f88a9800ba9119a66dcf2c87 Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 19 Sep 2019 01:20:59 -0700 Subject: [PATCH 09/18] Simplify install workflow --- contrib/packs/actions/workflows/install.yaml | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/contrib/packs/actions/workflows/install.yaml b/contrib/packs/actions/workflows/install.yaml index f87e864677..dbf9eaeb93 100644 --- a/contrib/packs/actions/workflows/install.yaml +++ b/contrib/packs/actions/workflows/install.yaml @@ -39,17 +39,13 @@ tasks: packs_status: <% task(download_pack).result.result %> packs_list: <% ctx().packs_list %> next: - - when: <% succeeded() %> + - when: <% succeeded() and (ctx().skip_dependencies or ctx().nested = 0) %> publish: - packs_list: <% task(make_a_prerun).result.result %> - do: find_next_action - - find_next_action: - action: core.noop - next: - - when: <% ctx().skip_dependencies or ctx().nested = 0 %> do: install_pack_requirements - - when: <% ctx().nested > 0 and not ctx().skip_dependencies %> + - when: <% succeeded() and (ctx().nested > 0 and not ctx().skip_dependencies) %> + publish: + - packs_list: <% task(make_a_prerun).result.result %> do: get_pack_dependencies get_pack_dependencies: From a8166d415f472904e90c4ffba812fec38386a1b5 Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 19 Sep 2019 01:21:48 -0700 Subject: [PATCH 10/18] Explicitly fail --- contrib/packs/actions/workflows/install.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/packs/actions/workflows/install.yaml b/contrib/packs/actions/workflows/install.yaml index dbf9eaeb93..545ba86fdb 100644 --- a/contrib/packs/actions/workflows/install.yaml +++ b/contrib/packs/actions/workflows/install.yaml @@ -75,7 +75,9 @@ tasks: action: packs.delete input: packs: <% ctx().packs_list %> - delete_env: <% false %> + delete_env: false + next: + - do: fail install_pack_requirements: action: packs.setup_virtualenv From 6560d36cfbce98312ab60fb3299c5ef2ef930bea Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 19 Sep 2019 01:22:29 -0700 Subject: [PATCH 11/18] Fixup pack_management.py to handle tags that aren't on branches --- st2common/st2common/util/pack_management.py | 30 +++++++++++++-------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/st2common/st2common/util/pack_management.py b/st2common/st2common/util/pack_management.py index ab323ffcb6..b32961b3d5 100644 --- a/st2common/st2common/util/pack_management.py +++ b/st2common/st2common/util/pack_management.py @@ -241,19 +241,27 @@ def clone_repo(temp_dir, repo_url, verify_ssl=True, ref='master'): # We're trying to figure out which branch the ref is actually on, # since there's no direct way to check for this in git-python. branches = repo.git.branch('-a', '--contains', gitref.hexsha) # pylint: disable=no-member - branches = branches.replace('*', '').split() - if active_branch.name not in branches or use_branch: - branch = 'origin/%s' % ref if use_branch else branches[0] - short_branch = ref if use_branch else branches[0].split('/')[-1] - repo.git.checkout('-b', short_branch, branch) - branch = repo.head.reference - else: - branch = repo.active_branch.name + # Git tags aren't necessarily on a branch. + # If this is the case, gitref will be the tag name, but branches will be + # empty. + # We also need to checkout tags slightly differently than branches. + if branches: + branches = branches.replace('*', '').split() + + if active_branch.name not in branches or use_branch: + branch = 'origin/%s' % ref if use_branch else branches[0] + short_branch = ref if use_branch else branches[0].split('/')[-1] + repo.git.checkout('-b', short_branch, branch) + branch = repo.head.reference + else: + branch = repo.active_branch.name - repo.git.checkout(gitref.hexsha) # pylint: disable=no-member - repo.git.branch('-f', branch, gitref.hexsha) # pylint: disable=no-member - repo.git.checkout(branch) + repo.git.checkout(gitref.hexsha) # pylint: disable=no-member + repo.git.branch('-f', branch, gitref.hexsha) # pylint: disable=no-member + repo.git.checkout(branch) + else: + repo.git.checkout('v%s' % ref) # pylint: disable=no-member return temp_dir From fc6a9ea91665b5d0fabda64ffef79dfc4576cf4d Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 19 Sep 2019 02:15:14 -0700 Subject: [PATCH 12/18] Further simplify install workflow --- contrib/packs/actions/workflows/install.yaml | 26 +++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/contrib/packs/actions/workflows/install.yaml b/contrib/packs/actions/workflows/install.yaml index 545ba86fdb..0252d9c359 100644 --- a/contrib/packs/actions/workflows/install.yaml +++ b/contrib/packs/actions/workflows/install.yaml @@ -54,21 +54,23 @@ tasks: packs_status: <% task(download_pack).result.result %> nested: <% ctx().nested%> next: - - when: <% succeeded() %> + - when: <% succeeded() and result().result.conflict_list %> publish: - - dependency_list: <% task(get_pack_dependencies).result.result.dependency_list %> - - conflict_list: <% task(get_pack_dependencies).result.result.conflict_list %> - - nested: <% task(get_pack_dependencies).result.result.nested %> - do: check_dependency_and_conflict_list - - check_dependency_and_conflict_list: - action: core.noop - next: - - when: <% ctx().conflict_list %> + - dependency_list: <% result().result.dependency_list %> + - conflict_list: <% result().result.conflict_list %> + - nested: <% result().result.nested %> do: stop_installation_and_cleanup_because_conflict - - when: <% not ctx().conflict_list and ctx().dependency_list %> + - when: <% succeeded() and ctx().dependency_list and not ctx().conflict_list %> + publish: + - dependency_list: <% result().result.dependency_list %> + - conflict_list: <% result().result.conflict_list %> + - nested: <% result().result.nested %> do: download_pack - - when: <% not ctx().conflict_list and not ctx().dependency_list %> + - when: <% succeeded() and not ctx().conflict_list and not ctx().dependency_list %> + publish: + - dependency_list: <% result().result.dependency_list %> + - conflict_list: <% result().result.conflict_list %> + - nested: <% result().result.nested %> do: install_pack_requirements stop_installation_and_cleanup_because_conflict: From fb2932166408d2572dae9ee57c8e64c212b2d1a3 Mon Sep 17 00:00:00 2001 From: blag Date: Thu, 19 Sep 2019 16:07:53 -0700 Subject: [PATCH 13/18] Fix overzealous simplification --- contrib/packs/actions/workflows/install.yaml | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/contrib/packs/actions/workflows/install.yaml b/contrib/packs/actions/workflows/install.yaml index 0252d9c359..e4c64408b2 100644 --- a/contrib/packs/actions/workflows/install.yaml +++ b/contrib/packs/actions/workflows/install.yaml @@ -54,23 +54,21 @@ tasks: packs_status: <% task(download_pack).result.result %> nested: <% ctx().nested%> next: - - when: <% succeeded() and result().result.conflict_list %> + - when: <% succeeded() %> publish: - dependency_list: <% result().result.dependency_list %> - conflict_list: <% result().result.conflict_list %> - nested: <% result().result.nested %> + do: check_dependency_and_conflict_list + + check_dependency_and_conflict_list: + action: core.noop + next: + - when: <% ctx().conflict_list %> do: stop_installation_and_cleanup_because_conflict - - when: <% succeeded() and ctx().dependency_list and not ctx().conflict_list %> - publish: - - dependency_list: <% result().result.dependency_list %> - - conflict_list: <% result().result.conflict_list %> - - nested: <% result().result.nested %> + - when: <% not ctx().conflict_list and ctx().dependency_list %> do: download_pack - - when: <% succeeded() and not ctx().conflict_list and not ctx().dependency_list %> - publish: - - dependency_list: <% result().result.dependency_list %> - - conflict_list: <% result().result.conflict_list %> - - nested: <% result().result.nested %> + - when: <% not ctx().conflict_list and not ctx().dependency_list %> do: install_pack_requirements stop_installation_and_cleanup_because_conflict: From b91a05a1858f3972592bb49c370ab7cd0a2a9c3f Mon Sep 17 00:00:00 2001 From: blag Date: Fri, 20 Sep 2019 14:16:47 -0700 Subject: [PATCH 14/18] Fix version comparisons --- contrib/packs/actions/pack_mgmt/get_pack_dependencies.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py index 6521d17f0e..d3919fa48f 100644 --- a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py +++ b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py @@ -65,7 +65,10 @@ def run(self, packs_status, nested): existing_pack_version = get_pack_version(pack_name.split('stackstorm-')[-1]) if existing_pack_version: - existing_pack_version = 'v' + existing_pack_version + if not existing_pack_version.startswith('v'): + existing_pack_version = 'v' + existing_pack_version + if not pack_version.startswith('v'): + pack_version = 'v' + pack_version if pack_version and existing_pack_version != pack_version \ and dep_pack not in conflict_list: conflict_list.append(dep_pack) From 1c3b243c48b9a181a32b3706fc54014477e8613f Mon Sep 17 00:00:00 2001 From: blag Date: Fri, 20 Sep 2019 14:17:00 -0700 Subject: [PATCH 15/18] Bump packs pack version --- contrib/packs/pack.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/packs/pack.yaml b/contrib/packs/pack.yaml index aa07727c4c..531bd24c34 100644 --- a/contrib/packs/pack.yaml +++ b/contrib/packs/pack.yaml @@ -1,7 +1,7 @@ --- name : packs description : Pack management functionality. -version : 1.0.1 +version : 2.0.0 python_versions: - "2" - "3" From b8b480f3fbc381488471adfa50400dbb34ad6cac Mon Sep 17 00:00:00 2001 From: blag Date: Fri, 20 Sep 2019 14:38:23 -0700 Subject: [PATCH 16/18] Handle pack versions that are None --- contrib/packs/actions/pack_mgmt/get_pack_dependencies.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py index d3919fa48f..b28776e182 100644 --- a/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py +++ b/contrib/packs/actions/pack_mgmt/get_pack_dependencies.py @@ -65,9 +65,9 @@ def run(self, packs_status, nested): existing_pack_version = get_pack_version(pack_name.split('stackstorm-')[-1]) if existing_pack_version: - if not existing_pack_version.startswith('v'): + if existing_pack_version and not existing_pack_version.startswith('v'): existing_pack_version = 'v' + existing_pack_version - if not pack_version.startswith('v'): + if pack_version and not pack_version.startswith('v'): pack_version = 'v' + pack_version if pack_version and existing_pack_version != pack_version \ and dep_pack not in conflict_list: From 105c5889dec8c16e26b1d15bb8fdad3e9850fb93 Mon Sep 17 00:00:00 2001 From: blag Date: Fri, 20 Sep 2019 16:20:10 -0700 Subject: [PATCH 17/18] Add dependencies list example to hello_st2/pack.yaml --- contrib/hello_st2/pack.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contrib/hello_st2/pack.yaml b/contrib/hello_st2/pack.yaml index 365ec13e40..afd34066c3 100644 --- a/contrib/hello_st2/pack.yaml +++ b/contrib/hello_st2/pack.yaml @@ -16,6 +16,12 @@ version: 0.1.0 python_versions: - "2" - "3" +# New in StackStorm 3.2 +# Specify a list of dependency packs to install. If the pack is in StackStorm Exchange you can use +# the pack name, or you can specify a full Git repository URL. Optionally, you can specify the +# exact version, tag, or branch. +dependencies: + - core # Name of the pack author. author: StackStorm, Inc. # Email of the pack author. From 32ad61f2b9a19e1c36b0e32c73a3e86009e4a620 Mon Sep 17 00:00:00 2001 From: blag Date: Fri, 20 Sep 2019 22:54:18 -0700 Subject: [PATCH 18/18] Betterize error messages and add a core.error action --- contrib/core/actions/error.yaml | 24 ++++++++++++++++++++ contrib/packs/actions/workflows/install.yaml | 19 +++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 contrib/core/actions/error.yaml diff --git a/contrib/core/actions/error.yaml b/contrib/core/actions/error.yaml new file mode 100644 index 0000000000..089aeb9737 --- /dev/null +++ b/contrib/core/actions/error.yaml @@ -0,0 +1,24 @@ +--- +description: Action that executes the Linux echo command (to stderr) on the localhost. +runner_type: "local-shell-cmd" +enabled: true +entry_point: '' +name: error +parameters: + message: + description: The message that the command will echo to stderr. + type: string + required: true + cmd: + description: Arbitrary Linux command to be executed on the local host. + required: true + type: string + default: '>&2 echo "{{message}}"' + immutable: true + kwarg_op: + immutable: true + sudo: + default: false + immutable: true + sudo_password: + immutable: true diff --git a/contrib/packs/actions/workflows/install.yaml b/contrib/packs/actions/workflows/install.yaml index e4c64408b2..ceb8091ce3 100644 --- a/contrib/packs/actions/workflows/install.yaml +++ b/contrib/packs/actions/workflows/install.yaml @@ -15,6 +15,7 @@ vars: - dependency_list: null - conflict_list: null - nested: 10 + - message: "" tasks: init_task: @@ -77,7 +78,18 @@ tasks: packs: <% ctx().packs_list %> delete_env: false next: - - do: fail + - do: echo_pack_conflicts + + echo_pack_conflicts: + action: core.noop + next: + - publish: + - message: >- + Unable to install packs due to conflicts. Review the + conflict_list and check the versions of corresponding installed + packs. You can also run the `st2 pack install` command with the + `--skip-dependencies` flag to skip installing dependent packs. + do: fail install_pack_requirements: action: packs.setup_virtualenv @@ -94,6 +106,11 @@ tasks: input: register: <% ctx().register %> packs: <% ctx().packs_list %> + next: + - publish: + - message: Successfully installed packs output: - packs_list: <% ctx().packs_list %> + - message: <% ctx().message %> + - conflict_list: <% ctx().conflict_list %>