Revert "Merge branch 'po-fix-cache-invalidator-no-repo' into 'master'"po-cache-fix-fix

This reverts commit d2dc9d4b87c5e0d3416a78b3d47321078e5f93f9, reversing
changes made to 10d40838934326be0bc68e28ea3450be2d3b039c.

6 files changed, 10 insertions, 58 deletions

diff --git a/changelogs/unreleased/po-fix-cache-invalidator-no-repo.yml b/changelogs/unreleased/po-fix-cache-invalidator-no-repo.yml
deleted file mode 100644
index fef61d144..000000000
--- a/changelogs/unreleased/po-fix-cache-invalidator-no-repo.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: Fix cache invalidator for Create* RPCs and health checks
-merge_request: 1494
-author:
-type: fixed
diff --git a/internal/cache/cachedb_test.go b/internal/cache/cachedb_test.go
index 1fe267fa8..1f006aedb 100644
--- a/internal/cache/cachedb_test.go
+++ b/internal/cache/cachedb_test.go
@@ -101,9 +101,4 @@ func TestStreamDBNaiveKeyer(t *testing.T) {
 	// only completing/removing the pending generation file will allow access
 	require.NoError(t, repo1Lease.EndLease(ctx))
 	expectGetMiss(req1)
-
-	// creating a lease on a repo that doesn't exist yet should succeed
-	req1.Repository.RelativePath += "-does-not-exist"
-	_, err = keyer.StartLease(req1.Repository)
-	require.NoError(t, err)
 }
