Skip to content

Commit adc414e

Browse files
creydrcreydr
authored
Trigger reconciliation on functions.knative.dev/ annotation changes (#89)
1 parent 7c22010 commit adc414e

4 files changed

Lines changed: 281 additions & 4 deletions

File tree

‎internal/controller/function_controller.go‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"k8s.io/apimachinery/pkg/runtime"
3333
"k8s.io/apimachinery/pkg/types"
3434
"k8s.io/client-go/tools/events"
35+
"k8s.io/client-go/util/retry"
3536
"k8s.io/utils/ptr"
3637
funcfn "knative.dev/func/pkg/functions"
3738
ctrl "sigs.k8s.io/controller-runtime"
@@ -116,10 +117,38 @@ func (r *FunctionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
116117
return ctrl.Result{}, reconcileErr
117118
}
118119

120+
if err := r.removeFuncAnnotations(ctx, function); err != nil {
121+
logger.Error(err, "Failed to remove func annotations")
122+
return ctrl.Result{}, err
123+
}
124+
119125
logger.Info("Reconciliation complete")
120126
return ctrl.Result{}, nil
121127
}
122128

129+
func (r *FunctionReconciler) removeFuncAnnotations(ctx context.Context, function *v1alpha1.Function) error {
130+
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
131+
latest := &v1alpha1.Function{}
132+
if err := r.Get(ctx, types.NamespacedName{Name: function.Name, Namespace: function.Namespace}, latest); err != nil {
133+
return err
134+
}
135+
136+
if !hasFuncAnnotations(latest) {
137+
return nil
138+
}
139+
140+
annotations := latest.GetAnnotations()
141+
for key := range annotations {
142+
if strings.HasPrefix(key, funcAnnotationPrefix) {
143+
delete(annotations, key)
144+
}
145+
}
146+
147+
latest.SetAnnotations(annotations)
148+
return r.Update(ctx, latest)
149+
})
150+
}
151+
123152
func (r *FunctionReconciler) reconcile(ctx context.Context, function *v1alpha1.Function) error {
124153
// Initialize conditions to start fresh each reconcile
125154
function.InitializeConditions()
@@ -525,10 +554,11 @@ func (r *FunctionReconciler) isDeployed(ctx context.Context, name, namespace str
525554
// SetupWithManager sets up the controller with the Manager.
526555
func (r *FunctionReconciler) SetupWithManager(mgr ctrl.Manager) error {
527556
return ctrl.NewControllerManagedBy(mgr).
528-
// Only reconcile Functions when their spec changes (not on status updates).
529-
// This predicate is applied to For() instead of WithEventFilter() to ensure
530-
// it doesn't filter out ConfigMap-triggered reconciliations.
531-
For(&v1alpha1.Function{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
557+
// Reconcile Functions on spec changes (generation change) or when
558+
// "functions.knative.dev/" annotations are present. This predicate is applied
559+
// to For() instead of WithEventFilter() to ensure it doesn't filter out
560+
// ConfigMap-triggered reconciliations.
561+
For(&v1alpha1.Function{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, FuncAnnotationChangedPredicate{}))).
532562
Watches(
533563
&v1.ConfigMap{},
534564
handler.EnqueueRequestsFromMapFunc(r.findFunctionsForConfigMap),

‎internal/controller/function_controller_test.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,10 @@ var _ = Describe("Function Controller", func() {
7979

8080
type reconcileTestCase struct {
8181
spec functionsdevv1alpha1.FunctionSpec
82+
annotations map[string]string
8283
configureMocks func(*funccli.MockManager, *git.MockManager)
8384
statusChecks func(*functionsdevv1alpha1.FunctionStatus)
85+
functionChecks func(*functionsdevv1alpha1.Function)
8486
operatorConfig map[string]string
8587
}
8688

@@ -90,6 +92,16 @@ var _ = Describe("Function Controller", func() {
9092
err := createFunctionResource(resourceName, resourceNamespace, tc.spec)
9193
Expect(err).NotTo(HaveOccurred())
9294

95+
if tc.annotations != nil {
96+
By("Adding annotations to the Function")
97+
f := &functionsdevv1alpha1.Function{}
98+
err = k8sClient.Get(ctx, typeNamespacedName, f)
99+
Expect(err).NotTo(HaveOccurred())
100+
f.SetAnnotations(tc.annotations)
101+
err = k8sClient.Update(ctx, f)
102+
Expect(err).NotTo(HaveOccurred())
103+
}
104+
93105
By("Setting up mocks")
94106
funcCliManagerMock := funccli.NewMockManager(GinkgoT())
95107
gitManagerMock := git.NewMockManager(GinkgoT())
@@ -127,6 +139,14 @@ var _ = Describe("Function Controller", func() {
127139

128140
tc.statusChecks(&f.Status)
129141
}
142+
143+
if tc.functionChecks != nil {
144+
f := &functionsdevv1alpha1.Function{}
145+
err := k8sClient.Get(ctx, typeNamespacedName, f)
146+
Expect(err).NotTo(HaveOccurred())
147+
148+
tc.functionChecks(f)
149+
}
130150
},
131151
Entry("should deploy when middleware update required", reconcileTestCase{
132152
spec: defaultSpec,
@@ -397,6 +417,52 @@ var _ = Describe("Function Controller", func() {
397417
Expect(messages).ToNot(ContainElement(ContainSubstring("Middleware updated")))
398418
},
399419
}),
420+
Entry("should remove func annotations after successful reconcile", reconcileTestCase{
421+
spec: defaultSpec,
422+
annotations: map[string]string{
423+
"functions.knative.dev/rebuild": "true",
424+
"other-annotation": "keep-me",
425+
},
426+
configureMocks: func(funcMock *funccli.MockManager, gitMock *git.MockManager) {
427+
funcMock.EXPECT().Describe(mock.Anything, functionName, resourceNamespace).Return(functions.Instance{
428+
Middleware: functions.Middleware{
429+
Version: "v1.0.0",
430+
},
431+
}, nil)
432+
funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil)
433+
funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil)
434+
435+
gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil)
436+
},
437+
functionChecks: func(f *functionsdevv1alpha1.Function) {
438+
annotations := f.GetAnnotations()
439+
Expect(annotations).NotTo(HaveKey("functions.knative.dev/rebuild"))
440+
Expect(annotations).To(HaveKeyWithValue("other-annotation", "keep-me"))
441+
},
442+
}),
443+
Entry("should remove multiple func annotations after successful reconcile", reconcileTestCase{
444+
spec: defaultSpec,
445+
annotations: map[string]string{
446+
"functions.knative.dev/rebuild": "true",
447+
"functions.knative.dev/reason": "config-change",
448+
},
449+
configureMocks: func(funcMock *funccli.MockManager, gitMock *git.MockManager) {
450+
funcMock.EXPECT().Describe(mock.Anything, functionName, resourceNamespace).Return(functions.Instance{
451+
Middleware: functions.Middleware{
452+
Version: "v1.0.0",
453+
},
454+
}, nil)
455+
funcMock.EXPECT().GetLatestMiddlewareVersion(mock.Anything, mock.Anything, mock.Anything).Return("v1.0.0", nil)
456+
funcMock.EXPECT().GetMiddlewareVersion(mock.Anything, functionName, resourceNamespace).Return("v1.0.0", nil)
457+
458+
gitMock.EXPECT().CloneRepository(mock.Anything, "https://github.com/foo/bar", "", "my-branch", mock.Anything).Return(createTmpGitRepo(functions.Function{Name: "func-go"}), nil)
459+
},
460+
functionChecks: func(f *functionsdevv1alpha1.Function) {
461+
annotations := f.GetAnnotations()
462+
Expect(annotations).NotTo(HaveKey("functions.knative.dev/rebuild"))
463+
Expect(annotations).NotTo(HaveKey("functions.knative.dev/reason"))
464+
},
465+
}),
400466
Entry("should set ServiceReady condition to false with unknown reason when ready status is empty", reconcileTestCase{
401467
spec: defaultSpec,
402468
configureMocks: func(funcMock *funccli.MockManager, gitMock *git.MockManager) {

‎internal/controller/function_predicate.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
Copyright 2025.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package controller
18+
19+
import (
20+
"strings"
21+
22+
"sigs.k8s.io/controller-runtime/pkg/client"
23+
"sigs.k8s.io/controller-runtime/pkg/event"
24+
"sigs.k8s.io/controller-runtime/pkg/predicate"
25+
)
26+
27+
const funcAnnotationPrefix = "functions.knative.dev/"
28+
29+
// FuncAnnotationChangedPredicate triggers reconciliation when annotations
30+
// with the "functions.knative.dev/" prefix are added or changed.
31+
type FuncAnnotationChangedPredicate struct {
32+
predicate.Funcs
33+
}
34+
35+
func (FuncAnnotationChangedPredicate) Update(e event.UpdateEvent) bool {
36+
if e.ObjectNew == nil {
37+
return false
38+
}
39+
return hasFuncAnnotations(e.ObjectNew)
40+
}
41+
42+
func hasFuncAnnotations(obj client.Object) bool {
43+
for key := range obj.GetAnnotations() {
44+
if strings.HasPrefix(key, funcAnnotationPrefix) {
45+
return true
46+
}
47+
}
48+
return false
49+
}

‎internal/controller/function_predicate_test.go‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/*
2+
Copyright 2025.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package controller
18+
19+
import (
20+
. "github.com/onsi/ginkgo/v2"
21+
. "github.com/onsi/gomega"
22+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
23+
"sigs.k8s.io/controller-runtime/pkg/event"
24+
25+
"github.com/functions-dev/func-operator/api/v1alpha1"
26+
)
27+
28+
var _ = Describe("FuncAnnotationChangedPredicate", func() {
29+
var p FuncAnnotationChangedPredicate
30+
31+
BeforeEach(func() {
32+
p = FuncAnnotationChangedPredicate{}
33+
})
34+
35+
Context("Update", func() {
36+
It("should trigger when func annotation is present", func() {
37+
result := p.Update(event.UpdateEvent{
38+
ObjectOld: &v1alpha1.Function{
39+
ObjectMeta: metav1.ObjectMeta{Generation: 1},
40+
},
41+
ObjectNew: &v1alpha1.Function{
42+
ObjectMeta: metav1.ObjectMeta{
43+
Generation: 1,
44+
Annotations: map[string]string{
45+
"functions.knative.dev/rebuild": "true",
46+
},
47+
},
48+
},
49+
})
50+
Expect(result).To(BeTrue())
51+
})
52+
53+
It("should trigger when multiple func annotations are present", func() {
54+
result := p.Update(event.UpdateEvent{
55+
ObjectOld: &v1alpha1.Function{
56+
ObjectMeta: metav1.ObjectMeta{Generation: 1},
57+
},
58+
ObjectNew: &v1alpha1.Function{
59+
ObjectMeta: metav1.ObjectMeta{
60+
Generation: 1,
61+
Annotations: map[string]string{
62+
"functions.knative.dev/rebuild": "true",
63+
"functions.knative.dev/reason": "config-change",
64+
},
65+
},
66+
},
67+
})
68+
Expect(result).To(BeTrue())
69+
})
70+
71+
It("should not trigger on non-func annotation change", func() {
72+
result := p.Update(event.UpdateEvent{
73+
ObjectOld: &v1alpha1.Function{
74+
ObjectMeta: metav1.ObjectMeta{Generation: 1},
75+
},
76+
ObjectNew: &v1alpha1.Function{
77+
ObjectMeta: metav1.ObjectMeta{
78+
Generation: 1,
79+
Annotations: map[string]string{
80+
"some-other-annotation": "value",
81+
},
82+
},
83+
},
84+
})
85+
Expect(result).To(BeFalse())
86+
})
87+
88+
It("should not trigger when nothing changed", func() {
89+
result := p.Update(event.UpdateEvent{
90+
ObjectOld: &v1alpha1.Function{
91+
ObjectMeta: metav1.ObjectMeta{Generation: 1},
92+
},
93+
ObjectNew: &v1alpha1.Function{
94+
ObjectMeta: metav1.ObjectMeta{Generation: 1},
95+
},
96+
})
97+
Expect(result).To(BeFalse())
98+
})
99+
100+
It("should not trigger when new object is nil", func() {
101+
result := p.Update(event.UpdateEvent{
102+
ObjectOld: &v1alpha1.Function{
103+
ObjectMeta: metav1.ObjectMeta{Generation: 1},
104+
},
105+
ObjectNew: nil,
106+
})
107+
Expect(result).To(BeFalse())
108+
})
109+
})
110+
111+
Context("Create", func() {
112+
It("should trigger on create", func() {
113+
result := p.Create(event.CreateEvent{
114+
Object: &v1alpha1.Function{
115+
ObjectMeta: metav1.ObjectMeta{Generation: 1},
116+
},
117+
})
118+
Expect(result).To(BeTrue())
119+
})
120+
})
121+
122+
Context("Delete", func() {
123+
It("should trigger on delete", func() {
124+
result := p.Delete(event.DeleteEvent{
125+
Object: &v1alpha1.Function{
126+
ObjectMeta: metav1.ObjectMeta{Generation: 1},
127+
},
128+
})
129+
Expect(result).To(BeTrue())
130+
})
131+
})
132+
})

0 commit comments

Comments
 (0)