-
Notifications
You must be signed in to change notification settings - Fork 24
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
Quick skip validation test if models or clouds mismatch #1562
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,10 +10,13 @@ | |
import requests | ||
import sh | ||
import shlex | ||
import subprocess | ||
import uuid | ||
import yaml | ||
|
||
from contextlib import contextmanager, asynccontextmanager | ||
from functools import cached_property | ||
|
||
from cilib.lp import Client as LPClient | ||
from datetime import datetime | ||
from juju.model import Model | ||
|
@@ -24,7 +27,6 @@ | |
asyncify, | ||
upgrade_charms, | ||
upgrade_snaps, | ||
arch, | ||
log_snap_versions, | ||
juju_run, | ||
) | ||
|
@@ -52,9 +54,7 @@ def pytest_addoption(parser): | |
default=os.environ.get("SERIES", "focal"), | ||
help="Base series", | ||
) | ||
parser.addoption( | ||
"--cloud", action="store", default="aws/us-east-2", help="Juju cloud to use" | ||
) | ||
parser.addoption("--cloud", action="store", help="Juju cloud to use") | ||
parser.addoption( | ||
"--charm-channel", action="store", default="", help="Charm channel to use" | ||
) | ||
|
@@ -143,41 +143,49 @@ def pytest_addoption(parser): | |
class Tools: | ||
"""Utility class for accessing juju related tools""" | ||
|
||
def __init__(self, request): | ||
self._request = request | ||
def __init__(self, config): | ||
self._config = config | ||
Comment on lines
-146
to
+147
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. construct with the pytest |
||
self.requests = requests | ||
self.requests_get = asyncify(requests.get) | ||
self.run = asyncify(self.exec) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the existing |
||
|
||
async def _load(self): | ||
request = self._request | ||
whoami, _ = await self.run("juju", "whoami", "--format=yaml") | ||
stdout, _ = await self.run("juju", "--version") | ||
def _load(self): | ||
whoami, _ = self.exec("juju", "whoami", "--format=yaml") | ||
stdout, _ = self.exec("juju", "--version") | ||
ver_str = stdout.splitlines()[-1].split("-")[0] | ||
self.juju_version = tuple(map(int, ver_str.split("."))) | ||
self.juju_user = yaml.safe_load(whoami)["user"] | ||
self.controller_name = request.config.getoption("--controller") | ||
self.model_name = request.config.getoption("--model") | ||
self.controller_name = self._config.getoption("--controller") | ||
self.model_name = self._config.getoption("--model") | ||
self.model_name_full = f"{self.juju_user}/{self.model_name}" | ||
self.k8s_model_name = f"{self.model_name}-k8s" | ||
self.k8s_model_name_full = f"{self.model_name_full}-k8s" | ||
self.series = request.config.getoption("--series") | ||
self.cloud = request.config.getoption("--cloud") | ||
self.series = self._config.getoption("--series") | ||
self.k8s_cloud = f"{self.k8s_model_name}-cloud" | ||
self.connection = f"{self.controller_name}:{self.model_name_full}" | ||
self.k8s_connection = f"{self.controller_name}:{self.k8s_model_name_full}" | ||
self.is_series_upgrade = request.config.getoption("--is-series-upgrade") | ||
self.is_series_upgrade = self._config.getoption("--is-series-upgrade") | ||
self.charm_channel = ( | ||
request.config.getoption("--charm-channel") # use specified channel | ||
self._config.getoption("--charm-channel") # use specified channel | ||
or os.environ.get("CHARM_CHANNEL_UPGRADE_TO") # fallback to upgrade env var | ||
or os.environ.get("JUJU_DEPLOY_CHANNEL") # fallback to env var | ||
or "edge" # default to edge | ||
) | ||
self.snap_channel = request.config.getoption("--snap-channel") | ||
self.vault_unseal_command = request.config.getoption("--vault-unseal-command") | ||
self.juju_ssh_proxy = request.config.getoption("--juju-ssh-proxy") | ||
self.use_existing_ceph_apps = request.config.getoption( | ||
"--use-existing-ceph-apps" | ||
self.snap_channel = self._config.getoption("--snap-channel") | ||
self.vault_unseal_command = self._config.getoption("--vault-unseal-command") | ||
self.juju_ssh_proxy = self._config.getoption("--juju-ssh-proxy") | ||
self.use_existing_ceph_apps = self._config.getoption("--use-existing-ceph-apps") | ||
|
||
@cached_property | ||
def cloud(self): | ||
if _it := self._config.getoption("--cloud"): | ||
return _it | ||
controller_data, _ = self.exec( | ||
"juju", "show-controller", self.controller_name, "--format", "yaml" | ||
) | ||
controller_infos = yaml.safe_load(controller_data) | ||
controller_info, *_ = controller_infos.values() | ||
return controller_info["details"]["cloud"] | ||
Comment on lines
+179
to
+188
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the cloud field can be loaded from the existing required controller name argument. |
||
|
||
def juju_base(self, series): | ||
"""Retrieve juju 3.x base from series.""" | ||
|
@@ -191,9 +199,9 @@ def juju_base(self, series): | |
} | ||
return f"--base={mapping[series]}" | ||
|
||
async def run(self, cmd: str, *args: str, stdin=None, _tee=False): | ||
def exec(self, cmd: str, *args: str, stdin=None, _tee=False): | ||
Comment on lines
-194
to
+202
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
""" | ||
asynchronously run a command as a subprocess | ||
exec a command as a subprocess | ||
@param str cmd: path to command on filesystem | ||
@param str *args: arguments to the command | ||
|
@@ -204,12 +212,11 @@ async def run(self, cmd: str, *args: str, stdin=None, _tee=False): | |
"out" -- stdout is tee'd to test stdout | ||
"err" -- stderr is tee'd to test stderr | ||
""" | ||
proc = await asyncio.create_subprocess_exec( | ||
cmd, | ||
*args, | ||
stdin=asyncio.subprocess.PIPE if stdin else None, | ||
stdout=asyncio.subprocess.PIPE, | ||
stderr=asyncio.subprocess.PIPE, | ||
process = subprocess.Popen( | ||
[cmd] + list(args), | ||
stdin=subprocess.PIPE if stdin else None, | ||
stdout=subprocess.PIPE, | ||
stderr=subprocess.PIPE, | ||
env=os.environ.copy(), | ||
) | ||
|
||
|
@@ -218,43 +225,21 @@ async def run(self, cmd: str, *args: str, stdin=None, _tee=False): | |
|
||
stdout, stderr = bytearray(), bytearray() | ||
|
||
def tee(line: bytes, sink: bytearray, fd: int): | ||
sink += line | ||
def tee(reader, sink: bytearray, fd: int): | ||
_read = reader.read(1024) | ||
sink += _read | ||
write = _tee == "out" and fd == 1 | ||
write |= _tee == "err" and fd == 2 | ||
if write or _tee is True: | ||
os.write(fd, line) | ||
|
||
async def _read_stream(stream, callback): | ||
while True: | ||
line = await stream.read(1024) | ||
if line: | ||
callback(line) | ||
else: | ||
break | ||
|
||
async def _feed_stream(input): | ||
if input: | ||
# replicates what proc.communicate() does with stdin | ||
await proc._feed_stdin(input) | ||
|
||
await asyncio.wait( | ||
map( | ||
asyncio.create_task, | ||
[ | ||
_read_stream(proc.stdout, lambda _l: tee(_l, stdout, 1)), | ||
_read_stream(proc.stderr, lambda _l: tee(_l, stderr, 2)), | ||
_feed_stream(input=stdin), | ||
], | ||
) | ||
) | ||
os.write(fd, _read) | ||
|
||
return_code = await proc.wait() | ||
if return_code != 0: | ||
raise Exception( | ||
f"Problem with run command {' '.join((cmd, *args))} (exit {return_code}):\n" | ||
f"stdout:\n{str(stdout, 'utf8')}\n" | ||
f"stderr:\n{str(stderr, 'utf8')}\n" | ||
while process.poll() is None: | ||
tee(process.stdout, stdout, 1) | ||
tee(process.stderr, stderr, 2) | ||
|
||
if rc := process.returncode: | ||
raise subprocess.CalledProcessError( | ||
rc, [cmd] + list(args), output=stdout, stderr=stderr | ||
) | ||
return str(stdout, "utf8"), str(stderr, "utf8") | ||
|
||
|
@@ -304,9 +289,7 @@ async def fast_forward( | |
|
||
@pytest.fixture(scope="module") | ||
async def tools(request): | ||
tools = Tools(request) | ||
await tools._load() | ||
return tools | ||
yield request.config.test_tools | ||
Comment on lines
-307
to
+292
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the old fixture now just has to yield the existing tools from the |
||
|
||
|
||
@pytest.fixture(scope="module") | ||
|
@@ -343,7 +326,8 @@ async def model(request, tools): | |
|
||
|
||
@pytest.fixture(scope="module") | ||
async def k8s_cloud(kubeconfig, tools): | ||
@pytest.mark.usefixtures("kubeconfig") | ||
async def k8s_cloud(tools): | ||
clouds = await tools.run( | ||
"juju", "clouds", "--format", "yaml", "-c", tools.controller_name | ||
) | ||
|
@@ -430,19 +414,6 @@ async def k8s_model(k8s_cloud, tools): | |
) | ||
|
||
|
||
@pytest.fixture | ||
def system_arch(): | ||
return arch | ||
|
||
|
||
@pytest.fixture(autouse=True) | ||
def skip_by_arch(request, system_arch): | ||
"""Skip tests on specified arches""" | ||
if request.node.get_closest_marker("skip_arch"): | ||
if system_arch in request.node.get_closest_marker("skip_arch").args[0]: | ||
pytest.skip("skipped on this arch: {}".format(system_arch)) | ||
|
||
|
||
Comment on lines
-433
to
-445
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This never would have really worked the way it was intended |
||
@pytest.fixture(scope="module") | ||
async def proxy_app(model): | ||
proxy_app = model.applications.get("squid-forwardproxy") | ||
|
@@ -485,7 +456,7 @@ def skip_unless_all_charms(request, model): | |
current_charms = set(map(_charm_name, model.applications.values())) | ||
all_are_available = all(charm in current_charms for charm in charms) | ||
if not all_are_available: | ||
pytest.skip("skipped, not all matching charms found: {}".format(charms)) | ||
pytest.skip("not all matching charms found: {}".format(charms)) | ||
|
||
|
||
@pytest.fixture() | ||
|
@@ -502,14 +473,15 @@ def _apps_by_charm(charm): | |
return _apps_by_charm | ||
|
||
|
||
@pytest.fixture(autouse=True) | ||
def skip_by_model(request, model): | ||
def skip_by_model(item) -> bool: | ||
"""Skips tests if model isn't referenced, ie validate-vault for only | ||
running tests applicable to vault | ||
""" | ||
if request.node.get_closest_marker("on_model"): | ||
if request.node.get_closest_marker("on_model").args[0] not in model.info.name: | ||
pytest.skip("skipped on this model: {}".format(model.info.name)) | ||
model_name = item.config.getoption("--model") | ||
on_models = [mark.args[0] for mark in item.iter_markers(name="on_model")] | ||
if on_models: | ||
if model_name not in on_models: | ||
pytest.skip(f"model {model_name!r} not in [{', '.join(on_models)}]") | ||
|
||
|
||
@pytest.fixture | ||
|
@@ -574,7 +546,6 @@ async def addons_model(request): | |
model_name = request.config.getoption("--addons-model") | ||
if not model_name: | ||
pytest.skip("--addons-model not specified") | ||
return | ||
model = Model() | ||
await model.connect(controller_name + ":" + model_name) | ||
yield model | ||
|
@@ -587,12 +558,15 @@ async def cloud(model): | |
return config["type"].value | ||
|
||
|
||
@pytest.fixture(autouse=True) | ||
def skip_by_cloud(request, cloud): | ||
clouds_marker = request.node.get_closest_marker("clouds") | ||
if not clouds_marker: | ||
def skip_by_cloud(item): | ||
allowed_clouds = set() | ||
for mark in item.iter_markers(name="clouds"): | ||
allowed_clouds |= set(mark.args[0]) | ||
|
||
if not allowed_clouds: | ||
# All clouds are allowed, as no restricts exist on this test | ||
return | ||
allowed_clouds = set(clouds_marker.args[0]) | ||
|
||
# from: juju add-cloud --help | ||
known_clouds = { | ||
# private clouds | ||
|
@@ -603,24 +577,23 @@ def skip_by_cloud(request, cloud): | |
"vsphere", | ||
# public clouds | ||
"azure", | ||
"cloudsigma", | ||
"ec2", | ||
"gce", | ||
"oci", | ||
} | ||
|
||
unknown_clouds = allowed_clouds - known_clouds | ||
if unknown_clouds: | ||
nodeid = request.node.nodeid | ||
nodeid = item.nodeid | ||
s = "s" if len(unknown_clouds) > 1 else "" | ||
unknown_clouds = ", ".join(unknown_clouds) | ||
raise ValueError( | ||
f"Unrecognized cloud{s} in marker for {nodeid}: {unknown_clouds}" | ||
) | ||
if cloud not in allowed_clouds: | ||
log( | ||
f"Skipping due to unsupported cloud: {cloud} not in [{', '.join(allowed_clouds)}]" | ||
) | ||
pytest.skip("unsupported cloud") | ||
_cloud = item.config.test_tools.cloud | ||
if _cloud not in allowed_clouds: | ||
msg = f"cloud '{_cloud}' not in [{', '.join(allowed_clouds)}]" | ||
pytest.skip(msg) | ||
|
||
|
||
@pytest.fixture() | ||
|
@@ -660,10 +633,10 @@ def skip_if_version(request, k8s_version): | |
if not skip_marker: | ||
return | ||
if k8s_version is None: | ||
pytest.skip("skipping, Couldn't determine k8s version yet.") | ||
pytest.skip("Couldn't determine k8s version yet.") | ||
version_predicate, *_ = skip_marker.args | ||
if version_predicate(k8s_version): | ||
pytest.skip(f"skipping, k8s version v{'.'.join(k8s_version)}") | ||
pytest.skip(f"k8s version v{'.'.join(k8s_version)}") | ||
|
||
|
||
# def pytest_itemcollected(item): | ||
|
@@ -719,6 +692,17 @@ def pytest_metadata(metadata): | |
) | ||
|
||
|
||
def pytest_runtest_setup(item): | ||
"""Called to perform the setup phase for a test item.""" | ||
skip_by_model(item) # skip tests if model marking on test mismatches | ||
skip_by_cloud(item) # skip tests if cloud marking on test mismatches | ||
Comment on lines
+695
to
+698
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. handles quick skipping of the tagged test by model or cloud |
||
|
||
|
||
def pytest_configure(config): | ||
config.test_tools = Tools(config) | ||
config.test_tools._load() | ||
Comment on lines
+701
to
+703
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Load the tools for the validation tests into the config's global space |
||
|
||
|
||
@pytest.fixture(scope="module") | ||
async def kubeconfig(model): | ||
control_planes = model.applications["kubernetes-control-plane"].units | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stop defaulting the cloud type to 'aws/us-east-2`. This isn't necessary