diff --git a/internal/controller/function_controller.go b/internal/controller/function_controller.go index 8d1246c..a7780be 100644 --- a/internal/controller/function_controller.go +++ b/internal/controller/function_controller.go @@ -32,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/events" + "k8s.io/client-go/util/retry" "k8s.io/utils/ptr" funcfn "knative.dev/func/pkg/functions" ctrl "sigs.k8s.io/controller-runtime" @@ -116,10 +117,38 @@ func (r *FunctionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c return ctrl.Result{}, reconcileErr } + if err := r.removeFuncAnnotations(ctx, function); err != nil { + logger.Error(err, "Failed to remove func annotations") + return ctrl.Result{}, err + } + logger.Info("Reconciliation complete") return ctrl.Result{}, nil } +func (r *FunctionReconciler) removeFuncAnnotations(ctx context.Context, function *v1alpha1.Function) error { + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + latest := &v1alpha1.Function{} + if err := r.Get(ctx, types.NamespacedName{Name: function.Name, Namespace: function.Namespace}, latest); err != nil { + return err + } + + if !hasFuncAnnotations(latest) { + return nil + } + + annotations := latest.GetAnnotations() + for key := range annotations { + if strings.HasPrefix(key, funcAnnotationPrefix) { + delete(annotations, key) + } + } + + latest.SetAnnotations(annotations) + return r.Update(ctx, latest) + }) +} + func (r *FunctionReconciler) reconcile(ctx context.Context, function *v1alpha1.Function) error { // Initialize conditions to start fresh each reconcile function.InitializeConditions() @@ -525,10 +554,11 @@ func (r *FunctionReconciler) isDeployed(ctx context.Context, name, namespace str // SetupWithManager sets up the controller with the Manager. func (r *FunctionReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - // Only reconcile Functions when their spec changes (not on status updates). - // This predicate is applied to For() instead of WithEventFilter() to ensure - // it doesn't filter out ConfigMap-triggered reconciliations. - For(&v1alpha1.Function{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). + // Reconcile Functions on spec changes (generation change) or when + // "functions.knative.dev/" annotations are present. This predicate is applied + // to For() instead of WithEventFilter() to ensure it doesn't filter out + // ConfigMap-triggered reconciliations. + For(&v1alpha1.Function{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, FuncAnnotationChangedPredicate{}))). Watches( &v1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.findFunctionsForConfigMap), diff --git a/internal/controller/function_controller_test.go b/internal/controller/function_controller_test.go index 4532712..e102347 100644 --- a/internal/controller/function_controller_test.go +++ b/internal/controller/function_controller_test.go @@ -79,8 +79,10 @@ var _ = Describe("Function Controller", func() { type reconcileTestCase struct { spec functionsdevv1alpha1.FunctionSpec + annotations map[string]string configureMocks func(*funccli.MockManager, *git.MockManager) statusChecks func(*functionsdevv1alpha1.FunctionStatus) + functionChecks func(*functionsdevv1alpha1.Function) operatorConfig map[string]string } @@ -90,6 +92,16 @@ var _ = Describe("Function Controller", func() { err := createFunctionResource(resourceName, resourceNamespace, tc.spec) Expect(err).NotTo(HaveOccurred()) + if tc.annotations != nil { + By("Adding annotations to the Function") + f := &functionsdevv1alpha1.Function{} + err = k8sClient.Get(ctx, typeNamespacedName, f) + Expect(err).NotTo(HaveOccurred()) + f.SetAnnotations(tc.annotations) + err = k8sClient.Update(ctx, f) + Expect(err).NotTo(HaveOccurred()) + } + By("Setting up mocks") funcCliManagerMock := funccli.NewMockManager(GinkgoT()) gitManagerMock := git.NewMockManager(GinkgoT()) @@ -127,6 +139,14 @@ var _ = Describe("Function Controller", func() { tc.statusChecks(&f.Status) } + + if tc.functionChecks != nil { + f := &functionsdevv1alpha1.Function{} + err := k8sClient.Get(ctx, typeNamespacedName, f) + Expect(err).NotTo(HaveOccurred()) + + tc.functionChecks(f) + } }, Entry("should deploy when middleware update required", reconcileTestCase{ spec: defaultSpec, @@ -397,6 +417,52 @@ var _ = Describe("Function Controller", func() { Expect(messages).ToNot(ContainElement(ContainSubstring("Middleware updated"))) }, }), + Entry("should remove func annotations after successful reconcile", reconcileTestCase{ + spec: defaultSpec, + annotations: map[string]string{ + "functions.knative.dev/rebuild": "true", + "other-annotation": "keep-me", + }, + configureMocks: func(funcMock *funccli.MockManager, gitMock *git.MockManager) { + funcMock.EXPECT().Describe(mock.Anything, functionName, resourceNamespace).Return(functions.Instance{ + Middleware: functions.Middleware{ + Version: "v1.0.0", + }, + }, nil) + funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) + funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) + + gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) + }, + functionChecks: func(f *functionsdevv1alpha1.Function) { + annotations := f.GetAnnotations() + Expect(annotations).NotTo(HaveKey("functions.knative.dev/rebuild")) + Expect(annotations).To(HaveKeyWithValue("other-annotation", "keep-me")) + }, + }), + Entry("should remove multiple func annotations after successful reconcile", reconcileTestCase{ + spec: defaultSpec, + annotations: map[string]string{ + "functions.knative.dev/rebuild": "true", + "functions.knative.dev/reason": "config-change", + }, + configureMocks: func(funcMock *funccli.MockManager, gitMock *git.MockManager) { + funcMock.EXPECT().Describe(mock.Anything, functionName, resourceNamespace).Return(functions.Instance{ + Middleware: functions.Middleware{ + Version: "v1.0.0", + }, + }, nil) + funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil) + funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil) + + gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil) + }, + functionChecks: func(f *functionsdevv1alpha1.Function) { + annotations := f.GetAnnotations() + Expect(annotations).NotTo(HaveKey("functions.knative.dev/rebuild")) + Expect(annotations).NotTo(HaveKey("functions.knative.dev/reason")) + }, + }), Entry("should set ServiceReady condition to false with unknown reason when ready status is empty", reconcileTestCase{ spec: defaultSpec, configureMocks: func(funcMock *funccli.MockManager, gitMock *git.MockManager) { diff --git a/internal/controller/function_predicate.go b/internal/controller/function_predicate.go new file mode 100644 index 0000000..8e39488 --- /dev/null +++ b/internal/controller/function_predicate.go @@ -0,0 +1,49 @@ +/* +Copyright 2025. + +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 controller + +import ( + "strings" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +const funcAnnotationPrefix = "functions.knative.dev/" + +// FuncAnnotationChangedPredicate triggers reconciliation when annotations +// with the "functions.knative.dev/" prefix are added or changed. +type FuncAnnotationChangedPredicate struct { + predicate.Funcs +} + +func (FuncAnnotationChangedPredicate) Update(e event.UpdateEvent) bool { + if e.ObjectNew == nil { + return false + } + return hasFuncAnnotations(e.ObjectNew) +} + +func hasFuncAnnotations(obj client.Object) bool { + for key := range obj.GetAnnotations() { + if strings.HasPrefix(key, funcAnnotationPrefix) { + return true + } + } + return false +} diff --git a/internal/controller/function_predicate_test.go b/internal/controller/function_predicate_test.go new file mode 100644 index 0000000..351358b --- /dev/null +++ b/internal/controller/function_predicate_test.go @@ -0,0 +1,132 @@ +/* +Copyright 2025. + +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 controller + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/event" + + "github.com/functions-dev/func-operator/api/v1alpha1" +) + +var _ = Describe("FuncAnnotationChangedPredicate", func() { + var p FuncAnnotationChangedPredicate + + BeforeEach(func() { + p = FuncAnnotationChangedPredicate{} + }) + + Context("Update", func() { + It("should trigger when func annotation is present", func() { + result := p.Update(event.UpdateEvent{ + ObjectOld: &v1alpha1.Function{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + }, + ObjectNew: &v1alpha1.Function{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + Annotations: map[string]string{ + "functions.knative.dev/rebuild": "true", + }, + }, + }, + }) + Expect(result).To(BeTrue()) + }) + + It("should trigger when multiple func annotations are present", func() { + result := p.Update(event.UpdateEvent{ + ObjectOld: &v1alpha1.Function{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + }, + ObjectNew: &v1alpha1.Function{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + Annotations: map[string]string{ + "functions.knative.dev/rebuild": "true", + "functions.knative.dev/reason": "config-change", + }, + }, + }, + }) + Expect(result).To(BeTrue()) + }) + + It("should not trigger on non-func annotation change", func() { + result := p.Update(event.UpdateEvent{ + ObjectOld: &v1alpha1.Function{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + }, + ObjectNew: &v1alpha1.Function{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + Annotations: map[string]string{ + "some-other-annotation": "value", + }, + }, + }, + }) + Expect(result).To(BeFalse()) + }) + + It("should not trigger when nothing changed", func() { + result := p.Update(event.UpdateEvent{ + ObjectOld: &v1alpha1.Function{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + }, + ObjectNew: &v1alpha1.Function{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + }, + }) + Expect(result).To(BeFalse()) + }) + + It("should not trigger when new object is nil", func() { + result := p.Update(event.UpdateEvent{ + ObjectOld: &v1alpha1.Function{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + }, + ObjectNew: nil, + }) + Expect(result).To(BeFalse()) + }) + }) + + Context("Create", func() { + It("should trigger on create", func() { + result := p.Create(event.CreateEvent{ + Object: &v1alpha1.Function{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + }, + }) + Expect(result).To(BeTrue()) + }) + }) + + Context("Delete", func() { + It("should trigger on delete", func() { + result := p.Delete(event.DeleteEvent{ + Object: &v1alpha1.Function{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + }, + }) + Expect(result).To(BeTrue()) + }) + }) +})