Update dependent resources (#8)

This takes a first stab at reconciling changes to `MarimoNotebook`:

- pvc: explicitly declare storage attribute immutable (i.e. no
reconciling)
- service: attempt in-place change of service
- recreate Marimo pod on changes

Fixes #6 .

Also, debug logging of desired state for pods.

---------

Co-authored-by: dmadisetti <dmadisetti@coreweave.com>

Author: bittrance

api/v1alpha1/marimonotebook_types.go

@@ -106,6 +106,7 @@ type MarimoNotebookSpec struct {
 
 	// Storage configures persistent storage for notebooks
 	// +optional
+	// +kubebuilder:validation:XValidation:rule="oldSelf == null || self == oldSelf",message="storage is immutable once set"
 	Storage *StorageSpec `json:"storage,omitempty"`
 
 	// Resources for the marimo container

config/crd/bases/marimo.io_marimos.yaml

@@ -767,6 +767,9 @@ spec:
                       if empty)
                     type: string
                 type: object
+                x-kubernetes-validations:
+                - message: storage is immutable once set
+                  rule: oldSelf == null || self == oldSelf
             type: object
             x-kubernetes-validations:
             - message: storage is required when sidecars are specified

deploy/install.yaml

@@ -775,6 +775,9 @@ spec:
                       if empty)
                     type: string
                 type: object
+                x-kubernetes-validations:
+                - message: storage is immutable once set
+                  rule: oldSelf == null || self == oldSelf
             type: object
             x-kubernetes-validations:
             - message: storage is required when sidecars are specified

internal/controller/marimonotebook_controller.go

@@ -23,6 +23,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -192,9 +193,19 @@ func (r *MarimoNotebookReconciler) reconcilePod(ctx context.Context, notebook *m
 		return nil, err
 	}
 
+	// Compute hash of desired pod spec for change detection
+	specHash, err := resources.PodSpecHash(desired)
+	if err != nil {
+		return nil, fmt.Errorf("computing pod spec hash: %w", err)
+	}
+	if desired.Annotations == nil {
+		desired.Annotations = make(map[string]string)
+	}
+	desired.Annotations[resources.PodSpecHashAnnotation] = specHash
+
 	// Check if Pod exists
 	existing := &corev1.Pod{}
-	err := r.Get(ctx, client.ObjectKeyFromObject(desired), existing)
+	err = r.Get(ctx, client.ObjectKeyFromObject(desired), existing)
 	if err != nil {
 		if k8serrors.IsNotFound(err) {
 			logger.Info("Creating Pod", "name", desired.Name)
@@ -213,41 +224,49 @@ func (r *MarimoNotebookReconciler) reconcilePod(ctx context.Context, notebook *m
 		return nil, err
 	}
 
-	// Pod exists - we don't update running pods (recreate strategy)
+	// Pod exists - check if spec has changed and recreate if so
+	if existingHash := existing.Annotations[resources.PodSpecHashAnnotation]; existingHash != specHash {
+		logger.Info("Pod spec changed, recreating", "name", existing.Name)
+		if err := r.Delete(ctx, existing); err != nil && !k8serrors.IsNotFound(err) {
+			return nil, err
+		}
+		// Pod deleted - next reconcile will create the updated pod
+		return nil, nil
+	}
+
 	return existing, nil
 }
 
 func (r *MarimoNotebookReconciler) reconcileService(ctx context.Context, notebook *marimov1alpha1.MarimoNotebook) (*corev1.Service, error) {
 	logger := logf.FromContext(ctx)
 	desired := resources.BuildService(notebook)
 
-	// Set owner reference
-	if err := controllerutil.SetControllerReference(notebook, desired, r.Scheme); err != nil {
-		return nil, err
+	svc := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      desired.Name,
+			Namespace: desired.Namespace,
+		},
 	}
 
-	// Check if Service exists
-	existing := &corev1.Service{}
-	err := r.Get(ctx, client.ObjectKeyFromObject(desired), existing)
-	if err != nil {
-		if k8serrors.IsNotFound(err) {
-			logger.Info("Creating Service", "name", desired.Name)
-			if err := r.Create(ctx, desired); err != nil {
-				if k8serrors.IsAlreadyExists(err) {
-					// Service was created between Get and Create, re-fetch
-					if err := r.Get(ctx, client.ObjectKeyFromObject(desired), existing); err != nil {
-						return nil, err
-					}
-					return existing, nil
-				}
-				return nil, err
-			}
-			return desired, nil
+	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, svc, func() error {
+		if err := controllerutil.SetControllerReference(notebook, svc, r.Scheme); err != nil {
+			return err
 		}
+		svc.Labels = desired.Labels
+		svc.Spec.Ports = desired.Spec.Ports
+		svc.Spec.Selector = desired.Spec.Selector
+		svc.Spec.Type = desired.Spec.Type
+		return nil
+	})
+	if err != nil {
 		return nil, err
 	}
 
-	return existing, nil
+	if op != controllerutil.OperationResultNone {
+		logger.Info("Reconciled Service", "name", svc.Name, "operation", op)
+	}
+
+	return svc, nil
 }
 
 func (r *MarimoNotebookReconciler) updateStatus(ctx context.Context, notebook *marimov1alpha1.MarimoNotebook, pod *corev1.Pod, svc *corev1.Service) (ctrl.Result, error) {

internal/controller/marimonotebook_controller_test.go

@@ -370,6 +370,120 @@ var _ = Describe("MarimoNotebook Controller", func() {
 		})
 	})
 
