Periodically clean up orphaned Sandcastle PVCs

[nforro]

No description provided.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:49 AM UTC · Completed 11:06 AM UTC
Commit: bcc8806 · View workflow run →

[centosinfra-prod-github-app]

Build succeeded.
https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/packit-service/buildset/304552f4647949f682b10a60864a81f7

✔️ pre-commit SUCCESS in 1m 53s
✔️ packit-service-tests SUCCESS in 3m 45s

[fullsend-ai-review]

Review

Findings

Low

• [test adequacy] packit_service/worker/tasks.py — No unit tests are added for cleanup_orphaned_pvcs. The sibling function cleanup_orphaned_pidbox_queues has a corresponding test in tests/unit/test_tasks.py that validates its scanning and TTL-setting logic via mocks. Adding similar mocked test coverage would catch regressions in the naming filter, age check, pod-volume matching, and 404 handling.
• [missing-authorization] No linked issue or PR description. While the change is well-scoped and follows established patterns, documenting the motivation for the new PVC cleanup task (e.g., frequency of PVC leaks, cluster impact) would benefit future maintainers.

Previous run

Review

Findings

Medium

• [race condition] packit_service/worker/tasks.py — Race condition between PVC creation and pod scheduling. Sandcastle creates a PVC before launching its pod. If the cleanup task runs between PVC creation and pod scheduling, the cleanup will see a PVC with no associated pod and delete it, causing the subsequently scheduled pod to fail. There is no minimum-age check on PVCs to provide a grace period.
  Remediation: Before deleting, check pvc.metadata.creation_timestamp and skip PVCs that are younger than a safe threshold (e.g., 30–60 minutes).

• [error-handling-idiom] packit_service/worker/tasks.py — The adjacent cleanup_orphaned_pidbox_queues task wraps its entire body in a try/except with a broad except Exception fallback, ensuring unexpected errors are logged rather than propagated as Celery task failures. The new cleanup_orphaned_pvcs task has no top-level exception guard — only targeted ApiException catches. An unexpected error would propagate unhandled, producing a Celery task failure rather than a logged warning.
  Remediation: Add a top-level try/except Exception around the task body, matching the pattern in cleanup_orphaned_pidbox_queues.

Low

• [missing-authorization] — This PR has no linked issue and no body text explaining the motivation. Non-trivial changes that add new operational behavior should trace to an authorized issue so that design decisions (e.g., 6-hour interval, naming convention filter, grace_period_seconds=0) are documented.

• [null handling] packit_service/worker/tasks.py — The nested generator expression accesses pod_obj.spec.volumes but only guards against volumes being None. If pod_obj.spec were None, this would raise AttributeError.

• [import-placement] packit_service/worker/tasks.py — The new task uses function-local imports. This is the only function in tasks.py with local imports — every other function uses module-level imports exclusively. Deferred imports can be valid for Celery tasks, but should be consistent or documented.

---

Labels: PR adds a new periodic Celery beat task for internal Kubernetes PVC cleanup

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 11:15 AM UTC · Ended 11:17 AM UTC
Commit: c73c7ab · View workflow run →

[fullsend-ai-review]

🤖 Review · ❌ Terminated · Started 11:18 AM UTC · Ended 11:32 AM UTC
Commit: 811afc7 · View workflow run →

[centosinfra-prod-github-app]

Build succeeded.
https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/packit-service/buildset/e83e3d4bac5b4519b9c8ab6f53b195c1

✔️ pre-commit SUCCESS in 1m 58s
✔️ packit-service-tests SUCCESS in 5m 41s

Superseded by updated review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:18 AM UTC · Completed 11:32 AM UTC
Commit: 811afc7 · View workflow run →

[centosinfra-prod-github-app]

Build succeeded (gate pipeline).
https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/packit-service/buildset/ad8006c85177431b9f2dbb6500aadac6

✔️ pre-commit SUCCESS in 1m 56s