|
| 1 | +// Copyright 2026 StreamNative |
| 2 | +// |
| 3 | +// Licensed under the Apache License, Version 2.0 (the "License"); |
| 4 | +// you may not use this file except in compliance with the License. |
| 5 | +// You may obtain a copy of the License at |
| 6 | +// |
| 7 | +// http://www.apache.org/licenses/LICENSE-2.0 |
| 8 | +// |
| 9 | +// Unless required by applicable law or agreed to in writing, software |
| 10 | +// distributed under the License is distributed on an "AS IS" BASIS, |
| 11 | +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
| 12 | +// See the License for the specific language governing permissions and |
| 13 | +// limitations under the License. |
| 14 | + |
| 15 | +package connection |
| 16 | + |
| 17 | +import ( |
| 18 | + "context" |
| 19 | + "testing" |
| 20 | + |
| 21 | + apierrors "k8s.io/apimachinery/pkg/api/errors" |
| 22 | + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" |
| 23 | + "sigs.k8s.io/controller-runtime/pkg/client" |
| 24 | + "sigs.k8s.io/controller-runtime/pkg/client/fake" |
| 25 | + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" |
| 26 | + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" |
| 27 | + |
| 28 | + resourcev1alpha1 "github.com/streamnative/pulsar-resources-operator/api/v1alpha1" |
| 29 | +) |
| 30 | + |
| 31 | +// TestRemoveFinalizerUsesLiveReaderResourceVersion proves the finalizer write is driven by the |
| 32 | +// reader, not by the caller's (possibly stale) in-hand object. A reader that keeps returning a |
| 33 | +// stale resourceVersion — as the manager's cached client can under informer lag — makes every |
| 34 | +// retry 409, so the removal fails (the original stuck-Terminating symptom). The same removal |
| 35 | +// with a live reader reads a fresh resourceVersion and succeeds. In production the reader is the |
| 36 | +// uncached mgr.GetAPIReader(), so a lagging informer cache cannot wedge deletion. |
| 37 | +// |
| 38 | +// This is the case ordinary single-fake-client tests cannot exercise: here the reader and the |
| 39 | +// write store are deliberately decoupled to simulate cache lag. |
| 40 | +func TestRemoveFinalizerUsesLiveReaderResourceVersion(t *testing.T) { |
| 41 | + scheme := newSourceTestScheme(t) |
| 42 | + topic := &resourcev1alpha1.PulsarTopic{ |
| 43 | + ObjectMeta: metav1.ObjectMeta{ |
| 44 | + Name: "t", |
| 45 | + Namespace: "ns", |
| 46 | + Finalizers: []string{resourcev1alpha1.FinalizerName}, |
| 47 | + }, |
| 48 | + } |
| 49 | + writer := fake.NewClientBuilder().WithScheme(scheme).WithObjects(topic).Build() |
| 50 | + key := client.ObjectKeyFromObject(topic) |
| 51 | + |
| 52 | + // Snapshot the object at its current resourceVersion, then advance the live RV out of band. |
| 53 | + staleObj := &resourcev1alpha1.PulsarTopic{} |
| 54 | + if err := writer.Get(context.Background(), key, staleObj); err != nil { |
| 55 | + t.Fatalf("get snapshot: %v", err) |
| 56 | + } |
| 57 | + bumped := staleObj.DeepCopy() |
| 58 | + bumped.Annotations = map[string]string{"bump": "1"} |
| 59 | + if err := writer.Update(context.Background(), bumped); err != nil { |
| 60 | + t.Fatalf("bump live resourceVersion: %v", err) |
| 61 | + } |
| 62 | + |
| 63 | + // staleReader always returns the pre-bump snapshot — a persistently stale resourceVersion. |
| 64 | + staleReader := interceptor.NewClient(writer, interceptor.Funcs{ |
| 65 | + Get: func(_ context.Context, _ client.WithWatch, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) error { |
| 66 | + staleObj.DeepCopyInto(obj.(*resourcev1alpha1.PulsarTopic)) |
| 67 | + return nil |
| 68 | + }, |
| 69 | + }) |
| 70 | + |
| 71 | + // Persistently stale reader: every retry re-reads the stale RV and the Update conflicts. |
| 72 | + if err := removeFinalizer(context.Background(), staleReader, writer, staleObj.DeepCopy(), |
| 73 | + resourcev1alpha1.FinalizerName); !apierrors.IsConflict(err) { |
| 74 | + t.Fatalf("expected a 409 conflict from a persistently stale reader, got: %v", err) |
| 75 | + } |
| 76 | + |
| 77 | + // Live (uncached) reader: the removal reads a fresh RV and succeeds. |
| 78 | + if err := removeFinalizer(context.Background(), writer, writer, staleObj.DeepCopy(), |
| 79 | + resourcev1alpha1.FinalizerName); err != nil { |
| 80 | + t.Fatalf("removeFinalizer with a live reader: %v", err) |
| 81 | + } |
| 82 | + got := &resourcev1alpha1.PulsarTopic{} |
| 83 | + if err := writer.Get(context.Background(), key, got); err != nil { |
| 84 | + t.Fatalf("get after removal: %v", err) |
| 85 | + } |
| 86 | + if controllerutil.ContainsFinalizer(got, resourcev1alpha1.FinalizerName) { |
| 87 | + t.Fatalf("operator finalizer should have been removed, got %v", got.Finalizers) |
| 88 | + } |
| 89 | +} |
| 90 | + |
| 91 | +// TestEnsureFinalizerGuardSkipsReadWhenPresent verifies the steady-state fast path: when the |
| 92 | +// cached object already carries the finalizer, ensureFinalizer issues no API calls at all, so |
| 93 | +// routing the read through the uncached API reader adds no per-reconcile apiserver load on the |
| 94 | +// common path. |
| 95 | +func TestEnsureFinalizerGuardSkipsReadWhenPresent(t *testing.T) { |
| 96 | + scheme := newSourceTestScheme(t) |
| 97 | + writer := fake.NewClientBuilder().WithScheme(scheme).Build() |
| 98 | + panicReader := interceptor.NewClient(writer, interceptor.Funcs{ |
| 99 | + Get: func(_ context.Context, _ client.WithWatch, _ client.ObjectKey, _ client.Object, _ ...client.GetOption) error { |
| 100 | + panic("reader.Get must not be called when the finalizer is already present") |
| 101 | + }, |
| 102 | + }) |
| 103 | + |
| 104 | + obj := &resourcev1alpha1.PulsarTopic{ |
| 105 | + ObjectMeta: metav1.ObjectMeta{ |
| 106 | + Name: "t", |
| 107 | + Namespace: "ns", |
| 108 | + Finalizers: []string{resourcev1alpha1.FinalizerName}, |
| 109 | + }, |
| 110 | + } |
| 111 | + if err := ensureFinalizer(context.Background(), panicReader, writer, obj, resourcev1alpha1.FinalizerName); err != nil { |
| 112 | + t.Fatalf("ensureFinalizer (guard fast path) returned error: %v", err) |
| 113 | + } |
| 114 | +} |
0 commit comments