fix: Update Funnel resource cleanup all K8s resources (including Worker + Executor jobs) by lbeckman314 · Pull Request #1423 · calypr/funnel

lbeckman314 · 2026-05-27T19:42:15Z

Overview 🌀

This PR addresses review comments from PR #1421 by removing all resources tagged with an ownership field from cleanOrphanedResources.

Resources with an ownership field will be automatically cleaned up by the Kubernetes Garbage Collector upon task completion.

Note

This support will likely be adapted into an external cronjob.yaml Helm Chart file to clean resources externally on a user-managed schedule!

Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

- K8s fake client does not support cascading delete from Owner field Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

[skip ci] Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

…ources Signed-off-by: Liam Beckman <lbeckman314@gmail.com> Assisted-by: Claude:claude-sonnet-4-5

github-actions · 2026-05-27T20:03:37Z

Gen3 Integration Tests

filepath	passed	skipped	SUBTOTAL
tests/test_gen3_workflow.py	13	5	18
TOTAL	13	5	18

Move backend.go and backend_test.go resource cleanup changes to review/service-account-deletion (PR #1423) per reviewer request. PR #1421 should only contain the ServiceAccount deletion race condition fix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Assisted-by: Claude Code:claude [Claude Code]

lbeckman314 · 2026-06-03T20:40:49Z

-		}
-	}
+	// TODO: Add Executor Jobs here beacause orphaned tasks can result in orphaned jobs



Never added support here to clean orphaned Executor Jobs (orphaned worker jobs are already handled in cleanOrphanedResources)

Recommended Steps (Credit: @nss10):

Need to make sure that job completed (terminal)

Ensure DeleteJob is triggered (to clean up the Executor job)

Important

Take initial pass to add this support along with new tests

Then update team to review/pair-program as needed!

github-actions · 2026-06-03T20:57:31Z

Gen3 Integration Tests

filepath	passed	skipped	SUBTOTAL
tests/test_gen3_workflow.py	25	4	29
TOTAL	25	4	29

@nss10

