From 03b870bd3306c17a333bbfa833f3855883e3ff45 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaime=20Pi=C3=B1a?= <variadico@users.noreply.github.com>
Date: Wed, 3 Nov 2021 15:14:41 -0700
Subject: [PATCH] Only send reload signal when necessary (#47)

Currently, we send multiple reload signals to the NATS server when we
detect any file changes within a watched directory. This causes
unnecessary logging and reloading.

This change only sends a signal if a file we're interested in has
changed. It ignores other file changes within the same directory. In
addition, we no longer log for every event, of which there can be many,
not all of them useful.
---
 pkg/natsreloader/natsreloader.go              | 33 +++++++++----------
 .../natsreloadertest/natsreloader_test.go     | 16 +++++++--
 2 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/pkg/natsreloader/natsreloader.go b/pkg/natsreloader/natsreloader.go
index 35244094..683ff788 100644
--- a/pkg/natsreloader/natsreloader.go
+++ b/pkg/natsreloader/natsreloader.go
@@ -141,14 +141,17 @@ func (r *Reloader) Run(ctx context.Context) error {
 			r.ConfigFiles)
 	}
 
-WaitForEvent:
+	var triggerName string
+	var updatedFiles bool
+
 	for {
 		select {
 		case <-ctx.Done():
 			return nil
 		case event := <-configWatcher.Events:
-			log.Printf("Event: %+v \n", event)
-			touchedInfo, err := os.Stat(event.Name)
+			triggerName = event.Name
+
+			_, err := os.Stat(event.Name)
 			if err != nil {
 				// Beware that this means that we won't reconfigure if a file
 				// is permanently removed.  We want to support transient
@@ -160,42 +163,38 @@ WaitForEvent:
 				continue
 			}
 
+			updatedFiles = false
 			for _, configFile := range r.ConfigFiles {
-				configInfo, err := os.Stat(configFile)
-				if err != nil {
-					log.Printf("Error: %s\n", err)
-					continue WaitForEvent
-				}
-				if !os.SameFile(touchedInfo, configInfo) {
-					continue
-				}
-
 				h := sha256.New()
 				f, err := os.Open(configFile)
 				if err != nil {
 					log.Printf("Error: %s\n", err)
-					continue WaitForEvent
+					continue
 				}
 				if _, err := io.Copy(h, f); err != nil {
 					log.Printf("Error: %s\n", err)
-					continue WaitForEvent
+					continue
 				}
 				digest := h.Sum(nil)
 				lastConfigHash, ok := lastConfigAppliedCache[configFile]
 				if ok && bytes.Equal(lastConfigHash, digest) {
 					// No meaningful change or this is the first time we've checked
-					continue WaitForEvent
+					continue
 				}
 				lastConfigAppliedCache[configFile] = digest
 
 				log.Printf("changed config; file=%q existing=%v total-files=%d",
 					configFile, ok, len(lastConfigAppliedCache))
 
+				updatedFiles = true
+
 				// We only get an event for one file at a time, we can stop checking
 				// config files here and continue with our business.
 				break
 			}
-
+			if !updatedFiles {
+				continue
+			}
 		case err := <-configWatcher.Errors:
 			log.Printf("Error: %s\n", err)
 			continue
@@ -205,7 +204,7 @@ WaitForEvent:
 		// otherwise give up and wait for next event.
 	TryReload:
 		for {
-			log.Println("Sending signal to server to reload configuration")
+			log.Println("Sending signal to server to reload configuration due to:", triggerName)
 			err := r.proc.Signal(syscall.SIGHUP)
 			if err != nil {
 				log.Printf("Error during reload: %s\n", err)
diff --git a/pkg/natsreloader/natsreloadertest/natsreloader_test.go b/pkg/natsreloader/natsreloadertest/natsreloader_test.go
index f8c8e8e7..e89e5b58 100644
--- a/pkg/natsreloader/natsreloadertest/natsreloader_test.go
+++ b/pkg/natsreloader/natsreloadertest/natsreloader_test.go
@@ -3,7 +3,6 @@ package natsreloadertest
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"os/signal"
 	"sync"
@@ -22,7 +21,7 @@ someOtherThing = "bar"
 func TestReloader(t *testing.T) {
 	// Setup a pidfile that points to us
 	pid := os.Getpid()
-	pidfile, err := ioutil.TempFile(os.TempDir(), "nats-pid-")
+	pidfile, err := os.CreateTemp(os.TempDir(), "nats-pid-")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -41,7 +40,7 @@ func TestReloader(t *testing.T) {
 
 	var configFiles []*os.File
 	for i := 0; i < 2; i++ {
-		configFile, err := ioutil.TempFile(os.TempDir(), "nats-conf-")
+		configFile, err := os.CreateTemp(os.TempDir(), "nats-conf-")
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -94,6 +93,17 @@ func TestReloader(t *testing.T) {
 				time.Sleep(10 * time.Millisecond)
 			}
 		}
+
+		// Create some random file in the same directory, shouldn't trigger an
+		// additional server signal.
+		configFile, err := os.CreateTemp(os.TempDir(), "foo")
+		if err != nil {
+			t.Log(err)
+			return
+		}
+		defer os.Remove(configFile.Name())
+		time.Sleep(100 * time.Millisecond)
+
 		cancel()
 	}()