From 7b4bc572deb75f92bb852b908525f26059bda43b Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Mon, 14 Jan 2019 19:57:32 -0800 Subject: [PATCH] Fix ansible-test target dependency issues. (#50908) * Log dependencies at verbosity level 4. This makes it easier to debug target dependency issues. * Scan symlinks for target dependencies. Some test targets use symlinks to files in other test targets. These dependencies were previously undetected. This could result in changes made to dependencies without triggering the dependent tests. * Track missing target deps with `needs/target/*`. Some existing test targets have untracked dependencies on other test targets. This can result in changes to those dependencies not triggering their dependent tests, resulting in test failures after a PR is merged. This PR adds the appropriate `needs/target/*` aliases to track those dependencies, along with appropriate processing in ansible-test to handle the new aliases. * Scan meta dependencies in script targets. Script targets are often former role targets which were converted to allow custom invocations of ansible-playbook. These targets still have their meta dependencies, but they were not being detected. This could result in changes to dependencies not triggering the targets which depend on them. --- .../targets/binary_modules_posix/aliases | 1 + .../targets/binary_modules_winrm/aliases | 1 + .../targets/connection_posix/aliases | 1 + .../targets/connection_psrp/aliases | 1 + .../targets/connection_winrm/aliases | 1 + .../targets/lookup_hashi_vault/aliases | 1 + .../targets/template_jinja2_latest/aliases | 1 + test/runner/lib/target.py | 49 +++++++++++++++++-- 8 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 test/integration/targets/connection_posix/aliases diff --git a/test/integration/targets/binary_modules_posix/aliases b/test/integration/targets/binary_modules_posix/aliases index b59832142f2..2c6e4a07c83 100644 --- a/test/integration/targets/binary_modules_posix/aliases +++ b/test/integration/targets/binary_modules_posix/aliases @@ -1 +1,2 @@ shippable/posix/group3 +needs/target/binary_modules diff --git a/test/integration/targets/binary_modules_winrm/aliases b/test/integration/targets/binary_modules_winrm/aliases index 93e9504f144..ba3d20001e6 100644 --- a/test/integration/targets/binary_modules_winrm/aliases +++ b/test/integration/targets/binary_modules_winrm/aliases @@ -1,3 +1,4 @@ shippable/windows/group1 shippable/windows/smoketest windows +needs/target/binary_modules diff --git a/test/integration/targets/connection_posix/aliases b/test/integration/targets/connection_posix/aliases new file mode 100644 index 00000000000..c0fdd23c557 --- /dev/null +++ b/test/integration/targets/connection_posix/aliases @@ -0,0 +1 @@ +needs/target/connection diff --git a/test/integration/targets/connection_psrp/aliases b/test/integration/targets/connection_psrp/aliases index cf714783f5c..b3e9b8bcf75 100644 --- a/test/integration/targets/connection_psrp/aliases +++ b/test/integration/targets/connection_psrp/aliases @@ -1,3 +1,4 @@ windows shippable/windows/group1 shippable/windows/smoketest +needs/target/connection diff --git a/test/integration/targets/connection_winrm/aliases b/test/integration/targets/connection_winrm/aliases index cf714783f5c..b3e9b8bcf75 100644 --- a/test/integration/targets/connection_winrm/aliases +++ b/test/integration/targets/connection_winrm/aliases @@ -1,3 +1,4 @@ windows shippable/windows/group1 shippable/windows/smoketest +needs/target/connection diff --git a/test/integration/targets/lookup_hashi_vault/aliases b/test/integration/targets/lookup_hashi_vault/aliases index ca7c9128514..ffded0edd48 100644 --- a/test/integration/targets/lookup_hashi_vault/aliases +++ b/test/integration/targets/lookup_hashi_vault/aliases @@ -1,2 +1,3 @@ shippable/posix/group2 destructive +needs/target/setup_openssl diff --git a/test/integration/targets/template_jinja2_latest/aliases b/test/integration/targets/template_jinja2_latest/aliases index 9af2d790425..14286dbda7d 100644 --- a/test/integration/targets/template_jinja2_latest/aliases +++ b/test/integration/targets/template_jinja2_latest/aliases @@ -1,2 +1,3 @@ needs/root shippable/posix/group2 +needs/target/template diff --git a/test/runner/lib/target.py b/test/runner/lib/target.py index 17a7f55b668..70fc91abf60 100644 --- a/test/runner/lib/target.py +++ b/test/runner/lib/target.py @@ -12,6 +12,7 @@ import sys from lib.util import ( ApplicationError, + display, read_lines_without_comments, ) @@ -342,6 +343,8 @@ def analyze_integration_target_dependencies(integration_targets): :type integration_targets: list[IntegrationTarget] :rtype: dict[str,set[str]] """ + real_target_root = os.path.realpath('test/integration/targets') + '/' + role_targets = [t for t in integration_targets if t.type == 'role'] hidden_role_target_names = set(t.name for t in role_targets if 'hidden/' in t.aliases) @@ -352,9 +355,37 @@ def analyze_integration_target_dependencies(integration_targets): for setup_target_name in target.setup_always + target.setup_once: dependencies[setup_target_name].add(target.name) + # handle target dependencies + for target in integration_targets: + for need_target in target.needs_target: + dependencies[need_target].add(target.name) + + # handle symlink dependencies between targets + # this use case is supported, but discouraged + for target in integration_targets: + for root, _dummy, file_names in os.walk(target.path): + for name in file_names: + path = os.path.join(root, name) + + if not os.path.islink(path): + continue + + real_link_path = os.path.realpath(path) + + if not real_link_path.startswith(real_target_root): + continue + + link_target = real_link_path[len(real_target_root):].split('/')[0] + + if link_target == target.name: + continue + + dependencies[link_target].add(target.name) + # intentionally primitive analysis of role meta to avoid a dependency on pyyaml - for role_target in role_targets: - meta_dir = os.path.join(role_target.path, 'meta') + # script based targets are scanned as they may execute a playbook with role dependencies + for target in integration_targets: + meta_dir = os.path.join(target.path, 'meta') if not os.path.isdir(meta_dir): continue @@ -375,7 +406,7 @@ def analyze_integration_target_dependencies(integration_targets): for hidden_target_name in hidden_role_target_names: if hidden_target_name in meta_line: - dependencies[hidden_target_name].add(role_target.name) + dependencies[hidden_target_name].add(target.name) while True: changes = 0 @@ -393,6 +424,17 @@ def analyze_integration_target_dependencies(integration_targets): if not changes: break + for target_name in sorted(dependencies): + consumers = dependencies[target_name] + + if not consumers: + continue + + display.info('%s:' % target_name, verbosity=4) + + for consumer in sorted(consumers): + display.info(' %s' % consumer, verbosity=4) + return dependencies @@ -608,6 +650,7 @@ class IntegrationTarget(CompletionTarget): self.setup_once = tuple(sorted(set(g.split('/')[2] for g in groups if g.startswith('setup/once/')))) self.setup_always = tuple(sorted(set(g.split('/')[2] for g in groups if g.startswith('setup/always/')))) + self.needs_target = tuple(sorted(set(g.split('/')[2] for g in groups if g.startswith('needs/target/')))) class TargetPatternsNotMatched(ApplicationError):