pkg/prometheus: enable Thanos uploads only when needed (#3485)

When the Thanos spec doesn't configure object storage, there's no need to
configure the Thanos sidecar for block uploads and mount the
Prometheus data volume.

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

pkg/prometheus/statefulset.go

@@ -729,7 +729,6 @@ func makeStatefulSetSpec(p monitoringv1.Prometheus, c *Config, ruleConfigMapName
 
 		thanosArgs := []string{"sidecar",
 			fmt.Sprintf("--prometheus.url=http://%s:9090%s", c.LocalHost, path.Clean(webRoutePrefix)),
-			fmt.Sprintf("--tsdb.path=%s", storageDir),
 			fmt.Sprintf("--grpc-address=%s:10901", bindAddress),
 			fmt.Sprintf("--http-address=%s:10902", bindAddress),
 		}
@@ -772,13 +771,6 @@ func makeStatefulSetSpec(p monitoringv1.Prometheus, c *Config, ruleConfigMapName
 					ContainerPort: 10901,
 				},
 			},
-			VolumeMounts: []v1.VolumeMount{
-				{
-					Name:      volName,
-					MountPath: storageDir,
-					SubPath:   subPathForStorage(p.Spec.Storage),
-				},
-			},
 			Resources: p.Spec.Thanos.Resources,
 		}
 
@@ -790,6 +782,17 @@ func makeStatefulSetSpec(p monitoringv1.Prometheus, c *Config, ruleConfigMapName
 					SecretKeyRef: p.Spec.Thanos.ObjectStorageConfig,
 				},
 			})
+
+			container.Args = append(container.Args, fmt.Sprintf("--tsdb.path=%s", storageDir))
+			container.VolumeMounts = append(
+				container.VolumeMounts,
+				v1.VolumeMount{
+					Name:      volName,
+					MountPath: storageDir,
+					SubPath:   subPathForStorage(p.Spec.Storage),
+				},
+			)
+
 			// 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.
 			disableCompaction = true

pkg/prometheus/statefulset_test.go

@@ -780,12 +780,24 @@ func TestThanosNoObjectStorage(t *testing.T) {
 	}
 
 	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)
+		t.Fatalf("expected 3rd container 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")
+			t.Fatal("Prometheus compaction should be disabled")
+		}
+	}
+
+	for _, arg := range sset.Spec.Template.Spec.Containers[2].Args {
+		if strings.HasPrefix(arg, "--tsdb.path=") {
+			t.Fatal("--tsdb.path argument should not be given to the Thanos sidecar")
+		}
+	}
+
+	for _, vol := range sset.Spec.Template.Spec.Containers[2].VolumeMounts {
+		if vol.MountPath == storageDir {
+			t.Fatal("Prometheus data volume should not be mounted in the Thanos sidecar")
 		}
 	}
 }
@@ -853,6 +865,32 @@ func TestThanosObjectStorage(t *testing.T) {
 			t.Fatalf("Prometheus is missing expected argument: %s", expectedArg)
 		}
 	}
+
+	{
+		var found bool
+		for _, arg := range sset.Spec.Template.Spec.Containers[2].Args {
+			if strings.HasPrefix(arg, "--tsdb.path=") {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Fatalf("--tsdb.path argument should be given to the Thanos sidecar, got %q", strings.Join(sset.Spec.Template.Spec.Containers[3].Args, " "))
+		}
+	}
+
+	{
+		var found bool
+		for _, vol := range sset.Spec.Template.Spec.Containers[2].VolumeMounts {
+			if vol.MountPath == storageDir {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Fatal("Prometheus data volume should be mounted in the Thanos sidecar")
+		}
+	}
 }
 
 func TestThanosTracing(t *testing.T) {