From de6e3bcf53bdd055e3dd5fc6235545033f3e2c96 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 5 Feb 2024 14:06:50 -0800 Subject: [PATCH] Add review feedback --- internal/framework/runnables/cronjob_test.go | 13 ++----------- internal/mode/static/manager.go | 6 +++--- internal/mode/static/telemetry/job_worker_test.go | 2 +- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/internal/framework/runnables/cronjob_test.go b/internal/framework/runnables/cronjob_test.go index 89ddf09d53..9a3a6bbf72 100644 --- a/internal/framework/runnables/cronjob_test.go +++ b/internal/framework/runnables/cronjob_test.go @@ -54,24 +54,15 @@ func TestCronJob_ContextCanceled(t *testing.T) { readyChannel := make(chan struct{}) - timeout := 10 * time.Second - var callCount int - - valCh := make(chan int, 128) - worker := func(context.Context) { - callCount++ - valCh <- callCount - } - cfg := CronJobConfig{ - Worker: worker, + Worker: func(ctx context.Context) {}, Logger: zap.New(), Period: 1 * time.Millisecond, // 1ms is much smaller than timeout so the CronJob should run a few times ReadyCh: readyChannel, } job := NewCronJob(cfg) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithCancel(context.Background()) errCh := make(chan error) go func() { diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 910a796bd7..712bb6c46f 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -219,7 +219,7 @@ func StartManager(cfg config.Config) error { ConfigurationGetter: eventHandler, Version: cfg.Version, }) - if err = mgr.Add(createTelemetryJob(cfg, dataCollector, nginxChecker)); err != nil { + if err = mgr.Add(createTelemetryJob(cfg, dataCollector, nginxChecker.getReadyCh())); err != nil { return fmt.Errorf("cannot register telemetry job: %w", err) } @@ -408,7 +408,7 @@ func registerControllers( func createTelemetryJob( cfg config.Config, dataCollector telemetry.DataCollector, - nginxChecker *nginxConfiguredOnStartChecker, + readyCh <-chan struct{}, ) *runnables.Leader { logger := cfg.Logger.WithName("telemetryJob") exporter := telemetry.NewLoggingExporter(cfg.Logger.WithName("telemetryExporter").V(1 /* debug */)) @@ -426,7 +426,7 @@ func createTelemetryJob( Logger: logger, Period: cfg.TelemetryReportPeriod, JitterFactor: jitterFactor, - ReadyCh: nginxChecker.getReadyCh(), + ReadyCh: readyCh, }, ), } diff --git a/internal/mode/static/telemetry/job_worker_test.go b/internal/mode/static/telemetry/job_worker_test.go index b1c119e13c..4e804ae26d 100644 --- a/internal/mode/static/telemetry/job_worker_test.go +++ b/internal/mode/static/telemetry/job_worker_test.go @@ -36,9 +36,9 @@ func TestCreateTelemetryJobWorker(t *testing.T) { timeout := 10 * time.Second ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() worker(ctx) _, data := exporter.ExportArgsForCall(0) g.Expect(data).To(Equal(expData)) - cancel() }