Do not disable compaction on sidecar without object storage configura…

…tion (prometheus-operator#2845)

* Do not disable compaction on sidecar without object storage configuration.

... as it's not required (:


Also:
* updated Thanos docs!
* added tests
* bumped default image.

Signed-off-by: Bartek Plotka <bwplotka@gmail.com>

* Addressed comments.

Signed-off-by: Bartek Plotka <bwplotka@gmail.com>

* Update pkg/prometheus/statefulset.go

Co-Authored-By: Lili Cosic <cosiclili@gmail.com>

* Reverted prom to 2.7.1

Signed-off-by: Bartek Plotka <bwplotka@gmail.com>

Documentation/thanos.md

@@ -2,32 +2,55 @@
 
 _Note: This guide is valid for Prometheus Operator v0.28+ and Thanos v0.2+ and above._
 
-[Thanos](https://github.com/thanos-io/thanos/) is a set of components
-that can be composed into a highly available
-metric system with unlimited storage capacity, if your Object Storage allows for it.
-The Prometheus Operator provides integration for allowing Prometheus to connect to Thanos.
+[Thanos](https://github.com/thanos-io/thanos/) is a set of components that can be composed into a highly available, multi
+Prometheus metric system with potentially unlimited storage capacity, if your Object Storage allows for it.
 
-Thanos components include the rulers, compactors, queries and stores, which Thanos needs to
-be fully functional, and should be deployed independently of the Prometheus
-Operator and its Thanos configuration. The
-[kube-thanos](https://github.com/metalmatze/kube-thanos/) project has some experimental
-starting points as well as the [thanos
-project](https://github.com/thanos-io/thanos/blob/master/tutorials/kubernetes-demo/manifests).
+## What Prometheus Operator helps with?
 
-In short, for the Thanos integration using the Prometheus Operator to work
-correctly you will need to have these extra components installed and
-configured.
+Prometheus Operator operates Prometheus, not Thanos. However Thanos system integrates with existing setup by adding 
+sidecar to each Prometheus running in the system.
+
+Please before continuing with Prometheus Operator Thanos integration, read more about Thanos in the [documentation](https://thanos.io/getting-started.md/).
 
-Now please take the time to look at the Thanos README and read through the documentation before continuing with the Prometheus Operator integration.
+Prometheus Operator allows your to optionally add Thanos sidecar to Prometheus. Sidecar allows to hook into Thanos
+querying system as well as **optionally** back up your data in object storage.
 
-https://github.com/thanos-io/thanos 
-https://github.com/thanos-io/thanos/blob/master/docs/getting-started.md
+Thanos system includes other components like queriers or rulers. To get the advantage of object storage it also requires compactors and stores.
+
+All beside the sidecar should be deployed independently of the Prometheus Operator and its Thanos configuration. The
+[kube-thanos](https://github.com/thanos-io/kube-thanos/) project has some starting points for other Thanos components deployments.
+
+In short, for the Thanos integration using the Prometheus Operator to work correctly you will need to have these extra
+components installed and configured.
 
 ## Prometheus Operator
 
-### Configuring Thanos Object Storage
+Let's walk through the process of adding Thanos sidecar to Prometheus Operator.
+
+### Prometheus Custom Resource with Thanos Sidecar
+
+The `Prometheus` CRD has support for adding a Thanos sidecar to the Prometheus
+Pod. To enable the sidecar, reference the following examples.
+
+This is the simplest configuration change that needs to be made to your
+Prometheus Custom Resource, after creating the secret.
 
-Beginning with Thanos v0.2 the sidecar assumes an existing Kubernetes Secret containing the Thanos configuration.
+```
+...
+spec:
+  ...
+  thanos:
+    baseImage: quay.io/thanos/thanos
+    version: v0.2.1
+...
+```
+Note: If you're using Istio you may need to also set `ListenLocal` on the Thanos spec due to Istio's forwarding of traffic to localhost.
+
+### Optional: Configuring Thanos Object Storage
+
+If you want sidecar to be able to upload blocks to object storage you need to tell Prometheus Operator about it.
+
+In this mode, sidecar assumes an existing Kubernetes Secret containing the Thanos configuration.
 Inside this secret you configure how to run Thanos with your object storage.
 
 For more information and examples about the configuration itself, take a look at the Thanos documentation:  
@@ -51,13 +74,7 @@ Let's assume you saved this file to `/tmp/thanos-config.yaml`. You can use the f
 kubectl -n monitoring create secret generic thanos-objstore-config --from-file=thanos.yaml=/tmp/thanos-config.yaml
 ```
 
-### Prometheus Custom Resource with Thanos Sidecar
-
-The `Prometheus` CRD has support for adding a Thanos sidecar to the Prometheus
-Pod. To enable the sidecar, reference the following examples.
-
-This is the simplest configuration change that needs to be made to your
-Prometheus Custom Resource, after creating the secret.
+And then you can specify this secret inside Thanose part of the Prometheus CRD we mentioned [earlier](#prometheus-custom-resource-with-thanos-sidecar):
 
 ```
 ...
@@ -71,18 +88,24 @@ spec:
       name: thanos-objstore-config
 ...
 ```
-Note: If you're using Istio you may need to also set `ListenLocal` on the Thanos spec due to Istio's forwarding of traffic to localhost.
+
+This will attach Thanos sidecar that will backup all *new blocks* that Prometheus produces every 2 hours to the object storage.
+
+NOTE: This option will also disable local Prometheus compaction. This means that Thanos compactor is the main singleton component 
+responsible for compactions on a global, object storage level.
 
 ## Thanos and kube-thanos
 
-Deploying the sidecar was the first step towards getting Thanos up and running, but there are more components to be deployed, that complete Thanos.
+Deploying the sidecar was the first step towards getting Thanos up and running, but there are more components to be deployed, that complete Thanos:
+
+* [Querier](https://thanos.io/components/query.md/)
+
+Additionally, when object storage backup is desired:
 
-* Store
-* Querier
-* Compactor
+* [Store](https://thanos.io/components/store.md/)
+* [Compactor](https://thanos.io/components/compact.md/)
 
-Again, take a look at the Thanos documentation for more details on these components:  
-https://github.com/thanos-io/thanos/blob/master/docs/getting-started.md#store-api
+Again, take a look at the Thanos documentation for more details on these components: https://thanos.io/quick-tutorial.md
 
-Although kube-thanos project is still in early stage, it has already supported several thanos components. 
-For more details, please checkout [kube-thanos](https://github.com/metalmatze/kube-thanos/).
+kube-thanos project has already supported several thanos components. 
+For more details, please checkout [kube-thanos](https://github.com/thanos-io/kube-thanos/).

README.md

@@ -149,6 +149,15 @@ kubectl delete --ignore-not-found customresourcedefinitions \
     docker image on minikube's docker
 3. `make test-e2e`
 
+#### Running *end-to-end* tests on local kind cluster:
+
+1. `kind create cluster --image=kindest/node:<latest>`. e.g `v1.16.2` version. 
+2. `export KUBECONFIG="$(kind get kubeconfig-path --name="kind")"`
+3. `make image` - build Prometheus Operator  docker image locally.
+4. `for n in "operator" "config-reloader"; do kind load docker-image "quay.io/coreos/prometheus-$n:$(git rev-parse --short HEAD)"; done` - publish 
+built locally images to be accessible inside kind. 
+5. `make test-e2e`
+
 ## Contributing
 
 Many files (documentation, manifests, ...) in this repository are

pkg/prometheus/statefulset.go

@@ -35,7 +35,7 @@ import (
 const (
 	governingServiceName            = "prometheus-operated"
 	DefaultPrometheusVersion        = "v2.7.1"
-	DefaultThanosVersion            = "v0.7.0"
+	DefaultThanosVersion            = "v0.8.1"
 	defaultRetention                = "24h"
 	defaultReplicaExternalLabelName = "prometheus_replica"
 	storageDir                      = "/prometheus"
@@ -808,6 +808,9 @@ func makeStatefulSetSpec(p monitoringv1.Prometheus, c *Config, ruleConfigMapName
 					SecretKeyRef: p.Spec.Thanos.ObjectStorageConfig,
 				},
 			})
+			// NOTE(bwplotka): As described in https://thanos.io/components/sidecar.md/ we have to turn off compaction of Prometheus
+			// to avoid races during upload, if the uploads are configured.
+			promArgs = append(promArgs, "--storage.tsdb.max-block-duration=2h")
 		}
 
 		if p.Spec.LogLevel != "" {
@@ -816,9 +819,7 @@ func makeStatefulSetSpec(p monitoringv1.Prometheus, c *Config, ruleConfigMapName
 		if p.Spec.LogFormat != "" {
 			container.Args = append(container.Args, fmt.Sprintf("--log.format=%s", p.Spec.LogFormat))
 		}
-
 		additionalContainers = append(additionalContainers, container)
-		promArgs = append(promArgs, "--storage.tsdb.min-block-duration=2h", "--storage.tsdb.max-block-duration=2h")
 	}
 
 	// Version is used by default.

pkg/prometheus/statefulset_test.go

@@ -17,6 +17,7 @@ package prometheus
 import (
 	"fmt"
 	"reflect"
+	"strings"
 	"testing"
 
 	monitoringv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
@@ -615,6 +616,31 @@ func TestThanosResourcesSet(t *testing.T) {
 	}
 }
 
+func TestThanosNoObjectStorage(t *testing.T) {
+	sset, err := makeStatefulSet(monitoringv1.Prometheus{
+		Spec: monitoringv1.PrometheusSpec{
+			Thanos: &monitoringv1.ThanosSpec{},
+		},
+	}, defaultTestConfig, nil, "")
+	if err != nil {
+		t.Fatalf("Unexpected error while making StatefulSet: %v", err)
+	}
+
+	if sset.Spec.Template.Spec.Containers[0].Name != "prometheus" {
+		t.Fatalf("expected 1st containers to be prometheus, got %s", sset.Spec.Template.Spec.Containers[0].Name)
+	}
+
+	if sset.Spec.Template.Spec.Containers[2].Name != "thanos-sidecar" {
+		t.Fatalf("expected 3rd containers to be thanos-sidecar, got %s", sset.Spec.Template.Spec.Containers[2].Name)
+	}
+
+	for _, arg := range sset.Spec.Template.Spec.Containers[0].Args {
+		if strings.HasPrefix(arg, "--storage.tsdb.max-block-duration=2h") {
+			t.Fatal("Prometheus compaction should be enabled")
+		}
+	}
+}
+
 func TestThanosObjectStorage(t *testing.T) {
 	testKey := "thanos-config-secret-test"
 
@@ -631,27 +657,52 @@ func TestThanosObjectStorage(t *testing.T) {
 		t.Fatalf("Unexpected error while making StatefulSet: %v", err)
 	}
 
+	if sset.Spec.Template.Spec.Containers[0].Name != "prometheus" {
+		t.Fatalf("expected 1st containers to be prometheus, got %s", sset.Spec.Template.Spec.Containers[0].Name)
+	}
+
+	if sset.Spec.Template.Spec.Containers[2].Name != "thanos-sidecar" {
+		t.Fatalf("expected 3rd containers to be thanos-sidecar, got %s", sset.Spec.Template.Spec.Containers[2].Name)
+	}
+
 	var containsEnvVar bool
 	for _, env := range sset.Spec.Template.Spec.Containers[2].Env {
 		if env.Name == "OBJSTORE_CONFIG" {
 			if env.ValueFrom.SecretKeyRef.Key == testKey {
 				containsEnvVar = true
+				break
 			}
 		}
 	}
 	if !containsEnvVar {
 		t.Fatalf("Thanos sidecar is missing expected OBJSTORE_CONFIG env var with correct value")
 	}
 
-	var containsArg bool
-	const expectedArg = "--objstore.config=$(OBJSTORE_CONFIG)"
-	for _, arg := range sset.Spec.Template.Spec.Containers[2].Args {
-		if arg == expectedArg {
-			containsArg = true
+	{
+		var containsArg bool
+		const expectedArg = "--objstore.config=$(OBJSTORE_CONFIG)"
+		for _, arg := range sset.Spec.Template.Spec.Containers[2].Args {
+			if arg == expectedArg {
+				containsArg = true
+				break
+			}
+		}
+		if !containsArg {
+			t.Fatalf("Thanos sidecar is missing expected argument: %s", expectedArg)
 		}
 	}
-	if !containsArg {
-		t.Fatalf("Thanos sidecar is missing expected argument: %s", expectedArg)
+	{
+		var containsArg bool
+		const expectedArg = "--storage.tsdb.max-block-duration=2h"
+		for _, arg := range sset.Spec.Template.Spec.Containers[0].Args {
+			if arg == expectedArg {
+				containsArg = true
+				break
+			}
+		}
+		if !containsArg {
+			t.Fatalf("Prometheus is missing expected argument: %s", expectedArg)
+		}
 	}
 }