Skip to content
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
6 changes: 3 additions & 3 deletions docusaurus/docs/commands.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ This document provides a comprehensive reference for all available CLI commands

Create a cluster with custom settings:
$ v8x cluster create mycluster --cloud-account-id 16 --settings
'{"autoscaler_enabled":true}'
'{"kubeflow_enabled":true}'

Create a cluster with an app:
$ v8x cluster create mycluster --cloud-account-id 16 --app
Expand Down Expand Up @@ -681,7 +681,7 @@ This document provides a comprehensive reference for all available CLI commands
│ [default: slurm] │
│ --settings -s <str> Cluster settings as JSON │
│ string (e.g., │
│ '{"autoscaler_enabled":tr… │
│ '{"kubeflow_enabled":tr… │
│ --config -c <str> Cluster config as JSON │
│ string (e.g., │
│ '{"key":"value"}'). │
Expand Down Expand Up @@ -883,7 +883,7 @@ This document provides a comprehensive reference for all available CLI commands
v8x cluster update my-cluster --status ready

# Replace all settings
v8x cluster update my-cluster --settings '{"autoscaler_enabled": true}'
v8x cluster update my-cluster --settings '{"kubeflow_enabled": true}'

# Merge new settings with existing settings
v8x cluster update my-cluster --settings '{"new_key": "value"}' --merge
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ dependencies = [
"pyyaml>=6.0.3",
"pyopenssl>=25.3.0",
"qrcode>=8.0.0",
"vantage-sdkpy>=0.1.23",
"vantage-sdkpy>=0.1.24",
]

[tool.hatch.build.targets.wheel]
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_cluster_sdk_delegation.py
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ async def test_disable_service_heavy_cascade_delegates_to_workflow_sdk(
async def test_cluster_update_triggers_workflow_sdk(monkeypatch: pytest.MonkeyPatch):
ctx = _ctx()
cluster_with_settings = SimpleNamespace(client_id="client-1")
merged_settings = {"autoscaler_enabled": True}
merged_settings = {"kubeflow_enabled": True}
monkeypatch.setattr(
"v8x.commands.cluster.update._merge_and_validate_settings",
AsyncMock(return_value=(cluster_with_settings, merged_settings)),
Expand Down Expand Up @@ -998,7 +998,7 @@ async def test_cluster_update_triggers_workflow_sdk(monkeypatch: pytest.MonkeyPa
cluster_name="cluster-a",
description=None,
status=None,
settings='{"autoscaler_enabled": true}',
settings='{"kubeflow_enabled": true}',
settings_file=None,
config=None,
config_file=None,
Expand Down
81 changes: 80 additions & 1 deletion tests/unit/test_lxd_slurm_provider_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ def fake_run(cmd: list[str], **_: object) -> subprocess.CompletedProcess[str]:
vantage_cluster_ctx=_cluster_ctx(
{
"default_control_plane_compute_pool": {
"name": "control-plane", "instance_type": "control-plane-smx"
"name": "control-plane",
"instance_type": "control-plane-smx",
}
}
),
Expand Down Expand Up @@ -142,3 +143,81 @@ def fake_run(cmd: list[str], **_: object) -> subprocess.CompletedProcess[str]:
assert cmd[cmd.index("--containerd-device") + 1] == "/dev/disk/by-id/custom-containerd"
assert cmd[cmd.index("--containerd-disk-size") + 1] == "250"
assert cmd[cmd.index("--containerd-storage-pool") + 1] == "fast-pool"


def test_provider_command_reads_organization_from_the_nested_keycloak_block(monkeypatch) -> None:
"""The gateway Istio org-claim policy depends on these two flags.

vdeployer nests keycloak settings, so this read moved from
settings["keycloak_organization_id"] to settings["keycloak"]["organization_id"].
Renaming only the *write* sites would have left this returning None and
dropped --organization-id/--organization-name from the command -- disabling
the auth policy with no error and no log line.
"""
captured: dict[str, list[str]] = {}

def fake_run(cmd: list[str], **_: object) -> subprocess.CompletedProcess[str]:
captured["cmd"] = cmd
return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")

monkeypatch.setattr(lxd_app.subprocess, "run", fake_run)

lxd_app._run_vantage_provider_provision(
ctx=_ctx(),
vantage_cluster_ctx=_cluster_ctx(
{"keycloak": {"organization_id": "org-uuid", "organization_name": "Acme"}}
),
binary_path=Path("/tmp/vantage-provider"),
)

cmd = captured["cmd"]
assert cmd[cmd.index("--organization-id") + 1] == "org-uuid"
assert cmd[cmd.index("--organization-name") + 1] == "Acme"


def test_provider_command_omits_organization_flags_when_absent(monkeypatch) -> None:
"""No keycloak block at all must not raise -- older clusters have none."""
captured: dict[str, list[str]] = {}

def fake_run(cmd: list[str], **_: object) -> subprocess.CompletedProcess[str]:
captured["cmd"] = cmd
return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")

monkeypatch.setattr(lxd_app.subprocess, "run", fake_run)

lxd_app._run_vantage_provider_provision(
ctx=_ctx(),
vantage_cluster_ctx=_cluster_ctx({}),
binary_path=Path("/tmp/vantage-provider"),
)

cmd = captured["cmd"]
assert "--organization-id" not in cmd
assert "--organization-name" not in cmd


def test_provider_command_ignores_the_stale_flat_organization_keys(monkeypatch) -> None:
"""A pre-nesting settings blob must not silently keep working here.

If this passed, it would mean the read still honours the flat keys and the
migration is incomplete somewhere.
"""
captured: dict[str, list[str]] = {}

def fake_run(cmd: list[str], **_: object) -> subprocess.CompletedProcess[str]:
captured["cmd"] = cmd
return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")

monkeypatch.setattr(lxd_app.subprocess, "run", fake_run)

lxd_app._run_vantage_provider_provision(
ctx=_ctx(),
vantage_cluster_ctx=_cluster_ctx(
{"keycloak_organization_id": "org-uuid", "keycloak_organization_name": "Acme"}
),
binary_path=Path("/tmp/vantage-provider"),
)

cmd = captured["cmd"]
assert "--organization-id" not in cmd
assert "--organization-name" not in cmd
84 changes: 84 additions & 0 deletions tests/unit/test_vdeployer_nested_settings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
"""vdeployer settings must be sent in the nested shape it now expects.

`vdeployer` moved keycloak_client_id / client_secret / organization_id /
organization_name under a nested `keycloak:` config class as a clean break --
no pydantic aliases. Any flat key v8x sends now binds to nothing and is dropped
by `extra="ignore"`, silently. `vdeployer` derives cluster_name from client_id
and raises ValueError building the tunnel URL without it, so a regression here
breaks deploys rather than degrading them.

These assert the wire shape at the boundary, which is the thing that has to stay
right; the flat form produced no error anywhere, which is why it needs a test
rather than trust.
"""

from __future__ import annotations

from types import SimpleNamespace

import pytest

from v8x.commands.cluster.utils import build_vdeployer_settings


def _cluster(**overrides):
base = {
"name": "mycluster",
"client_id": "cid-abc",
"client_secret": "sec-xyz",
"sssd_binder_password": "pw",
"creation_parameters": {},
}
base.update(overrides)
return SimpleNamespace(**base)


def _persona(org_id: str | None = "org-uuid"):
return SimpleNamespace(identity_data=SimpleNamespace(org_id=org_id))


def test_keycloak_credentials_are_sent_nested_not_flat() -> None:
out = build_vdeployer_settings({}, "lxd", _cluster(), _persona())

assert out["keycloak"] == {
"client_id": "cid-abc",
"client_secret": "sec-xyz",
"organization_id": "org-uuid",
}
assert not [k for k in out if k.startswith("keycloak_")], (
"flat keycloak_* keys bind to nothing on vdeployer and are dropped silently"
)


def test_a_caller_supplied_keycloak_block_survives() -> None:
"""Merged, not assigned over -- organization_name has no other source."""
out = build_vdeployer_settings(
{"keycloak": {"organization_name": "Acme"}}, "lxd", _cluster(), _persona()
)

assert out["keycloak"]["organization_name"] == "Acme"
assert out["keycloak"]["client_id"] == "cid-abc"


def test_the_callers_settings_dict_is_not_mutated() -> None:
"""`dict(settings)` is a shallow copy, so a nested block must be rebuilt."""
caller = {"keycloak": {"organization_name": "Acme"}}
build_vdeployer_settings(caller, "lxd", _cluster(), _persona())

assert caller == {"keycloak": {"organization_name": "Acme"}}


def test_unrelated_settings_pass_through() -> None:
out = build_vdeployer_settings({"kubeflow_enabled": True}, "aws", _cluster(), _persona())

assert out["kubeflow_enabled"] is True
assert out["provider"] == "aws"
assert out["cluster_name"] == "mycluster"


def test_missing_org_context_still_aborts() -> None:
"""The nesting change must not have weakened this guard."""
from vantage_sdk.exceptions import Abort

with pytest.raises(Abort):
build_vdeployer_settings({}, "lxd", _cluster(), _persona(org_id=None))
8 changes: 4 additions & 4 deletions uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 22 additions & 8 deletions v8x/apps/lxd/slurm/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import subprocess
import urllib.parse
from pathlib import Path
from typing import Annotated, Optional
from typing import Annotated, Any, Optional

import httpx
import typer
Expand Down Expand Up @@ -53,8 +53,16 @@
logger = logging.getLogger("v8x.apps.lxd.slurm")


def _asc(ctx: VantageClusterContext, key: str, default: object = None) -> object:
"""Read a key from the nested ``vantage_lxd_autoscaler`` settings block."""
def _asc(ctx: VantageClusterContext, key: str, default: object = None) -> Any:
"""Read a key from the nested ``vantage_lxd_autoscaler`` settings block.

Returns ``Any`` rather than ``object`` because the value comes out of an
untyped settings dict and callers legitimately feed it to ``int()`` and to
string contexts. ``object`` was not more truthful -- it only moved the
problem to the call sites, where pyright rejected
``int(_asc(...) or DEFAULT)`` as "object is incompatible with
SupportsInt".
"""
return ctx.settings.get("vantage_lxd_autoscaler", {}).get(key, default)


Expand Down Expand Up @@ -300,10 +308,11 @@ def _run_vantage_provider_provision( # noqa: C901
cmd.extend(["--extra-api-san", extra_sans])

# Organization info for gateway-level Istio auth policy
if org_id := vantage_cluster_ctx.settings.get("keycloak_organization_id"):
keycloak_settings = vantage_cluster_ctx.settings.get("keycloak") or {}
if org_id := keycloak_settings.get("organization_id"):
cmd.extend(["--organization-id", org_id])

if org_name := vantage_cluster_ctx.settings.get("keycloak_organization_name"):
if org_name := keycloak_settings.get("organization_name"):
cmd.extend(["--organization-name", org_name])

if ctx.obj.cloud_config_metadata.get("dev_mode"):
Expand Down Expand Up @@ -829,9 +838,14 @@ async def _trigger_vdeployer_deploy(

# Build settings from the cluster's stored creation_parameters.settings
deploy_settings = dict(vctx.settings)
deploy_settings["keycloak_client_id"] = vctx.client_id
deploy_settings["keycloak_client_secret"] = vctx.client_secret
deploy_settings["keycloak_organization_id"] = vctx.org_id
# vdeployer nests these under `keycloak:`; the flat keys it used to
# accept now bind to nothing and are dropped by `extra="ignore"`.
deploy_settings["keycloak"] = {
**(deploy_settings.get("keycloak") or {}),
"client_id": vctx.client_id,
"client_secret": vctx.client_secret,
"organization_id": vctx.org_id,
}
deploy_settings["sssd_binder_password"] = vctx.sssd_binder_password
deploy_settings["vantage_url"] = ctx.obj.settings.vantage_url
if vctx.jupyterhub_token:
Expand Down
23 changes: 13 additions & 10 deletions v8x/commands/cluster/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ async def create_cluster( # noqa: C901
typer.Option(
"--settings",
"-s",
help="Cluster settings as JSON string (e.g., '{\"autoscaler_enabled\":true}').",
help="Cluster settings as JSON string (e.g., '{\"kubeflow_enabled\":true}').",
),
] = None,
config: Annotated[
Expand Down Expand Up @@ -330,7 +330,7 @@ async def create_cluster( # noqa: C901
$ v8x cluster create mycluster --cloud-account my-aws-account

Create a cluster with custom settings:
$ v8x cluster create mycluster --cloud-account-id 16 --settings '{"autoscaler_enabled":true}'
$ v8x cluster create mycluster --cloud-account-id 16 --settings '{"kubeflow_enabled":true}'

Create a cluster with an app:
$ v8x cluster create mycluster --cloud-account-id 16 --app slurm-lxd-localhost
Expand Down Expand Up @@ -553,8 +553,11 @@ async def create_cluster( # noqa: C901
# Pass organization identity to vdeployer for Istio auth policies
if ctx.obj.persona and ctx.obj.persona.identity_data:
identity = ctx.obj.persona.identity_data
vdeployer_settings_dict["keycloak_organization_id"] = identity.org_id
vdeployer_settings_dict["keycloak_organization_name"] = identity.org_name
vdeployer_settings_dict["keycloak"] = {
**(vdeployer_settings_dict.get("keycloak") or {}),
"organization_id": identity.org_id,
"organization_name": identity.org_name,
}

# Set vantage_url from CLI context (applies to all providers)
vdeployer_settings_dict["vantage_url"] = ctx.obj.settings.vantage_url
Expand Down Expand Up @@ -595,15 +598,15 @@ async def create_cluster( # noqa: C901
"http://"
)
vdeployer_settings_dict["local_registry"] = local_registry
asc.setdefault("cloud_init", {})["local_registry"] = (
cloud_account_attributes["local_registry"]
)
asc.setdefault("cloud_init", {})["local_registry"] = cloud_account_attributes[
"local_registry"
]

cloud_init = asc.setdefault("cloud_init", {})
if "vantage_node_security_binary_url" in cloud_account_attributes:
cloud_init["node_security_binary_url"] = (
cloud_account_attributes["vantage_node_security_binary_url"]
)
cloud_init["node_security_binary_url"] = cloud_account_attributes[
"vantage_node_security_binary_url"
]

if "dev_mode" in cloud_account_attributes:
cloud_init["dev_mode"] = cloud_account_attributes["dev_mode"]
Expand Down
2 changes: 1 addition & 1 deletion v8x/commands/cluster/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ async def update_cluster(
v8x cluster update my-cluster --status ready

# Replace all settings
v8x cluster update my-cluster --settings '{"autoscaler_enabled": true}'
v8x cluster update my-cluster --settings '{"kubeflow_enabled": true}'

# Merge new settings with existing settings
v8x cluster update my-cluster --settings '{"new_key": "value"}' --merge
Expand Down
Loading
Loading