fix(catalog): treat missing placement run as deleted (FLPATH-4925)

[gabriel-farache]

Fix https://redhat.atlassian.net/browse/FLPATH-4925: Catalog instance deletion when its Placement Manager run has already been removed.

• Preserve HTTP status codes in Placement client errors so DELETE failures continue through status-specific service mappings: 406 → policy rejection, 422 → provider error, and 424 → policy dependency.
• Treat only a structured 404 from Placement Manager as already-cleaned-up success and delete the corresponding local catalog instance. Retain the local instance on other Placement Manager failures; keep unknown-instance behavior unchanged.
• Keep direct Placement Manager unknown-run DELETE behavior unchanged.

Verification:

• go test ./internal/catalog/placement -ginkgo.focus='DeleteRun' — passed.
• go test ./internal/catalog/service -ginkgo.focus='Delete with PM' — passed.
• git diff --check — passed.

[qodo-code-review]

PR Summary by Qodo

Treat missing placement runs as deleted catalog instances

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Preserve Placement Manager DELETE status codes through structured client errors.
• Treat a typed missing-run 404 as idempotent catalog instance deletion.
• Keep other Placement failures and direct unknown-run DELETE behavior unchanged.

Diagram

sequenceDiagram
  actor Caller
  participant API as Catalog API
  participant Service as Instance Service
  participant Client as Placement Client
  participant PM as Placement Manager
  participant Store as Catalog Store
  Caller->>API: Delete instance
  API->>Service: Delete by ID
  Service->>Client: Delete stored run
  Client->>PM: DELETE run
  PM-->>Client: 404 Not Found
  Client-->>Service: PlacementError 404
  Service->>Store: Delete instance
  Store-->>Service: Deleted
  Service-->>API: Success
  API-->>Caller: 204 No Content

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Ignore 404 in Placement client

• ➕ Centralizes idempotent DELETE handling inside the HTTP client.
• ➕ Simplifies catalog service deletion logic.
• ➖ Changes unknown-run behavior for every direct Placement client caller.
• ➖ Removes the service layer's ability to distinguish missing runs from successful deletion.

2. Check run existence before deletion

• ➕ Makes the missing-run case explicit before issuing DELETE.
• ➖ Adds an extra network request.
• ➖ Introduces a race between existence checking and deletion.
• ➖ Requires additional Placement Manager API and client behavior.

Recommendation: Keep the PR's service-level handling. Preserving the typed HTTP error lets the catalog workflow treat only a confirmed 404 as successful cleanup while retaining existing direct Placement client semantics and retry behavior for all other failures.

Files changed (6) +84 / -8

Bug fix (2) +10 / -6

client.goPreserve DELETE failure statuses in PlacementError +1/-1

Preserve DELETE failure statuses in PlacementError

• Returns the existing structured PlacementError for non-2xx run deletion responses, preserving the HTTP status and response body for service-level decisions.

internal/catalog/placement/client.go

catalog_item_instance.goComplete local deletion when the placement run is missing +9/-5

Complete local deletion when the placement run is missing

• Treats a typed Placement Manager 404 as an already-deleted run and proceeds with local instance removal. Untyped errors and all other statuses continue through existing failure mapping without deleting local state.

internal/catalog/service/catalog_item_instance.go

Tests (4) +74 / -2

client_test.goVerify structured 404 errors from run deletion +4/-2

Verify structured 404 errors from run deletion

• Updates the unknown-run deletion test to assert that the client returns a PlacementError containing HTTP 404.

internal/catalog/placement/client_test.go

catalog_item_instance_test.goTest idempotent deletion for missing placement runs +16/-0

Test idempotent deletion for missing placement runs

• Adds service coverage proving that a PlacementError with status 404 still removes the local catalog item instance and completes successfully.

internal/catalog/service/catalog_item_instance_test.go

catalog_item_instance_test.goAdd missing-run deletion subsystem regression +33/-0

Add missing-run deletion subsystem regression

• Adds an end-to-end catalog API scenario where Placement Manager returns 404 during deletion. The test verifies a 204 response, local instance removal, and successful subsequent catalog item cleanup.

test/subsystem/catalog/catalog_item_instance_test.go

setup_test.goStub Placement Manager missing-run responses +21/-0

Stub Placement Manager missing-run responses

• Adds a WireMock helper returning a structured HTTP 404 response for Placement Manager run deletion requests.

test/subsystem/catalog/setup_test.go

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[NoamNakash]

lgtm