Skip to content

Commit

Permalink
Overhaul ansible-test test path handling. (ansible#61416)
Browse files Browse the repository at this point in the history
* Remove .keep files from test/results/ dirs.

* Remove classification of test/results/ dir.

* Add results_relative to data context.

* Use variables in delegation paths.

* Standardize file writing and results paths.

* Fix issues reported by PyCharm.

* Clean up invocation of coverage command.

It now runs through the injector.

* Hack to allow intercept_command in cover.py.

* Simplify git ignore for test results.

* Use test result tmp dir instead of cache dir.

* Remove old .pytest_cache reference.

* Fix unit test docker delegation.

* Show HTML report link.

* Clean up more results references.

* Move import sanity test output to .tmp dir.

* Exclude test results dir from coverage.

* Fix import sanity test lib paths.

* Fix hard-coded import test paths.

* Fix most hard-coded integration test paths.

* Fix PyCharm warnings.

* Fix import placement.

* Fix integration test dir path.

* Fix Shippable scripts.

* Fix Shippable matrix check.

* Overhaul key pair management.
  • Loading branch information
mattclay authored Aug 28, 2019
1 parent bf108ee commit f5d8293
Show file tree
Hide file tree
Showing 38 changed files with 393 additions and 307 deletions.
9 changes: 1 addition & 8 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,7 @@ ansible.egg-info/
# Release directory
packaging/release/ansible_release
/.cache/
/test/results/coverage/*=coverage.*
/test/results/coverage/coverage*
/test/results/reports/coverage*.xml
/test/results/reports/coverage*/
/test/results/bot/*.json
/test/results/junit/*.xml
/test/results/logs/*.log
/test/results/data/*.json
/test/results/
/test/integration/cloud-config-aws.yml
/test/integration/inventory.networking
/test/integration/inventory.winrm
Expand Down
Empty file removed test/cache/.keep
Empty file.
6 changes: 3 additions & 3 deletions test/lib/ansible_test/_data/sanity/import/importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ def main():
import traceback
import warnings

import_dir = os.environ['SANITY_IMPORT_DIR']
minimal_dir = os.environ['SANITY_MINIMAL_DIR']

try:
import importlib.util
imp = None # pylint: disable=invalid-name
Expand Down Expand Up @@ -266,9 +269,6 @@ def capture_report(path, capture, messages):
filepath = os.path.relpath(warning.filename)
lineno = warning.lineno

import_dir = 'test/runner/.tox/import/'
minimal_dir = 'test/runner/.tox/minimal-'

if filepath.startswith('../') or filepath.startswith(minimal_dir):
# The warning occurred outside our source tree.
# The best we can do is to report the file which was tested that triggered the warning.
Expand Down
3 changes: 2 additions & 1 deletion test/lib/ansible_test/_internal/ansible_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from .util_common import (
run_command,
ResultType,
)

from .config import (
Expand Down Expand Up @@ -82,7 +83,7 @@ def ansible_environment(args, color=True, ansible_config=None):
if args.debug:
env.update(dict(
ANSIBLE_DEBUG='true',
ANSIBLE_LOG_PATH=os.path.join(data_context().results, 'logs', 'debug.log'),
ANSIBLE_LOG_PATH=os.path.join(ResultType.LOGS.name, 'debug.log'),
))

if data_context().content.collection:
Expand Down
40 changes: 6 additions & 34 deletions test/lib/ansible_test/_internal/classification.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ def get_dependent_paths_internal(self, path):
if ext == '.cs':
return self.get_csharp_module_utils_usage(path)

if path.startswith('test/integration/targets/'):
if is_subdir(path, data_context().content.integration_targets_path):
return self.get_integration_target_usage(path)

return []
Expand Down Expand Up @@ -338,7 +338,8 @@ def get_integration_target_usage(self, path):
:rtype: list[str]
"""
target_name = path.split('/')[3]
dependents = [os.path.join('test/integration/targets/%s/' % target) for target in sorted(self.integration_dependencies.get(target_name, set()))]
dependents = [os.path.join(data_context().content.integration_targets_path, target) + os.path.sep
for target in sorted(self.integration_dependencies.get(target_name, set()))]

return dependents

Expand Down Expand Up @@ -620,22 +621,10 @@ def _classify(self, path):
if path.startswith('test/ansible_test/'):
return minimal # these tests are not invoked from ansible-test

if path.startswith('test/cache/'):
return minimal

if path.startswith('test/results/'):
return minimal

if path.startswith('test/legacy/'):
return minimal

if path.startswith('test/env/'):
return minimal

if path.startswith('test/integration/roles/'):
return minimal

if path.startswith('test/integration/targets/'):
if is_subdir(path, data_context().content.integration_targets_path):
if not os.path.exists(path):
return minimal

Expand All @@ -655,25 +644,8 @@ def _classify(self, path):
FOCUSED_TARGET: True,
}

if path.startswith('test/integration/'):
if dirname == 'test/integration':
if self.prefixes.get(name) == 'network' and ext == '.yaml':
return minimal # network integration test playbooks are not used by ansible-test

if filename == 'network-all.yaml':
return minimal # network integration test playbook not used by ansible-test

if filename == 'platform_agnostic.yaml':
return minimal # network integration test playbook not used by ansible-test

if filename.startswith('inventory.') and filename.endswith('.template'):
return minimal # ansible-test does not use these inventory templates

if filename == 'inventory':
return {
'integration': self.integration_all_target,
}

if is_subdir(path, data_context().content.integration_path):
if dirname == data_context().content.integration_path:
for command in (
'integration',
'windows-integration',
Expand Down
2 changes: 1 addition & 1 deletion test/lib/ansible_test/_internal/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ def complete_network_testcase(prefix, parsed_args, **_):
if len(parsed_args.include) != 1:
return []

test_dir = 'test/integration/targets/%s/tests' % parsed_args.include[0]
test_dir = os.path.join(data_context().content.integration_targets_path, parsed_args.include[0], 'tests')
connection_dirs = data_context().content.get_dirs(test_dir)

for connection_dir in connection_dirs:
Expand Down
24 changes: 11 additions & 13 deletions test/lib/ansible_test/_internal/cloud/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import abc
import atexit
import datetime
import json
import time
import os
import platform
Expand All @@ -23,10 +22,14 @@
load_plugins,
ABC,
to_bytes,
make_dirs,
ANSIBLE_TEST_CONFIG_ROOT,
)

from ..util_common import (
write_json_test_results,
ResultType,
)

from ..target import (
TestTarget,
)
Expand Down Expand Up @@ -158,17 +161,14 @@ def cloud_init(args, targets):
)

if not args.explain and results:
results_path = os.path.join(data_context().results, 'data', '%s-%s.json' % (
args.command, re.sub(r'[^0-9]', '-', str(datetime.datetime.utcnow().replace(microsecond=0)))))
result_name = '%s-%s.json' % (
args.command, re.sub(r'[^0-9]', '-', str(datetime.datetime.utcnow().replace(microsecond=0))))

data = dict(
clouds=results,
)

make_dirs(os.path.dirname(results_path))

with open(results_path, 'w') as results_fd:
results_fd.write(json.dumps(data, sort_keys=True, indent=4))
write_json_test_results(ResultType.DATA, result_name, data)


class CloudBase(ABC):
Expand Down Expand Up @@ -280,8 +280,6 @@ def _set_cloud_config(self, key, value):

class CloudProvider(CloudBase):
"""Base class for cloud provider plugins. Sets up cloud resources before delegation."""
TEST_DIR = 'test/integration'

def __init__(self, args, config_extension='.ini'):
"""
:type args: IntegrationConfig
Expand All @@ -291,7 +289,7 @@ def __init__(self, args, config_extension='.ini'):

self.remove_config = False
self.config_static_name = 'cloud-config-%s%s' % (self.platform, config_extension)
self.config_static_path = os.path.join(self.TEST_DIR, self.config_static_name)
self.config_static_path = os.path.join(data_context().content.integration_path, self.config_static_name)
self.config_template_path = os.path.join(ANSIBLE_TEST_CONFIG_ROOT, '%s.template' % self.config_static_name)
self.config_extension = config_extension

Expand Down Expand Up @@ -352,8 +350,8 @@ def _write_config(self, content):
"""
prefix = '%s-' % os.path.splitext(os.path.basename(self.config_static_path))[0]

with tempfile.NamedTemporaryFile(dir=self.TEST_DIR, prefix=prefix, suffix=self.config_extension, delete=False) as config_fd:
filename = os.path.join(self.TEST_DIR, os.path.basename(config_fd.name))
with tempfile.NamedTemporaryFile(dir=data_context().content.integration_path, prefix=prefix, suffix=self.config_extension, delete=False) as config_fd:
filename = os.path.join(data_context().content.integration_path, os.path.basename(config_fd.name))

self.config_path = filename
self.remove_config = True
Expand Down
7 changes: 0 additions & 7 deletions test/lib/ansible_test/_internal/cloud/vcenter.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
__metaclass__ = type

import os
import time

from . import (
CloudProvider,
Expand All @@ -14,10 +13,8 @@
from ..util import (
find_executable,
display,
ApplicationError,
is_shippable,
ConfigParser,
SubprocessError,
)

from ..docker_util import (
Expand All @@ -32,10 +29,6 @@
AnsibleCoreCI,
)

from ..http import (
HttpClient,
)


class VcenterProvider(CloudProvider):
"""VMware vcenter/esx plugin. Sets up cloud resources for tests."""
Expand Down
4 changes: 2 additions & 2 deletions test/lib/ansible_test/_internal/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
generate_pip_command,
get_docker_completion,
ApplicationError,
INTEGRATION_DIR_RELATIVE,
)

from .util_common import (
Expand Down Expand Up @@ -247,7 +246,7 @@ def __init__(self, args, command):

def get_ansible_config(self): # type: () -> str
"""Return the path to the Ansible config for the given config."""
ansible_config_relative_path = os.path.join(INTEGRATION_DIR_RELATIVE, '%s.cfg' % self.command)
ansible_config_relative_path = os.path.join(data_context().content.integration_path, '%s.cfg' % self.command)
ansible_config_path = os.path.join(data_context().content.root, ansible_config_relative_path)

if not os.path.exists(ansible_config_path):
Expand Down Expand Up @@ -327,6 +326,7 @@ def __init__(self, args):
self.group_by = frozenset(args.group_by) if 'group_by' in args and args.group_by else set() # type: t.FrozenSet[str]
self.all = args.all if 'all' in args else False # type: bool
self.stub = args.stub if 'stub' in args else False # type: bool
self.coverage = False # temporary work-around to support intercept_command in cover.py


class CoverageReportConfig(CoverageConfig):
Expand Down
97 changes: 65 additions & 32 deletions test/lib/ansible_test/_internal/core_ci.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@

from .util_common import (
run_command,
write_json_file,
ResultType,
)

from .config import (
Expand Down Expand Up @@ -492,10 +494,7 @@ def _save(self):

config = self.save()

make_dirs(os.path.dirname(self.path))

with open(self.path, 'w') as instance_fd:
instance_fd.write(json.dumps(config, indent=4, sort_keys=True))
write_json_file(self.path, config, create_directories=True)

def save(self):
"""
Expand Down Expand Up @@ -559,47 +558,81 @@ def __init__(self, args):
"""
:type args: EnvironmentConfig
"""
cache_dir = os.path.join(data_context().content.root, 'test/cache')

self.key = os.path.join(cache_dir, self.KEY_NAME)
self.pub = os.path.join(cache_dir, self.PUB_NAME)

key_dst = os.path.relpath(self.key, data_context().content.root)
pub_dst = os.path.relpath(self.pub, data_context().content.root)
key_pair = self.get_key_pair()

if not os.path.isfile(self.key) or not os.path.isfile(self.pub):
base_dir = os.path.expanduser('~/.ansible/test/')
if not key_pair:
key_pair = self.generate_key_pair(args)

key = os.path.join(base_dir, self.KEY_NAME)
pub = os.path.join(base_dir, self.PUB_NAME)
key, pub = key_pair
key_dst, pub_dst = self.get_in_tree_key_pair_paths()

if not args.explain:
make_dirs(base_dir)

if not os.path.isfile(key) or not os.path.isfile(pub):
run_command(args, ['ssh-keygen', '-m', 'PEM', '-q', '-t', 'rsa', '-N', '', '-f', key])
def ssh_key_callback(files): # type: (t.List[t.Tuple[str, str]]) -> None
"""
Add the SSH keys to the payload file list.
They are either outside the source tree or in the cache dir which is ignored by default.
"""
if data_context().content.collection:
working_path = data_context().content.collection.directory
else:
working_path = ''

self.key = key
self.pub = pub
files.append((key, os.path.join(working_path, os.path.relpath(key_dst, data_context().content.root))))
files.append((pub, os.path.join(working_path, os.path.relpath(pub_dst, data_context().content.root))))

def ssh_key_callback(files): # type: (t.List[t.Tuple[str, str]]) -> None
"""Add the SSH keys to the payload file list."""
if data_context().content.collection:
working_path = data_context().content.collection.directory
else:
working_path = ''
data_context().register_payload_callback(ssh_key_callback)

files.append((key, os.path.join(working_path, key_dst)))
files.append((pub, os.path.join(working_path, pub_dst)))

data_context().register_payload_callback(ssh_key_callback)
self.key, self.pub = key, pub

if args.explain:
self.pub_contents = None
else:
with open(self.pub, 'r') as pub_fd:
self.pub_contents = pub_fd.read().strip()

def get_in_tree_key_pair_paths(self): # type: () -> t.Optional[t.Tuple[str, str]]
"""Return the ansible-test SSH key pair paths from the content tree."""
temp_dir = ResultType.TMP.path

key = os.path.join(temp_dir, self.KEY_NAME)
pub = os.path.join(temp_dir, self.PUB_NAME)

return key, pub

def get_source_key_pair_paths(self): # type: () -> t.Optional[t.Tuple[str, str]]
"""Return the ansible-test SSH key pair paths for the current user."""
base_dir = os.path.expanduser('~/.ansible/test/')

key = os.path.join(base_dir, self.KEY_NAME)
pub = os.path.join(base_dir, self.PUB_NAME)

return key, pub

def get_key_pair(self): # type: () -> t.Optional[t.Tuple[str, str]]
"""Return the ansible-test SSH key pair paths if present, otherwise return None."""
key, pub = self.get_in_tree_key_pair_paths()

if os.path.isfile(key) and os.path.isfile(pub):
return key, pub

key, pub = self.get_source_key_pair_paths()

if os.path.isfile(key) and os.path.isfile(pub):
return key, pub

return None

def generate_key_pair(self, args): # type: (EnvironmentConfig) -> t.Tuple[str, str]
"""Generate an SSH key pair for use by all ansible-test invocations for the current user."""
key, pub = self.get_source_key_pair_paths()

if not args.explain:
make_dirs(os.path.dirname(key))

if not os.path.isfile(key) or not os.path.isfile(pub):
run_command(args, ['ssh-keygen', '-m', 'PEM', '-q', '-t', 'rsa', '-N', '', '-f', key])

return key, pub


class InstanceConnection:
"""Container for remote instance status and connection details."""
Expand Down
Loading

0 comments on commit f5d8293

Please sign in to comment.