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

Improve exception tracing when setting up a k8s-cloud #1404

Merged
Merged
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
110 changes: 51 additions & 59 deletions jobs/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,14 @@
from pathlib import Path
from py.xml import html
from tempfile import NamedTemporaryFile
from traceback import format_exc
from .utils import (
asyncify,
upgrade_charms,
upgrade_snaps,
arch,
log_snap_versions,
scp_from,
juju_run,
juju_run_action,
)

from .logger import log
Expand Down Expand Up @@ -301,53 +300,42 @@ async def model(request, tools):


@pytest.fixture(scope="module")
async def k8s_cloud(model, tools):
kcp_app = model.applications["kubernetes-control-plane"]
kcp_unit = kcp_app.units[0]
created_k8s_cloud = False

with NamedTemporaryFile(dir=Path.home() / ".local" / "share" / "juju") as f:
await scp_from(
kcp_unit,
"config",
f.name,
async def k8s_cloud(kubeconfig, tools):
clouds = await tools.run(
"juju", "clouds", "--format", "yaml", "-c", tools.controller_name
)
if tools.k8s_cloud in yaml.safe_load(clouds[0]):
yield tools.k8s_cloud
return
Comment on lines +304 to +309
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first -- let's see if the cloud we want already exists -- to prevent a failure when adding it a second time


_created = False
click.echo("Adding k8s cloud")
try:
await tools.run(
"juju",
"add-k8s",
"--skip-storage",
"-c",
tools.controller_name,
tools.connection,
proxy=tools.juju_ssh_proxy,
tools.k8s_cloud,
)
Comment on lines +314 to 321
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

next, let's add-k8s. The kubeconfig fixture will handle adding the correct kubeconfig file into the os.environment

try:
click.echo("Adding k8s cloud")
os.environ["KUBECONFIG"] = f.name
_created = True
yield tools.k8s_cloud
finally:
if _created:
click.echo("Removing k8s cloud")
Comment on lines +325 to +326
Copy link
Member Author

@addyess addyess Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the remove portion is only run if _created was set

We're careful not to return during the finally so as to not lose any exceptions that may still need to bubble out

await tools.run(
"juju",
"add-k8s",
"--skip-storage",
"remove-cloud",
"-c",
tools.controller_name,
tools.k8s_cloud,
)
del os.environ["KUBECONFIG"]
created_k8s_cloud = True
yield tools.k8s_cloud
finally:
if not created_k8s_cloud:
return
click.echo("Removing k8s cloud")
try:
await tools.run(
"juju",
"remove-cloud",
"-c",
tools.controller_name,
tools.k8s_cloud,
)
except Exception:
click.echo(format_exc())


@pytest.fixture(scope="module")
async def k8s_model(k8s_cloud, tools):
k8s_model = None
_model_created = None
try:
click.echo("Adding k8s model")
await tools.run(
Expand All @@ -356,35 +344,36 @@ async def k8s_model(k8s_cloud, tools):
"-c",
tools.controller_name,
tools.k8s_model_name,
tools.k8s_cloud,
k8s_cloud,
"--config",
"test-mode=true",
"--no-switch",
)

k8s_model = Model()
await k8s_model.connect(tools.k8s_connection)
yield k8s_model
_model_created = Model()
await _model_created.connect(tools.k8s_connection)
yield _model_created
finally:
if not k8s_model:
return
await tools.run("juju-crashdump", "-a", "config", "-m", tools.k8s_connection)
click.echo("Cleaning up k8s model")
try:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also -- stop trying to catch errors in the finally -- just let the exceptions flow so we can find the issues

for relation in k8s_model.relations:
if _model_created:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're careful here to not return in the finally, so exceptions can freely bubble out

await tools.run(
"juju-crashdump", "-a", "config", "-m", tools.k8s_connection
)
click.echo("Cleaning up k8s model")

for relation in _model_created.relations:
click.echo(f"Removing relation {relation.name} from k8s model")
await relation.destroy()

for name, offer in k8s_model.application_offers.items():
for name, offer in _model_created.application_offers.items():
click.echo(f"Removing offer {name} from k8s model")
await offer.destroy()

for name, app in k8s_model.applications.items():
for name, app in _model_created.applications.items():
click.echo(f"Removing app {name} from k8s model")
await app.destroy()

click.echo("Disconnecting k8s model")
await k8s_model.disconnect()
await _model_created.disconnect()

click.echo("Destroying k8s model")
await tools.run(
Expand All @@ -396,8 +385,6 @@ async def k8s_model(k8s_cloud, tools):
"-y",
tools.k8s_connection,
)
except Exception:
click.echo(format_exc())


@pytest.fixture
Expand Down Expand Up @@ -688,13 +675,18 @@ def pytest_metadata(metadata):


@pytest.fixture(scope="module")
async def kubeconfig(tools, model, tmp_path_factory):
local = tmp_path_factory.getbasetemp() / "kubeconfig"
k8s_cp = model.applications["kubernetes-control-plane"].units[0]
await scp_from(
k8s_cp, "config", local, None, tools.connection, proxy=tools.juju_ssh_proxy
)
yield local
async def kubeconfig(model):
control_planes = model.applications["kubernetes-control-plane"].units
(unit,) = [u for u in control_planes if await u.is_leader_from_status()]
action = await juju_run_action(unit, "get-kubeconfig")
# kubeconfig needs to be somewhere the juju confined snap client can access it
path = Path.home() / ".local/share/juju"
with NamedTemporaryFile(dir=path) as f:
Comment on lines +683 to +684
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because juju is a strictly confined snap, there are use cases where this temporary file really should be in a path that juju can read it. That's really only under $HOME/.local/share/juju
so it's a temporary file in here

let's add it to the os.environ for things like kubectl or juju to be able to use naturally without have to feed its filename into the env

local = Path(f.name)
local.write_text(action.results["kubeconfig"])
os.environ["KUBECONFIG"] = str(local)
yield local
del os.environ["KUBECONFIG"]


@pytest.fixture(scope="module")
Expand Down
10 changes: 3 additions & 7 deletions jobs/integration/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,11 @@ async def upgrade_charms(model, channel, tools):
model_name = model.info.name
for app_name, app in model.applications.items():
log.info(f"Upgrading {app_name} from {app.charm_url} to --channel={channel}")
juju_2 = model.connection().info["server-version"].startswith("2.")
command = "upgrade-charm" if juju_2 else "refresh"
await tools.run(
"juju", "upgrade-charm", "-m", model_name, app.name, "--channel", channel
"juju", command, "-m", model_name, app.name, "--channel", channel
)
# Blocked on https://github.com/juju/python-libjuju/issues/728
# try:
# await app.refresh(channel=channel)
# except JujuError as e:
# if "already running charm" not in str(e):
# raise
await tools.juju_wait()


Expand Down
16 changes: 3 additions & 13 deletions jobs/integration/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
timeout_for_current_task,
retry_async_with_timeout,
scp_to,
scp_from,
disable_source_dest_check,
find_entities,
verify_deleted,
Expand Down Expand Up @@ -359,21 +358,12 @@ async def test_microbot(model, tools, teardown_microbot):


@pytest.mark.clouds(["ec2", "vsphere"])
@pytest.mark.usefixtures("log_dir")
@backoff.on_exception(backoff.expo, TypeError, max_tries=5)
async def test_dashboard(model, log_dir, tools):
async def test_dashboard(model, kubeconfig, tools):
"""Validate that the dashboard is operational"""
unit = model.applications["kubernetes-control-plane"].units[0]
with NamedTemporaryFile() as f:
await scp_from(
unit,
"config",
f.name,
tools.controller_name,
tools.connection,
proxy=tools.juju_ssh_proxy,
)
with open(f.name, "r") as stream:
config = yaml.safe_load(stream)
config = yaml.safe_load(kubeconfig.open())

async def query_dashboard(url, config):
# handle pre 1.19 authentication
Expand Down