[TASK] Implement Structured State Generation in Workspace Controller
- Dominant language
- No language data
- Stars
- 84
- Forks
- 149
- Avg merge
- 5d 15h
- Merged PRs (30d)
- 29
Description
### Certification
- [x] I certify I am an Epic Owner for Kubeflow Notebooks 2.0 and expected to create planning-related issues.
### Description
🛑 **Depends on:** #1192
Refactors the controller's state generation to populate the structured sub-states defined in
#1192 and derive the summary state from them. This replaces both the `generateWorkspaceState()`
function and the `updateWorkspaceState()` early returns in `Reconcile()`.
#### Refactoring Goals
1. **Separate I/O from derivation**
- Fetch events (StatefulSet events, Pod events) BEFORE entering state derivation
- Pass fetched data as inputs to a pure state derivation function
- This makes the derivation function unit-testable without envtest
2. **Populate sub-states independently**
- Each sub-state is populated from its own inputs
- No sub-state depends on another sub-state's value (they depend on raw inputs)
3. **Derive summary from sub-states**
- A single, pure function takes the populated sub-states and produces `(state, stateMessage)`
- This function codifies the priority/precedence rules documented in Task 1
- This function should be straightforward to unit test with table-driven tests
4. **Unify the two code paths**
- Config-level errors (currently early returns in `Reconcile()`) should populate sub-states
rather than short-circuiting to `updateWorkspaceState()`
- The reconciler should always flow through sub-state population → summary derivation →
status update
#### Testing Requirements
This task has significant testing requirements since it's restructuring the core state machine:
- **Unit tests for each sub-state derivation** — given specific pod conditions / container states /
event lists / config results, verify the correct sub-state is produced
- **Unit tests for summary derivation** — given specific sub-state combinations, verify the correct
summary state and message
- **Table-driven tests** covering all current state transitions (see the 17+ `stateMsg*` constants)
- **E2E tests** covering key state transitions on a real cluster:
- Happy path: create workspace → Pending → Running
- Pause/unpause: Running → Paused → Running
- Error: invalid imageConfig → Error (config)
- Error: image pull failure → Error (runtime)
- Error: pod unschedulable → Error (scheduling)
- Terminating: delete workspace → Terminating
- Pending restart: change WK options → pendingRestart reflected in sub-states
### Acceptance Criteria
- [ ] `generateWorkspaceState()` is replaced by structured sub-state population + summary derivation
- [ ] Config-level error paths in `Reconcile()` populate sub-states instead of early-returning
with `updateWorkspaceState()`
- [ ] The summary derivation is a pure function (no k8s API calls, no context dependency)
- [ ] Event fetching is separated from state derivation
- [ ] All 17+ current state message scenarios are covered by the new structure
- [ ] Unit tests verify sub-state derivation and summary derivation independently
- [ ] Table-driven unit tests cover all state transitions
- [ ] E2E tests verify key state transitions on a Kind cluster
- [ ] No behavioral regression — the summary `state` values produced for existing scenarios
match (or intentionally improve upon) the current behavior
- [ ] The `nolint:gocyclo` annotations on `generateWorkspaceState` and `Reconcile` can be removed
(or significantly reduced) after the refactor
Contributor guide
Research direction
Start with #1192, then inspect generateWorkspaceState(), updateWorkspaceState(), and Reconcile() to map the current stateMsg* transitions and config early returns. Use the listed unit, table-driven, and Kind E2E scenarios as completion checks; done means structured sub-states, pure summary derivation, separated event fetching, and no behavioral regression.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- kubernetes
- Domain
- infrastructure
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100