From dee284d669698107a8967edb36ed2f8f4dd40787 Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Sat, 8 Feb 2025 11:46:25 +0700 Subject: [PATCH 1/2] CLI: machine create use patch inventory for partial updates --- pkgs/clan-cli/clan_cli/machines/create.py | 17 +++++----- pkgs/clan-cli/tests/test_patch_inventory.py | 37 ++++++++++++++++----- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/machines/create.py b/pkgs/clan-cli/clan_cli/machines/create.py index a0f3e5e82..5ea2a4e67 100644 --- a/pkgs/clan-cli/clan_cli/machines/create.py +++ b/pkgs/clan-cli/clan_cli/machines/create.py @@ -10,11 +10,14 @@ from clan_cli.dirs import get_clan_flake_toplevel_or_env from clan_cli.errors import ClanError from clan_cli.flake import Flake from clan_cli.git import commit_file -from clan_cli.inventory import Machine as InventoryMachine +from clan_cli.inventory import ( + Machine as InventoryMachine, + patch_inventory_with, + dataclass_to_dict, +) from clan_cli.inventory import ( MachineDeploy, get_inventory, - set_inventory, ) from clan_cli.machines.list import list_nixos_machines from clan_cli.templates import ( @@ -100,20 +103,18 @@ def create_machine(opts: CreateOptions) -> None: copy_from_nixstore(src, dst) - inventory = get_inventory(clan_dir) - target_host = opts.target_host - # TODO: We should allow the template to specify machine metadata if not defined by user + new_machine = opts.machine if target_host: new_machine["deploy"] = {"targetHost": target_host} - inventory["machines"] = inventory.get("machines", {}) - inventory["machines"][machine_name] = new_machine + patch_inventory_with( + clan_dir, f"machines.{machine_name}", dataclass_to_dict(new_machine) + ) # Commit at the end in that order to avoid committing halve-baked machines # TODO: automatic rollbacks if something goes wrong - set_inventory(inventory, clan_dir, "Imported machine from template") commit_file( clan_dir / "machines" / machine_name, diff --git a/pkgs/clan-cli/tests/test_patch_inventory.py b/pkgs/clan-cli/tests/test_patch_inventory.py index cc2f73894..e1e1533b0 100644 --- a/pkgs/clan-cli/tests/test_patch_inventory.py +++ b/pkgs/clan-cli/tests/test_patch_inventory.py @@ -307,9 +307,7 @@ def test_update_list() -> None: assert writeables == {"writeable": {"foo"}, "non_writeable": set()} # Add "C" to the list - update = { - "foo": ["A", "B", "C"] # User wants to add "C" - } + update = {"foo": ["A", "B", "C"]} # User wants to add "C" patchset, _ = calc_patches( data_disk, update, all_values=data_eval, writeables=writeables @@ -320,9 +318,7 @@ def test_update_list() -> None: # "foo": ["A", "B"] # Remove "B" from the list # Expected is [ ] because ["A"] is defined in nix - update = { - "foo": ["A"] # User wants to remove "B" - } + update = {"foo": ["A"]} # User wants to remove "B" patchset, _ = calc_patches( data_disk, update, all_values=data_eval, writeables=writeables @@ -350,9 +346,7 @@ def test_update_list_duplicates() -> None: assert writeables == {"writeable": {"foo"}, "non_writeable": set()} # Add "A" to the list - update = { - "foo": ["A", "B", "A"] # User wants to add duplicate "A" - } + update = {"foo": ["A", "B", "A"]} # User wants to add duplicate "A" with pytest.raises(ClanError) as error: calc_patches(data_disk, update, all_values=data_eval, writeables=writeables) @@ -363,6 +357,31 @@ def test_update_list_duplicates() -> None: ) +def test_dont_persist_defaults() -> None: + """ + Default values should not be persisted to disk if not explicitly requested by the user. + """ + + prios = { + "enabled": {"__prio": 1500}, + "config": {"__prio": 100}, + } + data_eval = { + "enabled": True, + "config": {"foo": "bar"}, + } + data_disk = {} + writeables = determine_writeability(prios, data_eval, data_disk) + assert writeables == {"writeable": {"config", "enabled"}, "non_writeable": set()} + + update = {"config": {"foo": "foo"}} + patchset, delete_set = calc_patches( + data_disk, update, all_values=data_eval, writeables=writeables + ) + assert patchset == {"config.foo": "foo"} + assert delete_set == set() + + def test_update_mismatching_update_type() -> None: prios = { "foo": { From 9b706c148b3fa4046a6e2d6f99ed822b0f37bfba Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Mon, 10 Feb 2025 12:13:54 +0700 Subject: [PATCH 2/2] Inventory: automatically create emtpy file on write --- pkgs/clan-cli/clan_cli/inventory/__init__.py | 12 ++++++++++-- pkgs/clan-cli/clan_cli/machines/create.py | 5 ++--- pkgs/clan-cli/tests/test_patch_inventory.py | 4 +++- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/inventory/__init__.py b/pkgs/clan-cli/clan_cli/inventory/__init__.py index 9dba86ddf..0a7965f28 100644 --- a/pkgs/clan-cli/clan_cli/inventory/__init__.py +++ b/pkgs/clan-cli/clan_cli/inventory/__init__.py @@ -473,10 +473,18 @@ def patch(d: dict[str, Any], path: str, content: Any) -> None: @API.register def patch_inventory_with(base_dir: Path, section: str, content: dict[str, Any]) -> None: + """ + Pass only the section to update and the content to update with. + Make sure you pass only attributes that you would like to persist. + ATTENTION: Don't pass nix eval values unintentionally. + """ + inventory_file = get_inventory_path(base_dir) + curr_inventory = {} - with inventory_file.open("r") as f: - curr_inventory = json.load(f) + if inventory_file.exists(): + with inventory_file.open("r") as f: + curr_inventory = json.load(f) patch(curr_inventory, section, content) diff --git a/pkgs/clan-cli/clan_cli/machines/create.py b/pkgs/clan-cli/clan_cli/machines/create.py index 5ea2a4e67..c54309f86 100644 --- a/pkgs/clan-cli/clan_cli/machines/create.py +++ b/pkgs/clan-cli/clan_cli/machines/create.py @@ -12,12 +12,11 @@ from clan_cli.flake import Flake from clan_cli.git import commit_file from clan_cli.inventory import ( Machine as InventoryMachine, - patch_inventory_with, - dataclass_to_dict, ) from clan_cli.inventory import ( MachineDeploy, - get_inventory, + dataclass_to_dict, + patch_inventory_with, ) from clan_cli.machines.list import list_nixos_machines from clan_cli.templates import ( diff --git a/pkgs/clan-cli/tests/test_patch_inventory.py b/pkgs/clan-cli/tests/test_patch_inventory.py index e1e1533b0..387502db6 100644 --- a/pkgs/clan-cli/tests/test_patch_inventory.py +++ b/pkgs/clan-cli/tests/test_patch_inventory.py @@ -1,4 +1,6 @@ # Functions to test +from typing import Any + import pytest from clan_cli.errors import ClanError from clan_cli.inventory import ( @@ -370,7 +372,7 @@ def test_dont_persist_defaults() -> None: "enabled": True, "config": {"foo": "bar"}, } - data_disk = {} + data_disk: dict[str, Any] = {} writeables = determine_writeability(prios, data_eval, data_disk) assert writeables == {"writeable": {"config", "enabled"}, "non_writeable": set()}