kubeflow / kubeflow/notebooks

[TASK] Implement Structured State Generation in Workspace Controller

Open
#1,193 2 comments 0 reactions 0 assignees View on GitHub
kind/plan-task
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.