diff --git a/pkg/controllers/deploy/runtime_controllers.go b/pkg/controllers/deploy/runtime_controllers.go index 1720e28eaa1..b13d1c3e829 100644 --- a/pkg/controllers/deploy/runtime_controllers.go +++ b/pkg/controllers/deploy/runtime_controllers.go @@ -61,8 +61,9 @@ var ( resolveDefaultPrecheckFuncs = func() map[string]CheckFunc { return filterOutDisabledRuntimes(defaultPrecheckFuncs) } - precheckFuncs map[string]CheckFunc - precheckFuncsMu sync.Mutex + cachedPrecheckFuncs map[string]CheckFunc + precheckFuncs map[string]CheckFunc + precheckFuncsMu sync.Mutex ) func setPrecheckFunc(checks map[string]CheckFunc) { @@ -80,7 +81,11 @@ func getPrecheckFuncs() map[string]CheckFunc { return clonePrecheckFuncs(precheckFuncs) } - return clonePrecheckFuncs(resolveDefaultPrecheckFuncs()) + if cachedPrecheckFuncs == nil { + cachedPrecheckFuncs = resolveDefaultPrecheckFuncs() + } + + return clonePrecheckFuncs(cachedPrecheckFuncs) } func clonePrecheckFuncs(checks map[string]CheckFunc) map[string]CheckFunc { diff --git a/pkg/controllers/deploy/runtime_controllers_test.go b/pkg/controllers/deploy/runtime_controllers_test.go index e7c338f2d34..7990bdcf956 100644 --- a/pkg/controllers/deploy/runtime_controllers_test.go +++ b/pkg/controllers/deploy/runtime_controllers_test.go @@ -42,21 +42,35 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -const controllerNamespace = common.NamespaceFluidSystem +const ( + controllerNamespace = common.NamespaceFluidSystem + customControllerName = "custom-controller" +) var _ = Describe("runtime controller scaleout", func() { var originalPodNamespace string var hadOriginalPodNamespace bool var originalResolveDefaultPrecheckFuncs func() map[string]CheckFunc + var originalCachedPrecheckFuncs map[string]CheckFunc + var originalPrecheckFuncs map[string]CheckFunc BeforeEach(func() { originalPodNamespace, hadOriginalPodNamespace = os.LookupEnv(common.MyPodNamespace) + + precheckFuncsMu.Lock() originalResolveDefaultPrecheckFuncs = resolveDefaultPrecheckFuncs + originalCachedPrecheckFuncs = clonePrecheckFuncs(cachedPrecheckFuncs) + originalPrecheckFuncs = clonePrecheckFuncs(precheckFuncs) + precheckFuncsMu.Unlock() }) AfterEach(func() { - setPrecheckFunc(nil) + precheckFuncsMu.Lock() resolveDefaultPrecheckFuncs = originalResolveDefaultPrecheckFuncs + cachedPrecheckFuncs = clonePrecheckFuncs(originalCachedPrecheckFuncs) + precheckFuncs = clonePrecheckFuncs(originalPrecheckFuncs) + precheckFuncsMu.Unlock() + restoreEnv(common.MyPodNamespace, originalPodNamespace, hadOriginalPodNamespace) }) @@ -164,10 +178,49 @@ var _ = Describe("runtime controller scaleout", func() { Expect(*stored.Spec.Replicas).To(Equal(int32(1))) }) - It("keeps package-global precheck functions isolated between tests", func() { + It("uses injected precheck funcs without resolving defaults", func() { + fakeClient := newFakeClient(newDeployment(customControllerName, 0, nil)) + + resolverCalls := 0 + precheckCalls := 0 + + precheckFuncsMu.Lock() + resolveDefaultPrecheckFuncs = func() map[string]CheckFunc { + resolverCalls++ + return map[string]CheckFunc{} + } + precheckFuncsMu.Unlock() + + setPrecheckFunc(map[string]CheckFunc{ + customControllerName: func(client.Client, types.NamespacedName) (bool, error) { + precheckCalls++ + return true, nil + }, + }) + + controllerName, scaled, err := ScaleoutRuntimeControllerOnDemand(fakeClient, types.NamespacedName{ + Namespace: corev1.NamespaceDefault, + Name: "dataset", + }, fake.NullLogger()) + + Expect(err).NotTo(HaveOccurred()) + Expect(controllerName).To(Equal(customControllerName)) + Expect(scaled).To(BeTrue()) + Expect(resolverCalls).To(Equal(0)) + Expect(precheckCalls).To(Equal(1)) + + stored := &appsv1.Deployment{} + Expect(fakeClient.Get(context.TODO(), types.NamespacedName{ + Namespace: controllerNamespace, + Name: customControllerName, + }, stored)).To(Succeed()) + Expect(*stored.Spec.Replicas).To(Equal(int32(1))) + }) + + It("returns a not found error when an injected controller has no deployment", func() { fakeClient := newFakeClient(controllerDeployments()...) setPrecheckFunc(map[string]CheckFunc{ - "custom-controller": func(client.Client, types.NamespacedName) (bool, error) { + customControllerName: func(client.Client, types.NamespacedName) (bool, error) { return true, nil }, }) @@ -178,7 +231,8 @@ var _ = Describe("runtime controller scaleout", func() { }, fake.NullLogger()) Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError(ContainSubstring("custom-controller"))) + Expect(err).To(MatchError(ContainSubstring(customControllerName))) + Expect(err).To(MatchError(ContainSubstring("not found"))) Expect(controllerName).To(BeEmpty()) Expect(scaled).To(BeFalse()) }) @@ -203,7 +257,36 @@ var _ = Describe("runtime controller scaleout", func() { Expect(getPrecheckFuncs()).To(HaveKey("alluxioruntime-controller")) }) - It("does not pin discovery-filtered defaults into package-global state", func() { + It("resolves defaults lazily and caches the result", func() { + const lazyControllerName = "lazy-controller" + + calls := 0 + check := func(client.Client, types.NamespacedName) (bool, error) { + return false, nil + } + + precheckFuncsMu.Lock() + cachedPrecheckFuncs = nil + precheckFuncs = nil + resolveDefaultPrecheckFuncs = func() map[string]CheckFunc { + calls++ + return map[string]CheckFunc{lazyControllerName: check} + } + precheckFuncsMu.Unlock() + + checks := getPrecheckFuncs() + Expect(calls).To(Equal(1)) + Expect(checks[lazyControllerName]).NotTo(BeNil()) + + checksAgain := getPrecheckFuncs() + Expect(calls).To(Equal(1)) + Expect(checksAgain[lazyControllerName]).NotTo(BeNil()) + + delete(checks, lazyControllerName) + Expect(getPrecheckFuncs()[lazyControllerName]).NotTo(BeNil()) + }) + + It("does not pin resolved defaults into package-global state", func() { resolveDefaultPrecheckFuncs = func() map[string]CheckFunc { return runtimePrecheckFuncs() } diff --git a/pkg/controllers/v1alpha1/dataset/dataset_reconciler_test.go b/pkg/controllers/v1alpha1/dataset/dataset_reconciler_test.go new file mode 100644 index 00000000000..6e11ec4d95e --- /dev/null +++ b/pkg/controllers/v1alpha1/dataset/dataset_reconciler_test.go @@ -0,0 +1,508 @@ +/* +Copyright 2020 The Fluid Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package dataset + +import ( + "context" + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1" + "github.com/fluid-cloudnative/fluid/pkg/utils/fake" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +func newDatasetScheme() *runtime.Scheme { + s := runtime.NewScheme() + _ = datav1alpha1.AddToScheme(s) + _ = corev1.AddToScheme(s) + return s +} + +func newFakeDatasetClient(objs ...runtime.Object) client.Client { + return fake.NewFakeClientWithScheme(newDatasetScheme(), objs...) +} + +func newTestReconciler(objs ...runtime.Object) *DatasetReconciler { + c := newFakeDatasetClient(objs...) + return &DatasetReconciler{ + Client: c, + Recorder: record.NewFakeRecorder(100), + Log: zap.New(zap.UseDevMode(true)), + Scheme: newDatasetScheme(), + ResyncPeriod: 5 * time.Second, + } +} + +func newTestReconcilerWithInterceptor(interceptorFuncs interceptor.Funcs, objs ...runtime.Object) *DatasetReconciler { + s := newDatasetScheme() + var clientObjs []client.Object + for _, obj := range objs { + if co, ok := obj.(client.Object); ok { + clientObjs = append(clientObjs, co) + } + } + c := ctrlfake.NewClientBuilder(). + WithScheme(s). + WithRuntimeObjects(objs...). + WithStatusSubresource(clientObjs...). + WithInterceptorFuncs(interceptorFuncs). + Build() + return &DatasetReconciler{ + Client: c, + Recorder: record.NewFakeRecorder(100), + Log: zap.New(zap.UseDevMode(true)), + Scheme: s, + ResyncPeriod: 5 * time.Second, + } +} + +func makeReconcileCtx(r *DatasetReconciler, ds datav1alpha1.Dataset) reconcileRequestContext { + return reconcileRequestContext{ + Context: context.Background(), + Log: r.Log.WithValues("dataset", ds.Namespace+"/"+ds.Name), + NamespacedName: types.NamespacedName{ + Namespace: ds.Namespace, + Name: ds.Name, + }, + Dataset: ds, + } +} + +var _ = Describe("DatasetReconciler (fake client)", func() { + Describe("ControllerName", func() { + It("returns DatasetController", func() { + r := newTestReconciler() + Expect(r.ControllerName()).To(Equal("DatasetController")) + Expect(r.ControllerName()).To(Equal(controllerName)) + }) + }) + + Describe("addFinalizerAndRequeue", func() { + It("adds finalizer and requeues", func() { + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{Name: "af", Namespace: "default"}, + } + r := newTestReconciler(&ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.addFinalizerAndRequeue(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{Requeue: true})) + + stored := &datav1alpha1.Dataset{} + Expect(r.Get(ctx, types.NamespacedName{Namespace: "default", Name: "af"}, stored)).To(Succeed()) + Expect(stored.Finalizers).To(ContainElement(finalizer)) + }) + }) + + Describe("reconcileDataset", func() { + It("returns error for invalid DNS1035 name starting with digit", func() { + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{Name: "1invalid", Namespace: "default"}, + } + r := newTestReconciler(&ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDataset(ctx, false) + Expect(err).To(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + + It("adds finalizer when dataset has none", func() { + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{Name: "no-fin", Namespace: "default"}, + } + r := newTestReconciler(&ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDataset(ctx, false) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{Requeue: true})) + + stored := &datav1alpha1.Dataset{} + Expect(r.Get(ctx, types.NamespacedName{Namespace: "default", Name: "no-fin"}, stored)).To(Succeed()) + Expect(stored.Finalizers).To(ContainElement(finalizer)) + }) + + It("advances phase from None to NotBound when finalizer already present", func() { + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "none-phase", + Namespace: "default", + Finalizers: []string{finalizer}, + }, + Status: datav1alpha1.DatasetStatus{Phase: datav1alpha1.NoneDatasetPhase}, + } + r := newTestReconciler(&ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDataset(ctx, false) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + + It("requeues after ResyncPeriod when needRequeue is true", func() { + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "needs-requeue", + Namespace: "default", + Finalizers: []string{finalizer}, + }, + Status: datav1alpha1.DatasetStatus{Phase: datav1alpha1.NotBoundDatasetPhase}, + } + r := newTestReconciler(&ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDataset(ctx, true) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(r.ResyncPeriod)) + }) + + It("delegates to reconcileDatasetDeletion when DeletionTimestamp is set", func() { + now := metav1.Now() + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "with-deletion", + Namespace: "default", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + } + r := newTestReconciler(&ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDataset(ctx, false) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + + It("returns error when CheckReferenceDataset fails (multiple dataset mounts)", func() { + // Two dataset:// mounts → CheckReferenceDataset returns an error + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multi-ref", + Namespace: "default", + Finalizers: []string{finalizer}, + }, + Spec: datav1alpha1.DatasetSpec{ + Mounts: []datav1alpha1.Mount{ + {Name: "m1", MountPoint: "dataset://default/ds1"}, + {Name: "m2", MountPoint: "dataset://default/ds2"}, + }, + }, + Status: datav1alpha1.DatasetStatus{Phase: datav1alpha1.NotBoundDatasetPhase}, + } + r := newTestReconciler(&ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDataset(ctx, false) + Expect(err).To(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + + It("creates ThinRuntime for reference dataset (single dataset:// mount)", func() { + // Single dataset:// mount → checkReferenceDataset=true → CreateRuntimeForReferenceDatasetIfNotExist + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ref-ds", + Namespace: "default", + Finalizers: []string{finalizer}, + }, + Spec: datav1alpha1.DatasetSpec{ + Mounts: []datav1alpha1.Mount{ + {Name: "m1", MountPoint: "dataset://default/physical-ds"}, + }, + }, + Status: datav1alpha1.DatasetStatus{Phase: datav1alpha1.NotBoundDatasetPhase}, + } + r := newTestReconciler(&ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDataset(ctx, false) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + + It("returns error when CreateRuntimeForReferenceDatasetIfNotExist fails", func() { + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ref-ds-create-err", + Namespace: "default", + Finalizers: []string{finalizer}, + }, + Spec: datav1alpha1.DatasetSpec{ + Mounts: []datav1alpha1.Mount{ + {Name: "m1", MountPoint: "dataset://default/physical-ds"}, + }, + }, + Status: datav1alpha1.DatasetStatus{Phase: datav1alpha1.NotBoundDatasetPhase}, + } + createErr := fmt.Errorf("injected create error") + r := newTestReconcilerWithInterceptor(interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + return createErr + }, + }, &ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDataset(ctx, false) + Expect(err).To(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + }) + + Describe("reconcileDatasetDeletion", func() { + It("removes finalizer when no pods block and no DatasetRef", func() { + now := metav1.Now() + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "del-clean", + Namespace: "default", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + } + r := newTestReconciler(&ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDatasetDeletion(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + + stored := &datav1alpha1.Dataset{} + getErr := r.Get(ctx, types.NamespacedName{Namespace: "default", Name: "del-clean"}, stored) + Expect(apierrors.IsNotFound(getErr)).To(BeTrue()) + }) + + It("requeues when DatasetRef still has live entries", func() { + now := metav1.Now() + // Create both the main dataset and the referenced dataset in the fake store. + // With the reference alive, deletion is blocked. + ref := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{Name: "ref-dataset", Namespace: "default"}, + } + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "del-with-ref", + Namespace: "default", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + Status: datav1alpha1.DatasetStatus{ + DatasetRef: []string{"default/ref-dataset"}, + }, + } + r := newTestReconciler(&ref, &ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDatasetDeletion(ctx) + Expect(err).NotTo(HaveOccurred()) + // Blocked by live reference – must requeue + Expect(result.RequeueAfter > 0 || result.Requeue).To(BeTrue()) + }) + + It("updates status and requeues when stale DatasetRef entries are pruned", func() { + now := metav1.Now() + // Only ref-alive exists; ref-gone does NOT → RemoveNotFoundDatasetRef prunes it. + // Pruned list differs from original → status update + 1s requeue. + refAlive := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{Name: "ref-alive", Namespace: "default"}, + } + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "del-prune-ref", + Namespace: "default", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + Status: datav1alpha1.DatasetStatus{ + DatasetRef: []string{"default/ref-alive", "default/ref-gone"}, + }, + } + r := newTestReconciler(&refAlive, &ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDatasetDeletion(ctx) + Expect(err).NotTo(HaveOccurred()) + // After pruning, ref-alive is still live → blocked → requeue + Expect(result.RequeueAfter > 0 || result.Requeue).To(BeTrue()) + }) + + It("requeues when a PVC blocks dataset deletion", func() { + const blockedDatasetName = "del-blocked" + + now := metav1.Now() + // Create a PVC with the Fluid annotation in the same namespace/name as the dataset. + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: blockedDatasetName, + Namespace: "default", + Annotations: map[string]string{ + "CreatedBy": "fluid", + }, + }, + } + // Create a running pod that mounts the PVC. + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "using-pod", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "data", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: blockedDatasetName, + }, + }, + }, + }, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + }, + } + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: blockedDatasetName, + Namespace: "default", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + } + r := newTestReconciler(&ds, pvc, pod) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDatasetDeletion(ctx) + // ShouldDeleteDataset returns error (pod is still using the PVC) → requeue + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter > 0 || result.Requeue).To(BeTrue()) + }) + + It("returns error when r.Update fails while removing finalizer", func() { + now := metav1.Now() + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "del-update-err", + Namespace: "default", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + } + updateErr := fmt.Errorf("injected update error") + r := newTestReconcilerWithInterceptor(interceptor.Funcs{ + Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + return updateErr + }, + }, &ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDatasetDeletion(ctx) + Expect(err).To(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + }) + + Describe("addFinalizerAndRequeue error path", func() { + It("returns error when Update fails", func() { + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{Name: "af-err", Namespace: "default"}, + } + updateErr := fmt.Errorf("injected update error") + r := newTestReconcilerWithInterceptor(interceptor.Funcs{ + Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + return updateErr + }, + }, &ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.addFinalizerAndRequeue(ctx) + Expect(err).To(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + }) + + Describe("reconcileDataset status update error path", func() { + It("returns error when status update fails for NoneDatasetPhase", func() { + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "status-err", + Namespace: "default", + Finalizers: []string{finalizer}, + }, + Status: datav1alpha1.DatasetStatus{Phase: datav1alpha1.NoneDatasetPhase}, + } + statusErr := fmt.Errorf("injected status update error") + r := newTestReconcilerWithInterceptor(interceptor.Funcs{ + SubResourceUpdate: func(ctx context.Context, c client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error { + return statusErr + }, + }, &ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDataset(ctx, false) + Expect(err).To(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + }) + + Describe("reconcileDatasetDeletion error paths", func() { + It("requeues when status update fails after datasetRef is pruned", func() { + now := metav1.Now() + // Only ref-gone listed → will be pruned → triggers status update → inject error + ds := datav1alpha1.Dataset{ + ObjectMeta: metav1.ObjectMeta{ + Name: "del-status-err", + Namespace: "default", + Finalizers: []string{finalizer}, + DeletionTimestamp: &now, + }, + Status: datav1alpha1.DatasetStatus{ + DatasetRef: []string{"default/ref-gone"}, + }, + } + statusErr := fmt.Errorf("injected status update error") + r := newTestReconcilerWithInterceptor(interceptor.Funcs{ + SubResourceUpdate: func(ctx context.Context, c client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error { + return statusErr + }, + }, &ds) + ctx := makeReconcileCtx(r, ds) + + result, err := r.reconcileDatasetDeletion(ctx) + // Status update failed → returns RequeueAfterInterval(10s) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter > 0 || result.Requeue).To(BeTrue()) + }) + }) +}) diff --git a/pkg/controllers/v1alpha1/dataset/suite_test.go b/pkg/controllers/v1alpha1/dataset/suite_test.go index b511e57141a..9240bfd2126 100644 --- a/pkg/controllers/v1alpha1/dataset/suite_test.go +++ b/pkg/controllers/v1alpha1/dataset/suite_test.go @@ -20,6 +20,7 @@ import ( "context" "os" "path/filepath" + "strconv" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,6 +47,19 @@ var k8sClient client.Client var testEnv *envtest.Environment var testCtx = context.Background() var useExistingCluster = false +var testEnvStarted = false + +const skipEnvtestStartFailureEnvVar = "FLUID_DATASET_TEST_SKIP_ENVTEST_START_FAILURE" + +func allowSkippingEnvtestStartFailure() bool { + value, ok := os.LookupEnv(skipEnvtestStartFailureEnvVar) + if !ok { + return false + } + + allowed, err := strconv.ParseBool(value) + return err == nil && allowed +} func TestAPIs(t *testing.T) { RegisterFailHandler(Fail) @@ -54,7 +68,7 @@ func TestAPIs(t *testing.T) { "Controller Suite") } -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func(ctx context.Context) { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) if env := os.Getenv("USE_EXISTING_CLUSTER"); env != "" { useExistingCluster = true @@ -67,8 +81,20 @@ var _ = BeforeSuite(func(done Done) { var err error cfg, err = testEnv.Start() - Expect(err).ToNot(HaveOccurred()) + if err != nil { + if allowSkippingEnvtestStartFailure() { + GinkgoLogr.Info( + "envtest unavailable, skipping envtest-dependent specs due to explicit opt-in", + "envVar", skipEnvtestStartFailureEnvVar, + "error", err, + ) + return + } + + Expect(err).NotTo(HaveOccurred()) + } Expect(cfg).ToNot(BeNil()) + testEnvStarted = true err = datav1alpha1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) @@ -78,11 +104,13 @@ var _ = BeforeSuite(func(done Done) { k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) Expect(err).ToNot(HaveOccurred()) Expect(k8sClient).ToNot(BeNil()) - - close(done) -}, 60) +}) var _ = AfterSuite(func() { + if !testEnvStarted { + return + } + By("tearing down the test environment") err := testEnv.Stop() Expect(err).ToNot(HaveOccurred()) @@ -108,6 +136,9 @@ var _ = Describe("dataset", func() { }) It("Should create dataset successfully", func() { + if !testEnvStarted { + Skip("envtest not available") + } By("create dataset") err := k8sClient.Create(testCtx, &dataset) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/controllers/v1alpha1/dataset/suite_test_helpers_test.go b/pkg/controllers/v1alpha1/dataset/suite_test_helpers_test.go new file mode 100644 index 00000000000..58f8dbed0fd --- /dev/null +++ b/pkg/controllers/v1alpha1/dataset/suite_test_helpers_test.go @@ -0,0 +1,29 @@ +package dataset + +import "testing" + +func TestAllowSkippingEnvtestStartFailure(t *testing.T) { + t.Run("empty value stays disabled", func(t *testing.T) { + t.Setenv(skipEnvtestStartFailureEnvVar, "") + + if allowSkippingEnvtestStartFailure() { + t.Fatalf("expected empty %s value to keep opt-in skip disabled", skipEnvtestStartFailureEnvVar) + } + }) + + t.Run("accepts explicit opt in", func(t *testing.T) { + t.Setenv(skipEnvtestStartFailureEnvVar, "true") + + if !allowSkippingEnvtestStartFailure() { + t.Fatalf("expected %s=true to enable opt-in skip", skipEnvtestStartFailureEnvVar) + } + }) + + t.Run("rejects invalid values", func(t *testing.T) { + t.Setenv(skipEnvtestStartFailureEnvVar, "sometimes") + + if allowSkippingEnvtestStartFailure() { + t.Fatalf("expected invalid %s value to keep opt-in skip disabled", skipEnvtestStartFailureEnvVar) + } + }) +}