Repository navigation
fix: recreate the pod when inline content changes - #22
Open
blarghmatey wants to merge 1 commit into
Open
blarghmatey wants to merge 1 commit into
blarghmatey wants to merge 1 commit into
Conversation
Editing spec.content updated the ConfigMap but left the pod running the old notebook. The pod spec only names the ConfigMap, so the spec hash did not change, and the copy-content init container had already copied the previous file into the notebook volume. Content-mode pods now carry a marimo.io/content-hash annotation, and PodSpecHash folds it in, so a content change recreates the pod the same way an env or image change does. Source-mode pods have no annotation and hash exactly as before. Content-mode pods created by an older operator are recreated once after upgrading, because their stored hash did not include the content. Claude-Session: https://claude.ai/code/session_01Mznd5RYgzKGBShstpLnoFQ
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation addresses stale inline content with focused, compatible change detection and appropriate tests.
Review effort: Balanced
Findings: None
What changed in this PR
Ensures inline notebook content updates trigger pod recreation so the init container copies the latest content.
Changes:
- Adds content hashes to content-mode pods and pod change detection.
- Adds unit and controller reconciliation coverage.
- Preserves existing source-mode hashes.
| File | Description |
|---|---|
pkg/resources/pod.go |
Incorporates inline content into pod hashing. |
pkg/resources/pod_test.go |
Tests content and source-mode hashing. |
internal/controller/marimonotebook_controller_test.go |
Tests recreation after content updates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Editing
spec.contenton a running MarimoNotebook updates the ConfigMap, but the pod keeps serving the old notebook. The pod spec only names the ConfigMap, soPodSpecHashdoesn't change, and thecopy-contentinit container copied the previous file into the notebook volume when the pod started.This adds a
marimo.io/content-hashannotation to content-mode pods and folds it intoPodSpecHash. A content change now recreates the pod, the same way an env or image change already does. Source-mode pods get no annotation and hash exactly as before; a unit test pins that. Content-mode pods created by an older operator are recreated once after upgrading, because their stored hash didn't include the content.The new envtest case ("should recreate Pod when inline content is changed") times out on
mainand passes with this change.Checklist
make lintandmake testpass locally (operator).uv run pytestanduv run ty check kubectl_marimopass locally (plugin). (Not applicable, the plugin is unchanged.)make manifestshas been run and generated files are committed. (No CRD change.)https://claude.ai/code/session_01Mznd5RYgzKGBShstpLnoFQ