From 2c61a478bece868dd6aae930380ffbae9c2b9e50 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lucas=20Serv=C3=A9n=20Mar=C3=ADn?= <lserven@gmail.com>
Date: Mon, 4 May 2020 18:03:31 +0200
Subject: [PATCH] Revert "Merge 0.12 into master (#2559)" (#2560)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 003d245282bd683826304d25d1719c39d7401629.

Signed-off-by: Lucas Servén Marín <lserven@gmail.com>
---
 CHANGELOG.md                                  |   7 --
 VERSION                                       |   2 +-
 cmd/thanos/rule.go                            |   2 +-
 pkg/block/fetcher.go                          |   2 -
 pkg/compact/compact_e2e_test.go               | 100 ------------------
 test/e2e/rule_test.go                         |  59 ++---------
 .../thanos/1-globalview/courseBase.sh         |   2 +-
 .../katacoda/thanos/1-globalview/step2.md     |   8 +-
 .../katacoda/thanos/1-globalview/step3.md     |   2 +-
 9 files changed, 14 insertions(+), 170 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a91ab498a4..eb00c7444f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -29,13 +29,6 @@ We use *breaking* word for marking changes that are not backward compatible (rel
 moved `thanos check rules` to `thanos tools rules-check`. `thanos tools rules-check` also takes rules by `--rules` repeated flag not argument
 anymore.
 
-## [v0.12.2](https://github.com/thanos-io/thanos/releases/tag/v0.12.2) - 2020.04.30
-
-### Fixed
-
-- [#2459](https://github.com/thanos-io/thanos/issues/2459) Compact: Fixed issue with old blocks being marked and deleted in a (slow) loop.
-- [#2533](https://github.com/thanos-io/thanos/pull/2515) Rule: do not wrap reload endpoint with `/`. Makes `/-/reload` accessible again when no prefix has been specified.
-
 ## [v0.12.1](https://github.com/thanos-io/thanos/releases/tag/v0.12.1) - 2020.04.20
 
 ### Fixed
diff --git a/VERSION b/VERSION
index 26acbf080b..34a83616bb 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-0.12.2
+0.12.1
diff --git a/cmd/thanos/rule.go b/cmd/thanos/rule.go
index d49409627b..33889f7c99 100644
--- a/cmd/thanos/rule.go
+++ b/cmd/thanos/rule.go
@@ -561,7 +561,7 @@ func runRule(
 			router = router.WithPrefix(webRoutePrefix)
 		}
 
-		router.Post("/-/reload", func(w http.ResponseWriter, r *http.Request) {
+		router.WithPrefix(webRoutePrefix).Post("/-/reload", func(w http.ResponseWriter, r *http.Request) {
 			reloadMsg := make(chan error)
 			reloadWebhandler <- reloadMsg
 			if err := <-reloadMsg; err != nil {
diff --git a/pkg/block/fetcher.go b/pkg/block/fetcher.go
index 835f2ff191..8652596f58 100644
--- a/pkg/block/fetcher.go
+++ b/pkg/block/fetcher.go
@@ -559,8 +559,6 @@ func NewDeduplicateFilter() *DeduplicateFilter {
 // Filter filters out duplicate blocks that can be formed
 // from two or more overlapping blocks that fully submatches the source blocks of the older blocks.
 func (f *DeduplicateFilter) Filter(_ context.Context, metas map[ulid.ULID]*metadata.Meta, synced *extprom.TxGaugeVec) error {
-	f.duplicateIDs = f.duplicateIDs[:0]
-
 	var wg sync.WaitGroup
 
 	metasByResolution := make(map[int64][]*metadata.Meta)
diff --git a/pkg/compact/compact_e2e_test.go b/pkg/compact/compact_e2e_test.go
index 6bf8a44887..a6650fec0f 100644
--- a/pkg/compact/compact_e2e_test.go
+++ b/pkg/compact/compact_e2e_test.go
@@ -129,7 +129,6 @@ func TestSyncer_GarbageCollect_e2e(t *testing.T) {
 
 		// After another sync the changes should also be reflected in the local groups.
 		testutil.Ok(t, sy.SyncMetas(ctx))
-		testutil.Ok(t, sy.GarbageCollect(ctx))
 
 		// Only the level 3 block, the last source block in both resolutions should be left.
 		groups, err := sy.Groups()
@@ -417,102 +416,3 @@ func createAndUpload(t testing.TB, bkt objstore.Bucket, blocks []blockgenSpec) (
 	}
 	return metas
 }
-
-// Regression test for #2459 issue.
-func TestGarbageCollectDoesntCreateEmptyBlocksWithDeletionMarksOnly(t *testing.T) {
-	logger := log.NewLogfmtLogger(os.Stderr)
-
-	objtesting.ForeachStore(t, func(t *testing.T, bkt objstore.Bucket) {
-		ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
-		defer cancel()
-
-		// Generate two blocks, and then another block that covers both of them.
-		var metas []*metadata.Meta
-		var ids []ulid.ULID
-
-		for i := 0; i < 2; i++ {
-			var m metadata.Meta
-
-			m.Version = 1
-			m.ULID = ulid.MustNew(uint64(i), nil)
-			m.Compaction.Sources = []ulid.ULID{m.ULID}
-			m.Compaction.Level = 1
-
-			ids = append(ids, m.ULID)
-			metas = append(metas, &m)
-		}
-
-		var m1 metadata.Meta
-		m1.Version = 1
-		m1.ULID = ulid.MustNew(100, nil)
-		m1.Compaction.Level = 2
-		m1.Compaction.Sources = ids
-		m1.Thanos.Downsample.Resolution = 0
-
-		// Create all blocks in the bucket.
-		for _, m := range append(metas, &m1) {
-			fmt.Println("create", m.ULID)
-			var buf bytes.Buffer
-			testutil.Ok(t, json.NewEncoder(&buf).Encode(&m))
-			testutil.Ok(t, bkt.Upload(ctx, path.Join(m.ULID.String(), metadata.MetaFilename), &buf))
-		}
-
-		blocksMarkedForDeletion := promauto.With(nil).NewCounter(prometheus.CounterOpts{})
-		ignoreDeletionMarkFilter := block.NewIgnoreDeletionMarkFilter(nil, objstore.WithNoopInstr(bkt), 48*time.Hour)
-
-		duplicateBlocksFilter := block.NewDeduplicateFilter()
-		metaFetcher, err := block.NewMetaFetcher(nil, 32, objstore.WithNoopInstr(bkt), "", nil, []block.MetadataFilter{
-			ignoreDeletionMarkFilter,
-			duplicateBlocksFilter,
-		}, nil)
-		testutil.Ok(t, err)
-
-		sy, err := NewSyncer(nil, nil, bkt, metaFetcher, duplicateBlocksFilter, ignoreDeletionMarkFilter, blocksMarkedForDeletion, 1, false, false)
-		testutil.Ok(t, err)
-
-		// Do one initial synchronization with the bucket.
-		testutil.Ok(t, sy.SyncMetas(ctx))
-		testutil.Ok(t, sy.GarbageCollect(ctx))
-		testutil.Equals(t, 2.0, promtest.ToFloat64(sy.metrics.garbageCollectedBlocks))
-
-		rem, err := listBlocksMarkedForDeletion(ctx, bkt)
-		testutil.Ok(t, err)
-
-		sort.Slice(rem, func(i, j int) bool {
-			return rem[i].Compare(rem[j]) < 0
-		})
-
-		testutil.Equals(t, ids, rem)
-
-		// Delete source blocks.
-		for _, id := range ids {
-			testutil.Ok(t, block.Delete(ctx, logger, bkt, id))
-		}
-
-		// After another garbage-collect, we should not find new blocks that are deleted with new deletion mark files.
-		testutil.Ok(t, sy.SyncMetas(ctx))
-		testutil.Ok(t, sy.GarbageCollect(ctx))
-
-		rem, err = listBlocksMarkedForDeletion(ctx, bkt)
-		testutil.Ok(t, err)
-		testutil.Equals(t, 0, len(rem))
-	})
-}
-
-func listBlocksMarkedForDeletion(ctx context.Context, bkt objstore.Bucket) ([]ulid.ULID, error) {
-	var rem []ulid.ULID
-	err := bkt.Iter(ctx, "", func(n string) error {
-		id := ulid.MustParse(n[:len(n)-1])
-		deletionMarkFile := path.Join(id.String(), metadata.DeletionMarkFilename)
-
-		exists, err := bkt.Exists(ctx, deletionMarkFile)
-		if err != nil {
-			return err
-		}
-		if exists {
-			rem = append(rem, id)
-		}
-		return nil
-	})
-	return rem, err
-}
diff --git a/test/e2e/rule_test.go b/test/e2e/rule_test.go
index 0e87b4a9dd..3a601608b8 100644
--- a/test/e2e/rule_test.go
+++ b/test/e2e/rule_test.go
@@ -59,44 +59,18 @@ groups:
       severity: page
     annotations:
       summary: "I always complain and allow partial response in query."
-`
-	testAlertRuleAddedLaterWebHandler = `
-groups:
-- name: example
-  partial_response_strategy: "WARN"
-  rules:
-  - alert: TestAlert_HasBeenLoadedViaWebHandler
-    # It must be based on actual metric, otherwise call to StoreAPI would be not involved.
-    expr: absent(some_metric)
-    labels:
-      severity: page
-    annotations:
-      summary: "I always complain and I have been loaded via /-/reload."
 `
 )
 
-func createRuleFile(t *testing.T, path, content string) {
-	t.Helper()
-	err := ioutil.WriteFile(path, []byte(content), 0666)
-	testutil.Ok(t, err)
-}
-
 func createRuleFiles(t *testing.T, dir string) {
 	t.Helper()
 
 	for i, rule := range []string{testAlertRuleAbortOnPartialResponse, testAlertRuleWarnOnPartialResponse} {
-		createRuleFile(t, filepath.Join(dir, fmt.Sprintf("rules-%d.yaml", i)), rule)
+		err := ioutil.WriteFile(filepath.Join(dir, fmt.Sprintf("rules-%d.yaml", i)), []byte(rule), 0666)
+		testutil.Ok(t, err)
 	}
 }
 
-func reloadRulesHTTP(t *testing.T, ctx context.Context, endpoint string) {
-	req, err := http.NewRequestWithContext(ctx, "POST", "http://"+endpoint+"/-/reload", ioutil.NopCloser(bytes.NewReader(nil)))
-	testutil.Ok(t, err)
-	resp, err := http.DefaultClient.Do(req)
-	testutil.Ok(t, err)
-	testutil.Equals(t, 200, resp.StatusCode)
-}
-
 func writeTargets(t *testing.T, path string, addrs ...string) {
 	t.Helper()
 
@@ -295,14 +269,10 @@ func TestRule(t *testing.T) {
 	testutil.Ok(t, err)
 	defer s.Close()
 
-	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
-	defer cancel()
-
 	// Prepare work dirs.
 	rulesSubDir := filepath.Join("rules")
-	rulesPath := filepath.Join(s.SharedDir(), rulesSubDir)
-	testutil.Ok(t, os.MkdirAll(rulesPath, os.ModePerm))
-	createRuleFiles(t, rulesPath)
+	testutil.Ok(t, os.MkdirAll(filepath.Join(s.SharedDir(), rulesSubDir), os.ModePerm))
+	createRuleFiles(t, filepath.Join(s.SharedDir(), rulesSubDir))
 	amTargetsSubDir := filepath.Join("rules_am_targets")
 	testutil.Ok(t, os.MkdirAll(filepath.Join(s.SharedDir(), amTargetsSubDir), os.ModePerm))
 	queryTargetsSubDir := filepath.Join("rules_query_targets")
@@ -463,13 +433,8 @@ func TestRule(t *testing.T) {
 		testutil.Ok(t, r.WaitSumMetrics(e2e.Equals(1), "thanos_ruler_alertmanagers_dns_provider_results"))
 	})
 
-	t.Run("reload works", func(t *testing.T) {
-		// Add a new rule via /-/reload.
-		// TODO(GiedriusS): add a test for reloading via SIGHUP. Need to extend e2e framework to expose PIDs.
-
-		createRuleFile(t, fmt.Sprintf("%s/newrule.yaml", rulesPath), testAlertRuleAddedLaterWebHandler)
-		reloadRulesHTTP(t, ctx, r.HTTPEndpoint())
-	})
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
+	defer cancel()
 
 	queryAndAssertSeries(t, ctx, q.HTTPEndpoint(), "ALERTS", promclient.QueryOptions{
 		Deduplicate: false,
@@ -481,13 +446,6 @@ func TestRule(t *testing.T) {
 			"alertstate": "firing",
 			"replica":    "1",
 		},
-		{
-			"__name__":   "ALERTS",
-			"severity":   "page",
-			"alertname":  "TestAlert_HasBeenLoadedViaWebHandler",
-			"alertstate": "firing",
-			"replica":    "1",
-		},
 		{
 			"__name__":   "ALERTS",
 			"severity":   "page",
@@ -503,11 +461,6 @@ func TestRule(t *testing.T) {
 			"alertname": "TestAlert_AbortOnPartialResponse",
 			"replica":   "1",
 		},
-		{
-			"severity":  "page",
-			"alertname": "TestAlert_HasBeenLoadedViaWebHandler",
-			"replica":   "1",
-		},
 		{
 			"severity":  "page",
 			"alertname": "TestAlert_WarnOnPartialResponse",
diff --git a/tutorials/katacoda/thanos/1-globalview/courseBase.sh b/tutorials/katacoda/thanos/1-globalview/courseBase.sh
index 50febd39ec..c06dec0cf9 100644
--- a/tutorials/katacoda/thanos/1-globalview/courseBase.sh
+++ b/tutorials/katacoda/thanos/1-globalview/courseBase.sh
@@ -1,4 +1,4 @@
 #!/usr/bin/env bash
 
 docker pull quay.io/prometheus/prometheus:v2.16.0
-docker pull quay.io/thanos/thanos:v0.12.2
+docker pull quay.io/thanos/thanos:v0.12.1
diff --git a/tutorials/katacoda/thanos/1-globalview/step2.md b/tutorials/katacoda/thanos/1-globalview/step2.md
index 053f081e7b..af4b9d15b6 100644
--- a/tutorials/katacoda/thanos/1-globalview/step2.md
+++ b/tutorials/katacoda/thanos/1-globalview/step2.md
@@ -10,7 +10,7 @@ component and can be invoked in a single command.
 Let's take a look at all the Thanos commands:
 
 ```
-docker run --rm quay.io/thanos/thanos:v0.12.2 --help
+docker run --rm quay.io/thanos/thanos:v0.12.1 --help
 ```{{execute}}
 
 You should see multiple commands that solves different purposes.
@@ -53,7 +53,7 @@ docker run -d --net=host --rm \
     -v $(pwd)/prometheus0_eu1.yml:/etc/prometheus/prometheus.yml \
     --name prometheus-0-sidecar-eu1 \
     -u root \
-    quay.io/thanos/thanos:v0.12.2 \
+    quay.io/thanos/thanos:v0.12.1 \
     sidecar \
     --http-address 0.0.0.0:19090 \
     --grpc-address 0.0.0.0:19190 \
@@ -68,7 +68,7 @@ docker run -d --net=host --rm \
     -v $(pwd)/prometheus0_us1.yml:/etc/prometheus/prometheus.yml \
     --name prometheus-0-sidecar-us1 \
     -u root \
-    quay.io/thanos/thanos:v0.12.2 \
+    quay.io/thanos/thanos:v0.12.1 \
     sidecar \
     --http-address 0.0.0.0:19091 \
     --grpc-address 0.0.0.0:19191 \
@@ -81,7 +81,7 @@ docker run -d --net=host --rm \
     -v $(pwd)/prometheus1_us1.yml:/etc/prometheus/prometheus.yml \
     --name prometheus-1-sidecar-us1 \
     -u root \
-    quay.io/thanos/thanos:v0.12.2 \
+    quay.io/thanos/thanos:v0.12.1 \
     sidecar \
     --http-address 0.0.0.0:19092 \
     --grpc-address 0.0.0.0:19192 \
diff --git a/tutorials/katacoda/thanos/1-globalview/step3.md b/tutorials/katacoda/thanos/1-globalview/step3.md
index 23c58c34f1..8d4b17e84d 100644
--- a/tutorials/katacoda/thanos/1-globalview/step3.md
+++ b/tutorials/katacoda/thanos/1-globalview/step3.md
@@ -28,7 +28,7 @@ Click below snippet to start the Querier.
 ```
 docker run -d --net=host --rm \
     --name querier \
-    quay.io/thanos/thanos:v0.12.2 \
+    quay.io/thanos/thanos:v0.12.1 \
     query \
     --http-address 0.0.0.0:29090 \
     --query.replica-label replica \