diff --git a/internal/cache/keyer.go b/internal/cache/keyer.go
index cdebd4fa9..ab60e7f63 100644
--- a/internal/cache/keyer.go
+++ b/internal/cache/keyer.go
@@ -76,7 +76,7 @@ func (l lease) EndLease(ctx context.Context) error {
 }
 
 func updateLatest(repo *gitalypb.Repository) (string, error) {
-	repoPath, err := helper.GetPath(repo)
+	repoPath, err := helper.GetRepoPath(repo)
 	if err != nil {
 		return "", err
 	}
@@ -141,7 +141,7 @@ func (LeaseKeyer) KeyPath(ctx context.Context, repo *gitalypb.Repository, req pr
 		return "", err
 	}
 
-	repoPath, err := helper.GetPath(repo)
+	repoPath, err := helper.GetRepoPath(repo)
 	if err != nil {
 		return "", err
 	}
@@ -190,7 +190,7 @@ func radixPath(root, key string) (string, error) {
 }
 
 func newPendingLease(repo *gitalypb.Repository) (string, error) {
-	repoPath, err := helper.GetPath(repo)
+	repoPath, err := helper.GetRepoPath(repo)
 	if err != nil {
 		return "", err
 	}
@@ -223,7 +223,7 @@ func cacheDir(repo *gitalypb.Repository) (string, error) {
 }
 
 func currentLeases(repo *gitalypb.Repository) ([]os.FileInfo, error) {
-	repoPath, err := helper.GetPath(repo)
+	repoPath, err := helper.GetRepoPath(repo)
 	if err != nil {
 		return nil, err
 	}
@@ -243,7 +243,7 @@ func currentLeases(repo *gitalypb.Repository) ([]os.FileInfo, error) {
 }
 
 func currentGenID(repo *gitalypb.Repository) (string, error) {
-	repoPath, err := helper.GetPath(repo)
+	repoPath, err := helper.GetRepoPath(repo)
 	if err != nil {
 		return "", err
 	}
diff --git a/internal/middleware/cache/cache.go b/internal/middleware/cache/cache.go
index 3400f9b00..2a65c6447 100644
--- a/internal/middleware/cache/cache.go
+++ b/internal/middleware/cache/cache.go
@@ -3,7 +3,6 @@ package cache
 import (
 	"context"
 	"fmt"
-	"strings"
 	"sync"
 
 	"github.com/golang/protobuf/proto"
@@ -33,16 +32,11 @@ func methodErrLogger(method string) func(error) {
 	}
 }
 
-func shouldIgnore(fullMethod string) bool {
-	return strings.HasPrefix(fullMethod, "/grpc.health")
-}
-
 // StreamInvalidator will invalidate any mutating RPC that targets a
 // repository in a gRPC stream based RPC
 func StreamInvalidator(ci Invalidator, reg *protoregistry.Registry) grpc.StreamServerInterceptor {
 	return func(srv interface{}, ss grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error {
-		if !featureflag.IsEnabled(ss.Context(), FeatureFlag) ||
-			shouldIgnore(info.FullMethod) {
+		if !featureflag.IsEnabled(ss.Context(), FeatureFlag) {
 			return handler(srv, ss)
 		}
 
@@ -69,8 +63,7 @@ func StreamInvalidator(ci Invalidator, reg *protoregistry.Registry) grpc.StreamS
 // repository in a gRPC unary RPC
 func UnaryInvalidator(ci Invalidator, reg *protoregistry.Registry) grpc.UnaryServerInterceptor {
 	return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp interface{}, err error) {
-		if !featureflag.IsEnabled(ctx, FeatureFlag) ||
-			shouldIgnore(info.FullMethod) {
+		if !featureflag.IsEnabled(ctx, FeatureFlag) {
 			return handler(ctx, req)
 		}
 
diff --git a/internal/middleware/cache/cache_test.go b/internal/middleware/cache/cache_test.go
index 3f5f4065f..4296d2e4e 100644
--- a/internal/middleware/cache/cache_test.go
+++ b/internal/middleware/cache/cache_test.go
@@ -19,8 +19,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/protoregistry"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc"
-	"google.golang.org/grpc/health"
-	"google.golang.org/grpc/health/grpc_health_v1"
 	"google.golang.org/grpc/metadata"
 )
 
@@ -51,7 +49,7 @@ func TestInvalidators(t *testing.T) {
 
 	svc := &testSvc{}
 
-	cli, cc, cleanup := newTestSvc(t, ctx, srvr, svc)
+	cli, cleanup := newTestSvc(t, ctx, srvr, svc)
 	defer cleanup()
 
 	repo1 := &gitalypb.Repository{
@@ -120,12 +118,6 @@ func TestInvalidators(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	// Health checks should NOT trigger cache invalidation
-	hcr := &grpc_health_v1.HealthCheckRequest{Service: "TestService"}
-	_, err = grpc_health_v1.NewHealthClient(cc).Check(ctx, hcr)
-	require.NoError(t, err)
-	require.Equal(t, 0, cache.MethodErrCount.Method["/grpc.health.v1.Health/Check"])
-
 	require.Equal(t, expectedInvalidations, mCache.(*mockCache).invalidatedRepos)
 	require.Equal(t, expectedSvcRequests, svc.repoRequests)
 	require.Equal(t, 3, mCache.(*mockCache).endedLeases.count)
@@ -175,10 +167,7 @@ func streamFileDesc(t testing.TB) *descriptor.FileDescriptorProto {
 	return fdp
 }
 
-func newTestSvc(t testing.TB, ctx context.Context, srvr *grpc.Server, svc testdata.TestServiceServer) (testdata.TestServiceClient, *grpc.ClientConn, func()) {
-	healthSrvr := health.NewServer()
-	grpc_health_v1.RegisterHealthServer(srvr, healthSrvr)
-	healthSrvr.SetServingStatus("TestService", grpc_health_v1.HealthCheckResponse_SERVING)
+func newTestSvc(t testing.TB, ctx context.Context, srvr *grpc.Server, svc testdata.TestServiceServer) (testdata.TestServiceClient, func()) {
 	testdata.RegisterTestServiceServer(srvr, svc)
 
 	lis, err := net.Listen("tcp", ":0")
@@ -203,7 +192,7 @@ func newTestSvc(t testing.TB, ctx context.Context, srvr *grpc.Server, svc testda
 	)
 	require.NoError(t, err)
 
-	return testdata.NewTestServiceClient(cc), cc, cleanup
+	return testdata.NewTestServiceClient(cc), cleanup
 }
 
 type testSvc struct {
diff --git a/internal/middleware/cache/export_test.go b/internal/middleware/cache/export_test.go
deleted file mode 100644
index 48c1dab84..000000000
--- a/internal/middleware/cache/export_test.go
+++ /dev/null
@@ -1,20 +0,0 @@
-package cache
-
-import "sync"
-
-var MethodErrCount = struct {
-	sync.Mutex
-	Method map[string]int
-}{
-	Method: map[string]int{},
-}
-
-func init() {
-	// override prometheus counter to detect any errors logged for a specific
-	// method
-	countMethodErr = func(method string) {
-		MethodErrCount.Lock()
-		MethodErrCount.Method[method]++
-		MethodErrCount.Unlock()
-	}
-}