…1421) * fix: Resolve ServiceAccount deletion race condition (thanks @nss10!) Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * fix: K8s Unit Tests - K8s fake client does not support cascading delete from Owner field Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Revert function name in serviceaccount.go Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Update comment [skip ci] Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Remove unused parameter Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Restrict fix/service-account-deletion to SA race condition only Move backend.go and backend_test.go resource cleanup changes to review/service-account-deletion (PR #1423) per reviewer request. PR #1421 should only contain the ServiceAccount deletion race condition fix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Assisted-by: Claude Code:claude [Claude Code] --------- Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

github-actions · 2026-06-03T23:01:07Z

Gen3 Integration Tests

filepath	passed	skipped	SUBTOTAL
tests/test_gen3_workflow.py	25	4	29
TOTAL	25	4	29

github-actions · 2026-06-04T00:28:58Z

Gen3 Integration Tests

filepath	passed	skipped	SUBTOTAL
tests/test_gen3_workflow.py	25	4	29
TOTAL	25	4	29

github-actions · 2026-06-04T20:09:29Z

Gen3 Integration Tests

filepath	passed	skipped	SUBTOTAL
tests/test_gen3_workflow.py	25	4	29
TOTAL	25	4	29

github-actions · 2026-06-04T20:35:55Z

Gen3 Integration Tests

filepath	passed	skipped	SUBTOTAL
tests/test_gen3_workflow.py	25	4	29
TOTAL	25	4	29

lbeckman314 · 2026-06-05T18:25:49Z

Caution

Note to Self: The requested task above was not addressed before this PR was merged!

Moving forward ensure that all PR's are reviewed in full (or marked as draft) before merging...

@nss10

* fix: Resolve ServiceAccount deletion race condition (thanks @nss10!) Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * fix: K8s Unit Tests - K8s fake client does not support cascading delete from Owner field Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Revert function name in serviceaccount.go Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Update comment [skip ci] Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Remove unused parameter Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Restrict fix/service-account-deletion to SA race condition only Move backend.go and backend_test.go resource cleanup changes to review/service-account-deletion (PR #1423) per reviewer request. PR #1421 should only contain the ServiceAccount deletion race condition fix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Assisted-by: Claude Code:claude [Claude Code] * fix: Add support for cleaning orphaned Executor jobs Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * fix: Test + lint errors * fix: Linting errors Signed-off-by: Liam Beckman <lbeckman314@gmail.com> --------- Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

@nss10

…ackoffLimit`) (#1413) * fix: Ensure CreateContainerConfigError results in SYSTEM_ERROR Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * fix: Minor linting error in kubernetes backend test Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * feat: Update ContainerConfigError status check in K8s Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * fix: Correctly mark tasks as SYSTEM_ERROR when K8s Job creation fails Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * fix: enrich Kubernetes error diagnostics with pod warning events and exit info Surfaces human-readable detail from pod Warning events and container termination status into TES system logs, so users can see messages like 'secret "foo" not found' without needing kubectl access. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Assisted-by: Claude Code:claude [Claude Code] * fix: Resolve ServiceAccount deletion race condition (thanks @nss10!) Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * fix: K8s Unit Tests - K8s fake client does not support cascading delete from Owner field Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Revert function name in serviceaccount.go Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Update comment [skip ci] Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Remove unused parameter Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * feat: Add state filter in ListTasks (Credit: @nss10) - Original source: 7f60650 * feat: Add support for graceful FailedCreateEvents Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Combine `if` statements in kubernetes/backend.go Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * fix: Add Executor Pod status check Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * fix: Correctly set K8sExecutorErr reason fallback * feat: Add Executor-pod error events to SYSTEM_LOG * chore: Restrict fix/service-account-deletion to SA race condition only Move backend.go and backend_test.go resource cleanup changes to review/service-account-deletion (PR #1423) per reviewer request. PR #1421 should only contain the ServiceAccount deletion race condition fix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Assisted-by: Claude Code:claude [Claude Code] * fix: Add ImagePullBackOff to terminal pod states (SYSTEM_ERROR) * fix: Add remaining K8s terminal pod states (SYSTEM_ERROR) Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * chore: Remove debug fmt.Println statements from reconciler Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Assisted-by: Claude Code:claude [Claude Code] * refactor: Unify FetchPodWarningEvents as a shared exported function Rename fetchExecutorPodWarningEvents (worker) and the backend method fetchPodWarningEvents to a single exported FetchPodWarningEvents in the compute/kubernetes package. worker/kubernetes.go now imports and calls the shared function, eliminating duplicate logic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Assisted-by: Claude Code:claude [Claude Code] * fix: Replace maxErrEventWrites with failedCreateThreshold + minFailureSpan hasJobFailedCreateEvent now fetches all job events in one API call and requires both a minimum occurrence count (failedCreateThreshold=5) and a minimum time span between first and last failure (minFailureSpan=20s) before treating FailedCreate events as a persistent error. This avoids false-positives from rapid transient bursts early in a job's lifecycle. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Assisted-by: Claude Code:claude [Claude Code] * fix: Update ImagePullBackOff test to match current terminal waiting list ImagePullBackOff was added to terminalWaitingReasons but the corresponding test case was not updated. Correct the assertion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Assisted-by: Claude Code:claude [Claude Code] * fix: Minor linting fix * fix: Add support for cleaning orphaned Executor jobs Signed-off-by: Liam Beckman <lbeckman314@gmail.com> * refactor: Address remaining PR review comments - Move terminalWaitingReasons inside hasTerminalContainerWaitingError (no longer needed at package scope) - Remove InitContainers check (YAGNI — worker/executor pods don't use init containers) - Use slices.Contains in hasTerminalContainerWaitingError - Consolidate pod list fetch: fetch pods once per job and pass to both hasTerminalContainerWaitingError and FetchPodWarningEvents to avoid redundant K8s API calls - Update FetchPodWarningEvents signature to accept *corev1.PodList instead of jobName - Update all callers (reconciler, worker/kubernetes.go, tests) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Assisted-by: Claude Code:claude [Claude Code] * Update backend.go --------- Signed-off-by: Liam Beckman <lbeckman314@gmail.com> Co-authored-by: Sai Shanmukha <nss10@outlook.com>

lbeckman314 added 6 commits May 20, 2026 10:38

fix: Resolve ServiceAccount deletion race condition (thanks @nss10!)

d2e910b

Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

fix: K8s Unit Tests

0a8117a

- K8s fake client does not support cascading delete from Owner field Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

chore: Revert function name in serviceaccount.go

7a5616f

Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

chore: Update comment

fdc9b71

[skip ci] Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

chore: Remove unused parameter

7c134e7

Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

fix: remove GC-owned/namespace-scoped resources from CleanOrphanedRes…

142dfdd

…ources Signed-off-by: Liam Beckman <lbeckman314@gmail.com> Assisted-by: Claude:claude-sonnet-4-5

lbeckman314 added the enhancement label May 27, 2026

lbeckman314 self-assigned this May 27, 2026

lbeckman314 modified the milestones: GCP Batch Production Support, Gen3Workflow Production-Ready Support May 27, 2026

lbeckman314 changed the base branch from develop to fix/service-account-deletion May 27, 2026 19:42

lbeckman314 mentioned this pull request May 27, 2026

fix: Resolve ServiceAccount deletion race condition (thanks @nss10!) #1421

Merged

lbeckman314 changed the base branch from fix/service-account-deletion to develop May 27, 2026 20:39

Merge branch 'develop' into review/service-account-deletion

d804efb

lbeckman314 commented Jun 3, 2026

View reviewed changes

lbeckman314 changed the title ~~Review/service account deletion~~ fix: Update Funnel resource cleanup all K8s resources (including Worker + Executor jobs) Jun 3, 2026

Merge branch 'develop' into review/service-account-deletion

d0c6b39

Merge branch 'develop' into review/service-account-deletion

03ef2ad

Merge branch 'develop' into review/service-account-deletion

e42f2e8

Merge branch 'develop' into review/service-account-deletion

9d27519

lbeckman314 merged commit 2896399 into develop Jun 5, 2026
18 checks passed

lbeckman314 deleted the review/service-account-deletion branch June 5, 2026 00:36

lbeckman314 mentioned this pull request Jun 5, 2026

feat: Add initial support for cleaning orphaned jobs #1427

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Update Funnel resource cleanup all K8s resources (including Worker + Executor jobs)#1423

fix: Update Funnel resource cleanup all K8s resources (including Worker + Executor jobs)#1423
lbeckman314 merged 11 commits into
developfrom
review/service-account-deletion

lbeckman314 commented May 27, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 27, 2026

Uh oh!

lbeckman314 Jun 3, 2026

Uh oh!

github-actions Bot commented Jun 3, 2026

Uh oh!

github-actions Bot commented Jun 3, 2026

Uh oh!

github-actions Bot commented Jun 4, 2026

Uh oh!

github-actions Bot commented Jun 4, 2026

Uh oh!

github-actions Bot commented Jun 4, 2026

Uh oh!

Uh oh!

lbeckman314 commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

lbeckman314 commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview 🌀

Uh oh!

github-actions Bot commented May 27, 2026

Gen3 Integration Tests

Uh oh!

lbeckman314 Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 3, 2026

Gen3 Integration Tests

Uh oh!

github-actions Bot commented Jun 3, 2026

Gen3 Integration Tests

Uh oh!

github-actions Bot commented Jun 4, 2026

Gen3 Integration Tests

Uh oh!

github-actions Bot commented Jun 4, 2026

Gen3 Integration Tests

Uh oh!

github-actions Bot commented Jun 4, 2026

Gen3 Integration Tests

Uh oh!

Uh oh!

lbeckman314 commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lbeckman314 commented May 27, 2026 •

edited

Loading