Skip to content

Commit a686a38

Browse files
codablockcodablock
committed
fix: Perform watches on a per-object basis instead of globally on the kind
1 parent 619a05b commit a686a38

9 files changed

+272
-217
lines changed

‎api/v1alpha1/texttemplate_types.go

+4
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ import (
2020
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2121
)
2222

23+
const (
24+
TextTemplateFinalizer = "finalizers.templates.kluctl.io"
25+
)
26+
2327
// TextTemplateSpec defines the desired state of TextTemplate
2428
type TextTemplateSpec struct {
2529
// Suspend can be used to suspend the reconciliation of this object.

‎controllers/base_template_reconciler.go

+11-44
Original file line numberDiff line numberDiff line change
@@ -7,78 +7,45 @@ import (
77
"github.com/ohler55/ojg/jp"
88
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
99
"k8s.io/apimachinery/pkg/runtime"
10-
"k8s.io/apimachinery/pkg/runtime/schema"
1110
"k8s.io/apimachinery/pkg/types"
1211
"k8s.io/client-go/rest"
1312
"sigs.k8s.io/controller-runtime/pkg/client"
1413
"sigs.k8s.io/controller-runtime/pkg/controller"
15-
"sigs.k8s.io/controller-runtime/pkg/handler"
16-
"sigs.k8s.io/controller-runtime/pkg/log"
1714
"sigs.k8s.io/controller-runtime/pkg/manager"
18-
"sigs.k8s.io/controller-runtime/pkg/source"
1915
"sync"
2016
)
2117

2218
type BaseTemplateReconciler struct {
2319
client.Client
2420

21+
RawWatchContext context.Context
22+
2523
Manager manager.Manager
2624
Scheme *runtime.Scheme
2725
FieldManager string
2826

29-
controller controller.Controller
30-
watchedKinds map[schema.GroupVersionKind]bool
31-
mutex sync.Mutex
27+
controller controller.Controller
28+
29+
watchesUtil watchesUtil
30+
31+
mutex sync.Mutex
3232
}
3333

34-
func (r *BaseTemplateReconciler) getClientForObjects(serviceAccountName string, objNamespace string) (client.Client, error) {
34+
func (r *BaseTemplateReconciler) getClientForObjects(serviceAccountName string, objNamespace string) (client.WithWatch, string, error) {
3535
restConfig := rest.CopyConfig(r.Manager.GetConfig())
3636

3737
name := "default"
3838
if serviceAccountName != "" {
3939
name = serviceAccountName
4040
}
41-
if name == "" {
42-
return nil, fmt.Errorf("empty serviceAccountName not allowed")
43-
}
4441
username := fmt.Sprintf("system:serviceaccount:%s:%s", objNamespace, name)
4542
restConfig.Impersonate = rest.ImpersonationConfig{UserName: username}
4643

47-
c, err := client.New(restConfig, client.Options{Mapper: r.RESTMapper()})
48-
if err != nil {
49-
return nil, err
50-
}
51-
return c, nil
52-
}
53-
54-
func (r *BaseTemplateReconciler) addWatchForKind(ctx context.Context, gvk schema.GroupVersionKind, key string, eventHandler handler.TypedEventHandler[client.Object]) error {
55-
logger := log.FromContext(ctx)
56-
57-
r.mutex.Lock()
58-
defer r.mutex.Unlock()
59-
60-
if r.watchedKinds == nil {
61-
r.watchedKinds = map[schema.GroupVersionKind]bool{}
62-
}
63-
64-
keyGvk := gvk
65-
keyGvk.Kind += "+" + key
66-
if x, ok := r.watchedKinds[keyGvk]; ok && x {
67-
return nil
68-
}
69-
70-
logger.V(1).Info("Starting watch for new kind and key", "gvk", gvk, "key", key)
71-
72-
var dummy unstructured.Unstructured
73-
dummy.SetGroupVersionKind(gvk)
74-
75-
err := r.controller.Watch(source.Kind[client.Object](r.Manager.GetCache(), &dummy, eventHandler))
44+
c, err := client.NewWithWatch(restConfig, client.Options{Mapper: r.RESTMapper()})
7645
if err != nil {
77-
return err
46+
return nil, "", err
7847
}
79-
80-
r.watchedKinds[keyGvk] = true
81-
return nil
48+
return c, name, nil
8249
}
8350

8451
func (r *BaseTemplateReconciler) buildBaseVars(templateObj runtime.Object, objVarName string) (map[string]any, error) {

‎controllers/objecttemplate_controller.go

+22-59
Original file line numberDiff line numberDiff line change
@@ -28,24 +28,19 @@ import (
2828
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2929
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3030
"k8s.io/apimachinery/pkg/runtime"
31-
"k8s.io/apimachinery/pkg/types"
3231
"k8s.io/apimachinery/pkg/util/yaml"
3332
ctrl "sigs.k8s.io/controller-runtime"
3433
"sigs.k8s.io/controller-runtime/pkg/builder"
3534
"sigs.k8s.io/controller-runtime/pkg/client"
3635
"sigs.k8s.io/controller-runtime/pkg/controller"
3736
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
38-
"sigs.k8s.io/controller-runtime/pkg/handler"
3937
"sigs.k8s.io/controller-runtime/pkg/log"
4038
"sigs.k8s.io/controller-runtime/pkg/predicate"
41-
"sigs.k8s.io/controller-runtime/pkg/reconcile"
4239
"sort"
4340
"strings"
4441
"sync"
4542
)
4643

47-
const forMatrixObjectKey = "spec.matrix.object.ref"
48-
4944
// ObjectTemplateReconciler reconciles a ObjectTemplate object
5045
type ObjectTemplateReconciler struct {
5146
BaseTemplateReconciler
@@ -95,20 +90,6 @@ func (r *ObjectTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Reque
9590
return ctrl.Result{}, nil
9691
}
9792

98-
for _, me := range rt.Spec.Matrix {
99-
if me.Object != nil {
100-
gvk, err2 := me.Object.Ref.GroupVersionKind()
101-
if err2 != nil {
102-
err = err2
103-
return
104-
}
105-
err = r.addWatchForKind(ctx, gvk, forMatrixObjectKey, r.buildWatchEventHandler(forMatrixObjectKey))
106-
if err != nil {
107-
return
108-
}
109-
}
110-
}
111-
11293
patch := client.MergeFrom(rt.DeepCopy())
11394
err = r.doReconcile(ctx, &rt)
11495
if err != nil {
@@ -203,11 +184,25 @@ func (r *ObjectTemplateReconciler) doReconcile(ctx context.Context, rt *template
203184
var wg sync.WaitGroup
204185
var mutex sync.Mutex
205186

206-
objClient, err := r.getClientForObjects(rt.Spec.ServiceAccountName, rt.GetNamespace())
187+
objClient, saName, err := r.getClientForObjects(rt.Spec.ServiceAccountName, rt.GetNamespace())
207188
if err != nil {
208189
return err
209190
}
210191

192+
wt := r.watchesUtil.getWatchesForTemplate(client.ObjectKeyFromObject(rt))
193+
wt.setClient(objClient, saName)
194+
newObjects := map[templatesv1alpha1.ObjectRef]bool{}
195+
for _, me := range rt.Spec.Matrix {
196+
if me.Object != nil {
197+
newObjects[me.Object.Ref] = true
198+
err = wt.addWatchForObject(ctx, me.Object.Ref)
199+
if err != nil {
200+
return err
201+
}
202+
}
203+
}
204+
wt.removeDeletedWatches(newObjects)
205+
211206
matrixEntries, err := r.buildMatrixEntries(ctx, rt, objClient)
212207
if err != nil {
213208
return err
@@ -429,21 +424,6 @@ func (r *ObjectTemplateReconciler) renderTemplates(j2 *jinja2.Jinja2, rt *templa
429424
func (r *ObjectTemplateReconciler) SetupWithManager(mgr ctrl.Manager, concurrent int) error {
430425
r.Manager = mgr
431426

432-
// Index the ObjectTemplate by the objects they are for.
433-
if err := mgr.GetCache().IndexField(context.TODO(), &templatesv1alpha1.ObjectTemplate{}, forMatrixObjectKey,
434-
func(object client.Object) []string {
435-
o := object.(*templatesv1alpha1.ObjectTemplate)
436-
var ret []string
437-
for _, me := range o.Spec.Matrix {
438-
if me.Object != nil {
439-
ret = append(ret, BuildRefIndexValue(me.Object.Ref, o.GetNamespace()))
440-
}
441-
}
442-
return ret
443-
}); err != nil {
444-
return fmt.Errorf("failed setting index fields: %w", err)
445-
}
446-
447427
c, err := ctrl.NewControllerManagedBy(mgr).
448428
For(&templatesv1alpha1.ObjectTemplate{}, builder.WithPredicates(
449429
predicate.Or(predicate.GenerationChangedPredicate{}),
@@ -457,33 +437,16 @@ func (r *ObjectTemplateReconciler) SetupWithManager(mgr ctrl.Manager, concurrent
457437
}
458438
r.controller = c
459439

460-
return nil
461-
}
462-
463-
func (r *ObjectTemplateReconciler) buildWatchEventHandler(indexField string) handler.EventHandler {
464-
return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
465-
var list templatesv1alpha1.ObjectTemplateList
440+
err = r.watchesUtil.init(r.RawWatchContext, r.controller)
441+
if err != nil {
442+
return err
443+
}
466444

467-
err := r.List(context.Background(), &list, client.MatchingFields{
468-
indexField: BuildObjectIndexValue(object),
469-
})
470-
if err != nil {
471-
return nil
472-
}
473-
var reqs []reconcile.Request
474-
for _, x := range list.Items {
475-
reqs = append(reqs, reconcile.Request{
476-
NamespacedName: types.NamespacedName{
477-
Namespace: x.GetNamespace(),
478-
Name: x.GetName(),
479-
},
480-
})
481-
}
482-
return reqs
483-
})
445+
return nil
484446
}
485447

486448
func (r *ObjectTemplateReconciler) finalize(ctx context.Context, obj *templatesv1alpha1.ObjectTemplate) (ctrl.Result, error) {
449+
r.watchesUtil.removeWatchesForTemplate(client.ObjectKeyFromObject(obj))
487450
r.doFinalize(ctx, obj)
488451

489452
// Remove our finalizer from the list and update it
@@ -503,7 +466,7 @@ func (r *ObjectTemplateReconciler) doFinalize(ctx context.Context, obj *template
503466
return
504467
}
505468

506-
objClient, err := r.getClientForObjects(obj.Spec.ServiceAccountName, obj.GetNamespace())
469+
objClient, _, err := r.getClientForObjects(obj.Spec.ServiceAccountName, obj.GetNamespace())
507470
if err != nil {
508471
log.Error(err, "Failed to create objClient for deletion")
509472
return

‎controllers/objecttemplate_controller_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ var _ = Describe("ObjectTemplate controller", func() {
129129
t2 := getObjectTemplate(key)
130130
c := getReadyCondition(t2.GetConditions())
131131
Expect(c.Status).To(Equal(metav1.ConditionFalse))
132-
Expect(c.Message).To(ContainSubstring("secrets \"m1\" is forbidden"))
132+
Expect(c.Message).To(ContainSubstring("Secret \"m1\" is forbidden"))
133133
})
134134
It("Should succeed when RBAC is created", func() {
135135
createRoleWithBinding("default", ns, []string{"secrets"})
@@ -149,7 +149,7 @@ var _ = Describe("ObjectTemplate controller", func() {
149149
t2 := getObjectTemplate(key)
150150
c := getReadyCondition(t2.GetConditions())
151151
Expect(c.Status).To(Equal(metav1.ConditionFalse))
152-
Expect(c.Message).To(ContainSubstring("secrets \"m1\" is forbidden"))
152+
Expect(c.Message).To(ContainSubstring("Secret \"m1\" is forbidden"))
153153
})
154154
It("Should succeed after the SA is being created", func() {
155155
createServiceAccount("non-existent", ns)

‎controllers/suite_test.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,10 @@ var _ = BeforeSuite(func() {
9393

9494
err = (&ObjectTemplateReconciler{
9595
BaseTemplateReconciler: BaseTemplateReconciler{
96-
Client: k8sManager.GetClient(),
97-
Scheme: k8sManager.GetScheme(),
98-
FieldManager: "template-controller",
96+
Client: k8sManager.GetClient(),
97+
RawWatchContext: ctx,
98+
Scheme: k8sManager.GetScheme(),
99+
FieldManager: "template-controller",
99100
},
100101
}).SetupWithManager(k8sManager, 1)
101102
Expect(err).ToNot(HaveOccurred())

0 commit comments

Comments
 (0)