kubeflow / kubeflow/trainer

Cache initializer silently succeeds on Kubernetes API failures

Open
#3,835 2 comments 0 reactions 0 assignees View on GitHub
area/data-cache kind/bug
Dominant language
Go
Stars
2.2k
Forks
1.1k
Avg merge
3d 22h
Merged PRs (30d)
39

Description

### What happened?

The cache dataset initializer logs Kubernetes API failures and returns normally in multiple paths:

https://github.com/kubeflow/trainer/blob/7c08264b93d3b83de45d1152b169858cadf1d8b3/pkg/initializers/dataset/cache.py#L104-L135

https://github.com/kubeflow/trainer/blob/7c08264b93d3b83de45d1152b169858cadf1d8b3/pkg/initializers/dataset/cache.py#L335-L344

This lets the initializer process exit successfully even though the cache cluster was not created. It also makes retry cleanup unsafe: when the cache ServiceAccount already exists and LeaderWorkerSet creation returns 409, the outer error handler deletes the pre-existing ServiceAccount and then returns success.

I reproduced both behaviors with mocked Kubernetes clients:

```text
TrainJob GET 500 -> download_dataset returned None
pre-existing ServiceAccount + LeaderWorkerSet 409 -> returned None; ServiceAccount delete called once
```

### What did you expect to happen?

Non-recoverable Kubernetes API errors should propagate so the initializer container fails visibly. AlreadyExists responses should be handled idempotently, and cleanup should only delete resources created by the current invocation.

A focused fix would track resource ownership during creation, handle 409 consistently for the ServiceAccount, LeaderWorkerSet, and Service, perform best-effort cleanup only for newly created resources, and re-raise the original error. Unit tests can cover each failure and retry path.

### Environment

Kubeflow Trainer version: current master at `7c08264b93d3b83de45d1152b169858cadf1d8b3`

Kubernetes version: N/A (reproduced with Kubernetes Python client mocks)

Kubeflow Python SDK version: N/A

### Contribution

I'd like to work on this and submit a focused PR with regression tests once maintainers confirm the desired idempotency behavior.

_AI assistance disclosure: Codex assisted with repository inspection and technical drafting; I reproduced and verified the reported behavior locally._

Contributor guide

Open the contributing guide

Research direction

Start in pkg/initializers/dataset/cache.py at the referenced error paths and inspect the initializer's Kubernetes resource creation and cleanup flow. Reproduce the mocked API failure and retry cases, then add focused unit tests showing that non-recoverable errors propagate, AlreadyExists is idempotent, and cleanup removes only resources created by the current invocation.

Written by the indexing model from the issue text.

Assessment

Tech stack
kubernetes, python
Domain
infrastructure
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.