Use context to stop the upgradecheck goroutine

Author: cbandy

cmd/postgres-operator/main.go

@@ -85,12 +85,11 @@ func main() {
 
 	// Enable upgrade checking
 	upgradeCheckingDisabled := strings.EqualFold(os.Getenv("CHECK_FOR_UPGRADES"), "false")
-	done := make(chan bool, 1)
 	if !upgradeCheckingDisabled {
 		log.Info("upgrade checking enabled")
 		// get the URL for the check for upgrades endpoint if set in the env
 		upgradeCheckURL := os.Getenv("CHECK_FOR_UPGRADES_URL")
-		go upgradecheck.CheckForUpgradesScheduler(done, versionString, upgradeCheckURL,
+		go upgradecheck.CheckForUpgradesScheduler(ctx, versionString, upgradeCheckURL,
 			mgr.GetClient(), mgr.GetConfig(), isOpenshift(ctx, mgr.GetConfig()),
 			mgr.GetCache(),
 		)
@@ -100,10 +99,6 @@ func main() {
 
 	assertNoError(mgr.Start(ctx))
 	log.Info("signal received, exiting")
-	if !upgradeCheckingDisabled {
-		// Send true to channel to cancel ticker cleanly
-		done <- true
-	}
 }
 
 // addControllersToManager adds all PostgreSQL Operator controllers to the provided controller

internal/upgradecheck/http.go

@@ -141,13 +141,12 @@ func checkForUpgrades(ctx context.Context, versionString string, backoff wait.Ba
 }
 
 // CheckForUpgradesScheduler invokes the check func when the operator starts
-// and then on the given period schedule
-func CheckForUpgradesScheduler(channel chan bool,
+// and then on the given period schedule. It stops when the context is cancelled.
+func CheckForUpgradesScheduler(ctx context.Context,
 	versionString, url string, crclient crclient.Client,
 	cfg *rest.Config, isOpenShift bool,
 	cacheClient CacheWithWait,
 ) {
-	ctx := context.Background()
 	log := logging.FromContext(ctx)
 	defer func() {
 		if err := recover(); err != nil {
@@ -194,7 +193,7 @@ func CheckForUpgradesScheduler(channel chan bool,
 			} else {
 				log.V(1).Info(info)
 			}
-		case <-channel:
+		case <-ctx.Done():
 			ticker.Stop()
 			return
 		}

internal/upgradecheck/http_test.go

@@ -171,22 +171,24 @@ func TestCheckForUpgradesScheduler(t *testing.T) {
 	const testUpgradeCheckURL = "http://localhost:8080"
 
 	t.Run("panic from checkForUpgrades doesn't bubble up", func(t *testing.T) {
-		done := make(chan bool, 1)
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+
 		// capture logs
 		var calls []string
-		logging.SetLogFunc(1, func(input genericr.Entry) {
+		ctx = logging.NewContext(ctx, genericr.New(func(input genericr.Entry) {
 			calls = append(calls, input.Message)
-		})
+		}))
 
 		// A panicking call
 		funcFoo = func() (*http.Response, error) {
 			panic(fmt.Errorf("oh no!"))
 		}
 
-		go CheckForUpgradesScheduler(done, "4.7.3", testUpgradeCheckURL, fakeClient, cfg, false,
+		go CheckForUpgradesScheduler(ctx, "4.7.3", testUpgradeCheckURL, fakeClient, cfg, false,
 			&MockCacheClient{works: true})
 		time.Sleep(1 * time.Second)
-		done <- true
+		cancel()
 
 		// Sleeping leads to some non-deterministic results, but we expect at least 1 execution
 		// plus one log for the failure to apply the configmap
@@ -195,32 +197,36 @@ func TestCheckForUpgradesScheduler(t *testing.T) {
 	})
 
 	t.Run("cache sync fail leads to log and exit", func(t *testing.T) {
-		done := make(chan bool, 1)
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+
 		// capture logs
 		var calls []string
-		logging.SetLogFunc(1, func(input genericr.Entry) {
+		ctx = logging.NewContext(ctx, genericr.New(func(input genericr.Entry) {
 			calls = append(calls, input.Message)
-		})
+		}))
 
 		// Set loop time to 1s and sleep for 2s before sending the done signal -- though the cache sync
 		// failure will exit the func before the sleep ends
 		upgradeCheckPeriod = 1 * time.Second
-		go CheckForUpgradesScheduler(done, "4.7.3", testUpgradeCheckURL, fakeClient, cfg, false,
+		go CheckForUpgradesScheduler(ctx, "4.7.3", testUpgradeCheckURL, fakeClient, cfg, false,
 			&MockCacheClient{works: false})
 		time.Sleep(2 * time.Second)
-		done <- true
+		cancel()
 
 		assert.Assert(t, len(calls) == 1)
 		assert.Equal(t, calls[0], `unable to sync cache for upgrade check`)
 	})
 
 	t.Run("successful log each loop, ticker works", func(t *testing.T) {
-		done := make(chan bool, 1)
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+
 		// capture logs
 		var calls []string
-		logging.SetLogFunc(1, func(input genericr.Entry) {
+		ctx = logging.NewContext(ctx, genericr.New(func(input genericr.Entry) {
 			calls = append(calls, input.Message)
-		})
+		}))
 
 		// A successful call
 		funcFoo = func() (*http.Response, error) {
@@ -233,10 +239,10 @@ func TestCheckForUpgradesScheduler(t *testing.T) {
 
 		// Set loop time to 1s and sleep for 2s before sending the done signal
 		upgradeCheckPeriod = 1 * time.Second
-		go CheckForUpgradesScheduler(done, "4.7.3", testUpgradeCheckURL, fakeClient, cfg, false,
+		go CheckForUpgradesScheduler(ctx, "4.7.3", testUpgradeCheckURL, fakeClient, cfg, false,
 			&MockCacheClient{works: true})
 		time.Sleep(2 * time.Second)
-		done <- true
+		cancel()
 
 		// Sleeping leads to some non-deterministic results, but we expect at least 2 executions
 		// plus one log for the failure to apply the configmap