Skip to content

Commit e158d5a

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1880 from deads2k/recorder
API-1835: require passive clock for event recorder to avoid accidents
2 parents 144cb72 + a567942 commit e158d5a

File tree

68 files changed

+277
-149
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

68 files changed

+277
-149
lines changed

‎pkg/controller/controllercmd/builder.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package controllercmd
33
import (
44
"context"
55
"fmt"
6+
"k8s.io/utils/clock"
67
"os"
78
"strings"
89
"sync"
@@ -40,6 +41,9 @@ type StartFunc func(context.Context, *ControllerContext) error
4041
type ControllerContext struct {
4142
ComponentConfig *unstructured.Unstructured
4243

44+
// Clock is a potentially fake clock that must be used to run controllers.
45+
Clock clock.Clock
46+
4347
// KubeConfig provides the REST config with no content type (it will default to JSON).
4448
// Use this config for CR resources.
4549
KubeConfig *rest.Config
@@ -71,6 +75,7 @@ type ControllerBuilder struct {
7175
fileObserverReactorFn func(file string, action fileobserver.ActionType) error
7276
eventRecorderOptions record.CorrelatorOptions
7377
componentOwnerReference *corev1.ObjectReference
78+
clock clock.Clock
7479

7580
startFunc StartFunc
7681
componentName string
@@ -121,10 +126,11 @@ func (i infrastructureStatusTopologyDetector) DetectTopology(ctx context.Context
121126
var _ TopologyDetector = (*infrastructureStatusTopologyDetector)(nil)
122127

123128
// NewController returns a builder struct for constructing the command you want to run
124-
func NewController(componentName string, startFunc StartFunc) *ControllerBuilder {
129+
func NewController(componentName string, startFunc StartFunc, clock clock.Clock) *ControllerBuilder {
125130
return &ControllerBuilder{
126131
startFunc: startFunc,
127132
componentName: componentName,
133+
clock: clock,
128134
observerInterval: defaultObserverInterval,
129135
nonZeroExitFn: func(args ...interface{}) {
130136
klog.Warning(args...)
@@ -266,7 +272,7 @@ func (b *ControllerBuilder) Run(ctx context.Context, config *unstructured.Unstru
266272
klog.Warningf("unable to get owner reference (falling back to namespace): %v", err)
267273
}
268274
}
269-
eventRecorder := events.NewKubeRecorderWithOptions(kubeClient.CoreV1().Events(namespace), b.eventRecorderOptions, b.componentName, controllerRef)
275+
eventRecorder := events.NewKubeRecorderWithOptions(kubeClient.CoreV1().Events(namespace), b.eventRecorderOptions, b.componentName, controllerRef, b.clock)
270276

271277
utilruntime.PanicHandlers = append(utilruntime.PanicHandlers, func(c context.Context, r interface{}) {
272278
eventRecorder.Warningf(fmt.Sprintf("%sPanic", strings.Title(b.componentName)), "Panic observed: %v", r)
@@ -336,6 +342,7 @@ func (b *ControllerBuilder) Run(ctx context.Context, config *unstructured.Unstru
336342

337343
controllerContext := &ControllerContext{
338344
ComponentConfig: config,
345+
Clock: b.clock,
339346
KubeConfig: clientConfig,
340347
ProtoKubeConfig: protoConfig,
341348
EventRecorder: eventRecorder,

‎pkg/controller/controllercmd/cmd.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package controllercmd
33
import (
44
"context"
55
"fmt"
6+
"k8s.io/utils/clock"
67
"math/rand"
78
"os"
89
"path/filepath"
@@ -41,6 +42,7 @@ type ControllerCommandConfig struct {
4142
componentName string
4243
startFunc StartFunc
4344
version version.Info
45+
clock clock.Clock
4446

4547
basicFlags *ControllerFlags
4648

@@ -76,11 +78,12 @@ type ControllerCommandConfig struct {
7678

7779
// NewControllerConfig returns a new ControllerCommandConfig which can be used to wire up all the boiler plate of a controller
7880
// TODO add more methods around wiring health checks and the like
79-
func NewControllerCommandConfig(componentName string, version version.Info, startFunc StartFunc) *ControllerCommandConfig {
81+
func NewControllerCommandConfig(componentName string, version version.Info, startFunc StartFunc, clock clock.Clock) *ControllerCommandConfig {
8082
return &ControllerCommandConfig{
8183
startFunc: startFunc,
8284
componentName: componentName,
8385
version: version,
86+
clock: clock,
8487

8588
basicFlags: NewControllerFlags(),
8689

@@ -322,7 +325,7 @@ func (c *ControllerCommandConfig) StartController(ctx context.Context) error {
322325
config.LeaderElection.RenewDeadline = c.RenewDeadline
323326
config.LeaderElection.RetryPeriod = c.RetryPeriod
324327

325-
builder := NewController(c.componentName, c.startFunc).
328+
builder := NewController(c.componentName, c.startFunc, c.clock).
326329
WithKubeConfigFile(c.basicFlags.KubeConfigFile, nil).
327330
WithComponentNamespace(c.basicFlags.Namespace).
328331
WithLeaderElection(config.LeaderElection, c.basicFlags.Namespace, c.componentName+"-lock").

‎pkg/controller/factory/factory_test.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package factory
33
import (
44
"context"
55
"fmt"
6+
clocktesting "k8s.io/utils/clock/testing"
67
"sync"
78
"testing"
89
"time"
@@ -108,7 +109,7 @@ func TestResyncController(t *testing.T) {
108109
}
109110
t.Logf("controller sync called (%d)", syncCallCount)
110111
return nil
111-
}).ToController("PeriodicController", events.NewInMemoryRecorder("periodic-controller"))
112+
}).ToController("PeriodicController", events.NewInMemoryRecorder("periodic-controller", clocktesting.NewFakePassiveClock(time.Now())))
112113

113114
go controller.Run(ctx, 1)
114115
time.Sleep(1 * time.Second) // Give controller time to start
@@ -155,7 +156,7 @@ func TestMultiWorkerControllerShutdown(t *testing.T) {
155156
workersShutdownMutex.Unlock()
156157

157158
return nil
158-
}).ToController("ShutdownController", events.NewInMemoryRecorder("shutdown-controller"))
159+
}).ToController("ShutdownController", events.NewInMemoryRecorder("shutdown-controller", clocktesting.NewFakePassiveClock(time.Now())))
159160

160161
// wait for all workers to be busy, then signal shutdown
161162
go func() {
@@ -192,7 +193,7 @@ func TestControllerWithInformer(t *testing.T) {
192193
t.Errorf("expected queue key to be 'key', got %q", syncContext.QueueKey())
193194
}
194195
return nil
195-
}).ToController("FakeController", events.NewInMemoryRecorder("fake-controller"))
196+
}).ToController("FakeController", events.NewInMemoryRecorder("fake-controller", clocktesting.NewFakePassiveClock(time.Now())))
196197

197198
go controller.Run(ctx, 1)
198199
time.Sleep(1 * time.Second) // Give controller time to start
@@ -214,7 +215,7 @@ func TestControllerScheduled(t *testing.T) {
214215
controller := New().ResyncSchedule("@every 1s").WithSync(func(ctx context.Context, controllerContext SyncContext) error {
215216
syncCalled <- struct{}{}
216217
return nil
217-
}).ToController("test", events.NewInMemoryRecorder("fake-controller"))
218+
}).ToController("test", events.NewInMemoryRecorder("fake-controller", clocktesting.NewFakePassiveClock(time.Now())))
218219

219220
ctx, cancel := context.WithCancel(context.Background())
220221
defer cancel()
@@ -256,7 +257,7 @@ func TestControllerSyncAfterStart(t *testing.T) {
256257
controller := New().ResyncEvery(10*time.Second).WithSync(func(ctx context.Context, controllerContext SyncContext) error {
257258
close(syncCalled)
258259
return nil
259-
}).ToController("test", events.NewInMemoryRecorder("fake-controller"))
260+
}).ToController("test", events.NewInMemoryRecorder("fake-controller", clocktesting.NewFakePassiveClock(time.Now())))
260261

261262
go controller.Run(context.TODO(), 1)
262263
select {
@@ -294,7 +295,7 @@ func TestControllerWithQueueFunction(t *testing.T) {
294295
t.Errorf("expected queue key to be 'test/test-secret', got %q", syncContext.QueueKey())
295296
}
296297
return nil
297-
}).ToController("FakeController", events.NewInMemoryRecorder("fake-controller"))
298+
}).ToController("FakeController", events.NewInMemoryRecorder("fake-controller", clocktesting.NewFakePassiveClock(time.Now())))
298299

299300
go controller.Run(ctx, 1)
300301
time.Sleep(1 * time.Second) // Give controller time to start

‎pkg/operator/apiserver/controller/apiservice/apiservice_controller_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package apiservice
33
import (
44
"context"
55
"fmt"
6+
clocktesting "k8s.io/utils/clock/testing"
67
"time"
78

89
"sort"
@@ -191,7 +192,7 @@ func TestAvailableStatus(t *testing.T) {
191192
kubeAggregatorClient.PrependReactor("*", "apiservices", tc.apiServiceReactor)
192193
}
193194

194-
eventRecorder := events.NewInMemoryRecorder("")
195+
eventRecorder := events.NewInMemoryRecorder("", clocktesting.NewFakePassiveClock(time.Now()))
195196
fakeOperatorClient := operatorv1helpers.NewFakeOperatorClient(&operatorv1.OperatorSpec{ManagementState: operatorv1.Managed}, &operatorv1.OperatorStatus{}, nil)
196197
fakeAuthOperatorIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
197198
{
@@ -281,7 +282,7 @@ func TestDisabledAPIService(t *testing.T) {
281282
kubeAggregatorClient.PrependReactor("*", "apiservices", apiServiceReactor)
282283
}
283284

284-
eventRecorder := events.NewInMemoryRecorder("")
285+
eventRecorder := events.NewInMemoryRecorder("", clocktesting.NewFakePassiveClock(time.Now()))
285286
fakeOperatorClient := operatorv1helpers.NewFakeOperatorClient(&operatorv1.OperatorSpec{ManagementState: operatorv1.Managed}, &operatorv1.OperatorStatus{}, nil)
286287
fakeAuthOperatorIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
287288
{

‎pkg/operator/apiserver/controller/workload/workload_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package workload
33
import (
44
"context"
55
"fmt"
6+
clocktesting "k8s.io/utils/clock/testing"
67
"testing"
78
"time"
89

@@ -583,7 +584,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
583584
delegate: delegate,
584585
}
585586

586-
err := target.sync(context.TODO(), factory.NewSyncContext("workloadcontroller_test", events.NewInMemoryRecorder("workloadcontroller_test")))
587+
err := target.sync(context.TODO(), factory.NewSyncContext("workloadcontroller_test", events.NewInMemoryRecorder("workloadcontroller_test", clocktesting.NewFakePassiveClock(time.Now()))))
587588
if err != nil && len(scenario.errors) == 0 {
588589
t.Fatal(err)
589590
}

‎pkg/operator/certrotation/cabundle_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"crypto/x509"
88
"crypto/x509/pkix"
99
"errors"
10+
clocktesting "k8s.io/utils/clock/testing"
1011
"math/big"
1112
"os"
1213
"strings"
@@ -225,7 +226,7 @@ func TestEnsureConfigMapCABundle(t *testing.T) {
225226

226227
Client: client.CoreV1(),
227228
Lister: corev1listers.NewConfigMapLister(indexer),
228-
EventRecorder: events.NewInMemoryRecorder("test"),
229+
EventRecorder: events.NewInMemoryRecorder("test", clocktesting.NewFakePassiveClock(time.Now())),
229230
}
230231

231232
newCA, err := test.caFn()

‎pkg/operator/certrotation/signer_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package certrotation
22

33
import (
44
"context"
5+
clocktesting "k8s.io/utils/clock/testing"
56
"strings"
67
"testing"
78
"time"
@@ -158,7 +159,7 @@ func TestEnsureSigningCertKeyPair(t *testing.T) {
158159
Refresh: 12 * time.Hour,
159160
Client: client.CoreV1(),
160161
Lister: corev1listers.NewSecretLister(indexer),
161-
EventRecorder: events.NewInMemoryRecorder("test"),
162+
EventRecorder: events.NewInMemoryRecorder("test", clocktesting.NewFakePassiveClock(time.Now())),
162163
AdditionalAnnotations: AdditionalAnnotations{
163164
JiraComponent: "test",
164165
},

‎pkg/operator/certrotation/target_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package certrotation
33
import (
44
"context"
55
"crypto/x509/pkix"
6+
clocktesting "k8s.io/utils/clock/testing"
67
"strings"
78
"testing"
89
"time"
@@ -250,7 +251,7 @@ func TestEnsureTargetCertKeyPair(t *testing.T) {
250251

251252
Client: client.CoreV1(),
252253
Lister: corev1listers.NewSecretLister(indexer),
253-
EventRecorder: events.NewInMemoryRecorder("test"),
254+
EventRecorder: events.NewInMemoryRecorder("test", clocktesting.NewFakePassiveClock(time.Now())),
254255
AdditionalAnnotations: AdditionalAnnotations{
255256
JiraComponent: "test",
256257
},
@@ -447,7 +448,7 @@ func TestEnsureTargetSignerCertKeyPair(t *testing.T) {
447448

448449
Client: client.CoreV1(),
449450
Lister: corev1listers.NewSecretLister(indexer),
450-
EventRecorder: events.NewInMemoryRecorder("test"),
451+
EventRecorder: events.NewInMemoryRecorder("test", clocktesting.NewFakePassiveClock(time.Now())),
451452
}
452453

453454
newCA, err := test.caFn()

‎pkg/operator/configobserver/apiserver/observe_audit_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ package apiserver
22

33
import (
44
"fmt"
5+
clocktesting "k8s.io/utils/clock/testing"
56
"strings"
67
"testing"
8+
"time"
79

810
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
911
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -151,7 +153,7 @@ func TestAuditObserver(t *testing.T) {
151153
}
152154

153155
observer := NewAuditObserver(getter)
154-
recorder := events.NewInMemoryRecorder(t.Name())
156+
recorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now()))
155157
for i := 1; i <= 2; i++ {
156158
gotConfig, errs := observer(listers, recorder, test.existingConfig)
157159

‎pkg/operator/configobserver/apiserver/observe_cors_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package apiserver
22

33
import (
4+
clocktesting "k8s.io/utils/clock/testing"
45
"reflect"
56
"sort"
67
"testing"
8+
"time"
79

810
configv1 "github.com/openshift/api/config/v1"
911
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
@@ -72,9 +74,9 @@ func TestObserveAdditionalCORSAllowedOrigins(t *testing.T) {
7274
var gotConfig map[string]interface{}
7375
var errs []error
7476
if useAPIServerArguments {
75-
gotConfig, errs = ObserveAdditionalCORSAllowedOriginsToArguments(listers, events.NewInMemoryRecorder(t.Name()), tt.existingConfig)
77+
gotConfig, errs = ObserveAdditionalCORSAllowedOriginsToArguments(listers, events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now())), tt.existingConfig)
7678
} else {
77-
gotConfig, errs = ObserveAdditionalCORSAllowedOrigins(listers, events.NewInMemoryRecorder(t.Name()), tt.existingConfig)
79+
gotConfig, errs = ObserveAdditionalCORSAllowedOrigins(listers, events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now())), tt.existingConfig)
7880
}
7981
if len(errs) > 0 {
8082
t.Errorf("expected no errors, got %v", errs)

0 commit comments

Comments
 (0)