Skip to content

🐛 Fix predicate logging #11406

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 13, 2024
Merged

🐛 Fix predicate logging #11406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions controllers/external/tracker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,16 @@ import (
type ObjectTracker struct {
m sync.Map

Controller controller.Controller
Cache cache.Cache
Scheme *runtime.Scheme
Controller controller.Controller
Cache cache.Cache
Scheme *runtime.Scheme
PredicateLogger *logr.Logger
}

// Watch uses the controller to issue a Watch only if the object hasn't been seen before.
func (o *ObjectTracker) Watch(log logr.Logger, obj client.Object, handler handler.EventHandler, p ...predicate.Predicate) error {
if o.Controller == nil || o.Cache == nil || o.Scheme == nil {
return errors.New("all of controller, cache and scheme must be set for object tracker")
if o.Controller == nil || o.Cache == nil || o.Scheme == nil || o.PredicateLogger == nil {
return errors.New("all of Controller, Cache, Scheme and PredicateLogger must be set for object tracker")
}

gvk := obj.GetObjectKind().GroupVersionKind()
Expand All @@ -59,7 +60,7 @@ func (o *ObjectTracker) Watch(log logr.Logger, obj client.Object, handler handle
o.Cache,
obj.DeepCopyObject().(client.Object),
handler,
append(p, predicates.ResourceNotPaused(o.Scheme, log))...,
append(p, predicates.ResourceNotPaused(o.Scheme, *o.PredicateLogger))...,
))
if err != nil {
o.m.Delete(key)
Expand Down
5 changes: 3 additions & 2 deletions controllers/external/tracker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/cache/informertest"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/log"
Expand Down Expand Up @@ -66,7 +67,7 @@ func (c *watchCountController) Watch(_ source.Source) error {
func TestRetryWatch(t *testing.T) {
g := NewWithT(t)
ctrl := newWatchCountController(true)
tracker := ObjectTracker{Controller: ctrl, Scheme: runtime.NewScheme(), Cache: &informertest.FakeInformers{}}
tracker := ObjectTracker{Controller: ctrl, Scheme: runtime.NewScheme(), Cache: &informertest.FakeInformers{}, PredicateLogger: ptr.To(logr.New(log.NullLogSink{}))}

err := tracker.Watch(logger, &clusterv1.Cluster{}, nil)
g.Expect(err).To(HaveOccurred())
Expand All @@ -80,7 +81,7 @@ func TestRetryWatch(t *testing.T) {
func TestWatchMultipleTimes(t *testing.T) {
g := NewWithT(t)
ctrl := &watchCountController{}
tracker := ObjectTracker{Controller: ctrl, Scheme: runtime.NewScheme(), Cache: &informertest.FakeInformers{}}
tracker := ObjectTracker{Controller: ctrl, Scheme: runtime.NewScheme(), Cache: &informertest.FakeInformers{}, PredicateLogger: ptr.To(logr.New(log.NullLogSink{}))}

obj := &clusterv1.Cluster{
TypeMeta: metav1.TypeMeta{
Expand Down
7 changes: 4 additions & 3 deletions exp/internal/controllers/machinepool_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,10 @@ func (r *MachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M
r.controller = c
r.recorder = mgr.GetEventRecorderFor("machinepool-controller")
r.externalTracker = external.ObjectTracker{
Controller: c,
Cache: mgr.GetCache(),
Scheme: mgr.GetScheme(),
Controller: c,
Cache: mgr.GetCache(),
Scheme: mgr.GetScheme(),
PredicateLogger: &predicateLog,
}
r.ssaCache = ssa.NewCache()

Expand Down
142 changes: 82 additions & 60 deletions exp/internal/controllers/machinepool_controller_phases_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"
"time"

"github.com/go-logr/logr"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -33,6 +34,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/cache/informertest"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/log"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/controllers/clustercache"
Expand Down Expand Up @@ -131,9 +133,10 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -173,9 +176,10 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -212,9 +216,10 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -267,9 +272,10 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -334,9 +340,10 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -379,9 +386,10 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -431,9 +439,10 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -496,9 +505,10 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -567,9 +577,10 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -636,9 +647,10 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -727,9 +739,10 @@ func TestReconcileMachinePoolPhases(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -1049,9 +1062,10 @@ func TestReconcileMachinePoolBootstrap(t *testing.T) {
r := &MachinePoolReconciler{
Client: fakeClient,
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -1349,9 +1363,10 @@ func TestReconcileMachinePoolInfrastructure(t *testing.T) {
Client: fakeClient,
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: defaultCluster.Name, Namespace: defaultCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -1437,9 +1452,10 @@ func TestReconcileMachinePoolMachines(t *testing.T) {
Client: env,
ssaCache: ssa.NewCache(),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: env.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: env.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}
scope := &scope{
Expand Down Expand Up @@ -1505,9 +1521,10 @@ func TestReconcileMachinePoolMachines(t *testing.T) {
Client: env,
ssaCache: ssa.NewCache(),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: env.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: env.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -1868,9 +1885,10 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
ClusterCache: clustercache.NewFakeClusterCache(env.GetClient(), client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
recorder: record.NewFakeRecorder(32),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -1935,9 +1953,10 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
ClusterCache: clustercache.NewFakeClusterCache(env.GetClient(), client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
recorder: record.NewFakeRecorder(32),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -1985,9 +2004,10 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
recorder: record.NewFakeRecorder(32),
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -2031,9 +2051,10 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
recorder: record.NewFakeRecorder(32),
ClusterCache: clustercache.NewFakeClusterCache(fakeClient, client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down Expand Up @@ -2099,9 +2120,10 @@ func TestReconcileMachinePoolScaleToFromZero(t *testing.T) {
ClusterCache: clustercache.NewFakeClusterCache(env.GetClient(), client.ObjectKey{Name: testCluster.Name, Namespace: testCluster.Namespace}),
recorder: record.NewFakeRecorder(32),
externalTracker: external.ObjectTracker{
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
Controller: externalfake.Controller{},
Cache: &informertest.FakeInformers{},
Scheme: fakeClient.Scheme(),
PredicateLogger: ptr.To(logr.New(log.NullLogSink{})),
},
}

Expand Down
Loading