feat: move VM provisioning from exordos_core into gcl_sdk, and per-node agent encryption keys - #494
Open
VasilyStepanov wants to merge 17 commits into
Open
feat: move VM provisioning from exordos_core into gcl_sdk, and per-node agent encryption keys#494VasilyStepanov wants to merge 17 commits into
VasilyStepanov wants to merge 17 commits into
Conversation
Contributor
Author
|
Run |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the compute module by offloading machine pool driver implementations, related models, enums, and exceptions to the external gcl_sdk library (using the gcl_sdk[libvirt] extra). Local driver files, including the libvirt and dummy drivers, have been deleted, and local enums and models in constants.py and models.py have been replaced with imports from gcl_sdk. Additionally, the libvirt-python dependency and several local entry points have been removed from pyproject.toml. I have no further feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
VasilyStepanov
changed the base branch from
del/poolagentdriver
to
feat/machine-pool-migrate-spec-to-kind
July 17, 2026 16:02
VasilyStepanov
commented
Jul 17, 2026
VasilyStepanov
changed the title
Contributor
Author
|
/gemini review |
VasilyStepanov
marked this pull request as ready for review
July 19, 2026 12:14
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the codebase by removing local machine pool drivers (including libvirt and dummy drivers) and their associated universal agent service, delegating these responsibilities to the gcl_sdk package. It also introduces support for managing agent private keys for local hypervisors, including provisioning them on machine pool insertion and exposing an API endpoint to retrieve them. The review feedback highlights two critical issues in exordos_core/compute/dm/models.py: first, the need to update an existing node encryption key if a different agent_private_key is explicitly provided to prevent out-of-sync credentials; and second, the necessity of guarding the get_agent_private_key method to prevent AttributeError when called on non-local hypervisor driver specs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
VasilyStepanov
added a commit
that referenced
this pull request
Jul 19, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
Base automatically changed from
feat/machine-pool-migrate-spec-to-kind
to
master
July 20, 2026 09:00
|
|
||
| from gcl_sdk.agents.universal.api import crypto as ua_crypto | ||
| from gcl_sdk.agents.universal.dm import models as ua_models | ||
| from gcl_sdk.agents.universal.drivers.pool import AbstractPoolDriverSpec # noqa: F401 |
Member
There was a problem hiding this comment.
Please use import modules instead of import attributes. It's related to other files as well.
| volume_builder = volume_builder_svc.VolumeBuilderService( | ||
| iter_min_period=iter_min_period | ||
| ) | ||
| pool_driver = ua_pool_drivers.PoolAgentDriver( |
Member
There was a problem hiding this comment.
Let's keep the agent in the core for local dev realms and for other clouds.
|
|
||
| if driver_key in self.__driver_map__: | ||
| return self.__driver_map__[driver_key] | ||
| def get_agent_private_key(self): |
Member
There was a problem hiding this comment.
Seems it's a common story and should moved to UA API
VasilyStepanov
added a commit
that referenced
this pull request
Jul 20, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
VasilyStepanov
added a commit
that referenced
this pull request
Jul 22, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
VasilyStepanov
changed the base branch from
master
to
feat/agent-registration-key-issuance
July 22, 2026 14:38
VasilyStepanov
added a commit
that referenced
this pull request
Jul 22, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
VasilyStepanov
added a commit
that referenced
this pull request
Jul 22, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
VasilyStepanov
added a commit
that referenced
this pull request
Jul 23, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
VasilyStepanov
added a commit
that referenced
this pull request
Jul 23, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
VasilyStepanov
added a commit
that referenced
this pull request
Jul 27, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
VasilyStepanov
added a commit
that referenced
this pull request
Jul 27, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
VasilyStepanov
added a commit
that referenced
this pull request
Jul 27, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
VasilyStepanov
added a commit
that referenced
this pull request
Jul 27, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
VasilyStepanov
added a commit
that referenced
this pull request
Jul 29, 2026
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
The machine_pool_agent service wired PoolAgentDriver into a UniversalAgentService but nothing consumed its capability, and no docs or tests reference it, so drop the dead wiring from GeneralService.
The libvirt-based machine pool driver and its universal-agent bridge
(compute/agents/universal/drivers/pool.py) now live natively in gcl_sdk
as a MetaCoordinatorAgentDriver (gcl_sdk.agents.universal.drivers.pool /
.libvirt), so remove the exordos_core-side copies:
- exordos_core/compute/pool/drivers/{base,exceptions,libvirt}.py
- exordos_core/compute/agents/universal/drivers/pool.py (and the now-empty
compute/agents/ tree)
- the gcn_machine_pool_driver entry points and the PoolAgentDriver/
LocalPoolAgentDriver entries under gcl_sdk_universal_agent
- the libvirt-python dependency (now pulled in transitively via the
gcl_sdk[libvirt] extra)
compute/dm/models.py and compute/constants.py now re-export the moved
spec/storage-pool models and status enums from gcl_sdk instead of
defining them locally, so existing call sites (models.LibvirtPoolDriverSpec,
nc.MachineStatus, etc.) keep working unchanged. Also drop the dead
MachinePool.load_driver() method, which had no remaining callers.
compute/pool/dm/models.py (the control-plane Pool/Machine/MachineVolume
ORM models used by the scheduler/builder) is untouched.
Requires a gcl_sdk release containing gcl_sdk.agents.universal.drivers.pool/
.libvirt before `uv lock` can be re-run here; the gcl_sdk[libvirt] dependency
bump in pyproject.toml is in place but uv.lock is left as-is until then.
…iver exordos_universal_agent.conf.j2's caps_drivers never included PoolAgentDriver/LocalPoolAgentDriver, so the CORE VM never actually calls into libvirt - the gcl_sdk[libvirt] dependency added when the driver moved into gcl_sdk was unnecessary here. The host that runs LibvirtPoolDriver is the one provisioned via `exordos compute hypervisors init`, which now installs gcl_sdk[libvirt] itself. Also make bootstrap/defaults.py drop unset optional driver_spec fields (e.g. `node`, only used by the "exordos_local_hyper" kind) before restoring the MachinePool, since KindModelType.from_simple_type() rejects fields the target kind doesn't define - dataclasses.asdict() on exordos.stand.models.Hypervisor now always emits `node`, `None` for plain "libvirt" hypervisors.
…gents The local-pool-agent's self-registration to status_api/orch_api was failing with a 500: unencrypted communication never sends the X-Genesis-Node-UUID header the SDK middleware requires, so it can't work regardless of endpoint. Provision a NodeEncryptionKey for the agent's node uuid when a MachinePool is created for the exordos_local_hyper kind, accepting a pre-generated key so the bootstrap flow (which writes the same key to the agent's disk) stays in sync instead of getting a mismatched one. Also expose the key via a get_agent_private_key action for the non-bootstrap registration path.
…cting A machine can be both a registered compute Node and a local hypervisor pool - both provision a NodeEncryptionKey keyed by the same node uuid. MachinePool.insert() always tried to create a fresh one unconditionally, so registering a hypervisor on an already-provisioned node hit a duplicate-key conflict. The key is an identity for the node, not a specific agent process, so reuse whatever already exists instead.
- MachinePool.insert(): when an explicit agent_private_key is given and an existing NodeEncryptionKey for the node already differs from it, update the stored key to match instead of silently keeping the stale one. A caller that already generated and deployed a specific key (the bootstrap flow) must end up in sync with the DB - an existing key from unrelated prior provisioning (e.g. this node's own compute Node) would otherwise silently win, leaving the agent unable to authenticate with the key it was actually given. - get_agent_private_key(): raise a clear error for non-local hypervisors (driver_spec without a `node` field) instead of an AttributeError. This is reachable directly through the get_agent_private_key hypervisor action on any hypervisor UUID, regardless of kind.
exordos_core/compute/dm/models.py imported individual classes (LibvirtPoolDriverSpec, ExordosLocalHyperDriverSpec, ThinStoragePool, etc.) directly from gcl_sdk.agents.universal.drivers.pool, including a noqa'd re-export of AbstractPoolDriverSpec/AbstractStoragePool kept around only so other modules could keep referencing them via exordos_core's own models module. Import the pool module itself instead (`pool as ua_pool`), matching the convention used elsewhere in this codebase for gcl_sdk imports. Update the handful of call sites that relied on the models.py re-export to import the pool module directly too.
Wires gcl_sdk's PoolAgentDriver into GeneralService as a universal agent, so a MachinePool with driver_spec kind "libvirt" is actually managed again (previously removed as dead code before the pool driver existed in gcl_sdk). This is the counterpart to LocalPoolAgentDriver: it runs inside core itself and reaches libvirt over the network, instead of via a dedicated agent on the hypervisor host. Requires the gcl_sdk[libvirt] extra for the libvirt python bindings. Note: uv.lock is stale until a gcl_sdk release with the libvirt extra is published (the currently locked 3.1.1 predates it); regenerate the lock once that's out.
install.sh's dev-mode reinstall (LOCAL_GENESIS_SDK_PATH set) replaced the uv-synced gcl_sdk with a bare editable install, dropping the libvirt extra pyproject.toml declares and leaving libvirt-python missing - PoolAgentDriver then fails to import at gservice startup.
Both MachinePool.insert() and the new AgentController.issue_key action reimplemented the same get-or-create-by-node-uuid logic via raw NodeEncryptionKey ORM access. Delegate to gcl_sdk's new NodeEncryptionKey.get_or_create() instead, keeping MachinePool's bootstrap key-sync behavior (agent_private_key override) on top of it.
…te_key action It was added as a stopgap for a non-bootstrap agent registration path that didn't exist yet, has no callers anywhere in exordos_core/exordos, and that path is now properly served by AgentController.issue_key.
MachinePool.insert() reached directly into NodeEncryptionKey, making the model aware of a concern that belongs to whoever registers a local-hypervisor pool, not to the pool itself. Move key provisioning to the two actual callers - bootstrap's direct DM insert and HypervisorsController.create() - each now calling gcl_sdk's NodeEncryptionKey.get_or_create() explicitly after the pool exists.
Replace isinstance(driver_spec, ExordosLocalHyperDriverSpec) branching in HypervisorsController.create() and bootstrap's apply_startup_db with driver_spec.agent_key_node, a declarative property each pool driver spec exposes (None unless its agent needs a per-node key).
Drop the BootAlternative/BootType/MachinePoolStatus/MachineStatus/ NodeType/PortStatus/VolumeStatus re-exports (BootType had zero users anyway) and import them directly from gcl_sdk.agents.universal.drivers at each call site instead.
Node.insert() unconditionally created a NodeEncryptionKey for its uuid, so registering a compute Node for a machine that was already a local hypervisor's node (which provisions its own key the same way) crashed on the unique node uuid. Route through get_or_create like MachinePool already does.
Match the surrounding file's own convention (e.g. the set_agent block right below) instead of inlining the uuid5 call as a constructor arg.
gcl-sdk 3.2.2 doesn't publish the `libvirt` extra yet (that's still on gcl_sdk's own add/pool branch, unreleased) - uv resolves the request as a no-op for now, which is fine since libvirt-python is already a direct dependency here regardless.
VasilyStepanov
changed the base branch from
master
to
fix/boot-port-network-type
July 29, 2026 12:20
Member
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GeneralService.
both a compute Node and a hypervisor pool doesn't conflict with itself), exposed via a get_agent_private_key hypervisor action so a local agent can fetch and install the matching key.