+	Context("When updating a MarimoNotebook", func() {
+		var notebook *marimov1alpha1.MarimoNotebook
+		var namespacedName types.NamespacedName
+
+		BeforeEach(func() {
+			notebook = &marimov1alpha1.MarimoNotebook{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-update-" + randString(),
+					Namespace: "default",
+				},
+				Spec: marimov1alpha1.MarimoNotebookSpec{
+					Source:  "https://github.com/marimo-team/marimo.git",
+					Storage: &marimov1alpha1.StorageSpec{Size: "1Gi"},
+				},
+			}
+			namespacedName = types.NamespacedName{
+				Name:      notebook.Name,
+				Namespace: notebook.Namespace,
+			}
+		})
+
+		AfterEach(func() {
+			_ = k8sClient.Delete(ctx, notebook)
+		})
+
+		It("should update Service ports when a sidecar with a port is added", func() {
+			pausePort := int32(9090)
+
+			By("creating the MarimoNotebook with storage")
+			Expect(k8sClient.Create(ctx, notebook)).To(Succeed())
+
+			By("waiting for Service to be created with only the main port")
+			svc := &corev1.Service{}
+			Eventually(func() error {
+				return k8sClient.Get(ctx, namespacedName, svc)
+			}, timeout, interval).Should(Succeed())
+			Expect(svc.Spec.Ports).To(HaveLen(1))
+
+			By("updating the MarimoNotebook to add a pause sidecar with a container port")
+			nb := &marimov1alpha1.MarimoNotebook{}
+			Expect(k8sClient.Get(ctx, namespacedName, nb)).To(Succeed())
+			nb.Spec.Sidecars = []marimov1alpha1.SidecarSpec{
+				{
+					Name:       "pause",
+					Image:      "registry.k8s.io/pause:3.9",
+					ExposePort: &pausePort,
+				},
+			}
+			Expect(k8sClient.Update(ctx, nb)).To(Succeed())
+
+			By("checking Service is updated to include the sidecar port")
+			Eventually(func() bool {
+				if err := k8sClient.Get(ctx, namespacedName, svc); err != nil {
+					return false
+				}
+				for _, port := range svc.Spec.Ports {
+					if port.Name == "pause" && port.Port == pausePort {
+						return true
+					}
+				}
+				return false
+			}, timeout, interval).Should(BeTrue(), "Service should expose the pause sidecar port")
+
+			Expect(svc.Spec.Ports).To(HaveLen(2))
+		})
+
+		It("should recreate Pod when env vars are changed", func() {
+			By("creating the MarimoNotebook")
+			Expect(k8sClient.Create(ctx, notebook)).To(Succeed())
+
+			By("waiting for initial Pod to be created")
+			pod := &corev1.Pod{}
+			Eventually(func() error {
+				return k8sClient.Get(ctx, namespacedName, pod)
+			}, timeout, interval).Should(Succeed())
+			originalUID := pod.UID
+
+			By("updating the notebook to add an env var")
+			nb := &marimov1alpha1.MarimoNotebook{}
+			Expect(k8sClient.Get(ctx, namespacedName, nb)).To(Succeed())
+			nb.Spec.Env = []corev1.EnvVar{
+				{Name: "MY_VAR", Value: "hello"},
+			}
+			Expect(k8sClient.Update(ctx, nb)).To(Succeed())
+
+			By("verifying Pod is recreated with the new env var")
+			Eventually(func() bool {
+				if err := k8sClient.Get(ctx, namespacedName, pod); err != nil {
+					return false
+				}
+				if pod.UID == originalUID {
+					return false // still the old pod
+				}
+				for _, env := range pod.Spec.Containers[0].Env {
+					if env.Name == "MY_VAR" && env.Value == "hello" {
+						return true
+					}
+				}
+				return false
+			}, timeout, interval).Should(BeTrue(), "Pod should be recreated with MY_VAR env var")
+		})
+
+		It("should reject changes to the storage attribute", func() {
+			By("creating the MarimoNotebook with storage")
+			Expect(k8sClient.Create(ctx, notebook)).To(Succeed())
+
+			By("attempting to update the storage attribute")
+			nb := &marimov1alpha1.MarimoNotebook{}
+			Expect(k8sClient.Get(ctx, namespacedName, nb)).To(Succeed())
+			nb.Spec.Storage = &marimov1alpha1.StorageSpec{Size: "2Gi"}
+			Expect(k8sClient.Update(ctx, nb)).To(MatchError(ContainSubstring("storage is immutable once set")))
+		})
+	})
+
 	Context("When deleting a MarimoNotebook", func() {
 		It("should clean up owned resources via garbage collection", func() {
 			notebook := &marimov1alpha1.MarimoNotebook{

pkg/resources/pod.go

@@ -1,6 +1,8 @@
 package resources
 
 import (
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"strings"
@@ -18,8 +20,20 @@ const (
 	NotebookDir = "/home/marimo/notebooks"
 	// DefaultMode is the default mode for running marimo.
 	DefaultMode = "edit"
+	// PodSpecHashAnnotation is the annotation key used to store the hash of the desired pod spec.
+	PodSpecHashAnnotation = "marimo.io/pod-spec-hash"
 )
 
+// PodSpecHash returns a SHA-256 hash of the pod's spec for change detection.
+func PodSpecHash(pod *corev1.Pod) (string, error) {
+	data, err := json.Marshal(pod.Spec)
+	if err != nil {
+		return "", err
+	}
+	sum := sha256.Sum256(data)
+	return hex.EncodeToString(sum[:]), nil
+}
+
 // BuildPod creates a Pod spec for a MarimoNotebook.
 // Supports two content modes:
 // - source: clone from git URL