Move purger API endpoints to compactor (#2644)

* Move purger API endpoints to compactor Signed-off-by: Levi Harrison <git@leviharrison.dev> * Update changelog Signed-off-by: Levi Harrison <git@leviharrison.dev> * Remove purger from places Signed-off-by: Levi Harrison <git@leviharrison.dev> * Fixed indentation Signed-off-by: Marco Pracucci <marco@pracucci.com> * Update CHANGELOG.md Co-authored-by: Marco Pracucci <marco@pracucci.com>
grafana · Aug 8, 2022 · 23afa07 · 23afa07
1 parent 69a52e5
commit 23afa07
Show file tree

Hide file tree

Showing 11 changed files with 49 additions and 150 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@
 * [CHANGE] Distributor: removed previously deprecated `extend_writes` (see #1856) YAML key and `-distributor.extend-writes` CLI flag from the distributor config. #2551
 * [CHANGE] Ingester: removed previously deprecated `active_series_custom_trackers` (see #1188) YAML key from the ingester config. #2552
 * [CHANGE] The tenant ID `__mimir_cluster` is reserved by Mimir and not allowed to store metrics. #2643
+* [CHANGE] Purger: removed the purger component and moved its API endpoints `/purger/delete_tenant` and `/purger/delete_tenant_status` to the compactor at `/compactor/delete_tenant` and `/compactor/delete_tenant_status`. #2644
 * [FEATURE] Compactor: Adds the ability to delete partial blocks after a configurable delay. This option can be configured per tenant. #2285
   - `-compactor.partial-block-deletion-delay`, as a duration string, allows you to set the delay since a partial block has been modified before marking it for deletion. A value of `0`, the default, disables this feature.
   - The metric `cortex_compactor_blocks_marked_for_deletion_total` has a new value for the `reason` label `reason="partial"`, when a block deletion marker is triggered by the partial block deletion delay.

diff --git a/development/tsdb-blocks-storage-s3/docker-compose.jsonnet b/development/tsdb-blocks-storage-s3/docker-compose.jsonnet
@@ -26,7 +26,6 @@ std.manifestYamlDoc({
     self.compactor +
     self.rulers(2) +
     self.alertmanagers(3) +
-    self.purger +
     self.minio +
     self.prometheus +
     self.grafana_agent +
@@ -133,13 +132,6 @@ std.manifestYamlDoc({
     }),
   },
 
-  purger:: {
-    purger: mimirService({
-      target: 'purger',
-      httpPort: 8014,
-    }),
-  },
-
   local all_rings = ['-ingester.ring', '-distributor.ring', '-compactor.ring', '-store-gateway.sharding-ring', '-ruler.ring', '-alertmanager.sharding-ring'],
 
   // This function builds docker-compose declaration for Mimir service.

diff --git a/development/tsdb-blocks-storage-s3/docker-compose.yml b/development/tsdb-blocks-storage-s3/docker-compose.yml
@@ -228,29 +228,6 @@
       - "9090:9090"
     "volumes":
       - "./config:/etc/prometheus"
-  "purger":
-    "build":
-      "context": "."
-      "dockerfile": "dev.dockerfile"
-    "command":
-      - "sh"
-      - "-c"
-      - "sleep 3 && exec ./mimir -config.file=./config/mimir.yaml -target=purger -server.http-listen-port=8014 -server.grpc-listen-port=9014 -activity-tracker.filepath=/activity/purger-8014  -memberlist.nodename=purger -memberlist.bind-port=10014 -ingester.ring.store=memberlist -distributor.ring.store=memberlist -compactor.ring.store=memberlist -store-gateway.sharding-ring.store=memberlist -ruler.ring.store=memberlist -alertmanager.sharding-ring.store=memberlist"
-    "depends_on":
-      - "minio"
-      - "distributor-1"
-    "environment":
-      - "JAEGER_AGENT_HOST=jaeger"
-      - "JAEGER_AGENT_PORT=6831"
-      - "JAEGER_SAMPLER_PARAM=1"
-      - "JAEGER_SAMPLER_TYPE=const"
-      - "JAEGER_TAGS=app=purger"
-    "image": "mimir"
-    "ports":
-      - "8014:8014"
-    "volumes":
-      - "./config:/mimir/config"
-      - "./activity:/activity"
   "querier":
     "build":
       "context": "."

diff --git a/docs/sources/operators-guide/configure/about-versioning.md b/docs/sources/operators-guide/configure/about-versioning.md
@@ -54,7 +54,6 @@ The following features are currently experimental:
     - `-distributor.request-rate-limit`
     - `-distributor.request-burst-limit`
   - OTLP ingestion path
-- Purger: Tenant deletion API
 - Exemplar storage
   - `-ingester.max-global-exemplars-per-user`
   - `-ingester.exemplars-update-period`
@@ -96,6 +95,7 @@ The following features are currently experimental:
   - `-ruler-storage.storage-prefix`
 - Compactor
   - HTTP API for uploading TSDB blocks
+  - Tenant deletion API
 
 ## Deprecated features
 

diff --git a/docs/sources/operators-guide/reference-http-api/index.md b/docs/sources/operators-guide/reference-http-api/index.md
@@ -71,8 +71,6 @@ This document groups API endpoints by service. Note that the API endpoints are e
 | [Get Alertmanager configuration](#get-alertmanager-configuration)                     | Alertmanager            | `GET /api/v1/alerts`                                                      |
 | [Set Alertmanager configuration](#set-alertmanager-configuration)                     | Alertmanager            | `POST /api/v1/alerts`                                                     |
 | [Delete Alertmanager configuration](#delete-alertmanager-configuration)               | Alertmanager            | `DELETE /api/v1/alerts`                                                   |
-| [Tenant delete request](#tenant-delete-request)                                       | Purger                  | `POST /purger/delete_tenant`                                              |
-| [Tenant delete status](#tenant-delete-status)                                         | Purger                  | `GET /purger/delete_tenant_status`                                        |
 | [Store-gateway ring status](#store-gateway-ring-status)                               | Store-gateway           | `GET /store-gateway/ring`                                                 |
 | [Store-gateway tenants](#store-gateway-tenants)                                       | Store-gateway           | `GET /store-gateway/tenants`                                              |
 | [Store-gateway tenant blocks](#store-gateway-tenant-blocks)                           | Store-gateway           | `GET /store-gateway/tenant/{tenant}/blocks`                               |
@@ -81,6 +79,8 @@ This document groups API endpoints by service. Note that the API endpoints are e
 | [Upload block file](#upload-block-file)                                               | Compactor               | `POST /api/v1/upload/block/{block}/files?path={path}`                     |
 | [Complete block upload](#complete-block-upload)                                       | Compactor               | `POST /api/v1/upload/block/{block}/finish`                                |
 | [Check block upload](#check-block-upload)                                             | Compactor               | `GET /api/v1/upload/block/{block}/check`                                  |
+| [Tenant delete request](#tenant-delete-request)                                       | Compactor               | `POST /compactor/delete_tenant`                                           |
+| [Tenant delete status](#tenant-delete-status)                                         | Compactor               | `GET /compactor/delete_tenant_status`                                     |
 
 ### Path prefixes
 
@@ -862,30 +862,6 @@ Requires [authentication](#authentication).
 
 > **Note:** To delete a tenant's Alertmanager configuration from Mimir, use [`mimirtool alertmanager delete` command]({{< relref "../tools/mimirtool.md#delete-alertmanager-configuration" >}}).
 
-## Purger
-
-The Purger service provides APIs for requesting tenant deletion.
-
-### Tenant Delete Request
-
-```
-POST /purger/delete_tenant
-```
-
-Request deletion of ALL tenant data. Experimental.
-
-Requires [authentication](#authentication).
-
-### Tenant Delete Status
-
-```
-GET /purger/delete_tenant_status
-```
-
-Returns status of tenant deletion. Output format to be defined. Experimental.
-
-Requires [authentication](#authentication).
-
 ## Store-gateway
 
 ### Store-gateway ring status
@@ -1014,3 +990,23 @@ Returns state of the block upload. State is returned as JSON object with field `
 Requires [authentication](#authentication).
 
 This API endpoint is experimental and subject to change.
+
+### Tenant Delete Request
+
+```
+POST /compactor/delete_tenant
+```
+
+Request deletion of ALL tenant data. Experimental.
+
+Requires [authentication](#authentication).
+
+### Tenant Delete Status
+
+```
+GET /compactor/delete_tenant_status
+```
+
+Returns status of tenant deletion. Output format to be defined. Experimental.
+
+Requires [authentication](#authentication).
diff --git a/integration/asserts.go b/integration/asserts.go
@@ -28,7 +28,6 @@ const (
 	AlertManager
 	Ruler
 	StoreGateway
-	Purger
 )
 
 var (
@@ -42,7 +41,6 @@ var (
 		AlertManager:   {"cortex_alertmanager"},
 		Ruler:          {},
 		StoreGateway:   {"!cortex_storegateway_client", "cortex_storegateway"}, // The metrics prefix cortex_storegateway_client may be used by other components so we ignore it.
-		Purger:         {"cortex_purger"},
 	}
 
 	// Blacklisted metrics prefixes across any Mimir service.

diff --git a/integration/e2emimir/services.go b/integration/e2emimir/services.go
@@ -260,20 +260,6 @@ func NewRuler(name string, consulAddress string, flags map[string]string, option
 	)
 }
 
-func NewPurger(name string, flags map[string]string, options ...Option) *MimirService {
-	return newMimirServiceFromOptions(
-		name,
-		map[string]string{
-			"-target":                   "purger",
-			"-log.level":                "warn",
-			"-purger.object-store-type": "filesystem",
-			"-local.chunk-directory":    e2e.ContainerSharedDir,
-		},
-		flags,
-		options...,
-	)
-}
-
 // Options holds a set of options for running services, they can be altered passing Option funcs.
 type Options struct {
 	Image      string

diff --git a/pkg/api/api.go b/pkg/api/api.go
@@ -33,7 +33,6 @@ import (
 	"github.com/grafana/mimir/pkg/frontend/v2/frontendv2pb"
 	"github.com/grafana/mimir/pkg/ingester/client"
 	"github.com/grafana/mimir/pkg/mimirpb"
-	"github.com/grafana/mimir/pkg/purger"
 	"github.com/grafana/mimir/pkg/querier"
 	"github.com/grafana/mimir/pkg/ruler"
 	"github.com/grafana/mimir/pkg/scheduler"
@@ -280,11 +279,6 @@ func (a *API) RegisterIngester(i Ingester, pushConfig distributor.Config) {
 	a.RegisterRoute("/ingester/push", push.Handler(pushConfig.MaxRecvMsgSize, a.sourceIPs, a.cfg.SkipLabelNameValidationHeader, i.PushWithCleanup), true, false, "POST") // For testing and debugging.
 }
 
-func (a *API) RegisterTenantDeletion(api *purger.TenantDeletionAPI) {
-	a.RegisterRoute("/purger/delete_tenant", http.HandlerFunc(api.DeleteTenant), true, true, "POST")
-	a.RegisterRoute("/purger/delete_tenant_status", http.HandlerFunc(api.DeleteTenantStatus), true, true, "GET")
-}
-
 // RegisterRuler registers routes associated with the Ruler service.
 func (a *API) RegisterRuler(r *ruler.Ruler) {
 	a.indexPage.AddLinks(defaultWeight, "Ruler", []IndexPageLink{
@@ -351,6 +345,8 @@ func (a *API) RegisterCompactor(c *compactor.MultitenantCompactor) {
 	a.RegisterRoute("/api/v1/upload/block/{block}/files", http.HandlerFunc(c.UploadBlockFile), true, false, http.MethodPost)
 	a.RegisterRoute("/api/v1/upload/block/{block}/finish", http.HandlerFunc(c.FinishBlockUpload), true, false, http.MethodPost)
 	a.RegisterRoute("/api/v1/upload/block/{block}/check", http.HandlerFunc(c.GetBlockUploadStateHandler), true, false, http.MethodGet)
+	a.RegisterRoute("/compactor/delete_tenant", http.HandlerFunc(c.DeleteTenant), true, true, "POST")
+	a.RegisterRoute("/compactor/delete_tenant_status", http.HandlerFunc(c.DeleteTenantStatus), true, true, "GET")
 }
 
 type Distributor interface {

diff --git a/pkg/purger/tenant_deletion_api.go → pkg/compactor/tenant_deletion_api.go b/pkg/purger/tenant_deletion_api.go → pkg/compactor/tenant_deletion_api.go
@@ -3,20 +3,17 @@
 // Provenance-includes-license: Apache-2.0
 // Provenance-includes-copyright: The Cortex Authors.
 
-package purger
+package compactor
 
 import (
 	"context"
 	"net/http"
 	"strings"
 	"time"
 
-	"github.com/go-kit/log"
 	"github.com/go-kit/log/level"
 	"github.com/oklog/ulid"
 	"github.com/pkg/errors"
-	"github.com/prometheus/client_golang/prometheus"
-	"github.com/thanos-io/thanos/pkg/objstore"
 
 	"github.com/grafana/dskit/tenant"
 
@@ -25,30 +22,7 @@ import (
 	"github.com/grafana/mimir/pkg/util"
 )
 
-type TenantDeletionAPI struct {
-	bucketClient objstore.Bucket
-	logger       log.Logger
-	cfgProvider  bucket.TenantConfigProvider
-}
-
-func NewTenantDeletionAPI(storageCfg mimir_tsdb.BlocksStorageConfig, cfgProvider bucket.TenantConfigProvider, logger log.Logger, reg prometheus.Registerer) (*TenantDeletionAPI, error) {
-	bucketClient, err := createBucketClient(storageCfg, logger, reg)
-	if err != nil {
-		return nil, err
-	}
-
-	return newTenantDeletionAPI(bucketClient, cfgProvider, logger), nil
-}
-
-func newTenantDeletionAPI(bkt objstore.Bucket, cfgProvider bucket.TenantConfigProvider, logger log.Logger) *TenantDeletionAPI {
-	return &TenantDeletionAPI{
-		bucketClient: bkt,
-		cfgProvider:  cfgProvider,
-		logger:       logger,
-	}
-}
-
-func (api *TenantDeletionAPI) DeleteTenant(w http.ResponseWriter, r *http.Request) {
+func (c *MultitenantCompactor) DeleteTenant(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	userID, err := tenant.TenantID(ctx)
 	if err != nil {
@@ -58,15 +32,15 @@ func (api *TenantDeletionAPI) DeleteTenant(w http.ResponseWriter, r *http.Reques
 		return
 	}
 
-	err = mimir_tsdb.WriteTenantDeletionMark(r.Context(), api.bucketClient, userID, api.cfgProvider, mimir_tsdb.NewTenantDeletionMark(time.Now()))
+	err = mimir_tsdb.WriteTenantDeletionMark(r.Context(), c.bucketClient, userID, c.cfgProvider, mimir_tsdb.NewTenantDeletionMark(time.Now()))
 	if err != nil {
-		level.Error(api.logger).Log("msg", "failed to write tenant deletion mark", "user", userID, "err", err)
+		level.Error(c.logger).Log("msg", "failed to write tenant deletion mark", "user", userID, "err", err)
 
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
-	level.Info(api.logger).Log("msg", "tenant deletion mark in blocks storage created", "user", userID)
+	level.Info(c.logger).Log("msg", "tenant deletion mark in blocks storage created", "user", userID)
 
 	w.WriteHeader(http.StatusOK)
 }
@@ -76,7 +50,7 @@ type DeleteTenantStatusResponse struct {
 	BlocksDeleted bool   `json:"blocks_deleted"`
 }
 
-func (api *TenantDeletionAPI) DeleteTenantStatus(w http.ResponseWriter, r *http.Request) {
+func (c *MultitenantCompactor) DeleteTenantStatus(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	userID, err := tenant.TenantID(ctx)
 	if err != nil {
@@ -86,7 +60,7 @@ func (api *TenantDeletionAPI) DeleteTenantStatus(w http.ResponseWriter, r *http.
 
 	result := DeleteTenantStatusResponse{}
 	result.TenantID = userID
-	result.BlocksDeleted, err = api.isBlocksForUserDeleted(ctx, userID)
+	result.BlocksDeleted, err = c.isBlocksForUserDeleted(ctx, userID)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
@@ -95,10 +69,10 @@ func (api *TenantDeletionAPI) DeleteTenantStatus(w http.ResponseWriter, r *http.
 	util.WriteJSONResponse(w, result)
 }
 
-func (api *TenantDeletionAPI) isBlocksForUserDeleted(ctx context.Context, userID string) (bool, error) {
+func (c *MultitenantCompactor) isBlocksForUserDeleted(ctx context.Context, userID string) (bool, error) {
 	var errBlockFound = errors.New("block found")
 
-	userBucket := bucket.NewUserBucketClient(userID, api.bucketClient, api.cfgProvider)
+	userBucket := bucket.NewUserBucketClient(userID, c.bucketClient, c.cfgProvider)
 	err := userBucket.Iter(ctx, "", func(s string) error {
 		s = strings.TrimSuffix(s, "/")
 
@@ -123,12 +97,3 @@ func (api *TenantDeletionAPI) isBlocksForUserDeleted(ctx context.Context, userID
 	// No blocks found, all good.
 	return true, nil
 }
-
-func createBucketClient(cfg mimir_tsdb.BlocksStorageConfig, logger log.Logger, reg prometheus.Registerer) (objstore.Bucket, error) {
-	bucketClient, err := bucket.NewClient(context.Background(), cfg.Bucket, "purger", logger, reg)
-	if err != nil {
-		return nil, errors.Wrap(err, "create bucket client")
-	}
-
-	return bucketClient, nil
-}
diff --git a/pkg/purger/tenant_deletion_api_test.go → pkg/compactor/tenant_deletion_api_test.go b/pkg/purger/tenant_deletion_api_test.go → pkg/compactor/tenant_deletion_api_test.go
@@ -3,7 +3,7 @@
 // Provenance-includes-license: Apache-2.0
 // Provenance-includes-copyright: The Cortex Authors.
 
-package purger
+package compactor
 
 import (
 	"bytes"
@@ -13,7 +13,7 @@ import (
 	"path"
 	"testing"
 
-	"github.com/go-kit/log"
+	"github.com/grafana/dskit/services"
 	"github.com/stretchr/testify/require"
 	"github.com/thanos-io/thanos/pkg/objstore"
 	"github.com/weaveworks/common/user"
@@ -23,11 +23,14 @@ import (
 
 func TestDeleteTenant(t *testing.T) {
 	bkt := objstore.NewInMemBucket()
-	api := newTenantDeletionAPI(bkt, nil, log.NewNopLogger())
+	cfg := prepareConfig(t)
+	c, _, _, _, _ := prepare(t, cfg, bkt)
+	require.NoError(t, services.StartAndAwaitRunning(context.Background(), c))
+	t.Cleanup(stopServiceFn(t, c))
 
 	{
 		resp := httptest.NewRecorder()
-		api.DeleteTenant(resp, &http.Request{})
+		c.DeleteTenant(resp, &http.Request{})
 		require.Equal(t, http.StatusUnauthorized, resp.Code)
 	}
 
@@ -37,7 +40,7 @@ func TestDeleteTenant(t *testing.T) {
 
 		req := &http.Request{}
 		resp := httptest.NewRecorder()
-		api.DeleteTenant(resp, req.WithContext(ctx))
+		c.DeleteTenant(resp, req.WithContext(ctx))
 
 		require.Equal(t, http.StatusOK, resp.Code)
 		objs := bkt.Objects()
@@ -85,9 +88,12 @@ func TestDeleteTenantStatus(t *testing.T) {
 				require.NoError(t, bkt.Upload(context.Background(), objName, bytes.NewReader(data)))
 			}
 
-			api := newTenantDeletionAPI(bkt, nil, log.NewNopLogger())
+			cfg := prepareConfig(t)
+			c, _, _, _, _ := prepare(t, cfg, bkt)
+			require.NoError(t, services.StartAndAwaitRunning(context.Background(), c))
+			t.Cleanup(stopServiceFn(t, c))
 
-			res, err := api.isBlocksForUserDeleted(context.Background(), username)
+			res, err := c.isBlocksForUserDeleted(context.Background(), username)
 			require.NoError(t, err)
 			require.Equal(t, tc.expectedBlocksDeleted, res)
 		})