Skip to content

Commit 2f59e67

Browse files
committed
Update enhancement proposal with typed ExternallyDeleted signal design
Replace the (nil, nil) sentinel pattern with a typed ExternallyDeleted signal on ReconcileStatus. This is a design improvement over the original proposal: using a dedicated signal prevents ambiguity with actuator bugs that might also return nil, and restores the safety guard for unexpected nil returns from GetOSResourceByID.
1 parent da9fb9b commit 2f59e67

1 file changed

Lines changed: 6 additions & 4 deletions

File tree

enhancements/drift-detection.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,10 +190,11 @@ When a resource with `managementPolicy=managed` is deleted from OpenStack but th
190190
Currently, `GetOrCreateOSResource` returns a terminal error when fetching a resource by `status.id` results in a 404. To support resource recreation, this logic must be updated to:
191191

192192
1. Check if `managementPolicy == managed` and the resource was not imported (no `importID` or `importFilter`)
193-
2. If both conditions are met, clear `status.id` and proceed to the creation path instead of returning an error
193+
2. If both conditions are met, return a typed `ExternallyDeleted` signal via `ReconcileStatus` so the caller can clear `status.id` and trigger recreation on the next reconcile
194194
3. If the resource was imported or is unmanaged, retain the existing terminal error behavior
195+
4. If `GetOSResourceByID` returns a nil resource with no error (actuator bug), return an explicit error rather than silently misinterpreting it as external deletion
195196

196-
This ensures that managed resources created by ORC are automatically recreated, while imported or unmanaged resources correctly fail with a terminal error when deleted externally.
197+
This ensures that managed resources created by ORC are automatically recreated, while imported or unmanaged resources correctly fail with a terminal error when deleted externally. The typed signal avoids ambiguity with nil returns from actuator bugs.
197198

198199
**Behavior when drift detection is disabled** (`resyncPeriod: 0`): Periodic resyncs do not occur, so discovery of external deletion depends on other triggers (spec change, controller restart). When discovered, ORC will still recreate managed resources (not a terminal error). The difference is timing of discovery, not the recreation behavior itself.
199200

@@ -294,10 +295,11 @@ The following have been implemented:
294295
**External Deletion Handling**
295296
- `IsImported()` method added to `APIObjectAdapter` interface (all resource adapters)
296297
- `GetOrCreateOSResource` branches on management policy and import status when 404 is received:
297-
- Managed, non-imported resources → `(nil, nil)` to trigger recreation
298+
- Managed, non-imported resources → typed `ExternallyDeleted` signal via `ReconcileStatus`
298299
- Unmanaged or imported resources → terminal error
300+
- Nil resource with no error (actuator bug) → explicit programming error
299301
- `status.ClearStatusID` clears `status.id` before recreation (using JSON merge patch with explicit `null`)
300-
- `reconcileNormal` handles the `(nil, nil)` recreation signal from `GetOrCreateOSResource`
302+
- `reconcileNormal` checks `IsExternallyDeleted()` on the `ReconcileStatus` to trigger recreation
301303

302304
**E2E Tests**
303305
- `network-resync-period`: verifies `lastSyncTime` is updated after configured period

0 commit comments

Comments
 (0)