|
| 1 | +# Project Cleanup Subsystem — Design |
| 2 | + |
| 3 | +## Context |
| 4 | + |
| 5 | +The python `openstacksdk` implements `OpenStackCloud.project_cleanup()` |
| 6 | +(`openstack/cloud/openstackcloud.py`) as follows: |
| 7 | + |
| 8 | +- Each service proxy may implement `_get_cleanup_dependencies()` (returns |
| 9 | + `{before: [...], after: [...]}` service names) and `_service_cleanup(...)`. |
| 10 | +- `project_cleanup` builds a service-level DAG (`utils.TinyDAG`) from these |
| 11 | + dependency hints and walks it with a thread pool, invoking each service's |
| 12 | + `_service_cleanup` in dependency order. |
| 13 | +- Each service's `_service_cleanup` is a single large imperative function |
| 14 | + that lists, filters, and deletes its own resources. Resources discovered |
| 15 | + as "to delete" are pushed into a shared `identified_resources` dict so |
| 16 | + other services' cleanup functions can consult what's already marked, |
| 17 | + and into a `status_queue` for caller visibility. |
| 18 | +- `dry_run` gates whether `del_fn` actually gets called, but the same flag |
| 19 | + is reused mid-function as a probe (network proxy calls |
| 20 | + `_service_cleanup_del_res(..., dry_run=True)` to *check* whether a |
| 21 | + network needs deleting, before making its real delete decision) — this |
| 22 | + conflates "user asked for dry run" with "internal evaluation call." |
| 23 | + |
| 24 | +Problems this design causes, confirmed by reading |
| 25 | +`openstack/network/v2/_proxy.py::_service_cleanup` (lines ~9837–10108): |
| 26 | + |
| 27 | +1. **Dependency graph is service-level only.** Any ordering that depends on |
| 28 | + actual resource relationships (a specific port belongs to a specific |
| 29 | + network; a router is attached via a specific interface port) can't be |
| 30 | + expressed as a graph edge. The network proxy instead hand-codes this as |
| 31 | + ~270 lines of imperative logic: list networks, list ports per network, |
| 32 | + classify port by `device_owner`, decide if the network "has ports |
| 33 | + allocated," detach router interfaces, delete ports, delete subnets, |
| 34 | + delete network, delete orphaned routers — all inline, untestable in |
| 35 | + isolation, and specific to network. |
| 36 | +2. **No real plan/approve mode.** Selection ("should this resource be |
| 37 | + deleted") and deletion happen interleaved in the same imperative pass, |
| 38 | + with mutable shared state (`identified_resources`) mutated across |
| 39 | + threads as services run concurrently. There's no point where a |
| 40 | + complete, stable "here's what will be deleted and why" object exists |
| 41 | + that a caller could inspect, edit, and then apply. `dry_run=True` only |
| 42 | + suppresses the delete call; it doesn't produce an artifact. |
| 43 | +3. **Not extensible.** Only services shipped in openstacksdk itself can |
| 44 | + participate, by defining these two dunder-ish methods on their proxy. |
| 45 | + A caller can't inject a cleanup hook for a service the SDK doesn't |
| 46 | + support, or override/augment built-in behavior, without subclassing |
| 47 | + the proxy classes. |
| 48 | + |
| 49 | +## Goals |
| 50 | + |
| 51 | +Design a project-cleanup subsystem for the rust `openstack_sdk` crate that: |
| 52 | + |
| 53 | +- Expresses dependencies at both the service level (coarse ordering hints) |
| 54 | + and the resource level (relationships between actual discovered |
| 55 | + resources), so no service needs to hand-code cascade/ordering logic. |
| 56 | +- Produces a real two-phase plan/approve flow: a discovery phase builds a |
| 57 | + complete, inspectable `CleanupPlan`; a separate apply phase executes |
| 58 | + only what's selected in that plan. |
| 59 | +- Lets callers inject their own cleanup providers (for services the SDK |
| 60 | + doesn't support, or to customize/override built-in behavior) through |
| 61 | + the same interface used by built-in providers — no special-casing. |
| 62 | + |
| 63 | +## Non-goals (v1) |
| 64 | + |
| 65 | +- Sync execution support (SDK's `sync` feature). This subsystem targets |
| 66 | + the `async` feature only. |
| 67 | +- Full parity with every service python covers. v1 ships compute, |
| 68 | + network, block-storage, image, and identity-scoped resources; the |
| 69 | + extension mechanism is designed so other services attach later with no |
| 70 | + core changes. |
| 71 | +- Automatic re-validation of plan freshness (re-listing resources between |
| 72 | + discover and apply). Apply attempts deletes and tolerates 404s from |
| 73 | + resources that vanished in the interim; it does not re-run discovery. |
| 74 | + |
| 75 | +## Architecture |
| 76 | + |
| 77 | +### Resource envelope |
| 78 | + |
| 79 | +Cleanup logic must be able to reason generically about resources without |
| 80 | +being generic over every SDK resource type, mirroring python's untyped |
| 81 | +`resource.Resource` handling in the cleanup path: |
| 82 | + |
| 83 | +```rust |
| 84 | +pub struct PlannedResource { |
| 85 | + pub kind: ResourceKind, // e.g. ResourceKind::new("network", "network") |
| 86 | + pub id: String, |
| 87 | + pub name: Option<String>, |
| 88 | + pub raw: serde_json::Value, // full resource body, for relation matching/filters |
| 89 | + pub selected: bool, // discovery's filter verdict; caller may flip before apply |
| 90 | + pub reason: Option<String>, // why selected/skipped, for plan display |
| 91 | +} |
| 92 | + |
| 93 | +pub struct ResourceKind { |
| 94 | + pub service_type: &'static str, // "network", "compute", ... |
| 95 | + pub resource_type: &'static str, // "port", "server", ... |
| 96 | +} |
| 97 | +``` |
| 98 | + |
| 99 | +### Two dependency layers |
| 100 | + |
| 101 | +**Service-level (`CleanupDependency`)** — same shape as python's |
| 102 | +`{before, after}`, used only for ordering that isn't about specific |
| 103 | +resource relationships (e.g. identity-scoped project resources should be |
| 104 | +handled after everything else that lives inside the project). |
| 105 | + |
| 106 | +**Resource-level (`RelationRule`)** — declarative edges between resource |
| 107 | +*kinds*, evaluated against actually discovered `PlannedResource`s during |
| 108 | +the discovery phase: |
| 109 | + |
| 110 | +```rust |
| 111 | +pub struct RelationRule { |
| 112 | + pub parent_kind: ResourceKind, |
| 113 | + pub child_kind: ResourceKind, |
| 114 | + pub matches: fn(child: &PlannedResource, parent: &PlannedResource) -> bool, |
| 115 | + pub effect: RelationEffect, |
| 116 | +} |
| 117 | + |
| 118 | +pub enum RelationEffect { |
| 119 | + /// Parent cannot be deleted while a matching, still-selected-or-existing |
| 120 | + /// child exists. Generic replacement for network's |
| 121 | + /// `network_has_ports_allocated` check. |
| 122 | + Blocks, |
| 123 | + /// Selecting any member of the group selects every member; the group |
| 124 | + /// has its own internal sub-order. Generic replacement for network's |
| 125 | + /// "networks are crazy, delete router+net+subnet together" cascade. |
| 126 | + CascadeGroup { order: fn(&[PlannedResource]) -> Vec<usize> }, |
| 127 | + /// Before the parent is deleted, run this action to sever the |
| 128 | + /// relationship (does not delete the child). Generic replacement for |
| 129 | + /// `remove_interface_from_router`. |
| 130 | + Detach(fn(&CleanupContext, child: &PlannedResource) -> BoxFuture<'_, Result<(), CleanupError>>), |
| 131 | +} |
| 132 | +``` |
| 133 | + |
| 134 | +This is the direct fix for the network proxy's hacks: what's currently |
| 135 | +270 lines of one-off imperative code becomes three `RelationRule` values |
| 136 | +declared by the network provider, using primitives every other provider |
| 137 | +can reuse. |
| 138 | + |
| 139 | +### Two-phase execution |
| 140 | + |
| 141 | +**Discover phase.** Every registered `CleanupProvider` lists its resources |
| 142 | +concurrently (tokio tasks respecting only service-level `CleanupDependency` |
| 143 | +ordering where a provider genuinely needs another service's data to list |
| 144 | +its own — e.g. needing a project-scoped list). Each provider's discovered |
| 145 | +resources are merged into one node set; `RelationRule`s are evaluated |
| 146 | +against the merged set to compute edges and apply `Blocks`/`CascadeGroup` |
| 147 | +effects. The per-resource filter/evaluation callback (equivalent of |
| 148 | +python's `resource_evaluation_fn` and built-in filters like |
| 149 | +`created_at`/`updated_at`) runs here too, setting `selected`. Output is a |
| 150 | +`CleanupPlan { nodes: Vec<PlannedResource>, edges: Vec<(NodeIdx, NodeIdx, RelationEffect)> }`, |
| 151 | +which is `serde`-serializable — it can be printed as a table/tree, diffed, |
| 152 | +or handed back after a caller/CLI lets the user toggle `selected` flags. |
| 153 | +No deletions happen in this phase. |
| 154 | + |
| 155 | +**Apply phase.** Takes a `CleanupPlan` (possibly edited) and walks it as a |
| 156 | +DAG: for `Blocks` edges, children delete before parents; for |
| 157 | +`CascadeGroup`s, members delete in the group's declared internal order; |
| 158 | +`Detach` actions run immediately before their parent's delete call. Only |
| 159 | +`selected` nodes are touched. Deletes run concurrently across independent |
| 160 | +subgraphs (tokio tasks + a shared "node done" signal, the same shape as |
| 161 | +python's `TinyDAG.walk`/`node_done`, implemented with `petgraph` for graph |
| 162 | +structure and topological walking). A delete returning "not found" is |
| 163 | +treated as success (plan may be stale relative to real state). |
| 164 | + |
| 165 | +### Extensibility |
| 166 | + |
| 167 | +```rust |
| 168 | +#[async_trait] |
| 169 | +pub trait CleanupProvider: Send + Sync { |
| 170 | + fn service_type(&self) -> &'static str; |
| 171 | + fn dependencies(&self) -> CleanupDependency { CleanupDependency::default() } |
| 172 | + fn relations(&self) -> Vec<RelationRule> { vec![] } |
| 173 | + async fn discover(&self, ctx: &CleanupContext) -> Result<Vec<PlannedResource>, CleanupError>; |
| 174 | + async fn delete(&self, ctx: &CleanupContext, r: &PlannedResource) -> Result<(), CleanupError>; |
| 175 | +} |
| 176 | +``` |
| 177 | + |
| 178 | +A `ProjectCleanupBuilder` registers providers: |
| 179 | + |
| 180 | +```rust |
| 181 | +let cleanup = ProjectCleanupBuilder::new(session) |
| 182 | + .with_provider(NetworkCleanupProvider::default()) // built-in |
| 183 | + .with_provider(ComputeCleanupProvider::default()) // built-in |
| 184 | + .with_provider(MyOrgCustomCleanupProvider::new(...)) // caller-injected, same trait |
| 185 | + .build(); |
| 186 | + |
| 187 | +let plan = cleanup.discover(&filters).await?; |
| 188 | +// caller inspects/edits plan.nodes[*].selected |
| 189 | +let result = cleanup.apply(plan).await?; |
| 190 | +``` |
| 191 | + |
| 192 | +Built-in and caller-supplied providers are indistinguishable to the |
| 193 | +engine — this directly satisfies the extensibility requirement without |
| 194 | +subclassing or special-casing. |
| 195 | + |
| 196 | +### Placement |
| 197 | + |
| 198 | +New module `openstack_sdk::cleanup`, gated behind the existing `async` |
| 199 | +feature (matches the crate's current `#[cfg(feature = "async")]` |
| 200 | +structure in `lib.rs`). Reusable from `openstack_cli`/`openstack_tui` |
| 201 | +without duplicating logic. |
| 202 | + |
| 203 | +### Error handling |
| 204 | + |
| 205 | +Provider `discover`/`delete` errors are collected per-`PlannedResource` |
| 206 | +into the plan/apply result (not per-service, as python does) and don't |
| 207 | +abort the overall run — one resource failing to delete doesn't block |
| 208 | +unrelated subgraphs. This is a strict improvement over python's per-service |
| 209 | +`try/except` + log, since python's per-service scope hides which |
| 210 | +individual resource failed inside a service that touches many resource |
| 211 | +types. |
| 212 | + |
| 213 | +### Testing |
| 214 | + |
| 215 | +`RelationRule` evaluation, `Blocks`/`CascadeGroup` resolution, and DAG |
| 216 | +ordering are unit-testable against synthetic `PlannedResource` sets with |
| 217 | +no live cloud connection required — this was not possible in python, |
| 218 | +where the equivalent logic is inline in one large imperative |
| 219 | +`_service_cleanup` method per service. |
| 220 | + |
| 221 | +## v1 scope |
| 222 | + |
| 223 | +Providers: compute, network (proves `Blocks`/`CascadeGroup`/`Detach`), |
| 224 | +block-storage, image, identity-scoped resources. `petgraph` added as a new |
| 225 | +dependency for graph structure/topo-walk. Sync support and full |
| 226 | +service-parity with python are deferred; the trait-based extension point |
| 227 | +means later services need no core changes. |
0 commit comments