Repository: VictoriaMetrics/VictoriaMetrics
Pull request #928 (2020-11-27 15:55:36)

fixes checksum calculation
'for' rule param wasnt marshal properly during checksum calculation.
 It leads to incorrect rule reload, when for param was changed for rule. 

f41gh7: > LGTM!
> do you know why build fails?

@valyala  broke it with latest commit to master branch :)

Patch:
diff --git a/app/vmalert/config/config.go b/app/vmalert/config/config.go
index 1ea260e4e..da998ca5c 100644
--- a/app/vmalert/config/config.go
+++ b/app/vmalert/config/config.go
@@ -95,7 +95,7 @@ type Rule struct {
 	Record      string            `yaml:"record,omitempty"`
 	Alert       string            `yaml:"alert,omitempty"`
 	Expr        string            `yaml:"expr"`
-	For         PromDuration      `yaml:"for,omitempty"`
+	For         PromDuration      `yaml:"for"`
 	Labels      map[string]string `yaml:"labels,omitempty"`
 	Annotations map[string]string `yaml:"annotations,omitempty"`
 
@@ -115,6 +115,11 @@ func NewPromDuration(d time.Duration) PromDuration {
 	}
 }
 
+// MarshalYAML implements yaml.Marshaler interface.
+func (pd PromDuration) MarshalYAML() (interface{}, error) {
+	return pd.Duration().String(), nil
+}
+
 // UnmarshalYAML implements yaml.Unmarshaler interface.
 func (pd *PromDuration) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	var s string


Test patch:
diff --git a/app/vmalert/config/config_test.go b/app/vmalert/config/config_test.go
index a631e6ddc..0539986e3 100644
--- a/app/vmalert/config/config_test.go
+++ b/app/vmalert/config/config_test.go
@@ -323,34 +323,55 @@ func TestHashRule(t *testing.T) {
 }
 
 func TestGroupChecksum(t *testing.T) {
-	data := `
+	f := func(t *testing.T, data, newData string) {
+		t.Helper()
+		var g Group
+		if err := yaml.Unmarshal([]byte(data), &g); err != nil {
+			t.Fatalf("failed to unmarshal: %s", err)
+		}
+		if g.Checksum == "" {
+			t.Fatalf("expected to get non-empty checksum")
+		}
+
+		var ng Group
+		if err := yaml.Unmarshal([]byte(newData), &ng); err != nil {
+			t.Fatalf("failed to unmarshal: %s", err)
+		}
+		if g.Checksum == ng.Checksum {
+			t.Fatalf("expected to get different checksums")
+		}
+	}
+	t.Run("Ok", func(t *testing.T) {
+		f(t, `
 name: TestGroup
 rules:
   - alert: ExampleAlertAlwaysFiring
     expr: sum by(job) (up == 1)
   - record: handler:requests:rate5m
     expr: sum(rate(prometheus_http_requests_total[5m])) by (handler)
-`
-	var g Group
-	if err := yaml.Unmarshal([]byte(data), &g); err != nil {
-		t.Fatalf("failed to unmarshal: %s", err)
-	}
-	if g.Checksum == "" {
-		t.Fatalf("expected to get non-empty checksum")
-	}
-	newData := `
+`, `
 name: TestGroup
 rules:
   - record: handler:requests:rate5m
     expr: sum(rate(prometheus_http_requests_total[5m])) by (handler)
   - alert: ExampleAlertAlwaysFiring
     expr: sum by(job) (up == 1)
-`
-	var ng Group
-	if err := yaml.Unmarshal([]byte(newData), &g); err != nil {
-		t.Fatalf("failed to unmarshal: %s", err)
-	}
-	if g.Checksum == ng.Checksum {
-		t.Fatalf("expected to get different checksums")
-	}
+`)
+	})
+
+	t.Run("Ok, `for` must change cs", func(t *testing.T) {
+		f(t, `
+name: TestGroup
+rules:
+  - alert: ExampleAlertWithFor
+    expr: sum by(job) (up == 1)
+    for: 5m
+`, `
+name: TestGroup
+rules:
+  - alert: ExampleAlertWithFor
+    expr: sum by(job) (up == 1)
+`)
+	})
+
 }
diff --git a/lib/promscrape/scrapework_test.go b/lib/promscrape/scrapework_test.go
index 9d1e54f92..861b77f39 100644
--- a/lib/promscrape/scrapework_test.go
+++ b/lib/promscrape/scrapework_test.go
@@ -65,7 +65,7 @@ func TestScrapeWorkScrapeInternalFailure(t *testing.T) {
 		pushDataCalls++
 	}
 
-	timestamp := int64(123)
+	timestamp := int64(123000)
 	if err := sw.scrapeInternal(timestamp, timestamp); err == nil {
 		t.Fatalf("expecting non-nil error")
 	}
@@ -112,7 +112,7 @@ func TestScrapeWorkScrapeInternalSuccess(t *testing.T) {
 			}
 		}
 
-		timestamp := int64(123)
+		timestamp := int64(123000)
 		if err := sw.scrapeInternal(timestamp, timestamp); err != nil {
 			t.Fatalf("unexpected error: %s", err)
 		}