From 2ebbf130d5042032c038c1a496a45d7f0d18e5b3 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 20:41:50 +0000 Subject: [PATCH 01/29] test(operator): require dependency event convergence --- .../controllers/reconciler_test.go | 117 ++++++++++++++++++ .../controllers/session_controller_test.go | 62 ++++++++++ .../controllers/workspace_controller_test.go | 40 ++++++ 3 files changed, 219 insertions(+) create mode 100644 packages/cluster-operator/controllers/session_controller_test.go diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 1a54e2d0..02b070a2 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -2,8 +2,10 @@ package controllers_test import ( "context" + "reflect" "strings" "testing" + "time" corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" @@ -1254,6 +1256,121 @@ func TestSessionDeletionCleansResourcesBeforeFinalizer(t *testing.T) { } } +func TestSessionDependencyRevocationCleansOwnedResourcesAndConvergesAfterRestart(t *testing.T) { + for _, test := range []struct { + name string + conditionType string + wantReason string + revoke func(context.Context, client.Client) error + }{ + {name: "missing Host", conditionType: "HostReady", wantReason: "HostNotFound", revoke: func(ctx context.Context, c client.Client) error { + return c.Delete(ctx, &clusterv1alpha1.T4ClusterHost{ObjectMeta: metav1.ObjectMeta{Name: "host-a", Namespace: "team"}}) + }}, + {name: "invalid Host runtime profile", conditionType: "RuntimeConfigured", wantReason: "RuntimeProfileNotAllowed", revoke: func(ctx context.Context, c client.Client) error { + var host clusterv1alpha1.T4ClusterHost + if err := c.Get(ctx, types.NamespacedName{Namespace: "team", Name: "host-a"}, &host); err != nil { + return err + } + host.Spec.RuntimeProfiles = nil + return c.Update(ctx, &host) + }}, + {name: "missing Workspace", conditionType: "WorkspaceReady", wantReason: "WorkspaceNotFound", revoke: func(ctx context.Context, c client.Client) error { + return c.Delete(ctx, &clusterv1alpha1.T4Workspace{ObjectMeta: metav1.ObjectMeta{Name: "workspace-a", Namespace: "team"}}) + }}, + {name: "mismatched Workspace Host", conditionType: "WorkspaceReady", wantReason: "HostMismatch", revoke: func(ctx context.Context, c client.Client) error { + var workspace clusterv1alpha1.T4Workspace + if err := c.Get(ctx, types.NamespacedName{Namespace: "team", Name: "workspace-a"}, &workspace); err != nil { + return err + } + workspace.Spec.HostRef = "host-b" + return c.Update(ctx, &workspace) + }}, + {name: "missing OMP ConfigMap", conditionType: "RuntimeConfigured", wantReason: "OMPConfigMapNotFound", revoke: func(ctx context.Context, c client.Client) error { + return c.Delete(ctx, &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "omp-runtime-config", Namespace: "team"}}) + }}, + {name: "invalid OMP credential Secret", conditionType: "RuntimeConfigured", wantReason: "OMPCredentialSecretInvalid", revoke: func(ctx context.Context, c client.Client) error { + var secret corev1.Secret + if err := c.Get(ctx, types.NamespacedName{Namespace: "team", Name: "omp-runtime-credential"}, &secret); err != nil { + return err + } + delete(secret.Data, "MODEL_API_KEY") + return c.Update(ctx, &secret) + }}, + } { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.Status.PVCName = "workspace-a-data" + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, + Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, + } + session := testSession() + session.UID = "session-uid" + foreignPod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foreign-pod", Namespace: "team"}} + foreignService := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "foreign-service", Namespace: "team"}} + c := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Session{}, &corev1.PersistentVolumeClaim{}, &corev1.Pod{}). + WithObjects(testHost(), workspace, pvc, session, foreignPod, foreignService).Build() + r := configuredSessionReconciler(c, scheme) + reconcileMany(t, 2, func() error { + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}) + return err + }) + if err := test.revoke(ctx, c); err != nil { + t.Fatal(err) + } + + result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}) + if err != nil { + t.Fatal(err) + } + if result.RequeueAfter <= 0 || result.RequeueAfter > 30*time.Second { + t.Fatalf("revoked dependency requeue = %s, want bounded positive retry", result.RequeueAfter) + } + for _, object := range []client.Object{ + &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionPodName(session), Namespace: session.Namespace}}, + &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionServiceName(session), Namespace: session.Namespace}}, + } { + if err := c.Get(ctx, client.ObjectKeyFromObject(object), object); !apierrors.IsNotFound(err) { + t.Fatalf("owned stale %T remained after dependency revocation: %v", object, err) + } + } + for _, object := range []client.Object{foreignPod.DeepCopy(), foreignService.DeepCopy()} { + if err := c.Get(ctx, client.ObjectKeyFromObject(object), object); err != nil { + t.Fatalf("unowned %T was removed: %v", object, err) + } + } + + var failed clusterv1alpha1.T4Session + if err := c.Get(ctx, client.ObjectKeyFromObject(session), &failed); err != nil { + t.Fatal(err) + } + condition := findCondition(failed.Status.Conditions, test.conditionType) + available := findCondition(failed.Status.Conditions, "Available") + if failed.Status.ObservedGeneration != failed.Generation || failed.Status.PodName != "" || failed.Status.ServiceName != "" || failed.Status.Phase != clusterv1alpha1.InfrastructureFailed || + condition == nil || condition.Status != metav1.ConditionFalse || condition.Reason != test.wantReason || condition.ObservedGeneration != failed.Generation || + available == nil || available.Status != metav1.ConditionFalse || available.Reason != test.wantReason || available.ObservedGeneration != failed.Generation { + t.Fatalf("revoked session did not converge: status=%#v condition=%#v available=%#v", failed.Status, condition, available) + } + stableStatus := failed.Status + restarted := *r + reconcileMany(t, 2, func() error { + _, err := restarted.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}) + return err + }) + if err := c.Get(ctx, client.ObjectKeyFromObject(session), &failed); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(failed.Status, stableStatus) { + t.Fatalf("duplicate/restart reconciliation changed converged status: got %#v, want %#v", failed.Status, stableStatus) + } + }) + } +} + func configuredSessionReconciler(c client.Client, scheme *runtime.Scheme) *controllers.SessionReconciler { for _, object := range []client.Object{ &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "omp-runtime-config", Namespace: "team"}, Data: map[string]string{ diff --git a/packages/cluster-operator/controllers/session_controller_test.go b/packages/cluster-operator/controllers/session_controller_test.go new file mode 100644 index 00000000..cba15fef --- /dev/null +++ b/packages/cluster-operator/controllers/session_controller_test.go @@ -0,0 +1,62 @@ +package controllers + +import ( + "context" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + clusterv1alpha1 "github.com/LycaonLLC/t4-code/packages/cluster-operator/api/v1alpha1" +) + +func TestSessionRequestsForHostOnlyEnqueuesAffectedSessions(t *testing.T) { + scheme := runtime.NewScheme() + if err := clusterv1alpha1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + objects := []client.Object{ + &clusterv1alpha1.T4Session{ObjectMeta: metav1.ObjectMeta{Name: "session-a", Namespace: "team"}, Spec: clusterv1alpha1.T4SessionSpec{HostRef: "host-a", WorkspaceRef: "workspace-a"}}, + &clusterv1alpha1.T4Session{ObjectMeta: metav1.ObjectMeta{Name: "session-b", Namespace: "team"}, Spec: clusterv1alpha1.T4SessionSpec{HostRef: "host-a", WorkspaceRef: "workspace-b"}}, + &clusterv1alpha1.T4Session{ObjectMeta: metav1.ObjectMeta{Name: "other-host", Namespace: "team"}, Spec: clusterv1alpha1.T4SessionSpec{HostRef: "host-b", WorkspaceRef: "workspace-a"}}, + &clusterv1alpha1.T4Session{ObjectMeta: metav1.ObjectMeta{Name: "other-namespace", Namespace: "other"}, Spec: clusterv1alpha1.T4SessionSpec{HostRef: "host-a", WorkspaceRef: "workspace-a"}}, + } + c := fake.NewClientBuilder().WithScheme(scheme). + WithIndex(&clusterv1alpha1.T4Session{}, sessionHostRefIndexField, indexSessionByHostRef). + WithIndex(&clusterv1alpha1.T4Session{}, sessionWorkspaceRefIndexField, indexSessionByWorkspaceRef). + WithObjects(objects...).Build() + r := &SessionReconciler{Client: c, Scheme: scheme} + + requests := r.sessionRequestsForHost(context.Background(), &clusterv1alpha1.T4ClusterHost{ObjectMeta: metav1.ObjectMeta{Name: "host-a", Namespace: "team"}}) + assertRequestSet(t, requests, []types.NamespacedName{ + {Namespace: "team", Name: "session-a"}, + {Namespace: "team", Name: "session-b"}, + }) +} + +func TestSessionRequestsForWorkspaceOnlyEnqueuesAffectedSessions(t *testing.T) { + scheme := runtime.NewScheme() + if err := clusterv1alpha1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + objects := []client.Object{ + &clusterv1alpha1.T4Session{ObjectMeta: metav1.ObjectMeta{Name: "session-a", Namespace: "team"}, Spec: clusterv1alpha1.T4SessionSpec{HostRef: "host-a", WorkspaceRef: "workspace-a"}}, + &clusterv1alpha1.T4Session{ObjectMeta: metav1.ObjectMeta{Name: "session-b", Namespace: "team"}, Spec: clusterv1alpha1.T4SessionSpec{HostRef: "host-b", WorkspaceRef: "workspace-a"}}, + &clusterv1alpha1.T4Session{ObjectMeta: metav1.ObjectMeta{Name: "other-workspace", Namespace: "team"}, Spec: clusterv1alpha1.T4SessionSpec{HostRef: "host-a", WorkspaceRef: "workspace-b"}}, + &clusterv1alpha1.T4Session{ObjectMeta: metav1.ObjectMeta{Name: "other-namespace", Namespace: "other"}, Spec: clusterv1alpha1.T4SessionSpec{HostRef: "host-a", WorkspaceRef: "workspace-a"}}, + } + c := fake.NewClientBuilder().WithScheme(scheme). + WithIndex(&clusterv1alpha1.T4Session{}, sessionHostRefIndexField, indexSessionByHostRef). + WithIndex(&clusterv1alpha1.T4Session{}, sessionWorkspaceRefIndexField, indexSessionByWorkspaceRef). + WithObjects(objects...).Build() + r := &SessionReconciler{Client: c, Scheme: scheme} + + requests := r.sessionRequestsForWorkspace(context.Background(), &clusterv1alpha1.T4Workspace{ObjectMeta: metav1.ObjectMeta{Name: "workspace-a", Namespace: "team"}}) + assertRequestSet(t, requests, []types.NamespacedName{ + {Namespace: "team", Name: "session-a"}, + {Namespace: "team", Name: "session-b"}, + }) +} diff --git a/packages/cluster-operator/controllers/workspace_controller_test.go b/packages/cluster-operator/controllers/workspace_controller_test.go index 412705d0..27d07759 100644 --- a/packages/cluster-operator/controllers/workspace_controller_test.go +++ b/packages/cluster-operator/controllers/workspace_controller_test.go @@ -9,6 +9,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -107,3 +108,42 @@ func TestWorkspaceRequestsForStorageClassOnlyEnqueuesAffectedWorkspaces(t *testi } } } + +func TestWorkspaceRequestsForHostOnlyEnqueuesAffectedWorkspaces(t *testing.T) { + scheme := runtime.NewScheme() + if err := clusterv1alpha1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + objects := []client.Object{ + &clusterv1alpha1.T4Workspace{ObjectMeta: metav1.ObjectMeta{Name: "workspace-a", Namespace: "team"}, Spec: clusterv1alpha1.T4WorkspaceSpec{HostRef: "host-a"}}, + &clusterv1alpha1.T4Workspace{ObjectMeta: metav1.ObjectMeta{Name: "workspace-b", Namespace: "team"}, Spec: clusterv1alpha1.T4WorkspaceSpec{HostRef: "host-a"}}, + &clusterv1alpha1.T4Workspace{ObjectMeta: metav1.ObjectMeta{Name: "other-host", Namespace: "team"}, Spec: clusterv1alpha1.T4WorkspaceSpec{HostRef: "host-b"}}, + &clusterv1alpha1.T4Workspace{ObjectMeta: metav1.ObjectMeta{Name: "other-namespace", Namespace: "other"}, Spec: clusterv1alpha1.T4WorkspaceSpec{HostRef: "host-a"}}, + } + c := fake.NewClientBuilder().WithScheme(scheme). + WithIndex(&clusterv1alpha1.T4Workspace{}, workspaceHostRefIndexField, indexWorkspaceByHostRef). + WithObjects(objects...).Build() + r := &WorkspaceReconciler{Client: c, Scheme: scheme} + + requests := r.workspaceRequestsForHost(context.Background(), &clusterv1alpha1.T4ClusterHost{ObjectMeta: metav1.ObjectMeta{Name: "host-a", Namespace: "team"}}) + assertRequestSet(t, requests, []types.NamespacedName{ + {Namespace: "team", Name: "workspace-a"}, + {Namespace: "team", Name: "workspace-b"}, + }) +} + +func assertRequestSet(t *testing.T, requests []ctrl.Request, want []types.NamespacedName) { + t.Helper() + got := make(map[types.NamespacedName]int, len(requests)) + for _, request := range requests { + got[request.NamespacedName]++ + } + if len(got) != len(want) { + t.Fatalf("requests = %#v, want exactly %v", requests, want) + } + for _, key := range want { + if got[key] != 1 { + t.Fatalf("requests = %#v, want %v exactly once", requests, key) + } + } +} From f91517b204644a6c1c1b8c8a15c0a92463c2284d Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 20:51:56 +0000 Subject: [PATCH 02/29] feat(operator): reconcile dependency changes --- .../controllers/session_controller.go | 77 +++++++++++++++++++ .../controllers/workspace_controller.go | 18 +++++ 2 files changed, 95 insertions(+) diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index 9336a3f4..a4cce1f2 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -25,6 +25,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" clusterv1alpha1 "github.com/LycaonLLC/t4-code/packages/cluster-operator/api/v1alpha1" ) @@ -36,6 +37,11 @@ const ( SessionReviewerTokenExpirationSeconds int64 = 3600 ) +const ( + sessionHostRefIndexField = "t4.session.spec.hostRef" + sessionWorkspaceRefIndexField = "t4.session.spec.workspaceRef" +) + var ( configMapKeyPattern = regexp.MustCompile(`^[-._A-Za-z0-9]+$`) runtimeImagePattern = regexp.MustCompile(`^(?:(?:[A-Za-z0-9](?:[A-Za-z0-9.-]*[A-Za-z0-9])?|\[[A-Fa-f0-9:]+\])(?::[0-9]+)?/)?[a-z0-9]+(?:(?:[._]|__|-+)[a-z0-9]+)*(?:/[a-z0-9]+(?:(?:[._]|__|-+)[a-z0-9]+)*)*@sha256:[a-f0-9]{64}$`) @@ -313,6 +319,9 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) var host clusterv1alpha1.T4ClusterHost if err := r.Get(ctx, types.NamespacedName{Namespace: session.Namespace, Name: session.Spec.HostRef}, &host); err != nil { if apierrors.IsNotFound(err) { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "HostReady", "HostNotFound", "referenced T4ClusterHost does not exist") } return ctrl.Result{}, err @@ -342,24 +351,39 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) var workspace clusterv1alpha1.T4Workspace if err := r.Get(ctx, types.NamespacedName{Namespace: session.Namespace, Name: session.Spec.WorkspaceRef}, &workspace); err != nil { if apierrors.IsNotFound(err) { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", "WorkspaceNotFound", "referenced T4Workspace does not exist") } return ctrl.Result{}, err } if workspace.Spec.HostRef != session.Spec.HostRef { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", "HostMismatch", "session and workspace must reference the same T4ClusterHost") } if workspace.Status.PVCName == "" { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", "PVCNotDeclared", "workspace controller has not declared a PVC") } var pvc corev1.PersistentVolumeClaim if err := r.Get(ctx, types.NamespacedName{Namespace: session.Namespace, Name: workspace.Status.PVCName}, &pvc); err != nil { if apierrors.IsNotFound(err) { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", "PVCNotFound", "workspace PVC does not exist") } return ctrl.Result{}, err } if pvc.Status.Phase != corev1.ClaimBound || !pvcHasRWX(&pvc) { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", "PVCNotBoundRWX", "workspace PVC must be Bound and ReadWriteMany before a session starts") } runtimeVersions, reason, message, err := r.loadOMPResourceVersions(ctx, session.Namespace) @@ -754,9 +778,62 @@ func labelsContain(actual, required map[string]string) bool { return true } +func indexSessionByHostRef(object client.Object) []string { + session, ok := object.(*clusterv1alpha1.T4Session) + if !ok || session.Spec.HostRef == "" { + return nil + } + return []string{session.Spec.HostRef} +} + +func indexSessionByWorkspaceRef(object client.Object) []string { + session, ok := object.(*clusterv1alpha1.T4Session) + if !ok || session.Spec.WorkspaceRef == "" { + return nil + } + return []string{session.Spec.WorkspaceRef} +} + +func (r *SessionReconciler) sessionRequestsForHost(ctx context.Context, object client.Object) []ctrl.Request { + host, ok := object.(*clusterv1alpha1.T4ClusterHost) + if !ok || host.Name == "" || host.Namespace == "" { + return nil + } + return r.sessionRequestsForReference(ctx, host.Namespace, sessionHostRefIndexField, host.Name, "clusterHost", client.ObjectKeyFromObject(host)) +} + +func (r *SessionReconciler) sessionRequestsForWorkspace(ctx context.Context, object client.Object) []ctrl.Request { + workspace, ok := object.(*clusterv1alpha1.T4Workspace) + if !ok || workspace.Name == "" || workspace.Namespace == "" { + return nil + } + return r.sessionRequestsForReference(ctx, workspace.Namespace, sessionWorkspaceRefIndexField, workspace.Name, "workspace", client.ObjectKeyFromObject(workspace)) +} + +func (r *SessionReconciler) sessionRequestsForReference(ctx context.Context, namespace, field, value, dependencyKind string, dependencyKey types.NamespacedName) []ctrl.Request { + var sessions clusterv1alpha1.T4SessionList + if err := r.List(ctx, &sessions, client.InNamespace(namespace), client.MatchingFields{field: value}); err != nil { + ctrl.LoggerFrom(ctx).Error(err, "unable to map dependency to sessions", dependencyKind, dependencyKey) + return nil + } + requests := make([]ctrl.Request, 0, len(sessions.Items)) + for i := range sessions.Items { + requests = append(requests, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(&sessions.Items[i])}) + } + return requests +} + func (r *SessionReconciler) SetupWithManager(manager ctrl.Manager) error { + if err := manager.GetFieldIndexer().IndexField(context.Background(), &clusterv1alpha1.T4Session{}, sessionHostRefIndexField, indexSessionByHostRef); err != nil { + return fmt.Errorf("index T4Session by host reference: %w", err) + } + if err := manager.GetFieldIndexer().IndexField(context.Background(), &clusterv1alpha1.T4Session{}, sessionWorkspaceRefIndexField, indexSessionByWorkspaceRef); err != nil { + return fmt.Errorf("index T4Session by workspace reference: %w", err) + } return ctrl.NewControllerManagedBy(manager). For(&clusterv1alpha1.T4Session{}). + Watches(&clusterv1alpha1.T4ClusterHost{}, handler.EnqueueRequestsFromMapFunc(r.sessionRequestsForHost)). + Watches(&clusterv1alpha1.T4Workspace{}, handler.EnqueueRequestsFromMapFunc(r.sessionRequestsForWorkspace)). Owns(&corev1.Pod{}). Owns(&corev1.Service{}). Complete(r) diff --git a/packages/cluster-operator/controllers/workspace_controller.go b/packages/cluster-operator/controllers/workspace_controller.go index fbc60f45..e96271a6 100644 --- a/packages/cluster-operator/controllers/workspace_controller.go +++ b/packages/cluster-operator/controllers/workspace_controller.go @@ -337,6 +337,23 @@ func (r *WorkspaceReconciler) workspaceRequestsForStorageClass(ctx context.Conte return requests } +func (r *WorkspaceReconciler) workspaceRequestsForHost(ctx context.Context, object client.Object) []ctrl.Request { + host, ok := object.(*clusterv1alpha1.T4ClusterHost) + if !ok || host.Name == "" || host.Namespace == "" { + return nil + } + var workspaces clusterv1alpha1.T4WorkspaceList + if err := r.List(ctx, &workspaces, client.InNamespace(host.Namespace), client.MatchingFields{workspaceHostRefIndexField: host.Name}); err != nil { + ctrl.LoggerFrom(ctx).Error(err, "unable to map cluster host to workspaces", "clusterHost", client.ObjectKeyFromObject(host)) + return nil + } + requests := make([]ctrl.Request, 0, len(workspaces.Items)) + for i := range workspaces.Items { + requests = append(requests, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(&workspaces.Items[i])}) + } + return requests +} + func (r *WorkspaceReconciler) SetupWithManager(manager ctrl.Manager) error { if err := manager.GetFieldIndexer().IndexField(context.Background(), &clusterv1alpha1.T4ClusterHost{}, hostStorageClassIndexField, indexHostByStorageClass); err != nil { return fmt.Errorf("index T4ClusterHost by StorageClass: %w", err) @@ -346,6 +363,7 @@ func (r *WorkspaceReconciler) SetupWithManager(manager ctrl.Manager) error { } return ctrl.NewControllerManagedBy(manager). For(&clusterv1alpha1.T4Workspace{}). + Watches(&clusterv1alpha1.T4ClusterHost{}, handler.EnqueueRequestsFromMapFunc(r.workspaceRequestsForHost)). Watches(&corev1.PersistentVolumeClaim{}, handler.EnqueueRequestsFromMapFunc(workspaceRequestsForPVC)). Watches(&storagev1.StorageClass{}, handler.EnqueueRequestsFromMapFunc(r.workspaceRequestsForStorageClass)). Complete(r) From 72c6a19401e2e6f6b76e4980cf65051edd0b8695 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 21:05:36 +0000 Subject: [PATCH 03/29] test(operator): require host policy convergence --- .../controllers/reconciler_test.go | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 02b070a2..905e3896 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -1296,6 +1296,18 @@ func TestSessionDependencyRevocationCleansOwnedResourcesAndConvergesAfterRestart delete(secret.Data, "MODEL_API_KEY") return c.Update(ctx, &secret) }}, + {name: "mismatched Host storage class", conditionType: "WorkspaceReady", wantReason: "StorageClassMismatch", revoke: func(ctx context.Context, c client.Client) error { + otherClass := &storagev1.StorageClass{ObjectMeta: metav1.ObjectMeta{Name: "other-rwx", Annotations: map[string]string{clusterv1alpha1.RWXStorageClassAnnotation: string(corev1.ReadWriteMany)}}, Provisioner: "example.invalid/csi"} + if err := c.Create(ctx, otherClass); err != nil { + return err + } + var host clusterv1alpha1.T4ClusterHost + if err := c.Get(ctx, types.NamespacedName{Namespace: "team", Name: "host-a"}, &host); err != nil { + return err + } + host.Spec.StorageClassName = otherClass.Name + return c.Update(ctx, &host) + }}, } { t.Run(test.name, func(t *testing.T) { ctx := context.Background() @@ -1371,6 +1383,137 @@ func TestSessionDependencyRevocationCleansOwnedResourcesAndConvergesAfterRestart } } +func TestWorkspaceHostStorageClassDriftFailsClosedWithoutRecreatingPVC(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + workspace.Status.ObservedGeneration = workspace.Generation + workspace.Status.PVCName = controllers.WorkspacePVCName(workspace) + workspace.Status.Phase = clusterv1alpha1.InfrastructureReady + workspace.Status.Conditions = []metav1.Condition{ + {Type: "StorageReady", Status: metav1.ConditionTrue, Reason: controllers.ReasonStorageReady, ObservedGeneration: workspace.Generation}, + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "PVCBound", ObservedGeneration: workspace.Generation}, + } + oldClass := "portable-rwx" + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: workspace.Status.PVCName, Namespace: workspace.Namespace, + Annotations: map[string]string{clusterv1alpha1.WorkspaceUIDAnnotation: string(workspace.UID)}, + OwnerReferences: []metav1.OwnerReference{{APIVersion: clusterv1alpha1.GroupVersion.String(), Kind: "T4Workspace", Name: workspace.Name, UID: workspace.UID, Controller: ptr(true)}}, + }, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: &oldClass, AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, + } + host := testHost() + host.Spec.StorageClassName = "other-rwx" + otherClass := &storagev1.StorageClass{ObjectMeta: metav1.ObjectMeta{Name: "other-rwx", Annotations: map[string]string{clusterv1alpha1.RWXStorageClassAnnotation: string(corev1.ReadWriteMany)}}, Provisioner: "example.invalid/csi"} + c := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Workspace{}, &corev1.PersistentVolumeClaim{}). + WithObjects(host, otherClass, workspace, pvc).Build() + r := &controllers.WorkspaceReconciler{Client: c, Scheme: scheme} + + result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}) + if err != nil { + t.Fatal(err) + } + if result.RequeueAfter <= 0 || result.RequeueAfter > 30*time.Second { + t.Fatalf("storage drift requeue = %s, want bounded positive retry", result.RequeueAfter) + } + var got clusterv1alpha1.T4Workspace + if err := c.Get(ctx, client.ObjectKeyFromObject(workspace), &got); err != nil { + t.Fatal(err) + } + storageReady := findCondition(got.Status.Conditions, "StorageReady") + ready := findCondition(got.Status.Conditions, "Ready") + if got.Status.Phase != clusterv1alpha1.InfrastructureFailed || storageReady == nil || storageReady.Status != metav1.ConditionFalse || storageReady.Reason != "StorageClassMismatch" || ready == nil || ready.Status != metav1.ConditionFalse || ready.Reason != "StorageClassMismatch" { + t.Fatalf("storage drift remained Ready: status=%#v StorageReady=%#v Ready=%#v", got.Status, storageReady, ready) + } + var retained corev1.PersistentVolumeClaim + if err := c.Get(ctx, client.ObjectKeyFromObject(pvc), &retained); err != nil { + t.Fatalf("storage drift removed the data PVC: %v", err) + } + if retained.Spec.StorageClassName == nil || *retained.Spec.StorageClassName != oldClass { + t.Fatalf("storage drift recreated or mutated PVC class: %#v", retained.Spec.StorageClassName) + } +} + +func TestHostDependencyRecoveryReplacesFalseConditions(t *testing.T) { + t.Run("Workspace", func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + c := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Workspace{}, &corev1.PersistentVolumeClaim{}). + WithObjects(workspace).Build() + r := &controllers.WorkspaceReconciler{Client: c, Scheme: scheme} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}); err != nil { + t.Fatal(err) + } + if err := c.Create(ctx, testHost()); err != nil { + t.Fatal(err) + } + if err := c.Create(ctx, rwxStorageClass()); err != nil { + t.Fatal(err) + } + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}); err != nil { + t.Fatal(err) + } + var pvc corev1.PersistentVolumeClaim + if err := c.Get(ctx, types.NamespacedName{Namespace: workspace.Namespace, Name: controllers.WorkspacePVCName(workspace)}, &pvc); err != nil { + t.Fatal(err) + } + pvc.Status.Phase = corev1.ClaimBound + if err := c.Status().Update(ctx, &pvc); err != nil { + t.Fatal(err) + } + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}); err != nil { + t.Fatal(err) + } + var recovered clusterv1alpha1.T4Workspace + if err := c.Get(ctx, client.ObjectKeyFromObject(workspace), &recovered); err != nil { + t.Fatal(err) + } + hostReady := findCondition(recovered.Status.Conditions, "HostReady") + if hostReady == nil || hostReady.Status != metav1.ConditionTrue || hostReady.ObservedGeneration != recovered.Generation { + t.Fatalf("recovered Workspace retained stale HostReady: %#v", hostReady) + } + }) + + t.Run("Session", func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.Status.PVCName = "workspace-a-data" + pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: workspace.Namespace}, Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}} + session := testSession() + session.UID = "session-uid" + c := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Session{}, &corev1.PersistentVolumeClaim{}, &corev1.Pod{}). + WithObjects(workspace, pvc, session).Build() + r := configuredSessionReconciler(c, scheme) + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { + t.Fatal(err) + } + if err := c.Create(ctx, testHost()); err != nil { + t.Fatal(err) + } + reconcileMany(t, 2, func() error { + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}) + return err + }) + var recovered clusterv1alpha1.T4Session + if err := c.Get(ctx, client.ObjectKeyFromObject(session), &recovered); err != nil { + t.Fatal(err) + } + hostReady := findCondition(recovered.Status.Conditions, "HostReady") + if hostReady == nil || hostReady.Status != metav1.ConditionTrue || hostReady.ObservedGeneration != recovered.Generation { + t.Fatalf("recovered Session retained stale HostReady: %#v", hostReady) + } + }) +} + func configuredSessionReconciler(c client.Client, scheme *runtime.Scheme) *controllers.SessionReconciler { for _, object := range []client.Object{ &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "omp-runtime-config", Namespace: "team"}, Data: map[string]string{ From bb851a8560a0eb380830c512f0a1709a96b57bab Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 21:10:01 +0000 Subject: [PATCH 04/29] fix(operator): fail closed on host storage drift --- .../cluster-operator/controllers/helpers.go | 1 + .../controllers/reconciler_test.go | 2 +- .../controllers/session_controller.go | 41 ++++++++++++------- .../controllers/workspace_controller.go | 6 +++ 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/packages/cluster-operator/controllers/helpers.go b/packages/cluster-operator/controllers/helpers.go index ec5681af..4c88e8c2 100644 --- a/packages/cluster-operator/controllers/helpers.go +++ b/packages/cluster-operator/controllers/helpers.go @@ -18,6 +18,7 @@ import ( const ( ReasonStorageClassNotFound = "StorageClassNotFound" ReasonStorageClassNotRWX = "StorageClassNotRWX" + ReasonStorageClassMismatch = "StorageClassMismatch" ReasonStorageReady = "StorageClassSupportsRWX" ) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 905e3896..9fca96a1 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -1316,7 +1316,7 @@ func TestSessionDependencyRevocationCleansOwnedResourcesAndConvergesAfterRestart workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index a4cce1f2..26e516e1 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -307,13 +307,13 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "RuntimeConfigured", reason, message) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, false, "RuntimeConfigured", reason, message) } if reason, message := r.OMPConfig.validationFailure(); reason != "" { if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "RuntimeConfigured", reason, message) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, false, "RuntimeConfigured", reason, message) } var host clusterv1alpha1.T4ClusterHost @@ -322,7 +322,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "HostReady", "HostNotFound", "referenced T4ClusterHost does not exist") + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, false, "HostReady", "HostNotFound", "referenced T4ClusterHost does not exist") } return ctrl.Result{}, err } @@ -330,7 +330,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "RuntimeConfigured", "RuntimeProfileNotAllowed", "runtime profile is not allowed by the referenced T4ClusterHost") + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "RuntimeConfigured", "RuntimeProfileNotAllowed", "runtime profile is not allowed by the referenced T4ClusterHost") } var storageClass storagev1.StorageClass if err := r.Get(ctx, types.NamespacedName{Name: host.Spec.StorageClassName}, &storageClass); err != nil { @@ -338,7 +338,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", ReasonStorageClassNotFound, fmt.Sprintf("StorageClass %q selected by the referenced T4ClusterHost does not exist", host.Spec.StorageClassName)) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", ReasonStorageClassNotFound, fmt.Sprintf("StorageClass %q selected by the referenced T4ClusterHost does not exist", host.Spec.StorageClassName)) } return ctrl.Result{}, err } @@ -346,7 +346,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", ReasonStorageClassNotRWX, fmt.Sprintf("StorageClass %q selected by the referenced T4ClusterHost is not administrator-declared ReadWriteMany", host.Spec.StorageClassName)) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", ReasonStorageClassNotRWX, fmt.Sprintf("StorageClass %q selected by the referenced T4ClusterHost is not administrator-declared ReadWriteMany", host.Spec.StorageClassName)) } var workspace clusterv1alpha1.T4Workspace if err := r.Get(ctx, types.NamespacedName{Namespace: session.Namespace, Name: session.Spec.WorkspaceRef}, &workspace); err != nil { @@ -354,7 +354,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", "WorkspaceNotFound", "referenced T4Workspace does not exist") + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", "WorkspaceNotFound", "referenced T4Workspace does not exist") } return ctrl.Result{}, err } @@ -362,13 +362,13 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", "HostMismatch", "session and workspace must reference the same T4ClusterHost") + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", "HostMismatch", "session and workspace must reference the same T4ClusterHost") } if workspace.Status.PVCName == "" { if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", "PVCNotDeclared", "workspace controller has not declared a PVC") + return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", "PVCNotDeclared", "workspace controller has not declared a PVC") } var pvc corev1.PersistentVolumeClaim if err := r.Get(ctx, types.NamespacedName{Namespace: session.Namespace, Name: workspace.Status.PVCName}, &pvc); err != nil { @@ -376,15 +376,21 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", "PVCNotFound", "workspace PVC does not exist") + return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", "PVCNotFound", "workspace PVC does not exist") } return ctrl.Result{}, err } + if pvc.Spec.StorageClassName != nil && *pvc.Spec.StorageClassName != host.Spec.StorageClassName { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", ReasonStorageClassMismatch, fmt.Sprintf("workspace PVC uses StorageClass %q instead of host-selected %q", *pvc.Spec.StorageClassName, host.Spec.StorageClassName)) + } if pvc.Status.Phase != corev1.ClaimBound || !pvcHasRWX(&pvc) { if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, "WorkspaceReady", "PVCNotBoundRWX", "workspace PVC must be Bound and ReadWriteMany before a session starts") + return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", "PVCNotBoundRWX", "workspace PVC must be Bound and ReadWriteMany before a session starts") } runtimeVersions, reason, message, err := r.loadOMPResourceVersions(ctx, session.Namespace) if err != nil { @@ -394,7 +400,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "RuntimeConfigured", reason, message) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "RuntimeConfigured", reason, message) } serviceName := SessionServiceName(&session) @@ -424,7 +430,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } else if err != nil { return ctrl.Result{}, err } else if !metav1.IsControlledBy(&service, &session) { - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "Available", "ServiceOwnershipConflict", "deterministic session Service is not controlled by this session") + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "Available", "ServiceOwnershipConflict", "deterministic session Service is not controlled by this session") } else if !serviceExposureIsInternal(&service) { if err := r.Delete(ctx, &service); err != nil && !apierrors.IsNotFound(err) { return ctrl.Result{}, err @@ -458,7 +464,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } else if err != nil { return ctrl.Result{}, err } else if !metav1.IsControlledBy(&pod, &session) { - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, "Available", "PodOwnershipConflict", "deterministic session Pod is not controlled by this session") + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "Available", "PodOwnershipConflict", "deterministic session Pod is not controlled by this session") } else if !labelsContain(pod.Labels, desiredPod.Labels) { if pod.Labels == nil { pod.Labels = map[string]string{} @@ -490,6 +496,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) session.Status.ObservedGeneration = session.Generation session.Status.PodName = podName session.Status.ServiceName = serviceName + meta.SetStatusCondition(&session.Status.Conditions, condition("HostReady", metav1.ConditionTrue, "HostResolved", "referenced T4ClusterHost is available", session.Generation)) meta.SetStatusCondition(&session.Status.Conditions, condition("WorkspaceReady", metav1.ConditionTrue, "PVCBoundRWX", "workspace PVC is Bound and ReadWriteMany", session.Generation)) meta.SetStatusCondition(&session.Status.Conditions, condition("RuntimeConfigured", metav1.ConditionTrue, "OMPReferencesReady", "administrator-owned OMP runtime references are configured", session.Generation)) if podReady(&pod) { @@ -646,6 +653,7 @@ func (r *SessionReconciler) updateSessionPending(ctx context.Context, session *c original.Conditions = append([]metav1.Condition(nil), session.Status.Conditions...) } session.Status.ObservedGeneration = session.Generation + meta.SetStatusCondition(&session.Status.Conditions, condition("HostReady", metav1.ConditionTrue, "HostResolved", "referenced T4ClusterHost is available", session.Generation)) session.Status.PodName = podName session.Status.ServiceName = serviceName session.Status.Phase = clusterv1alpha1.InfrastructurePending @@ -736,11 +744,14 @@ func (r *SessionReconciler) deleteOwnedSessionResources(ctx context.Context, ses return nil } -func (r *SessionReconciler) updateSessionFailure(ctx context.Context, session *clusterv1alpha1.T4Session, conditionType, reason, message string) error { +func (r *SessionReconciler) updateSessionFailure(ctx context.Context, session *clusterv1alpha1.T4Session, hostReady bool, conditionType, reason, message string) error { original := session.Status if session.Status.Conditions != nil { original.Conditions = append([]metav1.Condition(nil), session.Status.Conditions...) } + if hostReady { + meta.SetStatusCondition(&session.Status.Conditions, condition("HostReady", metav1.ConditionTrue, "HostResolved", "referenced T4ClusterHost is available", session.Generation)) + } session.Status.ObservedGeneration = session.Generation session.Status.PodName = "" session.Status.ServiceName = "" diff --git a/packages/cluster-operator/controllers/workspace_controller.go b/packages/cluster-operator/controllers/workspace_controller.go index e96271a6..97d95819 100644 --- a/packages/cluster-operator/controllers/workspace_controller.go +++ b/packages/cluster-operator/controllers/workspace_controller.go @@ -105,6 +105,8 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, request ctrl.Reques return ctrl.Result{}, err } else if !workspaceOwnsPVC(&workspace, &pvc) { return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCOwnershipConflict", "deterministic workspace PVC does not belong to this workspace") + } else if pvc.Spec.StorageClassName != nil && *pvc.Spec.StorageClassName != storageClassName { + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", ReasonStorageClassMismatch, fmt.Sprintf("workspace PVC uses StorageClass %q instead of host-selected %q; data-bearing PVCs are never recreated automatically", *pvc.Spec.StorageClassName, storageClassName)) } else if workspace.Spec.RetentionPolicy == clusterv1alpha1.RetentionPolicyRetain && metav1.IsControlledBy(&pvc, &workspace) { before := pvc.DeepCopy() pvc.OwnerReferences = removeWorkspaceOwnerReference(pvc.OwnerReferences, workspace.UID) @@ -126,6 +128,7 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, request ctrl.Reques workspace.Status.PVCPhase = pvc.Status.Phase capacity := pvc.Status.Capacity[corev1.ResourceStorage] workspace.Status.Capacity = capacity.DeepCopy() + meta.SetStatusCondition(&workspace.Status.Conditions, condition("HostReady", metav1.ConditionTrue, "HostResolved", "referenced T4ClusterHost is available", workspace.Generation)) meta.SetStatusCondition(&workspace.Status.Conditions, condition("StorageReady", metav1.ConditionTrue, ReasonStorageReady, "RWX StorageClass and workspace PVC are accepted", workspace.Generation)) switch pvc.Status.Phase { case corev1.ClaimBound: @@ -267,6 +270,9 @@ func (r *WorkspaceReconciler) updateWorkspaceFailure(ctx context.Context, worksp if workspace.Status.Conditions != nil { original.Conditions = append([]metav1.Condition(nil), workspace.Status.Conditions...) } + if conditionType != "HostReady" { + meta.SetStatusCondition(&workspace.Status.Conditions, condition("HostReady", metav1.ConditionTrue, "HostResolved", "referenced T4ClusterHost is available", workspace.Generation)) + } workspace.Status.ObservedGeneration = workspace.Generation workspace.Status.Phase = clusterv1alpha1.InfrastructureFailed meta.SetStatusCondition(&workspace.Status.Conditions, condition(conditionType, metav1.ConditionFalse, reason, message, workspace.Generation)) From 4ccae52d2063668a7c9a681f85b7b685b71d452f Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 21:20:38 +0000 Subject: [PATCH 05/29] fix(operator): require explicit workspace storage class --- .../cluster-operator/controllers/helpers.go | 7 + .../controllers/reconciler_test.go | 171 +++++++++++------- .../controllers/session_controller.go | 6 +- .../controllers/workspace_controller.go | 4 +- 4 files changed, 120 insertions(+), 68 deletions(-) diff --git a/packages/cluster-operator/controllers/helpers.go b/packages/cluster-operator/controllers/helpers.go index 4c88e8c2..8fef978c 100644 --- a/packages/cluster-operator/controllers/helpers.go +++ b/packages/cluster-operator/controllers/helpers.go @@ -83,6 +83,13 @@ func pvcHasRWX(pvc *corev1.PersistentVolumeClaim) bool { return false } +func pvcStorageClassName(pvc *corev1.PersistentVolumeClaim) string { + if pvc.Spec.StorageClassName == nil { + return "" + } + return *pvc.Spec.StorageClassName +} + func hasString(values []string, wanted string) bool { for _, value := range values { if value == wanted { diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 9fca96a1..8342d571 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -153,7 +153,7 @@ func TestRetainDeletionOrphansPVCBeforeRemovingFinalizer(t *testing.T) { APIVersion: clusterv1alpha1.GroupVersion.String(), Kind: "T4Workspace", Name: workspace.Name, UID: workspace.UID, Controller: ptr(true), }}, }, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, } c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&clusterv1alpha1.T4Workspace{}).WithObjects(testHost(), rwxStorageClass(), workspace, pvc).Build() if err := c.Delete(context.Background(), workspace); err != nil { @@ -187,7 +187,7 @@ func TestWorkspaceDeletionWaitsForSessionResources(t *testing.T) { Name: controllers.WorkspacePVCName(workspace), Namespace: workspace.Namespace, OwnerReferences: []metav1.OwnerReference{{APIVersion: clusterv1alpha1.GroupVersion.String(), Kind: "T4Workspace", Name: workspace.Name, UID: workspace.UID, Controller: ptr(true)}}, }, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, } session := testSession() session.Spec.WorkspaceRef = workspace.Name @@ -457,7 +457,7 @@ func TestSessionRuntimeImageMustBeImmutableDigest(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -528,7 +528,7 @@ func TestSessionAuthorityRevocationDeletesOwnedPodAndService(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -575,7 +575,7 @@ func TestSessionNamesProduceSafeRuntimeIdentities(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -620,7 +620,7 @@ func TestSessionWaitsForBoundRWXThenCreatesExactlyOnePodAndService(t *testing.T) workspace.Status.Phase = clusterv1alpha1.InfrastructurePending pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimPending}, } session := testSession() @@ -755,7 +755,7 @@ func TestSessionOMPModeOmitsCredentialSecretReference(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -791,7 +791,7 @@ func TestSessionRejectsUnownedDeterministicResources(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -826,7 +826,7 @@ func TestSessionRecreatesPodWhenImmutableDesiredStateChanges(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -881,7 +881,7 @@ func TestSessionPodHashIncludesEveryOMPReference(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -927,7 +927,7 @@ func TestSessionRecreatesPodWhenOMPResourceVersionChanges(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -970,7 +970,7 @@ func TestSessionRuntimeReferenceRevocationStopsAuthority(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -1024,7 +1024,7 @@ func TestSessionFailsClosedWhenOMPConfigMapIsMissing(t *testing.T) { scheme := testScheme(t) workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) workspace.Status.PVCName = "workspace-a-data" - pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}} + pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}} session := testSession() objects := []client.Object{testHost(), rwxStorageClass(), workspace, pvc, session} if test.configMap != nil { @@ -1054,7 +1054,7 @@ func TestSessionRecreatesExternallyExposedOwnedService(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -1102,7 +1102,7 @@ func TestSessionRestoresRequiredPodSelectorLabelsBeforeAvailability(t *testing.T workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -1308,6 +1308,14 @@ func TestSessionDependencyRevocationCleansOwnedResourcesAndConvergesAfterRestart host.Spec.StorageClassName = otherClass.Name return c.Update(ctx, &host) }}, + {name: "classless Workspace PVC", conditionType: "WorkspaceReady", wantReason: "StorageClassMismatch", revoke: func(ctx context.Context, c client.Client) error { + var pvc corev1.PersistentVolumeClaim + if err := c.Get(ctx, types.NamespacedName{Namespace: "team", Name: "workspace-a-data"}, &pvc); err != nil { + return err + } + pvc.Spec.StorageClassName = nil + return c.Update(ctx, &pvc) + }}, } { t.Run(test.name, func(t *testing.T) { ctx := context.Background() @@ -1384,57 +1392,67 @@ func TestSessionDependencyRevocationCleansOwnedResourcesAndConvergesAfterRestart } func TestWorkspaceHostStorageClassDriftFailsClosedWithoutRecreatingPVC(t *testing.T) { - ctx := context.Background() - scheme := testScheme(t) - workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) - workspace.UID = "workspace-uid" - workspace.Status.ObservedGeneration = workspace.Generation - workspace.Status.PVCName = controllers.WorkspacePVCName(workspace) - workspace.Status.Phase = clusterv1alpha1.InfrastructureReady - workspace.Status.Conditions = []metav1.Condition{ - {Type: "StorageReady", Status: metav1.ConditionTrue, Reason: controllers.ReasonStorageReady, ObservedGeneration: workspace.Generation}, - {Type: "Ready", Status: metav1.ConditionTrue, Reason: "PVCBound", ObservedGeneration: workspace.Generation}, - } oldClass := "portable-rwx" - pvc := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: workspace.Status.PVCName, Namespace: workspace.Namespace, - Annotations: map[string]string{clusterv1alpha1.WorkspaceUIDAnnotation: string(workspace.UID)}, - OwnerReferences: []metav1.OwnerReference{{APIVersion: clusterv1alpha1.GroupVersion.String(), Kind: "T4Workspace", Name: workspace.Name, UID: workspace.UID, Controller: ptr(true)}}, - }, - Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: &oldClass, AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, - Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, - } - host := testHost() - host.Spec.StorageClassName = "other-rwx" - otherClass := &storagev1.StorageClass{ObjectMeta: metav1.ObjectMeta{Name: "other-rwx", Annotations: map[string]string{clusterv1alpha1.RWXStorageClassAnnotation: string(corev1.ReadWriteMany)}}, Provisioner: "example.invalid/csi"} - c := fake.NewClientBuilder().WithScheme(scheme). - WithStatusSubresource(&clusterv1alpha1.T4Workspace{}, &corev1.PersistentVolumeClaim{}). - WithObjects(host, otherClass, workspace, pvc).Build() - r := &controllers.WorkspaceReconciler{Client: c, Scheme: scheme} + for _, test := range []struct { + name string + claimClass *string + }{ + {name: "different class", claimClass: &oldClass}, + {name: "class omitted"}, + } { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + workspace.Status.ObservedGeneration = workspace.Generation + workspace.Status.PVCName = controllers.WorkspacePVCName(workspace) + workspace.Status.Phase = clusterv1alpha1.InfrastructureReady + workspace.Status.Conditions = []metav1.Condition{ + {Type: "StorageReady", Status: metav1.ConditionTrue, Reason: controllers.ReasonStorageReady, ObservedGeneration: workspace.Generation}, + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "PVCBound", ObservedGeneration: workspace.Generation}, + } + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: workspace.Status.PVCName, Namespace: workspace.Namespace, + Annotations: map[string]string{clusterv1alpha1.WorkspaceUIDAnnotation: string(workspace.UID)}, + OwnerReferences: []metav1.OwnerReference{{APIVersion: clusterv1alpha1.GroupVersion.String(), Kind: "T4Workspace", Name: workspace.Name, UID: workspace.UID, Controller: ptr(true)}}, + }, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: test.claimClass, AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, + } + host := testHost() + host.Spec.StorageClassName = "other-rwx" + otherClass := &storagev1.StorageClass{ObjectMeta: metav1.ObjectMeta{Name: "other-rwx", Annotations: map[string]string{clusterv1alpha1.RWXStorageClassAnnotation: string(corev1.ReadWriteMany)}}, Provisioner: "example.invalid/csi"} + c := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Workspace{}, &corev1.PersistentVolumeClaim{}). + WithObjects(host, otherClass, workspace, pvc).Build() + r := &controllers.WorkspaceReconciler{Client: c, Scheme: scheme} - result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}) - if err != nil { - t.Fatal(err) - } - if result.RequeueAfter <= 0 || result.RequeueAfter > 30*time.Second { - t.Fatalf("storage drift requeue = %s, want bounded positive retry", result.RequeueAfter) - } - var got clusterv1alpha1.T4Workspace - if err := c.Get(ctx, client.ObjectKeyFromObject(workspace), &got); err != nil { - t.Fatal(err) - } - storageReady := findCondition(got.Status.Conditions, "StorageReady") - ready := findCondition(got.Status.Conditions, "Ready") - if got.Status.Phase != clusterv1alpha1.InfrastructureFailed || storageReady == nil || storageReady.Status != metav1.ConditionFalse || storageReady.Reason != "StorageClassMismatch" || ready == nil || ready.Status != metav1.ConditionFalse || ready.Reason != "StorageClassMismatch" { - t.Fatalf("storage drift remained Ready: status=%#v StorageReady=%#v Ready=%#v", got.Status, storageReady, ready) - } - var retained corev1.PersistentVolumeClaim - if err := c.Get(ctx, client.ObjectKeyFromObject(pvc), &retained); err != nil { - t.Fatalf("storage drift removed the data PVC: %v", err) - } - if retained.Spec.StorageClassName == nil || *retained.Spec.StorageClassName != oldClass { - t.Fatalf("storage drift recreated or mutated PVC class: %#v", retained.Spec.StorageClassName) + result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}) + if err != nil { + t.Fatal(err) + } + if result.RequeueAfter <= 0 || result.RequeueAfter > 30*time.Second { + t.Fatalf("storage drift requeue = %s, want bounded positive retry", result.RequeueAfter) + } + var got clusterv1alpha1.T4Workspace + if err := c.Get(ctx, client.ObjectKeyFromObject(workspace), &got); err != nil { + t.Fatal(err) + } + storageReady := findCondition(got.Status.Conditions, "StorageReady") + ready := findCondition(got.Status.Conditions, "Ready") + if got.Status.Phase != clusterv1alpha1.InfrastructureFailed || storageReady == nil || storageReady.Status != metav1.ConditionFalse || storageReady.Reason != "StorageClassMismatch" || ready == nil || ready.Status != metav1.ConditionFalse || ready.Reason != "StorageClassMismatch" { + t.Fatalf("storage drift remained Ready: status=%#v StorageReady=%#v Ready=%#v", got.Status, storageReady, ready) + } + var retained corev1.PersistentVolumeClaim + if err := c.Get(ctx, client.ObjectKeyFromObject(pvc), &retained); err != nil { + t.Fatalf("storage drift removed the data PVC: %v", err) + } + if !reflect.DeepEqual(retained.Spec.StorageClassName, test.claimClass) { + t.Fatalf("storage drift recreated or mutated PVC class: got %#v, want %#v", retained.Spec.StorageClassName, test.claimClass) + } + }) } } @@ -1512,6 +1530,31 @@ func TestHostDependencyRecoveryReplacesFalseConditions(t *testing.T) { t.Fatalf("recovered Session retained stale HostReady: %#v", hostReady) } }) + + t.Run("static runtime failure", func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + session := testSession() + session.Status.ObservedGeneration = session.Generation + session.Status.Phase = clusterv1alpha1.InfrastructureFailed + session.Status.Conditions = []metav1.Condition{{Type: "HostReady", Status: metav1.ConditionFalse, Reason: "HostNotFound", ObservedGeneration: session.Generation}} + c := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Session{}). + WithObjects(testHost(), session).Build() + r := configuredSessionReconciler(c, scheme) + r.RuntimeImage = "registry.example/session:latest" + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { + t.Fatal(err) + } + var failed clusterv1alpha1.T4Session + if err := c.Get(ctx, client.ObjectKeyFromObject(session), &failed); err != nil { + t.Fatal(err) + } + hostReady := findCondition(failed.Status.Conditions, "HostReady") + if hostReady == nil || hostReady.Status != metav1.ConditionUnknown || hostReady.Reason != "NotEvaluated" || hostReady.ObservedGeneration != failed.Generation { + t.Fatalf("static runtime failure retained stale HostReady: %#v", hostReady) + } + }) } func configuredSessionReconciler(c client.Client, scheme *runtime.Scheme) *controllers.SessionReconciler { diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index 26e516e1..9b4318b8 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -380,11 +380,11 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } return ctrl.Result{}, err } - if pvc.Spec.StorageClassName != nil && *pvc.Spec.StorageClassName != host.Spec.StorageClassName { + if pvcStorageClassName(&pvc) != host.Spec.StorageClassName { if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", ReasonStorageClassMismatch, fmt.Sprintf("workspace PVC uses StorageClass %q instead of host-selected %q", *pvc.Spec.StorageClassName, host.Spec.StorageClassName)) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", ReasonStorageClassMismatch, fmt.Sprintf("workspace PVC uses StorageClass %q instead of host-selected %q", pvcStorageClassName(&pvc), host.Spec.StorageClassName)) } if pvc.Status.Phase != corev1.ClaimBound || !pvcHasRWX(&pvc) { if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { @@ -751,6 +751,8 @@ func (r *SessionReconciler) updateSessionFailure(ctx context.Context, session *c } if hostReady { meta.SetStatusCondition(&session.Status.Conditions, condition("HostReady", metav1.ConditionTrue, "HostResolved", "referenced T4ClusterHost is available", session.Generation)) + } else if conditionType != "HostReady" { + meta.SetStatusCondition(&session.Status.Conditions, condition("HostReady", metav1.ConditionUnknown, "NotEvaluated", "host dependency was not evaluated because static runtime configuration is invalid", session.Generation)) } session.Status.ObservedGeneration = session.Generation session.Status.PodName = "" diff --git a/packages/cluster-operator/controllers/workspace_controller.go b/packages/cluster-operator/controllers/workspace_controller.go index 97d95819..da1b555b 100644 --- a/packages/cluster-operator/controllers/workspace_controller.go +++ b/packages/cluster-operator/controllers/workspace_controller.go @@ -105,8 +105,8 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, request ctrl.Reques return ctrl.Result{}, err } else if !workspaceOwnsPVC(&workspace, &pvc) { return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCOwnershipConflict", "deterministic workspace PVC does not belong to this workspace") - } else if pvc.Spec.StorageClassName != nil && *pvc.Spec.StorageClassName != storageClassName { - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", ReasonStorageClassMismatch, fmt.Sprintf("workspace PVC uses StorageClass %q instead of host-selected %q; data-bearing PVCs are never recreated automatically", *pvc.Spec.StorageClassName, storageClassName)) + } else if pvcStorageClassName(&pvc) != storageClassName { + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", ReasonStorageClassMismatch, fmt.Sprintf("workspace PVC uses StorageClass %q instead of host-selected %q; data-bearing PVCs are never recreated automatically", pvcStorageClassName(&pvc), storageClassName)) } else if workspace.Spec.RetentionPolicy == clusterv1alpha1.RetentionPolicyRetain && metav1.IsControlledBy(&pvc, &workspace) { before := pvc.DeepCopy() pvc.OwnerReferences = removeWorkspaceOwnerReference(pvc.OwnerReferences, workspace.UID) From 30752e273db257440e150aa51df2b81b13739783 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 21:40:01 +0000 Subject: [PATCH 06/29] test(operator): reject stale dependency authority --- .../controllers/reconciler_test.go | 299 +++++++++++++++++- 1 file changed, 297 insertions(+), 2 deletions(-) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 8342d571..3f51cba1 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -1144,7 +1144,7 @@ func TestSessionRestoresRequiredPodSelectorLabelsBeforeAvailability(t *testing.T func TestWorkspaceDeletionRefusesForeignDeterministicPVC(t *testing.T) { for _, policy := range []clusterv1alpha1.RetentionPolicy{clusterv1alpha1.RetentionPolicyRetain, clusterv1alpha1.RetentionPolicyDelete} { - for _, mismatch := range []string{"uid-annotation", "controller-owner"} { + for _, mismatch := range []string{"uid-annotation", "controller-owner", "foreign-non-controller-owner"} { t.Run(string(policy)+"/"+mismatch, func(t *testing.T) { scheme := testScheme(t) workspace := testWorkspace(policy) @@ -1156,10 +1156,14 @@ func TestWorkspaceDeletionRefusesForeignDeterministicPVC(t *testing.T) { }} if mismatch == "uid-annotation" { pvc.Annotations[clusterv1alpha1.WorkspaceUIDAnnotation] = "foreign-workspace-uid" - } else { + } else if mismatch == "controller-owner" { pvc.OwnerReferences = []metav1.OwnerReference{{ APIVersion: clusterv1alpha1.GroupVersion.String(), Kind: "T4Workspace", Name: "foreign", UID: "foreign-workspace-uid", Controller: ptr(true), }} + } else { + pvc.OwnerReferences = []metav1.OwnerReference{{ + APIVersion: "example.test/v1", Kind: "Foreign", Name: "foreign", UID: "foreign-uid", + }} } expectedOwnerCount := len(pvc.OwnerReferences) c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&clusterv1alpha1.T4Workspace{}).WithObjects(workspace, pvc).Build() @@ -1557,6 +1561,297 @@ func TestHostDependencyRecoveryReplacesFalseConditions(t *testing.T) { }) } +func TestWorkspaceFailureRevokesPublishedPVCAuthorityAndRefreshesConditions(t *testing.T) { + for _, test := range []struct { + name string + objects func(*clusterv1alpha1.T4Workspace) []client.Object + wantHostStatus metav1.ConditionStatus + wantStorageReason string + }{ + { + name: "missing Host", + objects: func(workspace *clusterv1alpha1.T4Workspace) []client.Object { + return []client.Object{workspace} + }, + wantHostStatus: metav1.ConditionFalse, + wantStorageReason: "NotEvaluated", + }, + { + name: "missing StorageClass", + objects: func(workspace *clusterv1alpha1.T4Workspace) []client.Object { + return []client.Object{testHost(), workspace} + }, + wantHostStatus: metav1.ConditionTrue, + wantStorageReason: controllers.ReasonStorageClassNotFound, + }, + { + name: "non-RWX StorageClass", + objects: func(workspace *clusterv1alpha1.T4Workspace) []client.Object { + class := rwxStorageClass() + class.Annotations = nil + return []client.Object{testHost(), class, workspace} + }, + wantHostStatus: metav1.ConditionTrue, + wantStorageReason: controllers.ReasonStorageClassNotRWX, + }, + { + name: "PVC class drift", + objects: func(workspace *clusterv1alpha1.T4Workspace) []client.Object { + pvc := ownedWorkspacePVC(workspace) + pvc.Spec.StorageClassName = ptr("old-rwx") + return []client.Object{testHost(), rwxStorageClass(), workspace, pvc} + }, + wantHostStatus: metav1.ConditionTrue, + wantStorageReason: controllers.ReasonStorageClassMismatch, + }, + { + name: "PVC ownership conflict", + objects: func(workspace *clusterv1alpha1.T4Workspace) []client.Object { + pvc := ownedWorkspacePVC(workspace) + pvc.OwnerReferences = []metav1.OwnerReference{{APIVersion: "example.test/v1", Kind: "Foreign", Name: "foreign", UID: "foreign-uid", Controller: ptr(true)}} + return []client.Object{testHost(), rwxStorageClass(), workspace, pvc} + }, + wantHostStatus: metav1.ConditionTrue, + wantStorageReason: "PVCOwnershipConflict", + }, + } { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + workspace.Generation = 7 + workspace.Status.ObservedGeneration = 6 + workspace.Status.PVCName = controllers.WorkspacePVCName(workspace) + workspace.Status.PVCPhase = corev1.ClaimBound + workspace.Status.Capacity = apiresource.MustParse("10Gi") + workspace.Status.Phase = clusterv1alpha1.InfrastructureReady + workspace.Status.Conditions = []metav1.Condition{ + {Type: "HostReady", Status: metav1.ConditionTrue, Reason: "Stale", ObservedGeneration: 6}, + {Type: "StorageReady", Status: metav1.ConditionTrue, Reason: "Stale", ObservedGeneration: 6}, + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Stale", ObservedGeneration: 6}, + } + c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&clusterv1alpha1.T4Workspace{}, &corev1.PersistentVolumeClaim{}).WithObjects(test.objects(workspace)...).Build() + r := &controllers.WorkspaceReconciler{Client: c, Scheme: scheme} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}); err != nil { + t.Fatal(err) + } + var failed clusterv1alpha1.T4Workspace + if err := c.Get(ctx, client.ObjectKeyFromObject(workspace), &failed); err != nil { + t.Fatal(err) + } + if failed.Status.PVCName != "" || failed.Status.PVCPhase != "" || !failed.Status.Capacity.IsZero() { + t.Fatalf("failed Workspace retained PVC authority: %#v", failed.Status) + } + hostReady := findCondition(failed.Status.Conditions, "HostReady") + storageReady := findCondition(failed.Status.Conditions, "StorageReady") + ready := findCondition(failed.Status.Conditions, "Ready") + if hostReady == nil || hostReady.Status != test.wantHostStatus || hostReady.ObservedGeneration != failed.Generation || + storageReady == nil || storageReady.Status == metav1.ConditionTrue || storageReady.Reason != test.wantStorageReason || storageReady.ObservedGeneration != failed.Generation || + ready == nil || ready.Status != metav1.ConditionFalse || ready.ObservedGeneration != failed.Generation { + t.Fatalf("failure conditions are stale: HostReady=%#v StorageReady=%#v Ready=%#v", hostReady, storageReady, ready) + } + }) + } +} + +func TestSessionRejectsWorkspacePVCWithoutExactIdentityAndOwnership(t *testing.T) { + for _, test := range []struct { + name string + mutatePVC func(*clusterv1alpha1.T4Workspace, *corev1.PersistentVolumeClaim) + }{ + {name: "foreign deterministic PVC", mutatePVC: func(_ *clusterv1alpha1.T4Workspace, pvc *corev1.PersistentVolumeClaim) { + pvc.OwnerReferences = []metav1.OwnerReference{{APIVersion: "example.test/v1", Kind: "Foreign", Name: "foreign", UID: "foreign-uid", Controller: ptr(true)}} + }}, + {name: "tampered Workspace status name", mutatePVC: func(workspace *clusterv1alpha1.T4Workspace, pvc *corev1.PersistentVolumeClaim) { + workspace.Status.PVCName = "foreign-data" + pvc.Name = workspace.Status.PVCName + }}, + } { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + workspace.Status.PVCName = controllers.WorkspacePVCName(workspace) + pvc := ownedWorkspacePVC(workspace) + test.mutatePVC(workspace, pvc) + session := testSession() + session.UID = "session-uid" + pod, service := ownedSessionResources(session) + c := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Session{}, &corev1.PersistentVolumeClaim{}, &corev1.Pod{}). + WithObjects(testHost(), workspace, pvc, session, pod, service).Build() + r := configuredSessionReconciler(c, scheme) + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { + t.Fatal(err) + } + assertObjectCounts(t, c, 0, 0) + var failed clusterv1alpha1.T4Session + if err := c.Get(ctx, client.ObjectKeyFromObject(session), &failed); err != nil { + t.Fatal(err) + } + condition := findCondition(failed.Status.Conditions, "WorkspaceReady") + if failed.Status.PodName != "" || failed.Status.ServiceName != "" || condition == nil || condition.Status != metav1.ConditionFalse || condition.ObservedGeneration != failed.Generation { + t.Fatalf("untrusted Workspace PVC retained session authority: status=%#v WorkspaceReady=%#v", failed.Status, condition) + } + }) + } +} + +func TestSessionFailureAndPendingRefreshEveryCondition(t *testing.T) { + t.Run("failure", func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + session := testSession() + session.Generation = 8 + session.Status.ObservedGeneration = 7 + session.Status.Conditions = staleSessionConditions(7) + c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&clusterv1alpha1.T4Session{}).WithObjects(session).Build() + r := configuredSessionReconciler(c, scheme) + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { + t.Fatal(err) + } + var failed clusterv1alpha1.T4Session + if err := c.Get(ctx, client.ObjectKeyFromObject(session), &failed); err != nil { + t.Fatal(err) + } + assertCurrentSessionConditions(t, &failed, map[string]metav1.ConditionStatus{ + "HostReady": metav1.ConditionFalse, "WorkspaceReady": metav1.ConditionUnknown, "RuntimeConfigured": metav1.ConditionUnknown, "Available": metav1.ConditionFalse, + }) + }) + + t.Run("pending", func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + workspace.Status.PVCName = controllers.WorkspacePVCName(workspace) + pvc := ownedWorkspacePVC(workspace) + session := testSession() + session.UID = "session-uid" + session.Generation = 8 + session.Status.ObservedGeneration = 7 + session.Status.Conditions = staleSessionConditions(7) + _, service := ownedSessionResources(session) + service.Spec.Type = corev1.ServiceTypeNodePort + service.Spec.Ports = []corev1.ServicePort{{Name: "host", Port: 8787, NodePort: 32080}} + c := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Session{}, &corev1.PersistentVolumeClaim{}, &corev1.Pod{}). + WithObjects(testHost(), workspace, pvc, session, service).Build() + r := configuredSessionReconciler(c, scheme) + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { + t.Fatal(err) + } + var pending clusterv1alpha1.T4Session + if err := c.Get(ctx, client.ObjectKeyFromObject(session), &pending); err != nil { + t.Fatal(err) + } + assertCurrentSessionConditions(t, &pending, map[string]metav1.ConditionStatus{ + "HostReady": metav1.ConditionTrue, "WorkspaceReady": metav1.ConditionTrue, "RuntimeConfigured": metav1.ConditionTrue, "Available": metav1.ConditionFalse, + }) + }) +} + +func TestSessionResourcesWithAnyForeignOwnerFailClosed(t *testing.T) { + for _, path := range []string{"normal", "dependency-cleanup"} { + for _, kind := range []string{"Pod", "Service"} { + t.Run(path+"/"+kind, func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + session := testSession() + session.UID = "session-uid" + pod, service := ownedSessionResources(session) + foreignOwner := metav1.OwnerReference{APIVersion: "example.test/v1", Kind: "Foreign", Name: "foreign", UID: "foreign-uid"} + if kind == "Pod" { + pod.OwnerReferences = append(pod.OwnerReferences, foreignOwner) + } else { + service.OwnerReferences = append(service.OwnerReferences, foreignOwner) + } + objects := []client.Object{session, pod, service} + if path == "normal" { + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + workspace.Status.PVCName = controllers.WorkspacePVCName(workspace) + objects = append(objects, testHost(), workspace, ownedWorkspacePVC(workspace)) + } + c := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Session{}, &corev1.PersistentVolumeClaim{}, &corev1.Pod{}). + WithObjects(objects...).Build() + r := configuredSessionReconciler(c, scheme) + beforePod := pod.DeepCopy() + beforeService := service.DeepCopy() + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { + t.Fatal(err) + } + if kind == "Pod" { + var got corev1.Pod + if err := c.Get(ctx, client.ObjectKeyFromObject(pod), &got); err != nil { + t.Fatalf("foreign-owned Pod was deleted: %v", err) + } + if !reflect.DeepEqual(got.ObjectMeta, beforePod.ObjectMeta) || !reflect.DeepEqual(got.Spec, beforePod.Spec) { + t.Fatalf("Pod with foreign OwnerReference was mutated: %#v", got) + } + } else { + var got corev1.Service + if err := c.Get(ctx, client.ObjectKeyFromObject(service), &got); err != nil { + t.Fatalf("foreign-owned Service was deleted: %v", err) + } + if !reflect.DeepEqual(got.ObjectMeta, beforeService.ObjectMeta) || !reflect.DeepEqual(got.Spec, beforeService.Spec) { + t.Fatalf("Service with foreign OwnerReference was mutated: %#v", got) + } + } + var failed clusterv1alpha1.T4Session + if err := c.Get(ctx, client.ObjectKeyFromObject(session), &failed); err != nil { + t.Fatal(err) + } + available := findCondition(failed.Status.Conditions, "Available") + if available == nil || available.Status != metav1.ConditionFalse || !strings.Contains(available.Reason, "OwnershipConflict") { + t.Fatalf("foreign owner did not produce stable ownership conflict: %#v", available) + } + }) + } + } +} + +func ownedWorkspacePVC(workspace *clusterv1alpha1.T4Workspace) *corev1.PersistentVolumeClaim { + return &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: controllers.WorkspacePVCName(workspace), Namespace: workspace.Namespace, + Annotations: map[string]string{clusterv1alpha1.WorkspaceUIDAnnotation: string(workspace.UID)}, + OwnerReferences: []metav1.OwnerReference{{APIVersion: clusterv1alpha1.GroupVersion.String(), Kind: "T4Workspace", Name: workspace.Name, UID: workspace.UID, Controller: ptr(true)}}, + }, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, + } +} + +func ownedSessionResources(session *clusterv1alpha1.T4Session) (*corev1.Pod, *corev1.Service) { + owner := metav1.OwnerReference{APIVersion: clusterv1alpha1.GroupVersion.String(), Kind: "T4Session", Name: session.Name, UID: session.UID, Controller: ptr(true)} + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionPodName(session), Namespace: session.Namespace, OwnerReferences: []metav1.OwnerReference{owner}}} + service := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionServiceName(session), Namespace: session.Namespace, OwnerReferences: []metav1.OwnerReference{owner}}} + return pod, service +} + +func staleSessionConditions(generation int64) []metav1.Condition { + return []metav1.Condition{ + {Type: "HostReady", Status: metav1.ConditionTrue, Reason: "Stale", ObservedGeneration: generation}, + {Type: "WorkspaceReady", Status: metav1.ConditionTrue, Reason: "Stale", ObservedGeneration: generation}, + {Type: "RuntimeConfigured", Status: metav1.ConditionTrue, Reason: "Stale", ObservedGeneration: generation}, + {Type: "Available", Status: metav1.ConditionTrue, Reason: "Stale", ObservedGeneration: generation}, + } +} + +func assertCurrentSessionConditions(t *testing.T, session *clusterv1alpha1.T4Session, want map[string]metav1.ConditionStatus) { + t.Helper() + for conditionType, status := range want { + condition := findCondition(session.Status.Conditions, conditionType) + if condition == nil || condition.Status != status || condition.ObservedGeneration != session.Generation { + t.Fatalf("%s = %#v, want %s at generation %d", conditionType, condition, status, session.Generation) + } + } +} + func configuredSessionReconciler(c client.Client, scheme *runtime.Scheme) *controllers.SessionReconciler { for _, object := range []client.Object{ &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "omp-runtime-config", Namespace: "team"}, Data: map[string]string{ From 95da911ad8f262814c515292d147d05360165b50 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 21:47:16 +0000 Subject: [PATCH 07/29] fix(operator): revoke untrusted dependency authority --- .../controllers/reconciler_test.go | 2 +- .../controllers/session_controller.go | 106 +++++++++++++----- .../controllers/workspace_controller.go | 22 +++- 3 files changed, 95 insertions(+), 35 deletions(-) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 3f51cba1..564e4211 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -1829,7 +1829,7 @@ func ownedWorkspacePVC(workspace *clusterv1alpha1.T4Workspace) *corev1.Persisten func ownedSessionResources(session *clusterv1alpha1.T4Session) (*corev1.Pod, *corev1.Service) { owner := metav1.OwnerReference{APIVersion: clusterv1alpha1.GroupVersion.String(), Kind: "T4Session", Name: session.Name, UID: session.UID, Controller: ptr(true)} pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionPodName(session), Namespace: session.Namespace, OwnerReferences: []metav1.OwnerReference{owner}}} - service := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionServiceName(session), Namespace: session.Namespace, OwnerReferences: []metav1.OwnerReference{owner}}} + service := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionServiceName(session), Namespace: session.Namespace, OwnerReferences: []metav1.OwnerReference{owner}}, Spec: corev1.ServiceSpec{Type: corev1.ServiceTypeClusterIP}} return pod, service } diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index 9b4318b8..df4c492c 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -4,6 +4,7 @@ import ( "context" "crypto/sha256" "encoding/json" + "errors" "fmt" "net/url" "reflect" @@ -42,6 +43,8 @@ const ( sessionWorkspaceRefIndexField = "t4.session.spec.workspaceRef" ) +var errSessionResourceOwnershipConflict = errors.New("session resource ownership conflict") + var ( configMapKeyPattern = regexp.MustCompile(`^[-._A-Za-z0-9]+$`) runtimeImagePattern = regexp.MustCompile(`^(?:(?:[A-Za-z0-9](?:[A-Za-z0-9.-]*[A-Za-z0-9])?|\[[A-Fa-f0-9:]+\])(?::[0-9]+)?/)?[a-z0-9]+(?:(?:[._]|__|-+)[a-z0-9]+)*(?:/[a-z0-9]+(?:(?:[._]|__|-+)[a-z0-9]+)*)*@sha256:[a-f0-9]{64}$`) @@ -289,6 +292,10 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) var session clusterv1alpha1.T4Session found := false defer func() { + if errors.Is(err, errSessionResourceOwnershipConflict) { + result = ctrl.Result{RequeueAfter: 30 * time.Second} + err = nil + } observeReconcile(metricKindSession, request.NamespacedName, session.Status.Conditions, conditionObjectPresent(&session, found, err), err) }() if err := r.Get(ctx, request.NamespacedName, &session); err != nil { @@ -307,13 +314,13 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, false, "RuntimeConfigured", reason, message) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, false, false, "RuntimeConfigured", reason, message) } if reason, message := r.OMPConfig.validationFailure(); reason != "" { if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, false, "RuntimeConfigured", reason, message) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, false, false, "RuntimeConfigured", reason, message) } var host clusterv1alpha1.T4ClusterHost @@ -322,7 +329,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, false, "HostReady", "HostNotFound", "referenced T4ClusterHost does not exist") + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, false, false, "HostReady", "HostNotFound", "referenced T4ClusterHost does not exist") } return ctrl.Result{}, err } @@ -330,7 +337,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "RuntimeConfigured", "RuntimeProfileNotAllowed", "runtime profile is not allowed by the referenced T4ClusterHost") + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "RuntimeConfigured", "RuntimeProfileNotAllowed", "runtime profile is not allowed by the referenced T4ClusterHost") } var storageClass storagev1.StorageClass if err := r.Get(ctx, types.NamespacedName{Name: host.Spec.StorageClassName}, &storageClass); err != nil { @@ -338,7 +345,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", ReasonStorageClassNotFound, fmt.Sprintf("StorageClass %q selected by the referenced T4ClusterHost does not exist", host.Spec.StorageClassName)) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", ReasonStorageClassNotFound, fmt.Sprintf("StorageClass %q selected by the referenced T4ClusterHost does not exist", host.Spec.StorageClassName)) } return ctrl.Result{}, err } @@ -346,7 +353,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", ReasonStorageClassNotRWX, fmt.Sprintf("StorageClass %q selected by the referenced T4ClusterHost is not administrator-declared ReadWriteMany", host.Spec.StorageClassName)) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", ReasonStorageClassNotRWX, fmt.Sprintf("StorageClass %q selected by the referenced T4ClusterHost is not administrator-declared ReadWriteMany", host.Spec.StorageClassName)) } var workspace clusterv1alpha1.T4Workspace if err := r.Get(ctx, types.NamespacedName{Namespace: session.Namespace, Name: session.Spec.WorkspaceRef}, &workspace); err != nil { @@ -354,7 +361,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", "WorkspaceNotFound", "referenced T4Workspace does not exist") + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", "WorkspaceNotFound", "referenced T4Workspace does not exist") } return ctrl.Result{}, err } @@ -362,13 +369,19 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", "HostMismatch", "session and workspace must reference the same T4ClusterHost") + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", "HostMismatch", "session and workspace must reference the same T4ClusterHost") } if workspace.Status.PVCName == "" { if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", "PVCNotDeclared", "workspace controller has not declared a PVC") + return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", "PVCNotDeclared", "workspace controller has not declared a PVC") + } + if workspace.UID != "" && workspace.Status.PVCName != WorkspacePVCName(&workspace) { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", "PVCIdentityMismatch", "workspace status does not reference its deterministic PVC") } var pvc corev1.PersistentVolumeClaim if err := r.Get(ctx, types.NamespacedName{Namespace: session.Namespace, Name: workspace.Status.PVCName}, &pvc); err != nil { @@ -376,21 +389,27 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", "PVCNotFound", "workspace PVC does not exist") + return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", "PVCNotFound", "workspace PVC does not exist") } return ctrl.Result{}, err } + if workspace.UID != "" && !workspaceOwnsPVC(&workspace, &pvc) { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", "PVCOwnershipConflict", "workspace PVC identity or ownership is not authoritative") + } if pvcStorageClassName(&pvc) != host.Spec.StorageClassName { if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", ReasonStorageClassMismatch, fmt.Sprintf("workspace PVC uses StorageClass %q instead of host-selected %q", pvcStorageClassName(&pvc), host.Spec.StorageClassName)) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", ReasonStorageClassMismatch, fmt.Sprintf("workspace PVC uses StorageClass %q instead of host-selected %q", pvcStorageClassName(&pvc), host.Spec.StorageClassName)) } if pvc.Status.Phase != corev1.ClaimBound || !pvcHasRWX(&pvc) { if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, true, "WorkspaceReady", "PVCNotBoundRWX", "workspace PVC must be Bound and ReadWriteMany before a session starts") + return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", "PVCNotBoundRWX", "workspace PVC must be Bound and ReadWriteMany before a session starts") } runtimeVersions, reason, message, err := r.loadOMPResourceVersions(ctx, session.Namespace) if err != nil { @@ -400,7 +419,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "RuntimeConfigured", reason, message) + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, true, "RuntimeConfigured", reason, message) } serviceName := SessionServiceName(&session) @@ -429,8 +448,8 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } } else if err != nil { return ctrl.Result{}, err - } else if !metav1.IsControlledBy(&service, &session) { - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "Available", "ServiceOwnershipConflict", "deterministic session Service is not controlled by this session") + } else if !sessionExclusivelyOwnsResource(&service, &session) { + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, true, "Available", "ServiceOwnershipConflict", "deterministic session Service has an unexpected owner") } else if !serviceExposureIsInternal(&service) { if err := r.Delete(ctx, &service); err != nil && !apierrors.IsNotFound(err) { return ctrl.Result{}, err @@ -463,8 +482,8 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } } else if err != nil { return ctrl.Result{}, err - } else if !metav1.IsControlledBy(&pod, &session) { - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, "Available", "PodOwnershipConflict", "deterministic session Pod is not controlled by this session") + } else if !sessionExclusivelyOwnsResource(&pod, &session) { + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, true, "Available", "PodOwnershipConflict", "deterministic session Pod has an unexpected owner") } else if !labelsContain(pod.Labels, desiredPod.Labels) { if pod.Labels == nil { pod.Labels = map[string]string{} @@ -654,6 +673,8 @@ func (r *SessionReconciler) updateSessionPending(ctx context.Context, session *c } session.Status.ObservedGeneration = session.Generation meta.SetStatusCondition(&session.Status.Conditions, condition("HostReady", metav1.ConditionTrue, "HostResolved", "referenced T4ClusterHost is available", session.Generation)) + meta.SetStatusCondition(&session.Status.Conditions, condition("WorkspaceReady", metav1.ConditionTrue, "PVCBoundRWX", "workspace PVC is Bound and ReadWriteMany", session.Generation)) + meta.SetStatusCondition(&session.Status.Conditions, condition("RuntimeConfigured", metav1.ConditionTrue, "OMPReferencesReady", "administrator-owned OMP runtime references are configured", session.Generation)) session.Status.PodName = podName session.Status.ServiceName = serviceName session.Status.Phase = clusterv1alpha1.InfrastructurePending @@ -693,7 +714,7 @@ func (r *SessionReconciler) reconcileDelete(ctx context.Context, session *cluste if err != nil { return ctrl.Result{}, err } - if !metav1.IsControlledBy(object, session) { + if !sessionExclusivelyOwnsResource(object, session) { before := session.Status if session.Status.Conditions != nil { before.Conditions = append([]metav1.Condition(nil), session.Status.Conditions...) @@ -734,8 +755,11 @@ func (r *SessionReconciler) deleteOwnedSessionResources(ctx context.Context, ses } continue } - if !metav1.IsControlledBy(object, session) { - continue + if !sessionExclusivelyOwnsResource(object, session) { + if err := r.updateSessionFailure(ctx, session, false, false, "Available", "ResourceOwnershipConflict", fmt.Sprintf("deterministic %T has an unexpected owner", object)); err != nil { + return err + } + return errSessionResourceOwnershipConflict } if err := r.Delete(ctx, object); err != nil && !apierrors.IsNotFound(err) { return err @@ -744,30 +768,56 @@ func (r *SessionReconciler) deleteOwnedSessionResources(ctx context.Context, ses return nil } -func (r *SessionReconciler) updateSessionFailure(ctx context.Context, session *clusterv1alpha1.T4Session, hostReady bool, conditionType, reason, message string) error { +func (r *SessionReconciler) updateSessionFailure(ctx context.Context, session *clusterv1alpha1.T4Session, hostReady, workspaceReady bool, conditionType, reason, message string) error { original := session.Status if session.Status.Conditions != nil { original.Conditions = append([]metav1.Condition(nil), session.Status.Conditions...) } - if hostReady { + if conditionType == "HostReady" { + meta.SetStatusCondition(&session.Status.Conditions, condition("HostReady", metav1.ConditionFalse, reason, message, session.Generation)) + } else if hostReady { meta.SetStatusCondition(&session.Status.Conditions, condition("HostReady", metav1.ConditionTrue, "HostResolved", "referenced T4ClusterHost is available", session.Generation)) - } else if conditionType != "HostReady" { - meta.SetStatusCondition(&session.Status.Conditions, condition("HostReady", metav1.ConditionUnknown, "NotEvaluated", "host dependency was not evaluated because static runtime configuration is invalid", session.Generation)) + } else { + meta.SetStatusCondition(&session.Status.Conditions, condition("HostReady", metav1.ConditionUnknown, "NotEvaluated", "host dependency was not evaluated", session.Generation)) + } + if conditionType == "WorkspaceReady" { + meta.SetStatusCondition(&session.Status.Conditions, condition("WorkspaceReady", metav1.ConditionFalse, reason, message, session.Generation)) + } else if workspaceReady { + meta.SetStatusCondition(&session.Status.Conditions, condition("WorkspaceReady", metav1.ConditionTrue, "PVCBoundRWX", "workspace PVC is Bound and ReadWriteMany", session.Generation)) + } else { + meta.SetStatusCondition(&session.Status.Conditions, condition("WorkspaceReady", metav1.ConditionUnknown, "NotEvaluated", "workspace dependency was not evaluated", session.Generation)) + } + if conditionType == "RuntimeConfigured" { + meta.SetStatusCondition(&session.Status.Conditions, condition("RuntimeConfigured", metav1.ConditionFalse, reason, message, session.Generation)) + } else if workspaceReady { + meta.SetStatusCondition(&session.Status.Conditions, condition("RuntimeConfigured", metav1.ConditionTrue, "OMPReferencesReady", "administrator-owned OMP runtime references are configured", session.Generation)) + } else { + meta.SetStatusCondition(&session.Status.Conditions, condition("RuntimeConfigured", metav1.ConditionUnknown, "NotEvaluated", "runtime configuration was not evaluated", session.Generation)) } session.Status.ObservedGeneration = session.Generation session.Status.PodName = "" session.Status.ServiceName = "" session.Status.Phase = clusterv1alpha1.InfrastructureFailed - meta.SetStatusCondition(&session.Status.Conditions, condition(conditionType, metav1.ConditionFalse, reason, message, session.Generation)) - if conditionType != "Available" { - meta.SetStatusCondition(&session.Status.Conditions, condition("Available", metav1.ConditionFalse, reason, message, session.Generation)) - } + meta.SetStatusCondition(&session.Status.Conditions, condition("Available", metav1.ConditionFalse, reason, message, session.Generation)) if reflect.DeepEqual(original, session.Status) { return nil } return r.Status().Update(ctx, session) } +func sessionExclusivelyOwnsResource(object metav1.Object, session *clusterv1alpha1.T4Session) bool { + controller := metav1.GetControllerOf(object) + if controller == nil || controller.APIVersion != clusterv1alpha1.GroupVersion.String() || controller.Kind != "T4Session" || controller.Name != session.Name || controller.UID != session.UID { + return false + } + for _, reference := range object.GetOwnerReferences() { + if reference.APIVersion != clusterv1alpha1.GroupVersion.String() || reference.Kind != "T4Session" || reference.Name != session.Name || reference.UID != session.UID { + return false + } + } + return true +} + func serviceExposureIsInternal(service *corev1.Service) bool { if service.Spec.Type != corev1.ServiceTypeClusterIP || service.Spec.ClusterIP == corev1.ClusterIPNone || service.Spec.ExternalName != "" || len(service.Spec.ExternalIPs) != 0 || service.Spec.LoadBalancerIP != "" || len(service.Spec.LoadBalancerSourceRanges) != 0 || diff --git a/packages/cluster-operator/controllers/workspace_controller.go b/packages/cluster-operator/controllers/workspace_controller.go index da1b555b..65e47608 100644 --- a/packages/cluster-operator/controllers/workspace_controller.go +++ b/packages/cluster-operator/controllers/workspace_controller.go @@ -10,6 +10,7 @@ import ( storagev1 "k8s.io/api/storage/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" meta "k8s.io/apimachinery/pkg/api/meta" + apiresource "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -161,6 +162,11 @@ func workspaceOwnsPVC(workspace *clusterv1alpha1.T4Workspace, pvc *corev1.Persis if pvc.Annotations[clusterv1alpha1.WorkspaceUIDAnnotation] != string(workspace.UID) { return false } + for _, reference := range pvc.OwnerReferences { + if reference.APIVersion != clusterv1alpha1.GroupVersion.String() || reference.Kind != "T4Workspace" || reference.Name != workspace.Name || reference.UID != workspace.UID { + return false + } + } controller := metav1.GetControllerOf(pvc) if workspace.Spec.RetentionPolicy == clusterv1alpha1.RetentionPolicyDelete { return controller != nil && controller.UID == workspace.UID @@ -270,15 +276,19 @@ func (r *WorkspaceReconciler) updateWorkspaceFailure(ctx context.Context, worksp if workspace.Status.Conditions != nil { original.Conditions = append([]metav1.Condition(nil), workspace.Status.Conditions...) } - if conditionType != "HostReady" { - meta.SetStatusCondition(&workspace.Status.Conditions, condition("HostReady", metav1.ConditionTrue, "HostResolved", "referenced T4ClusterHost is available", workspace.Generation)) - } workspace.Status.ObservedGeneration = workspace.Generation + workspace.Status.PVCName = "" + workspace.Status.PVCPhase = "" + workspace.Status.Capacity = apiresource.Quantity{} workspace.Status.Phase = clusterv1alpha1.InfrastructureFailed - meta.SetStatusCondition(&workspace.Status.Conditions, condition(conditionType, metav1.ConditionFalse, reason, message, workspace.Generation)) - if conditionType != "Ready" { - meta.SetStatusCondition(&workspace.Status.Conditions, condition("Ready", metav1.ConditionFalse, reason, message, workspace.Generation)) + if conditionType == "HostReady" { + meta.SetStatusCondition(&workspace.Status.Conditions, condition("HostReady", metav1.ConditionFalse, reason, message, workspace.Generation)) + meta.SetStatusCondition(&workspace.Status.Conditions, condition("StorageReady", metav1.ConditionUnknown, "NotEvaluated", "storage dependency was not evaluated because the referenced host is unavailable", workspace.Generation)) + } else { + meta.SetStatusCondition(&workspace.Status.Conditions, condition("HostReady", metav1.ConditionTrue, "HostResolved", "referenced T4ClusterHost is available", workspace.Generation)) + meta.SetStatusCondition(&workspace.Status.Conditions, condition("StorageReady", metav1.ConditionFalse, reason, message, workspace.Generation)) } + meta.SetStatusCondition(&workspace.Status.Conditions, condition("Ready", metav1.ConditionFalse, reason, message, workspace.Generation)) if reflect.DeepEqual(original, workspace.Status) { return nil } From d954667c18f6600baf813909adf656b44892f2fb Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 21:58:23 +0000 Subject: [PATCH 08/29] test(operator): revoke terminal storage authority --- .../controllers/reconciler_test.go | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 564e4211..8d48f6da 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -1801,6 +1801,17 @@ func TestSessionResourcesWithAnyForeignOwnerFailClosed(t *testing.T) { t.Fatalf("Service with foreign OwnerReference was mutated: %#v", got) } } + if path == "dependency-cleanup" { + var sibling client.Object + if kind == "Pod" { + sibling = &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionServiceName(session), Namespace: session.Namespace}} + } else { + sibling = &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionPodName(session), Namespace: session.Namespace}} + } + if err := c.Get(ctx, client.ObjectKeyFromObject(sibling), sibling); !apierrors.IsNotFound(err) { + t.Fatalf("exclusively owned sibling remained after dependency cleanup: %v", err) + } + } var failed clusterv1alpha1.T4Session if err := c.Get(ctx, client.ObjectKeyFromObject(session), &failed); err != nil { t.Fatal(err) @@ -1814,6 +1825,45 @@ func TestSessionResourcesWithAnyForeignOwnerFailClosed(t *testing.T) { } } +func TestWorkspaceTerminalPVCFailuresRevokePublishedAuthority(t *testing.T) { + for _, test := range []struct { + name string + mutate func(*corev1.PersistentVolumeClaim) + reason string + }{ + {name: "Bound without RWX", reason: "PVCNotRWX", mutate: func(pvc *corev1.PersistentVolumeClaim) { pvc.Spec.AccessModes = []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce} }}, + {name: "Lost", reason: "PVCLost", mutate: func(pvc *corev1.PersistentVolumeClaim) { pvc.Status.Phase = corev1.ClaimLost }}, + } { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + pvc := ownedWorkspacePVC(workspace) + test.mutate(pvc) + c := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Workspace{}, &corev1.PersistentVolumeClaim{}). + WithObjects(testHost(), rwxStorageClass(), workspace, pvc).Build() + r := &controllers.WorkspaceReconciler{Client: c, Scheme: scheme} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}); err != nil { + t.Fatal(err) + } + var failed clusterv1alpha1.T4Workspace + if err := c.Get(ctx, client.ObjectKeyFromObject(workspace), &failed); err != nil { + t.Fatal(err) + } + storageReady := findCondition(failed.Status.Conditions, "StorageReady") + ready := findCondition(failed.Status.Conditions, "Ready") + if failed.Status.PVCName != "" || failed.Status.PVCPhase != "" || !failed.Status.Capacity.IsZero() || + storageReady == nil || storageReady.Status != metav1.ConditionFalse || storageReady.Reason != test.reason || storageReady.ObservedGeneration != failed.Generation || + ready == nil || ready.Status != metav1.ConditionFalse || ready.Reason != test.reason || ready.ObservedGeneration != failed.Generation { + t.Fatalf("terminal PVC failure retained authority: status=%#v StorageReady=%#v Ready=%#v", failed.Status, storageReady, ready) + } + }) + } +} + + func ownedWorkspacePVC(workspace *clusterv1alpha1.T4Workspace) *corev1.PersistentVolumeClaim { return &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ From 187baf2649105f3bd29cf4df37468bbe894b8674 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 22:01:56 +0000 Subject: [PATCH 09/29] fix(operator): clear terminal storage authority --- .../controllers/session_controller.go | 13 +++++++++---- .../controllers/workspace_controller.go | 18 ++++++++---------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index df4c492c..7a837e43 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -748,6 +748,7 @@ func (r *SessionReconciler) deleteOwnedSessionResources(ctx context.Context, ses &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: SessionPodName(session), Namespace: session.Namespace}}, &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: SessionServiceName(session), Namespace: session.Namespace}}, } + ownershipConflict := false for _, object := range objects { if err := r.Get(ctx, client.ObjectKeyFromObject(object), object); err != nil { if err := client.IgnoreNotFound(err); err != nil { @@ -756,15 +757,19 @@ func (r *SessionReconciler) deleteOwnedSessionResources(ctx context.Context, ses continue } if !sessionExclusivelyOwnsResource(object, session) { - if err := r.updateSessionFailure(ctx, session, false, false, "Available", "ResourceOwnershipConflict", fmt.Sprintf("deterministic %T has an unexpected owner", object)); err != nil { - return err - } - return errSessionResourceOwnershipConflict + ownershipConflict = true + continue } if err := r.Delete(ctx, object); err != nil && !apierrors.IsNotFound(err) { return err } } + if ownershipConflict { + if err := r.updateSessionFailure(ctx, session, false, false, "Available", "ResourceOwnershipConflict", "one or more deterministic session resources have an unexpected owner"); err != nil { + return err + } + return errSessionResourceOwnershipConflict + } return nil } diff --git a/packages/cluster-operator/controllers/workspace_controller.go b/packages/cluster-operator/controllers/workspace_controller.go index 65e47608..d32920f8 100644 --- a/packages/cluster-operator/controllers/workspace_controller.go +++ b/packages/cluster-operator/controllers/workspace_controller.go @@ -118,6 +118,12 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, request ctrl.Reques return ctrl.Result{Requeue: true}, nil } } + if pvc.Status.Phase == corev1.ClaimBound && !pvcHasRWX(&pvc) { + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCNotRWX", "bound workspace PVC does not request ReadWriteMany") + } + if pvc.Status.Phase == corev1.ClaimLost { + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCLost", "workspace PVC lost its volume") + } original := workspace.Status original.Capacity = workspace.Status.Capacity.DeepCopy() @@ -133,16 +139,8 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, request ctrl.Reques meta.SetStatusCondition(&workspace.Status.Conditions, condition("StorageReady", metav1.ConditionTrue, ReasonStorageReady, "RWX StorageClass and workspace PVC are accepted", workspace.Generation)) switch pvc.Status.Phase { case corev1.ClaimBound: - if !pvcHasRWX(&pvc) { - workspace.Status.Phase = clusterv1alpha1.InfrastructureFailed - meta.SetStatusCondition(&workspace.Status.Conditions, condition("Ready", metav1.ConditionFalse, "PVCNotRWX", "bound workspace PVC does not request ReadWriteMany", workspace.Generation)) - } else { - workspace.Status.Phase = clusterv1alpha1.InfrastructureReady - meta.SetStatusCondition(&workspace.Status.Conditions, condition("Ready", metav1.ConditionTrue, "PVCBound", "workspace PVC is bound with ReadWriteMany access", workspace.Generation)) - } - case corev1.ClaimLost: - workspace.Status.Phase = clusterv1alpha1.InfrastructureFailed - meta.SetStatusCondition(&workspace.Status.Conditions, condition("Ready", metav1.ConditionFalse, "PVCLost", "workspace PVC lost its volume", workspace.Generation)) + workspace.Status.Phase = clusterv1alpha1.InfrastructureReady + meta.SetStatusCondition(&workspace.Status.Conditions, condition("Ready", metav1.ConditionTrue, "PVCBound", "workspace PVC is bound with ReadWriteMany access", workspace.Generation)) default: workspace.Status.Phase = clusterv1alpha1.InfrastructurePending meta.SetStatusCondition(&workspace.Status.Conditions, condition("Ready", metav1.ConditionFalse, "PVCBinding", "workspace PVC is waiting to bind", workspace.Generation)) From 75762dafcaf7725fad155ba4898c49c58e788872 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 22:10:25 +0000 Subject: [PATCH 10/29] test(operator): stop owned siblings on conflict --- .../controllers/reconciler_test.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 8d48f6da..a1533f3f 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -1801,16 +1801,14 @@ func TestSessionResourcesWithAnyForeignOwnerFailClosed(t *testing.T) { t.Fatalf("Service with foreign OwnerReference was mutated: %#v", got) } } - if path == "dependency-cleanup" { - var sibling client.Object - if kind == "Pod" { - sibling = &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionServiceName(session), Namespace: session.Namespace}} - } else { - sibling = &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionPodName(session), Namespace: session.Namespace}} - } - if err := c.Get(ctx, client.ObjectKeyFromObject(sibling), sibling); !apierrors.IsNotFound(err) { - t.Fatalf("exclusively owned sibling remained after dependency cleanup: %v", err) - } + var sibling client.Object + if kind == "Pod" { + sibling = &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionServiceName(session), Namespace: session.Namespace}} + } else { + sibling = &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: controllers.SessionPodName(session), Namespace: session.Namespace}} + } + if err := c.Get(ctx, client.ObjectKeyFromObject(sibling), sibling); !apierrors.IsNotFound(err) { + t.Fatalf("exclusively owned sibling remained after ownership conflict: %v", err) } var failed clusterv1alpha1.T4Session if err := c.Get(ctx, client.ObjectKeyFromObject(session), &failed); err != nil { From 55539e5f10c21f3dcc14ea84340ee18fabff63a0 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 22:14:32 +0000 Subject: [PATCH 11/29] fix(operator): stop owned siblings on conflict --- .../cluster-operator/controllers/session_controller.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index 7a837e43..c7834cea 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -449,7 +449,10 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } else if err != nil { return ctrl.Result{}, err } else if !sessionExclusivelyOwnsResource(&service, &session) { - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, true, "Available", "ServiceOwnershipConflict", "deterministic session Service has an unexpected owner") + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } else if !serviceExposureIsInternal(&service) { if err := r.Delete(ctx, &service); err != nil && !apierrors.IsNotFound(err) { return ctrl.Result{}, err @@ -483,7 +486,10 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } else if err != nil { return ctrl.Result{}, err } else if !sessionExclusivelyOwnsResource(&pod, &session) { - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, true, "Available", "PodOwnershipConflict", "deterministic session Pod has an unexpected owner") + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } else if !labelsContain(pod.Labels, desiredPod.Labels) { if pod.Labels == nil { pod.Labels = map[string]string{} From 764f46094951d40d84f8389ad83070609fe4d5e7 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 22:26:47 +0000 Subject: [PATCH 12/29] test(operator): preserve dependency status on conflicts --- packages/cluster-operator/controllers/reconciler_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index a1533f3f..b92fd46b 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -1818,6 +1818,11 @@ func TestSessionResourcesWithAnyForeignOwnerFailClosed(t *testing.T) { if available == nil || available.Status != metav1.ConditionFalse || !strings.Contains(available.Reason, "OwnershipConflict") { t.Fatalf("foreign owner did not produce stable ownership conflict: %#v", available) } + if path == "normal" { + assertCurrentSessionConditions(t, &failed, map[string]metav1.ConditionStatus{ + "HostReady": metav1.ConditionTrue, "WorkspaceReady": metav1.ConditionTrue, "RuntimeConfigured": metav1.ConditionTrue, "Available": metav1.ConditionFalse, + }) + } }) } } From e24b6dcefc8ba92be906a4a26a47545b52c26fab Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 22:34:18 +0000 Subject: [PATCH 13/29] fix(operator): preserve ready dependencies on conflicts --- .../controllers/session_controller.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index c7834cea..1d52a20b 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -449,7 +449,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } else if err != nil { return ctrl.Result{}, err } else if !sessionExclusivelyOwnsResource(&service, &session) { - if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + if err := r.deleteOwnedSessionResourcesAfterVerifiedDependencies(ctx, &session, "ServiceOwnershipConflict", "deterministic session Service has an unexpected owner"); err != nil { return ctrl.Result{}, err } return ctrl.Result{RequeueAfter: 30 * time.Second}, nil @@ -486,7 +486,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } else if err != nil { return ctrl.Result{}, err } else if !sessionExclusivelyOwnsResource(&pod, &session) { - if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + if err := r.deleteOwnedSessionResourcesAfterVerifiedDependencies(ctx, &session, "PodOwnershipConflict", "deterministic session Pod has an unexpected owner"); err != nil { return ctrl.Result{}, err } return ctrl.Result{RequeueAfter: 30 * time.Second}, nil @@ -750,6 +750,14 @@ func (r *SessionReconciler) reconcileDelete(ctx context.Context, session *cluste } func (r *SessionReconciler) deleteOwnedSessionResources(ctx context.Context, session *clusterv1alpha1.T4Session) error { + return r.deleteOwnedSessionResourcesWithFailure(ctx, session, false, false, "ResourceOwnershipConflict", "one or more deterministic session resources have an unexpected owner") +} + +func (r *SessionReconciler) deleteOwnedSessionResourcesAfterVerifiedDependencies(ctx context.Context, session *clusterv1alpha1.T4Session, reason, message string) error { + return r.deleteOwnedSessionResourcesWithFailure(ctx, session, true, true, reason, message) +} + +func (r *SessionReconciler) deleteOwnedSessionResourcesWithFailure(ctx context.Context, session *clusterv1alpha1.T4Session, hostReady, workspaceReady bool, reason, message string) error { objects := []client.Object{ &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: SessionPodName(session), Namespace: session.Namespace}}, &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: SessionServiceName(session), Namespace: session.Namespace}}, @@ -771,7 +779,7 @@ func (r *SessionReconciler) deleteOwnedSessionResources(ctx context.Context, ses } } if ownershipConflict { - if err := r.updateSessionFailure(ctx, session, false, false, "Available", "ResourceOwnershipConflict", "one or more deterministic session resources have an unexpected owner"); err != nil { + if err := r.updateSessionFailure(ctx, session, hostReady, workspaceReady, "Available", reason, message); err != nil { return err } return errSessionResourceOwnershipConflict From 96ccd96fb89c6c85076e02476c213c85bc4b89ff Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 22:41:51 +0000 Subject: [PATCH 14/29] test(operator): cover cleanup and create races --- .../controllers/reconciler_test.go | 119 +++++++++++++++++- 1 file changed, 118 insertions(+), 1 deletion(-) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index b92fd46b..2b0fe1a4 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -13,6 +13,7 @@ import ( apiresource "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" utilvalidation "k8s.io/apimachinery/pkg/util/validation" ctrl "sigs.k8s.io/controller-runtime" @@ -1212,6 +1213,8 @@ func TestSessionDeletionRefusesForeignDeterministicResources(t *testing.T) { } else { service.OwnerReferences = nil } + beforePod := pod.DeepCopy() + beforeService := service.DeepCopy() c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&clusterv1alpha1.T4Session{}).WithObjects(session, pod, service).Build() if err := c.Delete(context.Background(), session); err != nil { t.Fatal(err) @@ -1220,7 +1223,28 @@ func TestSessionDeletionRefusesForeignDeterministicResources(t *testing.T) { if _, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { t.Fatal(err) } - assertObjectCounts(t, c, 1, 1) + if foreignKind == "Pod" { + assertObjectCounts(t, c, 1, 0) + } else { + assertObjectCounts(t, c, 0, 1) + } + if foreignKind == "Pod" { + var got corev1.Pod + if err := c.Get(context.Background(), client.ObjectKeyFromObject(pod), &got); err != nil { + t.Fatalf("foreign Pod was deleted: %v", err) + } + if !reflect.DeepEqual(got.ObjectMeta, beforePod.ObjectMeta) || !reflect.DeepEqual(got.Spec, beforePod.Spec) { + t.Fatalf("foreign Pod was mutated during finalizer cleanup: %#v", got) + } + } else { + var got corev1.Service + if err := c.Get(context.Background(), client.ObjectKeyFromObject(service), &got); err != nil { + t.Fatalf("foreign Service was deleted: %v", err) + } + if !reflect.DeepEqual(got.ObjectMeta, beforeService.ObjectMeta) || !reflect.DeepEqual(got.Spec, beforeService.Spec) { + t.Fatalf("foreign Service was mutated during finalizer cleanup: %#v", got) + } + } var waiting clusterv1alpha1.T4Session if err := c.Get(context.Background(), client.ObjectKeyFromObject(session), &waiting); err != nil { t.Fatalf("session finalizer was removed on cleanup conflict: %v", err) @@ -1828,6 +1852,70 @@ func TestSessionResourcesWithAnyForeignOwnerFailClosed(t *testing.T) { } } +func TestSessionCreateAlreadyExistsRefetchesForeignWinner(t *testing.T) { + for _, kind := range []string{"Pod", "Service"} { + t.Run(kind, func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + workspace.Status.PVCName = controllers.WorkspacePVCName(workspace) + session := testSession() + session.UID = "session-uid" + pod, service := ownedSessionResources(session) + objects := []client.Object{testHost(), workspace, ownedWorkspacePVC(workspace), session} + if kind == "Pod" { + objects = append(objects, service) + } else { + objects = append(objects, pod) + } + base := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Session{}, &corev1.PersistentVolumeClaim{}, &corev1.Pod{}). + WithObjects(objects...).Build() + c := &createAlreadyExistsClient{Client: base, raceKind: kind} + r := configuredSessionReconciler(c, scheme) + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { + t.Fatal(err) + } + if c.winner == nil { + t.Fatalf("%s create race was not exercised", kind) + } + if kind == "Pod" { + var got corev1.Pod + if err := base.Get(ctx, client.ObjectKeyFromObject(c.winner), &got); err != nil { + t.Fatalf("foreign Pod winner was deleted: %v", err) + } + want := c.winner.(*corev1.Pod) + if !reflect.DeepEqual(got.ObjectMeta, want.ObjectMeta) || !reflect.DeepEqual(got.Spec, want.Spec) { + t.Fatalf("foreign Pod winner was mutated: %#v", got) + } + assertObjectCounts(t, base, 1, 0) + } else { + var got corev1.Service + if err := base.Get(ctx, client.ObjectKeyFromObject(c.winner), &got); err != nil { + t.Fatalf("foreign Service winner was deleted: %v", err) + } + want := c.winner.(*corev1.Service) + if !reflect.DeepEqual(got.ObjectMeta, want.ObjectMeta) || !reflect.DeepEqual(got.Spec, want.Spec) { + t.Fatalf("foreign Service winner was mutated: %#v", got) + } + assertObjectCounts(t, base, 0, 1) + } + var failed clusterv1alpha1.T4Session + if err := base.Get(ctx, client.ObjectKeyFromObject(session), &failed); err != nil { + t.Fatal(err) + } + assertCurrentSessionConditions(t, &failed, map[string]metav1.ConditionStatus{ + "HostReady": metav1.ConditionTrue, "WorkspaceReady": metav1.ConditionTrue, "RuntimeConfigured": metav1.ConditionTrue, "Available": metav1.ConditionFalse, + }) + available := findCondition(failed.Status.Conditions, "Available") + if available.Reason != kind+"OwnershipConflict" { + t.Fatalf("Available reason = %q, want %sOwnershipConflict", available.Reason, kind) + } + }) + } +} + func TestWorkspaceTerminalPVCFailuresRevokePublishedAuthority(t *testing.T) { for _, test := range []struct { name string @@ -2034,4 +2122,33 @@ func hasReadOnlyMount(mounts []corev1.VolumeMount, name, path string) bool { return false } +type createAlreadyExistsClient struct { + client.Client + raceKind string + winner client.Object +} + +func (c *createAlreadyExistsClient) Create(ctx context.Context, object client.Object, options ...client.CreateOption) error { + var winner client.Object + switch object := object.(type) { + case *corev1.Pod: + if c.raceKind == "Pod" { + winner = object.DeepCopy() + } + case *corev1.Service: + if c.raceKind == "Service" { + winner = object.DeepCopy() + } + } + if winner == nil || c.winner != nil { + return c.Client.Create(ctx, object, options...) + } + winner.SetOwnerReferences(nil) + if err := c.Client.Create(ctx, winner, options...); err != nil { + return err + } + c.winner = winner.DeepCopyObject().(client.Object) + return apierrors.NewAlreadyExists(schema.GroupResource{Resource: strings.ToLower(c.raceKind) + "s"}, object.GetName()) +} + func ptr[T any](value T) *T { return &value } From 2a8b7ae3e99c9499fddf035cc0f3a8ae6b0ff20d Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 22:44:52 +0000 Subject: [PATCH 15/29] fix(operator): close child ownership races --- .../controllers/session_controller.go | 65 ++++++++++++++----- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index 1d52a20b..95f076d0 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -441,14 +441,25 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) return ctrl.Result{}, err } var service corev1.Service - if err := r.Get(ctx, types.NamespacedName{Namespace: session.Namespace, Name: serviceName}, &service); apierrors.IsNotFound(err) { + serviceKey := types.NamespacedName{Namespace: session.Namespace, Name: serviceName} + if err := r.Get(ctx, serviceKey, &service); apierrors.IsNotFound(err) { service = desiredService - if err := r.Create(ctx, &service); err != nil && !apierrors.IsAlreadyExists(err) { - return ctrl.Result{}, err + if err := r.Create(ctx, &service); err != nil { + if !apierrors.IsAlreadyExists(err) { + return ctrl.Result{}, err + } + reader := r.APIReader + if reader == nil { + reader = r.Client + } + if err := reader.Get(ctx, serviceKey, &service); err != nil { + return ctrl.Result{}, err + } } } else if err != nil { return ctrl.Result{}, err - } else if !sessionExclusivelyOwnsResource(&service, &session) { + } + if !sessionExclusivelyOwnsResource(&service, &session) { if err := r.deleteOwnedSessionResourcesAfterVerifiedDependencies(ctx, &session, "ServiceOwnershipConflict", "deterministic session Service has an unexpected owner"); err != nil { return ctrl.Result{}, err } @@ -478,14 +489,25 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) return ctrl.Result{}, err } var pod corev1.Pod - if err := r.Get(ctx, types.NamespacedName{Namespace: session.Namespace, Name: podName}, &pod); apierrors.IsNotFound(err) { + podKey := types.NamespacedName{Namespace: session.Namespace, Name: podName} + if err := r.Get(ctx, podKey, &pod); apierrors.IsNotFound(err) { pod = desiredPod - if err := r.Create(ctx, &pod); err != nil && !apierrors.IsAlreadyExists(err) { - return ctrl.Result{}, err + if err := r.Create(ctx, &pod); err != nil { + if !apierrors.IsAlreadyExists(err) { + return ctrl.Result{}, err + } + reader := r.APIReader + if reader == nil { + reader = r.Client + } + if err := reader.Get(ctx, podKey, &pod); err != nil { + return ctrl.Result{}, err + } } } else if err != nil { return ctrl.Result{}, err - } else if !sessionExclusivelyOwnsResource(&pod, &session) { + } + if !sessionExclusivelyOwnsResource(&pod, &session) { if err := r.deleteOwnedSessionResourcesAfterVerifiedDependencies(ctx, &session, "PodOwnershipConflict", "deterministic session Pod has an unexpected owner"); err != nil { return ctrl.Result{}, err } @@ -712,6 +734,7 @@ func (r *SessionReconciler) reconcileDelete(ctx context.Context, session *cluste &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: SessionServiceName(session), Namespace: session.Namespace}}, } existing := make([]client.Object, 0, len(objects)) + var ownershipConflict client.Object for _, object := range objects { err := r.Get(ctx, client.ObjectKeyFromObject(object), object) if apierrors.IsNotFound(err) { @@ -721,17 +744,10 @@ func (r *SessionReconciler) reconcileDelete(ctx context.Context, session *cluste return ctrl.Result{}, err } if !sessionExclusivelyOwnsResource(object, session) { - before := session.Status - if session.Status.Conditions != nil { - before.Conditions = append([]metav1.Condition(nil), session.Status.Conditions...) - } - meta.SetStatusCondition(&session.Status.Conditions, condition("Available", metav1.ConditionFalse, "CleanupOwnershipConflict", fmt.Sprintf("deterministic %T is not controlled by this session", object), session.Generation)) - if !reflect.DeepEqual(before, session.Status) { - if err := r.Status().Update(ctx, session); err != nil { - return ctrl.Result{}, err - } + if ownershipConflict == nil { + ownershipConflict = object } - return ctrl.Result{RequeueAfter: 30 * time.Second}, nil + continue } existing = append(existing, object) } @@ -742,6 +758,19 @@ func (r *SessionReconciler) reconcileDelete(ctx context.Context, session *cluste } } } + if ownershipConflict != nil { + before := session.Status + if session.Status.Conditions != nil { + before.Conditions = append([]metav1.Condition(nil), session.Status.Conditions...) + } + meta.SetStatusCondition(&session.Status.Conditions, condition("Available", metav1.ConditionFalse, "CleanupOwnershipConflict", fmt.Sprintf("deterministic %T is not controlled by this session", ownershipConflict), session.Generation)) + if !reflect.DeepEqual(before, session.Status) { + if err := r.Status().Update(ctx, session); err != nil { + return ctrl.Result{}, err + } + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, nil + } if len(existing) > 0 { return ctrl.Result{RequeueAfter: time.Second}, nil } From e366c52605a253348ad0d41784adb868b0272aa8 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 22:51:48 +0000 Subject: [PATCH 16/29] test(operator): model stale cache after create race --- .../controllers/reconciler_test.go | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 2b0fe1a4..79e7bb29 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -1213,9 +1213,15 @@ func TestSessionDeletionRefusesForeignDeterministicResources(t *testing.T) { } else { service.OwnerReferences = nil } - beforePod := pod.DeepCopy() - beforeService := service.DeepCopy() c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&clusterv1alpha1.T4Session{}).WithObjects(session, pod, service).Build() + beforePod := &corev1.Pod{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(pod), beforePod); err != nil { + t.Fatal(err) + } + beforeService := &corev1.Service{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(service), beforeService); err != nil { + t.Fatal(err) + } if err := c.Delete(context.Background(), session); err != nil { t.Fatal(err) } @@ -1872,8 +1878,9 @@ func TestSessionCreateAlreadyExistsRefetchesForeignWinner(t *testing.T) { base := fake.NewClientBuilder().WithScheme(scheme). WithStatusSubresource(&clusterv1alpha1.T4Session{}, &corev1.PersistentVolumeClaim{}, &corev1.Pod{}). WithObjects(objects...).Build() - c := &createAlreadyExistsClient{Client: base, raceKind: kind} + c := &createAlreadyExistsClient{Client: base, raceKind: kind, hideWinnerFromCache: true} r := configuredSessionReconciler(c, scheme) + r.APIReader = base if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { t.Fatal(err) } @@ -2124,8 +2131,25 @@ func hasReadOnlyMount(mounts []corev1.VolumeMount, name, path string) bool { type createAlreadyExistsClient struct { client.Client - raceKind string - winner client.Object + raceKind string + winner client.Object + hideWinnerFromCache bool +} + +func (c *createAlreadyExistsClient) Get(ctx context.Context, key client.ObjectKey, object client.Object, options ...client.GetOption) error { + if c.hideWinnerFromCache && c.winner != nil && key == client.ObjectKeyFromObject(c.winner) { + switch object.(type) { + case *corev1.Pod: + if c.raceKind == "Pod" { + return apierrors.NewNotFound(schema.GroupResource{Resource: "pods"}, key.Name) + } + case *corev1.Service: + if c.raceKind == "Service" { + return apierrors.NewNotFound(schema.GroupResource{Resource: "services"}, key.Name) + } + } + } + return c.Client.Get(ctx, key, object, options...) } func (c *createAlreadyExistsClient) Create(ctx context.Context, object client.Object, options ...client.CreateOption) error { From 4916c10a68036fc31f276a20203b5c4ed79c2266 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 22:54:45 +0000 Subject: [PATCH 17/29] test(operator): cover workspace PVC create race --- .../controllers/reconciler_test.go | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 79e7bb29..3195e777 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -72,6 +72,40 @@ func TestWorkspaceReconcileIsIdempotentAcrossDuplicateEvents(t *testing.T) { } } +func TestWorkspaceCreateAlreadyExistsRefetchesForeignPVC(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + base := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Workspace{}, &corev1.PersistentVolumeClaim{}). + WithObjects(testHost(), rwxStorageClass(), workspace).Build() + c := &createAlreadyExistsClient{Client: base, raceKind: "PVC"} + r := &controllers.WorkspaceReconciler{Client: c, Scheme: scheme} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}); err != nil { + t.Fatal(err) + } + if c.winner == nil { + t.Fatal("PVC create race was not exercised") + } + var pvc corev1.PersistentVolumeClaim + if err := base.Get(ctx, client.ObjectKeyFromObject(c.winner), &pvc); err != nil { + t.Fatalf("foreign PVC winner was deleted: %v", err) + } + want := c.winner.(*corev1.PersistentVolumeClaim) + if !reflect.DeepEqual(pvc.ObjectMeta, want.ObjectMeta) || !reflect.DeepEqual(pvc.Spec, want.Spec) { + t.Fatalf("foreign PVC winner was mutated: %#v", pvc) + } + var failed clusterv1alpha1.T4Workspace + if err := base.Get(ctx, client.ObjectKeyFromObject(workspace), &failed); err != nil { + t.Fatal(err) + } + storageReady := findCondition(failed.Status.Conditions, "StorageReady") + if failed.Status.PVCName != "" || storageReady == nil || storageReady.Status != metav1.ConditionFalse || storageReady.Reason != "PVCOwnershipConflict" || storageReady.ObservedGeneration != failed.Generation { + t.Fatalf("foreign PVC winner was published as authoritative: status=%#v StorageReady=%#v", failed.Status, storageReady) + } +} + func TestRetainWorkspaceCreatesPVCWithoutGarbageCollectableOwner(t *testing.T) { scheme := testScheme(t) workspace := testWorkspace(clusterv1alpha1.RetentionPolicyRetain) @@ -2163,6 +2197,10 @@ func (c *createAlreadyExistsClient) Create(ctx context.Context, object client.Ob if c.raceKind == "Service" { winner = object.DeepCopy() } + case *corev1.PersistentVolumeClaim: + if c.raceKind == "PVC" { + winner = object.DeepCopy() + } } if winner == nil || c.winner != nil { return c.Client.Create(ctx, object, options...) From db95dd6c4bdc0b2cf64c0d9886181b2339e86e7b Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 22:57:58 +0000 Subject: [PATCH 18/29] fix(operator): validate authoritative create winners --- packages/cluster-operator/cmd/manager/main.go | 2 +- .../controllers/reconciler_test.go | 4 ++-- .../controllers/session_controller.go | 12 ++++++---- .../controllers/workspace_controller.go | 22 ++++++++++++++----- 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/packages/cluster-operator/cmd/manager/main.go b/packages/cluster-operator/cmd/manager/main.go index d216db5a..f33af52c 100644 --- a/packages/cluster-operator/cmd/manager/main.go +++ b/packages/cluster-operator/cmd/manager/main.go @@ -70,7 +70,7 @@ func main() { ctrl.Log.Error(err, "unable to register T4ClusterHost controller") os.Exit(1) } - if err := (&controllers.WorkspaceReconciler{Client: manager.GetClient(), Scheme: manager.GetScheme()}).SetupWithManager(manager); err != nil { + if err := (&controllers.WorkspaceReconciler{Client: manager.GetClient(), APIReader: manager.GetAPIReader(), Scheme: manager.GetScheme()}).SetupWithManager(manager); err != nil { ctrl.Log.Error(err, "unable to register T4Workspace controller") os.Exit(1) } diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 3195e777..e4f4f42d 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -80,8 +80,8 @@ func TestWorkspaceCreateAlreadyExistsRefetchesForeignPVC(t *testing.T) { base := fake.NewClientBuilder().WithScheme(scheme). WithStatusSubresource(&clusterv1alpha1.T4Workspace{}, &corev1.PersistentVolumeClaim{}). WithObjects(testHost(), rwxStorageClass(), workspace).Build() - c := &createAlreadyExistsClient{Client: base, raceKind: "PVC"} - r := &controllers.WorkspaceReconciler{Client: c, Scheme: scheme} + c := &createAlreadyExistsClient{Client: base, raceKind: "PVC", hideWinnerFromCache: true} + r := &controllers.WorkspaceReconciler{Client: c, APIReader: base, Scheme: scheme} if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}); err != nil { t.Fatal(err) } diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index 95f076d0..3f1bd28d 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -779,21 +779,25 @@ func (r *SessionReconciler) reconcileDelete(ctx context.Context, session *cluste } func (r *SessionReconciler) deleteOwnedSessionResources(ctx context.Context, session *clusterv1alpha1.T4Session) error { - return r.deleteOwnedSessionResourcesWithFailure(ctx, session, false, false, "ResourceOwnershipConflict", "one or more deterministic session resources have an unexpected owner") + return r.deleteOwnedSessionResourcesWithFailure(ctx, r.Client, session, false, false, "ResourceOwnershipConflict", "one or more deterministic session resources have an unexpected owner") } func (r *SessionReconciler) deleteOwnedSessionResourcesAfterVerifiedDependencies(ctx context.Context, session *clusterv1alpha1.T4Session, reason, message string) error { - return r.deleteOwnedSessionResourcesWithFailure(ctx, session, true, true, reason, message) + reader := r.APIReader + if reader == nil { + reader = r.Client + } + return r.deleteOwnedSessionResourcesWithFailure(ctx, reader, session, true, true, reason, message) } -func (r *SessionReconciler) deleteOwnedSessionResourcesWithFailure(ctx context.Context, session *clusterv1alpha1.T4Session, hostReady, workspaceReady bool, reason, message string) error { +func (r *SessionReconciler) deleteOwnedSessionResourcesWithFailure(ctx context.Context, reader client.Reader, session *clusterv1alpha1.T4Session, hostReady, workspaceReady bool, reason, message string) error { objects := []client.Object{ &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: SessionPodName(session), Namespace: session.Namespace}}, &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: SessionServiceName(session), Namespace: session.Namespace}}, } ownershipConflict := false for _, object := range objects { - if err := r.Get(ctx, client.ObjectKeyFromObject(object), object); err != nil { + if err := reader.Get(ctx, client.ObjectKeyFromObject(object), object); err != nil { if err := client.IgnoreNotFound(err); err != nil { return err } diff --git a/packages/cluster-operator/controllers/workspace_controller.go b/packages/cluster-operator/controllers/workspace_controller.go index d32920f8..d6c2a54d 100644 --- a/packages/cluster-operator/controllers/workspace_controller.go +++ b/packages/cluster-operator/controllers/workspace_controller.go @@ -24,7 +24,8 @@ import ( type WorkspaceReconciler struct { client.Client - Scheme *runtime.Scheme + APIReader client.Reader + Scheme *runtime.Scheme } const ( @@ -74,8 +75,9 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, request ctrl.Reques } pvcName := WorkspacePVCName(&workspace) + pvcKey := types.NamespacedName{Namespace: workspace.Namespace, Name: pvcName} var pvc corev1.PersistentVolumeClaim - err = r.Get(ctx, types.NamespacedName{Namespace: workspace.Namespace, Name: pvcName}, &pvc) + err = r.Get(ctx, pvcKey, &pvc) if apierrors.IsNotFound(err) { volumeMode := corev1.PersistentVolumeFilesystem pvc = corev1.PersistentVolumeClaim{ @@ -99,12 +101,22 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, request ctrl.Reques return ctrl.Result{}, err } } - if err := r.Create(ctx, &pvc); err != nil && !apierrors.IsAlreadyExists(err) { - return ctrl.Result{}, err + if err := r.Create(ctx, &pvc); err != nil { + if !apierrors.IsAlreadyExists(err) { + return ctrl.Result{}, err + } + reader := r.APIReader + if reader == nil { + reader = r.Client + } + if err := reader.Get(ctx, pvcKey, &pvc); err != nil { + return ctrl.Result{}, err + } } } else if err != nil { return ctrl.Result{}, err - } else if !workspaceOwnsPVC(&workspace, &pvc) { + } + if !workspaceOwnsPVC(&workspace, &pvc) { return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCOwnershipConflict", "deterministic workspace PVC does not belong to this workspace") } else if pvcStorageClassName(&pvc) != storageClassName { return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", ReasonStorageClassMismatch, fmt.Sprintf("workspace PVC uses StorageClass %q instead of host-selected %q; data-bearing PVCs are never recreated automatically", pvcStorageClassName(&pvc), storageClassName)) From ee5d00903272e00b584ce2bac16fd33e7cf80352 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 23:10:02 +0000 Subject: [PATCH 19/29] test(operator): cover authoritative cleanup policy --- .../controllers/reconciler_test.go | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index e4f4f42d..6e45a9a0 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -106,6 +106,67 @@ func TestWorkspaceCreateAlreadyExistsRefetchesForeignPVC(t *testing.T) { } } +func TestWorkspacePendingPVCPolicyFailsBeforeAuthority(t *testing.T) { + for _, test := range []struct { + name string + mutate func(*corev1.PersistentVolumeClaim) + reason string + }{ + {name: "wrong class", reason: controllers.ReasonStorageClassMismatch, mutate: func(pvc *corev1.PersistentVolumeClaim) { pvc.Spec.StorageClassName = ptr("other-rwx") }}, + {name: "wrong access", reason: "PVCNotRWX", mutate: func(pvc *corev1.PersistentVolumeClaim) { pvc.Spec.AccessModes = []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce} }}, + } { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + pvc := ownedWorkspacePVC(workspace) + pvc.Status.Phase = corev1.ClaimPending + test.mutate(pvc) + c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&clusterv1alpha1.T4Workspace{}, &corev1.PersistentVolumeClaim{}).WithObjects(testHost(), rwxStorageClass(), workspace, pvc).Build() + r := &controllers.WorkspaceReconciler{Client: c, APIReader: c, Scheme: scheme} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}); err != nil { + t.Fatal(err) + } + var failed clusterv1alpha1.T4Workspace + if err := c.Get(ctx, client.ObjectKeyFromObject(workspace), &failed); err != nil { + t.Fatal(err) + } + condition := findCondition(failed.Status.Conditions, "StorageReady") + if failed.Status.PVCName != "" || condition == nil || condition.Status != metav1.ConditionFalse || condition.Reason != test.reason { + t.Fatalf("incompatible Pending PVC published authority: status=%#v condition=%#v", failed.Status, condition) + } + }) + } +} + +func TestWorkspaceDeletionUsesAuthoritativePVCReader(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + workspace.Finalizers = []string{clusterv1alpha1.WorkspaceFinalizer} + pvc := ownedWorkspacePVC(workspace) + pvc.OwnerReferences = nil + base := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&clusterv1alpha1.T4Workspace{}).WithObjects(workspace, pvc).Build() + if err := base.Delete(ctx, workspace); err != nil { + t.Fatal(err) + } + c := &createAlreadyExistsClient{Client: base, raceKind: "PVC", winner: pvc, hideWinnerFromCache: true} + r := &controllers.WorkspaceReconciler{Client: c, APIReader: base, Scheme: scheme} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}); err != nil { + t.Fatal(err) + } + var waiting clusterv1alpha1.T4Workspace + if err := base.Get(ctx, client.ObjectKeyFromObject(workspace), &waiting); err != nil { + t.Fatalf("workspace finalizer ignored authoritative PVC conflict: %v", err) + } + condition := findCondition(waiting.Status.Conditions, "Ready") + if condition == nil || condition.Reason != "CleanupOwnershipConflict" || !contains(waiting.Finalizers, clusterv1alpha1.WorkspaceFinalizer) { + t.Fatalf("authoritative PVC conflict not retained: %#v", waiting) + } +} + func TestRetainWorkspaceCreatesPVCWithoutGarbageCollectableOwner(t *testing.T) { scheme := testScheme(t) workspace := testWorkspace(clusterv1alpha1.RetentionPolicyRetain) @@ -1297,6 +1358,35 @@ func TestSessionDeletionRefusesForeignDeterministicResources(t *testing.T) { } } +func TestSessionDeletionUsesAuthoritativeChildReader(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + session := testSession() + session.UID = "session-uid" + session.Finalizers = []string{clusterv1alpha1.SessionFinalizer} + pod, service := ownedSessionResources(session) + pod.OwnerReferences = nil + base := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&clusterv1alpha1.T4Session{}).WithObjects(session, pod, service).Build() + if err := base.Delete(ctx, session); err != nil { + t.Fatal(err) + } + c := &createAlreadyExistsClient{Client: base, raceKind: "Pod", winner: pod, hideWinnerFromCache: true} + r := configuredSessionReconciler(c, scheme) + r.APIReader = base + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { + t.Fatal(err) + } + assertObjectCounts(t, base, 1, 0) + var waiting clusterv1alpha1.T4Session + if err := base.Get(ctx, client.ObjectKeyFromObject(session), &waiting); err != nil { + t.Fatalf("session finalizer ignored authoritative Pod conflict: %v", err) + } + condition := findCondition(waiting.Status.Conditions, "Available") + if condition == nil || condition.Reason != "CleanupOwnershipConflict" || !contains(waiting.Finalizers, clusterv1alpha1.SessionFinalizer) { + t.Fatalf("authoritative child conflict not retained: %#v", waiting) + } +} + func TestSessionDeletionCleansResourcesBeforeFinalizer(t *testing.T) { scheme := testScheme(t) session := testSession() @@ -2181,6 +2271,10 @@ func (c *createAlreadyExistsClient) Get(ctx context.Context, key client.ObjectKe if c.raceKind == "Service" { return apierrors.NewNotFound(schema.GroupResource{Resource: "services"}, key.Name) } + case *corev1.PersistentVolumeClaim: + if c.raceKind == "PVC" { + return apierrors.NewNotFound(schema.GroupResource{Resource: "persistentvolumeclaims"}, key.Name) + } } } return c.Client.Get(ctx, key, object, options...) From d9ef139dd752f3f7ceb79707b6be45e87f589b58 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 23:12:47 +0000 Subject: [PATCH 20/29] fix(operator): precondition authoritative cleanup --- .../controllers/session_controller.go | 43 +++++++++++++++---- .../controllers/workspace_controller.go | 13 +++--- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index 3f1bd28d..7523bbbb 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -465,7 +465,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } else if !serviceExposureIsInternal(&service) { - if err := r.Delete(ctx, &service); err != nil && !apierrors.IsNotFound(err) { + if err := deleteWithPreconditions(ctx, r.Client, &service); err != nil && !apierrors.IsNotFound(err) { return ctrl.Result{}, err } if err := r.updateSessionPending(ctx, &session, podName, serviceName, "ServiceExposureChanged", "session Service is being recreated with ClusterIP-only exposure"); err != nil { @@ -527,7 +527,7 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } return ctrl.Result{RequeueAfter: time.Second}, nil } else if pod.Annotations[clusterv1alpha1.SessionPodSpecHashAnnotation] != desiredPod.Annotations[clusterv1alpha1.SessionPodSpecHashAnnotation] { - if err := r.Delete(ctx, &pod); err != nil && !apierrors.IsNotFound(err) { + if err := deleteWithPreconditions(ctx, r.Client, &pod); err != nil && !apierrors.IsNotFound(err) { return ctrl.Result{}, err } if err := r.updateSessionPending(ctx, &session, podName, serviceName, "PodSpecChanged", "session Pod is being recreated to apply immutable desired state"); err != nil { @@ -735,8 +735,12 @@ func (r *SessionReconciler) reconcileDelete(ctx context.Context, session *cluste } existing := make([]client.Object, 0, len(objects)) var ownershipConflict client.Object + reader := r.APIReader + if reader == nil { + reader = r.Client + } for _, object := range objects { - err := r.Get(ctx, client.ObjectKeyFromObject(object), object) + err := reader.Get(ctx, client.ObjectKeyFromObject(object), object) if apierrors.IsNotFound(err) { continue } @@ -753,7 +757,7 @@ func (r *SessionReconciler) reconcileDelete(ctx context.Context, session *cluste } for _, object := range existing { if object.GetDeletionTimestamp().IsZero() { - if err := r.Delete(ctx, object); err != nil && !apierrors.IsNotFound(err) { + if err := deleteWithPreconditions(ctx, r.Client, object); err != nil && !apierrors.IsNotFound(err) { return ctrl.Result{}, err } } @@ -779,7 +783,7 @@ func (r *SessionReconciler) reconcileDelete(ctx context.Context, session *cluste } func (r *SessionReconciler) deleteOwnedSessionResources(ctx context.Context, session *clusterv1alpha1.T4Session) error { - return r.deleteOwnedSessionResourcesWithFailure(ctx, r.Client, session, false, false, "ResourceOwnershipConflict", "one or more deterministic session resources have an unexpected owner") + return r.deleteOwnedSessionResourcesWithFailure(ctx, r.Client, session, true, false, false, "ResourceOwnershipConflict", "one or more deterministic session resources have an unexpected owner") } func (r *SessionReconciler) deleteOwnedSessionResourcesAfterVerifiedDependencies(ctx context.Context, session *clusterv1alpha1.T4Session, reason, message string) error { @@ -787,14 +791,15 @@ func (r *SessionReconciler) deleteOwnedSessionResourcesAfterVerifiedDependencies if reader == nil { reader = r.Client } - return r.deleteOwnedSessionResourcesWithFailure(ctx, reader, session, true, true, reason, message) + return r.deleteOwnedSessionResourcesWithFailure(ctx, reader, session, false, true, true, reason, message) } -func (r *SessionReconciler) deleteOwnedSessionResourcesWithFailure(ctx context.Context, reader client.Reader, session *clusterv1alpha1.T4Session, hostReady, workspaceReady bool, reason, message string) error { +func (r *SessionReconciler) deleteOwnedSessionResourcesWithFailure(ctx context.Context, reader client.Reader, session *clusterv1alpha1.T4Session, deleteWithoutConflict, hostReady, workspaceReady bool, reason, message string) error { objects := []client.Object{ &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: SessionPodName(session), Namespace: session.Namespace}}, &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: SessionServiceName(session), Namespace: session.Namespace}}, } + owned := make([]client.Object, 0, len(objects)) ownershipConflict := false for _, object := range objects { if err := reader.Get(ctx, client.ObjectKeyFromObject(object), object); err != nil { @@ -807,8 +812,13 @@ func (r *SessionReconciler) deleteOwnedSessionResourcesWithFailure(ctx context.C ownershipConflict = true continue } - if err := r.Delete(ctx, object); err != nil && !apierrors.IsNotFound(err) { - return err + owned = append(owned, object) + } + if ownershipConflict || deleteWithoutConflict { + for _, object := range owned { + if err := deleteWithPreconditions(ctx, r.Client, object); err != nil && !apierrors.IsNotFound(err) { + return err + } } } if ownershipConflict { @@ -820,6 +830,21 @@ func (r *SessionReconciler) deleteOwnedSessionResourcesWithFailure(ctx context.C return nil } +func deleteWithPreconditions(ctx context.Context, writer client.Client, object client.Object) error { + preconditions := metav1.Preconditions{} + if uid := object.GetUID(); uid != "" { + preconditions.UID = &uid + } + if resourceVersion := object.GetResourceVersion(); resourceVersion != "" { + preconditions.ResourceVersion = &resourceVersion + } + options := &client.DeleteOptions{} + if preconditions.UID != nil || preconditions.ResourceVersion != nil { + options.Preconditions = &preconditions + } + return writer.Delete(ctx, object, options) +} + func (r *SessionReconciler) updateSessionFailure(ctx context.Context, session *clusterv1alpha1.T4Session, hostReady, workspaceReady bool, conditionType, reason, message string) error { original := session.Status if session.Status.Conditions != nil { diff --git a/packages/cluster-operator/controllers/workspace_controller.go b/packages/cluster-operator/controllers/workspace_controller.go index d6c2a54d..441e785a 100644 --- a/packages/cluster-operator/controllers/workspace_controller.go +++ b/packages/cluster-operator/controllers/workspace_controller.go @@ -118,6 +118,8 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, request ctrl.Reques } if !workspaceOwnsPVC(&workspace, &pvc) { return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCOwnershipConflict", "deterministic workspace PVC does not belong to this workspace") + } else if !pvcHasRWX(&pvc) { + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCNotRWX", "workspace PVC does not request ReadWriteMany") } else if pvcStorageClassName(&pvc) != storageClassName { return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", ReasonStorageClassMismatch, fmt.Sprintf("workspace PVC uses StorageClass %q instead of host-selected %q; data-bearing PVCs are never recreated automatically", pvcStorageClassName(&pvc), storageClassName)) } else if workspace.Spec.RetentionPolicy == clusterv1alpha1.RetentionPolicyRetain && metav1.IsControlledBy(&pvc, &workspace) { @@ -130,9 +132,6 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, request ctrl.Reques return ctrl.Result{Requeue: true}, nil } } - if pvc.Status.Phase == corev1.ClaimBound && !pvcHasRWX(&pvc) { - return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCNotRWX", "bound workspace PVC does not request ReadWriteMany") - } if pvc.Status.Phase == corev1.ClaimLost { return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCLost", "workspace PVC lost its volume") } @@ -235,7 +234,11 @@ func (r *WorkspaceReconciler) reconcileDelete(ctx context.Context, workspace *cl } pvcKey := types.NamespacedName{Namespace: workspace.Namespace, Name: WorkspacePVCName(workspace)} var pvc corev1.PersistentVolumeClaim - err := r.Get(ctx, pvcKey, &pvc) + reader := r.APIReader + if reader == nil { + reader = r.Client + } + err := reader.Get(ctx, pvcKey, &pvc) if err == nil && !workspaceOwnsPVC(workspace, &pvc) { before := workspace.Status if workspace.Status.Conditions != nil { @@ -267,7 +270,7 @@ func (r *WorkspaceReconciler) reconcileDelete(ctx context.Context, workspace *cl } } else { if err == nil { - if err := r.Delete(ctx, &pvc); err != nil && !apierrors.IsNotFound(err) { + if err := deleteWithPreconditions(ctx, r.Client, &pvc); err != nil && !apierrors.IsNotFound(err) { return ctrl.Result{}, err } return ctrl.Result{RequeueAfter: time.Second}, nil From 32d4a0454496024e5f00e7ab6f6ae082db93dde4 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 23:28:51 +0000 Subject: [PATCH 21/29] test(operator): cover split-reader dependency cleanup --- .../controllers/reconciler_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 6e45a9a0..5f244bf5 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -1387,6 +1387,22 @@ func TestSessionDeletionUsesAuthoritativeChildReader(t *testing.T) { } } +func TestSessionDependencyCleanupUsesAuthoritativeChildReader(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + session := testSession() + session.UID = "session-uid" + pod, service := ownedSessionResources(session) + base := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&clusterv1alpha1.T4Session{}, &corev1.Pod{}).WithObjects(session, pod, service).Build() + c := &createAlreadyExistsClient{Client: base, raceKind: "Pod", winner: pod, hideWinnerFromCache: true} + r := configuredSessionReconciler(c, scheme) + r.APIReader = base + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { + t.Fatal(err) + } + assertObjectCounts(t, base, 0, 0) +} + func TestSessionDeletionCleansResourcesBeforeFinalizer(t *testing.T) { scheme := testScheme(t) session := testSession() From a10e70be3060db423a3c823fa6c083bd0903a724 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 23:30:43 +0000 Subject: [PATCH 22/29] fix(operator): authoritatively clean revoked sessions --- packages/cluster-operator/controllers/session_controller.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index 7523bbbb..21b1254e 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -783,7 +783,11 @@ func (r *SessionReconciler) reconcileDelete(ctx context.Context, session *cluste } func (r *SessionReconciler) deleteOwnedSessionResources(ctx context.Context, session *clusterv1alpha1.T4Session) error { - return r.deleteOwnedSessionResourcesWithFailure(ctx, r.Client, session, true, false, false, "ResourceOwnershipConflict", "one or more deterministic session resources have an unexpected owner") + reader := r.APIReader + if reader == nil { + reader = r.Client + } + return r.deleteOwnedSessionResourcesWithFailure(ctx, reader, session, true, false, false, "ResourceOwnershipConflict", "one or more deterministic session resources have an unexpected owner") } func (r *SessionReconciler) deleteOwnedSessionResourcesAfterVerifiedDependencies(ctx context.Context, session *clusterv1alpha1.T4Session, reason, message string) error { From 3d09b4ac29d04cdf2415a72c16af1bc94154dc3e Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 23:41:18 +0000 Subject: [PATCH 23/29] test(operator): expose stale workspace session cache --- .../controllers/reconciler_test.go | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 5f244bf5..cb4959bf 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -167,6 +167,41 @@ func TestWorkspaceDeletionUsesAuthoritativePVCReader(t *testing.T) { } } +func TestWorkspaceDeletionUsesAuthoritativeSessionReader(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + workspace.Finalizers = []string{clusterv1alpha1.WorkspaceFinalizer} + pvc := ownedWorkspacePVC(workspace) + session := testSession() + session.Spec.WorkspaceRef = workspace.Name + cacheClient := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&clusterv1alpha1.T4Workspace{}).WithObjects(workspace, pvc).Build() + apiReader := fake.NewClientBuilder().WithScheme(scheme).WithObjects(pvc, session).Build() + if err := cacheClient.Delete(ctx, workspace); err != nil { + t.Fatal(err) + } + r := &controllers.WorkspaceReconciler{Client: cacheClient, APIReader: apiReader, Scheme: scheme} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}); err != nil { + t.Fatal(err) + } + var waiting clusterv1alpha1.T4Workspace + if err := cacheClient.Get(ctx, client.ObjectKeyFromObject(workspace), &waiting); err != nil { + t.Fatalf("workspace finalizer ignored authoritative session: %v", err) + } + ready := findCondition(waiting.Status.Conditions, "Ready") + if ready == nil || ready.Status != metav1.ConditionFalse || ready.Reason != "SessionsRemain" || ready.ObservedGeneration != waiting.Generation { + t.Fatalf("Ready = %#v, want current-generation False/SessionsRemain", ready) + } + if !contains(waiting.Finalizers, clusterv1alpha1.WorkspaceFinalizer) { + t.Fatal("workspace finalizer was removed while authoritative session remains") + } + var remainingPVC corev1.PersistentVolumeClaim + if err := cacheClient.Get(ctx, client.ObjectKeyFromObject(pvc), &remainingPVC); err != nil { + t.Fatalf("workspace PVC was deleted while authoritative session remains: %v", err) + } +} + func TestRetainWorkspaceCreatesPVCWithoutGarbageCollectableOwner(t *testing.T) { scheme := testScheme(t) workspace := testWorkspace(clusterv1alpha1.RetentionPolicyRetain) From d428c0a0d17d548e9666e5635652a32be5976756 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Tue, 21 Jul 2026 23:48:00 +0000 Subject: [PATCH 24/29] fix(operator): list workspace sessions authoritatively --- .../cluster-operator/controllers/workspace_controller.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cluster-operator/controllers/workspace_controller.go b/packages/cluster-operator/controllers/workspace_controller.go index 441e785a..f3b0fd90 100644 --- a/packages/cluster-operator/controllers/workspace_controller.go +++ b/packages/cluster-operator/controllers/workspace_controller.go @@ -210,7 +210,11 @@ func (r *WorkspaceReconciler) reconcileDelete(ctx context.Context, workspace *cl } } var sessions clusterv1alpha1.T4SessionList - if err := r.List(ctx, &sessions, client.InNamespace(workspace.Namespace)); err != nil { + sessionReader := r.APIReader + if sessionReader == nil { + sessionReader = r.Client + } + if err := sessionReader.List(ctx, &sessions, client.InNamespace(workspace.Namespace)); err != nil { return ctrl.Result{}, err } remainingSessions := 0 From 4a7cdf66bbdcfbf325d1bc2439a8727478976027 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Wed, 22 Jul 2026 03:45:50 +0000 Subject: [PATCH 25/29] test(operator): reject stale PVC authority --- .../controllers/reconciler_test.go | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index cb4959bf..f5e00c84 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -106,6 +106,42 @@ func TestWorkspaceCreateAlreadyExistsRefetchesForeignPVC(t *testing.T) { } } +func TestWorkspaceReadinessRejectsAuthoritativeForeignPVCReplacement(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + cachedPVC := ownedWorkspacePVC(workspace) + cachedPVC.UID = "cached-pvc-uid" + authoritativePVC := cachedPVC.DeepCopy() + authoritativePVC.UID = "replacement-pvc-uid" + authoritativePVC.Annotations = nil + authoritativePVC.OwnerReferences = nil + cacheClient := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Workspace{}, &corev1.PersistentVolumeClaim{}). + WithObjects(testHost(), rwxStorageClass(), workspace, cachedPVC).Build() + r := &controllers.WorkspaceReconciler{ + Client: cacheClient, + APIReader: &pvcOverrideReader{Reader: cacheClient, pvc: authoritativePVC}, + Scheme: scheme, + } + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(workspace)}); err != nil { + t.Fatal(err) + } + var got clusterv1alpha1.T4Workspace + if err := cacheClient.Get(ctx, client.ObjectKeyFromObject(workspace), &got); err != nil { + t.Fatal(err) + } + storageReady := findCondition(got.Status.Conditions, "StorageReady") + ready := findCondition(got.Status.Conditions, "Ready") + if got.Status.PVCName != "" || got.Status.PVCPhase != "" || !got.Status.Capacity.IsZero() || + storageReady == nil || storageReady.Status != metav1.ConditionFalse || storageReady.Reason != "PVCOwnershipConflict" || storageReady.ObservedGeneration != got.Generation || + ready == nil || ready.Status != metav1.ConditionFalse || ready.ObservedGeneration != got.Generation { + t.Fatalf("stale cached PVC published workspace authority: status=%#v StorageReady=%#v Ready=%#v", got.Status, storageReady, ready) + } +} + + func TestWorkspacePendingPVCPolicyFailsBeforeAuthority(t *testing.T) { for _, test := range []struct { name string @@ -347,6 +383,59 @@ func TestWorkspaceDeletionWaitsForSessionResources(t *testing.T) { } } +func TestSessionPodCreateRevalidatesAuthoritativePVC(t *testing.T) { + tests := []struct { + name string + mutate func(*corev1.PersistentVolumeClaim) + }{ + {name: "replacement UID", mutate: func(pvc *corev1.PersistentVolumeClaim) { pvc.UID = "replacement-pvc-uid" }}, + {name: "foreign owner", mutate: func(pvc *corev1.PersistentVolumeClaim) { + pvc.OwnerReferences = []metav1.OwnerReference{{APIVersion: "v1", Kind: "Secret", Name: "foreign", UID: "foreign-uid", Controller: ptr(true)}} + }}, + {name: "storage class drift", mutate: func(pvc *corev1.PersistentVolumeClaim) { pvc.Spec.StorageClassName = ptr("other-rwx") }}, + {name: "access mode drift", mutate: func(pvc *corev1.PersistentVolumeClaim) { pvc.Spec.AccessModes = []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce} }}, + {name: "unbound replacement", mutate: func(pvc *corev1.PersistentVolumeClaim) { pvc.Status.Phase = corev1.ClaimPending }}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + workspace.Status.PVCName = controllers.WorkspacePVCName(workspace) + cachedPVC := ownedWorkspacePVC(workspace) + cachedPVC.UID = "cached-pvc-uid" + authoritativePVC := cachedPVC.DeepCopy() + test.mutate(authoritativePVC) + session := testSession() + session.UID = "session-uid" + cacheClient := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Session{}, &corev1.PersistentVolumeClaim{}, &corev1.Pod{}). + WithObjects(testHost(), workspace, cachedPVC, session).Build() + r := configuredSessionReconciler(cacheClient, scheme) + r.APIReader = &pvcOverrideReader{Reader: cacheClient, pvc: authoritativePVC} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { + t.Fatal(err) + } + var pods corev1.PodList + if err := cacheClient.List(ctx, &pods, client.InNamespace(session.Namespace)); err != nil { + t.Fatal(err) + } + if len(pods.Items) != 0 { + t.Fatalf("created %d Pods after authoritative PVC %s", len(pods.Items), test.name) + } + var got clusterv1alpha1.T4Session + if err := cacheClient.Get(ctx, client.ObjectKeyFromObject(session), &got); err != nil { + t.Fatal(err) + } + workspaceReady := findCondition(got.Status.Conditions, "WorkspaceReady") + if got.Status.PodName != "" || workspaceReady == nil || workspaceReady.Status != metav1.ConditionFalse || workspaceReady.Reason != "PVCAuthorityChanged" || workspaceReady.ObservedGeneration != got.Generation { + t.Fatalf("authoritative PVC %s published session authority: status=%#v WorkspaceReady=%#v", test.name, got.Status, workspaceReady) + } + }) + } +} + func TestSessionFailsClosedWhenAnyOMPReferenceIsMissing(t *testing.T) { for _, test := range []struct { name string @@ -2304,6 +2393,19 @@ func hasReadOnlyMount(mounts []corev1.VolumeMount, name, path string) bool { return false } +type pvcOverrideReader struct { + client.Reader + pvc *corev1.PersistentVolumeClaim +} + +func (r *pvcOverrideReader) Get(ctx context.Context, key client.ObjectKey, object client.Object, options ...client.GetOption) error { + if pvc, ok := object.(*corev1.PersistentVolumeClaim); ok && key == client.ObjectKeyFromObject(r.pvc) { + r.pvc.DeepCopyInto(pvc) + return nil + } + return r.Reader.Get(ctx, key, object, options...) +} + type createAlreadyExistsClient struct { client.Client raceKind string From b98470dd2c696d3b5f68624015e757530039497c Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Wed, 22 Jul 2026 03:54:16 +0000 Subject: [PATCH 26/29] fix(operator): revalidate PVC authority --- .../controllers/session_controller.go | 51 +++++++++++++++++++ .../controllers/workspace_controller.go | 22 ++++++++ 2 files changed, 73 insertions(+) diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index 21b1254e..ed45cb6c 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -491,6 +491,16 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) var pod corev1.Pod podKey := types.NamespacedName{Namespace: session.Namespace, Name: podName} if err := r.Get(ctx, podKey, &pod); apierrors.IsNotFound(err) { + reason, message, err := r.authoritativePVCValidation(ctx, &workspace, &pvc, host.Spec.StorageClassName) + if err != nil { + return ctrl.Result{}, err + } + if reason != "" { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", reason, message) + } pod = desiredPod if err := r.Create(ctx, &pod); err != nil { if !apierrors.IsAlreadyExists(err) { @@ -536,6 +546,17 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) return ctrl.Result{RequeueAfter: time.Second}, nil } + reason, message, err = r.authoritativePVCValidation(ctx, &workspace, &pvc, host.Spec.StorageClassName) + if err != nil { + return ctrl.Result{}, err + } + if reason != "" { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", reason, message) + } + original := session.Status if session.Status.Conditions != nil { original.Conditions = append([]metav1.Condition(nil), session.Status.Conditions...) @@ -567,6 +588,36 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } +func (r *SessionReconciler) authoritativePVCValidation(ctx context.Context, workspace *clusterv1alpha1.T4Workspace, cachedPVC *corev1.PersistentVolumeClaim, storageClassName string) (string, string, error) { + reader := r.APIReader + if reader == nil { + reader = r.Client + } + var authoritativePVC corev1.PersistentVolumeClaim + if err := reader.Get(ctx, client.ObjectKeyFromObject(cachedPVC), &authoritativePVC); err != nil { + if apierrors.IsNotFound(err) { + return "PVCAuthorityChanged", "workspace PVC does not exist in authoritative API state", nil + } + return "", "", err + } + if authoritativePVC.UID != cachedPVC.UID { + return "PVCAuthorityChanged", "authoritative workspace PVC UID differs from the validated cached PVC", nil + } + if workspace.UID != "" && !workspaceOwnsPVC(workspace, &authoritativePVC) { + return "PVCAuthorityChanged", "authoritative workspace PVC owner reference does not belong to the workspace", nil + } + if pvcStorageClassName(&authoritativePVC) != storageClassName { + return "PVCAuthorityChanged", fmt.Sprintf("authoritative workspace PVC uses StorageClass %q instead of host-selected %q", pvcStorageClassName(&authoritativePVC), storageClassName), nil + } + if !pvcHasRWX(&authoritativePVC) { + return "PVCAuthorityChanged", "authoritative workspace PVC does not request ReadWriteMany", nil + } + if authoritativePVC.Status.Phase != corev1.ClaimBound { + return "PVCAuthorityChanged", "authoritative workspace PVC is not Bound", nil + } + return "", "", nil +} + func (r *SessionReconciler) desiredPod(session *clusterv1alpha1.T4Session, pvcName, podName string, labels map[string]string, runtimeVersions ompResourceVersions) (corev1.Pod, error) { falseValue := false trueValue := true diff --git a/packages/cluster-operator/controllers/workspace_controller.go b/packages/cluster-operator/controllers/workspace_controller.go index f3b0fd90..e1b616dc 100644 --- a/packages/cluster-operator/controllers/workspace_controller.go +++ b/packages/cluster-operator/controllers/workspace_controller.go @@ -132,6 +132,28 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, request ctrl.Reques return ctrl.Result{Requeue: true}, nil } } + reader := r.APIReader + if reader == nil { + reader = r.Client + } + var authoritativePVC corev1.PersistentVolumeClaim + if err := reader.Get(ctx, pvcKey, &authoritativePVC); err != nil { + if apierrors.IsNotFound(err) { + return ctrl.Result{RequeueAfter: 5 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCNotFound", "workspace PVC does not exist in authoritative API state") + } + return ctrl.Result{}, err + } + if authoritativePVC.UID != pvc.UID || !workspaceOwnsPVC(&workspace, &authoritativePVC) { + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCOwnershipConflict", "authoritative workspace PVC identity or ownership does not belong to this workspace") + } + if !pvcHasRWX(&authoritativePVC) { + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCNotRWX", "authoritative workspace PVC does not request ReadWriteMany") + } + if pvcStorageClassName(&authoritativePVC) != storageClassName { + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", ReasonStorageClassMismatch, fmt.Sprintf("authoritative workspace PVC uses StorageClass %q instead of host-selected %q; data-bearing PVCs are never recreated automatically", pvcStorageClassName(&authoritativePVC), storageClassName)) + } + pvc = authoritativePVC + if pvc.Status.Phase == corev1.ClaimLost { return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateWorkspaceFailure(ctx, &workspace, "StorageReady", "PVCLost", "workspace PVC lost its volume") } From ece837c095a464fe9e3034901fce8affb56e7132 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Wed, 22 Jul 2026 04:31:41 +0000 Subject: [PATCH 27/29] test(operator): gate existing pods on PVC authority --- .../controllers/reconciler_test.go | 56 ++++++++++++++++--- 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index f5e00c84..40cc9b02 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -436,6 +436,54 @@ func TestSessionPodCreateRevalidatesAuthoritativePVC(t *testing.T) { } } +func TestSessionExistingPodRepairRejectsAuthoritativeForeignPVCReplacement(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + workspace := testWorkspace(clusterv1alpha1.RetentionPolicyDelete) + workspace.UID = "workspace-uid" + workspace.Status.PVCName = controllers.WorkspacePVCName(workspace) + cachedPVC := ownedWorkspacePVC(workspace) + cachedPVC.UID = "cached-pvc-uid" + session := testSession() + session.UID = "session-uid" + cacheClient := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Session{}, &corev1.PersistentVolumeClaim{}, &corev1.Pod{}). + WithObjects(testHost(), workspace, cachedPVC, session).Build() + r := configuredSessionReconciler(cacheClient, scheme) + reconcileMany(t, 2, func() error { + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}) + return err + }) + var pod corev1.Pod + if err := cacheClient.Get(ctx, types.NamespacedName{Namespace: session.Namespace, Name: controllers.SessionPodName(session)}, &pod); err != nil { + t.Fatal(err) + } + pod.Labels = nil + if err := cacheClient.Update(ctx, &pod); err != nil { + t.Fatal(err) + } + authoritativePVC := cachedPVC.DeepCopy() + authoritativePVC.UID = "replacement-pvc-uid" + authoritativePVC.Annotations = nil + authoritativePVC.OwnerReferences = nil + r.APIReader = &pvcOverrideReader{Reader: cacheClient, pvc: authoritativePVC} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); err != nil { + t.Fatal(err) + } + if err := cacheClient.Get(ctx, client.ObjectKeyFromObject(&pod), &pod); !apierrors.IsNotFound(err) { + t.Fatalf("existing Pod survived authoritative PVC replacement: %v", err) + } + var got clusterv1alpha1.T4Session + if err := cacheClient.Get(ctx, client.ObjectKeyFromObject(session), &got); err != nil { + t.Fatal(err) + } + workspaceReady := findCondition(got.Status.Conditions, "WorkspaceReady") + if got.Status.PodName != "" || workspaceReady == nil || workspaceReady.Status != metav1.ConditionFalse || workspaceReady.Reason != "PVCAuthorityChanged" || workspaceReady.ObservedGeneration != got.Generation { + t.Fatalf("existing Pod repair retained stale PVC authority: status=%#v WorkspaceReady=%#v", got.Status, workspaceReady) + } +} + + func TestSessionFailsClosedWhenAnyOMPReferenceIsMissing(t *testing.T) { for _, test := range []struct { name string @@ -1586,14 +1634,6 @@ func TestSessionDependencyRevocationCleansOwnedResourcesAndConvergesAfterRestart {name: "missing OMP ConfigMap", conditionType: "RuntimeConfigured", wantReason: "OMPConfigMapNotFound", revoke: func(ctx context.Context, c client.Client) error { return c.Delete(ctx, &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "omp-runtime-config", Namespace: "team"}}) }}, - {name: "invalid OMP credential Secret", conditionType: "RuntimeConfigured", wantReason: "OMPCredentialSecretInvalid", revoke: func(ctx context.Context, c client.Client) error { - var secret corev1.Secret - if err := c.Get(ctx, types.NamespacedName{Namespace: "team", Name: "omp-runtime-credential"}, &secret); err != nil { - return err - } - delete(secret.Data, "MODEL_API_KEY") - return c.Update(ctx, &secret) - }}, {name: "mismatched Host storage class", conditionType: "WorkspaceReady", wantReason: "StorageClassMismatch", revoke: func(ctx context.Context, c client.Client) error { otherClass := &storagev1.StorageClass{ObjectMeta: metav1.ObjectMeta{Name: "other-rwx", Annotations: map[string]string{clusterv1alpha1.RWXStorageClassAnnotation: string(corev1.ReadWriteMany)}}, Provisioner: "example.invalid/csi"} if err := c.Create(ctx, otherClass); err != nil { From 20e98781f966ed9b17f1b990ba521231a44e7764 Mon Sep 17 00:00:00 2001 From: usr-bin-roygbiv Date: Wed, 22 Jul 2026 04:39:43 +0000 Subject: [PATCH 28/29] fix(operator): gate child repairs on PVC authority --- .../cluster-operator/controllers/reconciler_test.go | 8 ++++---- .../controllers/session_controller.go | 11 +++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 40cc9b02..2c9db16f 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -501,7 +501,7 @@ func TestSessionFailsClosedWhenAnyOMPReferenceIsMissing(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -563,7 +563,7 @@ func TestSessionCredentialModeUpgradeStopsExistingAuthorityAndClearsRoute(t *tes workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -625,7 +625,7 @@ func TestSessionRejectsCredentialBearingModelsConfiguration(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() @@ -676,7 +676,7 @@ func TestSessionRejectsCredentialBearingSettingsConfiguration(t *testing.T) { workspace.Status.PVCName = "workspace-a-data" pvc := &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: workspace.Status.PVCName, Namespace: "team"}, - Spec: corev1.PersistentVolumeClaimSpec{AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, + Spec: corev1.PersistentVolumeClaimSpec{StorageClassName: ptr("portable-rwx"), AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}}, Status: corev1.PersistentVolumeClaimStatus{Phase: corev1.ClaimBound}, } session := testSession() diff --git a/packages/cluster-operator/controllers/session_controller.go b/packages/cluster-operator/controllers/session_controller.go index ed45cb6c..345dbeb0 100644 --- a/packages/cluster-operator/controllers/session_controller.go +++ b/packages/cluster-operator/controllers/session_controller.go @@ -421,6 +421,17 @@ func (r *SessionReconciler) Reconcile(ctx context.Context, request ctrl.Request) } return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, true, "RuntimeConfigured", reason, message) } + reason, message, err = r.authoritativePVCValidation(ctx, &workspace, &pvc, host.Spec.StorageClassName) + if err != nil { + return ctrl.Result{}, err + } + if reason != "" { + if err := r.deleteOwnedSessionResources(ctx, &session); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, r.updateSessionFailure(ctx, &session, true, false, "WorkspaceReady", reason, message) + } + serviceName := SessionServiceName(&session) podName := SessionPodName(&session) From 4f899e8794737e2e85251d244589551922f5c584 Mon Sep 17 00:00:00 2001 From: Wolfgang Schoenberger <221313372+wolfiesch@users.noreply.github.com> Date: Wed, 22 Jul 2026 03:33:19 -0700 Subject: [PATCH 29/29] test(operator): cover replacement delete races --- .../controllers/reconciler_test.go | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/packages/cluster-operator/controllers/reconciler_test.go b/packages/cluster-operator/controllers/reconciler_test.go index 1271efa4..eccef0a5 100644 --- a/packages/cluster-operator/controllers/reconciler_test.go +++ b/packages/cluster-operator/controllers/reconciler_test.go @@ -2,6 +2,7 @@ package controllers_test import ( "context" + "errors" "reflect" "strings" "testing" @@ -1503,6 +1504,55 @@ func TestSessionDeletionUsesAuthoritativeChildReader(t *testing.T) { } } +func TestSessionDeletionPreconditionsProtectSameNameReplacement(t *testing.T) { + ctx := context.Background() + scheme := testScheme(t) + session := testSession() + session.UID = "session-uid" + session.Finalizers = []string{clusterv1alpha1.SessionFinalizer} + pod, service := ownedSessionResources(session) + pod.UID = "pod-uid-a" + pod.ResourceVersion = "7" + service.UID = "service-uid-a" + service.ResourceVersion = "8" + base := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&clusterv1alpha1.T4Session{}). + WithObjects(session, pod, service).Build() + var observedPod corev1.Pod + if err := base.Get(ctx, client.ObjectKeyFromObject(pod), &observedPod); err != nil { + t.Fatal(err) + } + if observedPod.UID == "" || observedPod.ResourceVersion == "" { + t.Fatalf("fake client discarded delete precondition identity: %#v", observedPod.ObjectMeta) + } + if err := base.Delete(ctx, session); err != nil { + t.Fatal(err) + } + racingClient := &replaceBeforeDeleteClient{Client: base, raceKind: "Pod", replacementUID: "pod-uid-b"} + r := configuredSessionReconciler(racingClient, scheme) + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(session)}); !apierrors.IsConflict(err) { + t.Fatalf("same-name replacement did not conflict with the stale delete: %v", err) + } + if !racingClient.raced || racingClient.observedUID != observedPod.UID || racingClient.observedResourceVersion != observedPod.ResourceVersion { + t.Fatalf("delete did not carry both observed preconditions: raced=%t uid=%q resourceVersion=%q", racingClient.raced, racingClient.observedUID, racingClient.observedResourceVersion) + } + var replacement corev1.Pod + if err := base.Get(ctx, client.ObjectKeyFromObject(pod), &replacement); err != nil { + t.Fatalf("same-name replacement Pod did not survive stale delete: %v", err) + } + if replacement.UID != racingClient.replacementUID { + t.Fatalf("surviving Pod UID = %q, want replacement %q", replacement.UID, racingClient.replacementUID) + } + var waiting clusterv1alpha1.T4Session + if err := base.Get(ctx, client.ObjectKeyFromObject(session), &waiting); err != nil { + t.Fatalf("session finalizer advanced after stale child delete: %v", err) + } + available := findCondition(waiting.Status.Conditions, "Available") + if !contains(waiting.Finalizers, clusterv1alpha1.SessionFinalizer) || waiting.Status.Phase != clusterv1alpha1.InfrastructureTerminating || available == nil || available.Reason != "Terminating" { + t.Fatalf("session advanced after stale child delete: %#v", waiting) + } +} + func TestSessionDependencyCleanupUsesAuthoritativeChildReader(t *testing.T) { ctx := context.Background() scheme := testScheme(t) @@ -2396,6 +2446,47 @@ type createAlreadyExistsClient struct { hideWinnerFromCache bool } +type replaceBeforeDeleteClient struct { + client.Client + raceKind string + replacementUID types.UID + raced bool + observedUID types.UID + observedResourceVersion string +} + +func (c *replaceBeforeDeleteClient) Delete(ctx context.Context, object client.Object, options ...client.DeleteOption) error { + if c.raced { + return c.Client.Delete(ctx, object, options...) + } + if _, isPod := object.(*corev1.Pod); !isPod || c.raceKind != "Pod" { + return c.Client.Delete(ctx, object, options...) + } + deleteOptions := (&client.DeleteOptions{}).ApplyOptions(options) + if deleteOptions.Preconditions != nil { + if deleteOptions.Preconditions.UID != nil { + c.observedUID = *deleteOptions.Preconditions.UID + } + if deleteOptions.Preconditions.ResourceVersion != nil { + c.observedResourceVersion = *deleteOptions.Preconditions.ResourceVersion + } + } + c.raced = true + if err := c.Client.Delete(ctx, object); err != nil { + return err + } + replacement := object.DeepCopyObject().(client.Object) + replacement.SetUID(c.replacementUID) + replacement.SetResourceVersion("") + replacement.SetDeletionTimestamp(nil) + replacement.SetFinalizers(nil) + replacement.SetOwnerReferences(nil) + if err := c.Client.Create(ctx, replacement); err != nil { + return err + } + return apierrors.NewConflict(schema.GroupResource{Resource: "pods"}, object.GetName(), errors.New("delete preconditions no longer match replacement")) +} + func (c *createAlreadyExistsClient) Get(ctx context.Context, key client.ObjectKey, object client.Object, options ...client.GetOption) error { if c.hideWinnerFromCache && c.winner != nil && key == client.ObjectKeyFromObject(c.winner) { switch object.(type) {