*: Remove unnecessary configuration reload from ContentPathReloader…

… and improve its tests (#6496) * Fix and improve PathContentReloader tests Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> * Run tests in parallel Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> * Add changelog entry Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> * Fix table tests Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> * Fix typo Co-authored-by: Filip Petkovski <filip.petkovsky@gmail.com> Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> * Rerun CI Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> * Rerun CI Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> --------- Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com> Co-authored-by: Filip Petkovski <filip.petkovsky@gmail.com>
thanos-io · Jul 5, 2023 · c6623f8 · GiedriusS · Jul 5, 2023 · c6623f8
1 parent 81fdea9
commit c6623f8
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,6 +27,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
 - [#6467](https://github.com/thanos-io/thanos/pull/6467) Mixin (Receive): add alert for tenant reaching head series limit.
 
 ### Fixed
+- [#6496](https://github.com/thanos-io/thanos/pull/6496): *: Remove unnecessary configuration reload from `ContentPathReloader` and improve its tests.
 - [#6456](https://github.com/thanos-io/thanos/pull/6456) Store: fix crash when computing set matches from regex pattern
 - [#6427](https://github.com/thanos-io/thanos/pull/6427) Receive: increasing log level for failed uploads to error
 - [#6172](https://github.com/thanos-io/thanos/pull/6172) query-frontend: return JSON formatted errors for invalid PromQL expression in the split by interval middleware.

diff --git a/pkg/extkingpin/path_content_reloader.go b/pkg/extkingpin/path_content_reloader.go
@@ -49,6 +49,7 @@ func PathContentReloader(ctx context.Context, fileContent fileContent, logger lo
 				reloadFunc()
 				level.Debug(logger).Log("msg", "configuration reloaded after debouncing")
 			})
+			reloadTimer.Stop()
 		}
 		defer watcher.Close()
 		for {

diff --git a/pkg/extkingpin/path_content_reloader_test.go b/pkg/extkingpin/path_content_reloader_test.go
@@ -57,13 +57,15 @@ func TestPathContentReloader(t *testing.T) {
 					testutil.Ok(t, pathContent.Rewrite([]byte("test modified again")))
 				},
 			},
-			wantReloads: 1,
+			wantReloads: 2,
 		},
 		{
 			name: "Chmod doesn't trigger reload",
 			args: args{
 				runSteps: func(t *testing.T, testFile string, pathContent *staticPathContent) {
 					testutil.Ok(t, os.Chmod(testFile, 0777))
+					testutil.Ok(t, os.Chmod(testFile, 0666))
+					testutil.Ok(t, os.Chmod(testFile, 0777))
 				},
 			},
 			wantReloads: 0,
@@ -79,7 +81,9 @@ func TestPathContentReloader(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
+		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
 			testFile := path.Join(t.TempDir(), "test")
 			testutil.Ok(t, os.WriteFile(testFile, []byte("test"), 0666))
 			pathContent, err := NewStaticPathContent(testFile)
@@ -98,6 +102,10 @@ func TestPathContentReloader(t *testing.T) {
 			testutil.Ok(t, err)
 
 			tt.args.runSteps(t, testFile, pathContent)
+			if tt.wantReloads == 0 {
+				// Give things a little time to sync. The fs watcher events are heavily async and could be delayed.
+				time.Sleep(1 * time.Second)
+			}
 			wg.Wait()
 			testutil.Equals(t, tt.wantReloads, reloadCount)
 		})