Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] tools to handle slow cpus and low mem #239

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion galaxy_importer/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ class Config(object):
DEFAULTS = {
"ansible_local_tmp": "~/.ansible/tmp",
"ansible_test_local_image": False,
"ansible_lint_dynamic_exclusions": True,
"ansible_lint_serialize": False,
"ansible_lint_timeout": 120,
"check_required_tags": False,
"infra_osd": False,
"local_image_docker": False,
Expand All @@ -67,7 +70,12 @@ def __init__(self, config_data=None):
for key in self.__dict__.keys():
env_key = "GALAXY_IMPORTER_" + key.upper()
if env_key in os.environ:
self.__dict__[key] = parse_string_to_bool(os.environ[env_key])
if key == "ansible_lint_timeout":
self.__dict__[key] = int(os.environ[env_key])
elif key in ["ansible_local_tmp", "log_level_main"]:
self.__dict__[key] = os.environ[env_key].upper()
else:
self.__dict__[key] = parse_string_to_bool(os.environ[env_key])


class ConfigFile(object):
Expand Down
170 changes: 152 additions & 18 deletions galaxy_importer/loaders/collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,40 @@
DOCUMENTATION_DIR = "docs"


def make_exclusions(basepath):
exclusions = []

for root, dirnames, filenames in os.walk(basepath):
if root != basepath:
continue
for dirname in dirnames:
if dirname == "roles":
continue
if dirname == "sanity":
continue

if dirname == "tests":
tests_dir = os.path.join(root, dirname)
for tests_root, tests_dirnames, tests_filenames in os.walk(tests_dir):
if tests_root != tests_dir:
continue
for tests_dirname in tests_dirnames:
if tests_dirname != "sanity":
continue
dn = (
os.path.join(tests_root, tests_dirname).replace(basepath + "/", "")
+ "/"
)
if dn not in exclusions:
exclusions.append(dn)

dn = os.path.join(root, dirname + "/").replace(basepath + "/", "")
if dn not in exclusions:
exclusions.append(dn)

return exclusions


class CollectionLoader(object):
"""Loads collection and content info."""

Expand All @@ -55,6 +89,17 @@ def __init__(self, path, filename, cfg=None, logger=None):
self.docs_blob = None
self.contents = None

# build the collections path for lint's module resolution
self.collections_path = self.path
if not callable(self.path):
if hasattr(self.path, "strpath"):
paths = self.path.strpath.split(os.sep)
else:
paths = self.path.split(os.sep)
if "ansible_collections" in paths:
ix = paths.index("ansible_collections")
self.collections_path = os.sep.join(paths[: ix + 1])

def load(self):
# NOTE: If we knew the chksum for MANIFEST.json, we could check it here first
self.manifest = self._load_manifest()
Expand Down Expand Up @@ -95,7 +140,10 @@ def load(self):
self._check_collection_changelog()

if self.cfg.run_ansible_lint:
self._lint_collection()
if self.cfg.ansible_lint_serialize:
self._lint_collection_roles_individually()
else:
self._lint_collection()
self._check_ansible_test_ignore_files()

return schema.ImportResult(
Expand All @@ -105,6 +153,27 @@ def load(self):
requires_ansible=self.requires_ansible,
)

def _process_lint_results(self, proc, outs, errs, timedout=False):
# EXIT CODES ...
# 0 all good
# 2 lint found problems
# -9 lint process timed out OR was killed

for line in outs.splitlines():
self.log.warning(line.strip())

for line in errs.splitlines():
if line.startswith(constants.ANSIBLE_LINT_ERROR_PREFIXES):
self.log.warning(line.rstrip())

# The prevous code tries to be intelligent about what to display or not display
# but we have serious errors from lint that are hidden by that logic. We should
# attempt to inform the users of these errors (especially tracebacks).
if proc.returncode != 0 and errs and "Traceback (most recent call last):" in errs:
self.log.error(errs)

self.log.debug(f"\tansible-lint returncode:{proc.returncode} timedout:{timedout}")

def _lint_collection(self):
"""Log ansible-lint output.

Expand All @@ -124,21 +193,27 @@ def _lint_collection(self):

cmd = [
"/usr/bin/env",
f"ANSIBLE_COLLECTIONS_PATH={self.collections_path}",
f"ANSIBLE_LOCAL_TEMP={self.cfg.ansible_local_tmp}",
"ansible-lint",
"--profile",
"production",
"--exclude",
"tests/integration/",
"--exclude",
"tests/unit/",
"--parseable",
"--nocolor",
]
if self.cfg.offline_ansible_lint:
cmd.append("--offline")

self.log.debug("CMD: " + " ".join(cmd))
if self.cfg.ansible_lint_dynamic_exclusions:
exclusions = make_exclusions(self.path)
else:
exclusions = ["tests/integration/", "tests/unit/"]

for exclusion in exclusions:
cmd.append("--exclude")
cmd.append(exclusion)

self.log.info("CMD: " + " ".join(cmd))
proc = Popen(
cmd,
cwd=self.path,
Expand All @@ -147,29 +222,88 @@ def _lint_collection(self):
stderr=PIPE,
)

timedout = False
try:
outs, errs = proc.communicate(timeout=120)
outs, errs = proc.communicate(timeout=self.cfg.ansible_lint_timeout)
except (
TimeoutExpired
): # pragma: no cover - a TimeoutExpired mock would apply to both calls to commnicate()
timedout = True
self.log.error("Timeout on call to ansible-lint")
proc.kill()
outs, errs = proc.communicate()

for line in outs.splitlines():
self.log.warning(line.strip())
self._process_lint_results(proc, outs, errs, timedout=timedout)

for line in errs.splitlines():
if line.startswith(constants.ANSIBLE_LINT_ERROR_PREFIXES):
self.log.warning(line.rstrip())
self.log.info("...ansible-lint run complete")

# The prevous code tries to be intelligent about what to display or not display
# but we have serious errors from lint that are hidden by that logic. We should
# attempt to inform the users of these errors (especially tracebacks).
if proc.returncode != 0 and errs and "Traceback (most recent call last):" in errs:
self.log.error(errs)
def _lint_collection_roles_individually(self):
roles_path = os.path.join(self.path, "roles")
if not os.path.exists(roles_path):
self.log.info("No roles found, skipping ansible-lint")

self.log.info("...ansible-lint run complete")
role_paths = []
for root, dirnames, filenames in os.walk(roles_path):
if root != roles_path:
continue
for dirname in dirnames:
role_paths.append(os.path.join(root, dirname))

role_paths = sorted(role_paths)
for rp in role_paths:
self._lint_collection_role(rp)

def _lint_collection_role(self, path):
"""Log ansible-lint output.
ansible-lint stdout are linter violations, they are logged as warnings.
ansible-lint stderr includes info about vars, file discovery,
summary of linter violations, config suggestions, and raised errors.
Only raised errors are logged, they are logged as errors.
"""

path_name = os.path.basename(path)
self.log.info(f"Linting role {path_name} via ansible-lint...")

if not shutil.which("ansible-lint"):
self.log.warning("ansible-lint not found, skipping lint of role")
return

cmd = [
"/usr/bin/env",
f"ANSIBLE_LOCAL_TEMP={self.cfg.ansible_local_tmp}",
f"ANSIBLE_COLLECTIONS_PATH={self.collections_path}",
"ansible-lint",
path,
"--profile",
"production",
"--parseable",
"--nocolor",
"--skip-list",
"metadata",
"--project-dir",
os.path.dirname(path),
]
self.log.info("CMD: " + " ".join(cmd))
proc = Popen(
cmd,
cwd=self.path,
encoding="utf-8",
stdout=PIPE,
stderr=PIPE,
)

timedout = False
try:
outs, errs = proc.communicate(timeout=self.cfg.ansible_lint_timeout)
except (
TimeoutExpired
): # pragma: no cover - a TimeoutExpired mock would apply to both calls to commnicate()
timedout = True
self.log.error("Timeout on call to ansible-lint")
proc.kill()
outs, errs = proc.communicate()

self._process_lint_results(proc, outs, errs, timedout=timedout)

def _check_ansible_test_ignore_files(self): # pragma: no cover
"""Log a warning when ansible test sanity ignore files are present.
Expand Down
19 changes: 18 additions & 1 deletion tests/unit/test_loader_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ def test_changelog(changelog_path, tmpdir, caplog):
fh.write("Changelog info")

collection_loader = CollectionLoader(
tmpdir, "filename", cfg=SimpleNamespace(run_ansible_doc=False)
tmpdir, "filename", cfg=SimpleNamespace(run_ansible_doc=False, ansible_lint_serialize=False)
)
collection_loader._check_collection_changelog()
assert len(caplog.records) == 0
Expand Down Expand Up @@ -536,6 +536,8 @@ def test_ansiblelint_playbook_errors(populated_collection_root, tmp_collection_r
run_ansible_lint=True,
offline_ansible_lint=True,
ansible_local_tmp=tmp_collection_root,
ansible_lint_dynamic_exclusions=False,
ansible_lint_timeout=120,
),
)
collection_loader._lint_collection()
Expand All @@ -553,6 +555,8 @@ def test_ansiblelint_collection_pass(populated_collection_root, tmp_collection_r
run_ansible_lint=True,
offline_ansible_lint=True,
ansible_local_tmp=tmp_collection_root,
ansible_lint_dynamic_exclusions=False,
ansible_lint_timeout=120,
),
)
collection_loader._lint_collection()
Expand All @@ -569,6 +573,9 @@ def test_ansiblelint_true_loader(populated_collection_root, tmp_collection_root,
run_ansible_lint=True,
offline_ansible_lint=True,
ansible_local_tmp=tmp_collection_root,
ansible_lint_dynamic_exclusions=False,
ansible_lint_timeout=120,
ansible_lint_serialize=False,
),
)
collection_loader.load()
Expand All @@ -591,6 +598,8 @@ def test_ansiblelint_collection_role_errors(populated_collection_root, tmp_colle
run_ansible_lint=True,
offline_ansible_lint=True,
ansible_local_tmp=tmp_collection_root,
ansible_lint_dynamic_exclusions=False,
ansible_lint_timeout=120,
),
)
collection_loader._lint_collection()
Expand All @@ -617,6 +626,8 @@ def test_ansiblelint_collection_meta_runtime_errors(
run_ansible_lint=True,
offline_ansible_lint=True,
ansible_local_tmp=tmp_collection_root,
ansible_lint_dynamic_exclusions=False,
ansible_lint_timeout=120,
),
)
collection_loader._lint_collection()
Expand Down Expand Up @@ -649,6 +660,8 @@ def test_ansiblelint_stderr_filter(mocked_popen, caplog):
run_ansible_lint=True,
offline_ansible_lint=True,
ansible_local_tmp=tmp_collection_root,
ansible_lint_dynamic_exclusions=False,
ansible_lint_timeout=120,
),
)
collection_loader._lint_collection()
Expand All @@ -672,6 +685,8 @@ def test_ansiblelint_warning_log(mocked_popen, caplog):
run_ansible_lint=True,
offline_ansible_lint=True,
ansible_local_tmp=tmp_collection_root,
ansible_lint_dynamic_exclusions=False,
ansible_lint_timeout=120,
),
)
collection_loader._lint_collection()
Expand All @@ -689,6 +704,8 @@ def test_no_ansible_lint_bin(mocked_shutil_which, tmp_collection_root, caplog):
run_ansible_doc=False,
run_ansible_lint=True,
ansible_local_tmp=tmp_collection_root,
ansible_lint_dynamic_exclusions=False,
ansible_lint_timeout=120,
),
)
collection_loader._lint_collection()
Expand Down