Merge pull request 'CLI: use partial update for machine create' (#2848) from hsjobeki/clan-core:hsjobeki-main into main

Reviewed-on: https://git.clan.lol/clan/clan-core/pulls/2848

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)
 

pkgs/clan-cli/clan_cli/machines/create.py

@@ -10,11 +10,13 @@ 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,
+)
 from clan_cli.inventory import (
     MachineDeploy,
-    get_inventory,
-    set_inventory,
+    dataclass_to_dict,
+    patch_inventory_with,
 )
 from clan_cli.machines.list import list_nixos_machines
 from clan_cli.templates import (
@@ -100,20 +102,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,

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 (
@@ -307,9 +309,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 +320,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 +348,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 +359,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: dict[str, Any] = {}
+    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": {