Merge branch 'jc-remove-ruby-get-all-lfs' into 'master'

Remove ruby script approach to GetAllLFSPointers

Closes #2219

See merge request gitlab-org/gitaly!1695

4 files changed, 5 insertions, 173 deletions

diff --git a/changelogs/unreleased/jc-remove-ruby-get-all-lfs.yml b/changelogs/unreleased/jc-remove-ruby-get-all-lfs.yml
new file mode 100644
index 000000000..69aba10d1
--- /dev/null
+++ b/changelogs/unreleased/jc-remove-ruby-get-all-lfs.yml
@@ -0,0 +1,5 @@
+---
+title: Remove ruby script approach to GetAllLFSPointers
+merge_request: 1695
+author:
+type: other
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index f220bd5ac..3a4a33031 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -4,8 +4,6 @@ const (
 	// UploadPackFilter enables partial clones by sending uploadpack.allowFilter and uploadpack.allowAnySHA1InWant
 	// to upload-pack
 	UploadPackFilter = "upload_pack_filter"
-	// GetAllLFSPointersGo will cause the GetAllLFSPointers RPC to use the go implementation when set
-	GetAllLFSPointersGo = "get_all_lfs_pointers_go"
 	// GetTagMessagesGo will cause the GetTagMessages RPC to use the go implementation when set
 	GetTagMessagesGo = "get_tag_messages_go"
 	// FilterShasWithSignaturesGo will cause the FilterShasWithSignatures RPC to use the go implementation when set
diff --git a/internal/service/blob/lfs_pointers.go b/internal/service/blob/lfs_pointers.go
index e904a5139..4dfbd27ab 100644
--- a/internal/service/blob/lfs_pointers.go
+++ b/internal/service/blob/lfs_pointers.go
@@ -1,22 +1,10 @@
 package blob
 
 import (
-	"bufio"
-	"bytes"
 	"fmt"
-	"io"
-	"os"
-	"os/exec"
-	"strings"
 
-	"github.com/prometheus/client_golang/prometheus"
-	"gitlab.com/gitlab-org/gitaly/internal/command"
-	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
-	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/internal/helper/chunk"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -117,18 +105,6 @@ func (s *server) GetNewLFSPointers(in *gitalypb.GetNewLFSPointersRequest, stream
 	})
 }
 
-var getAllLFSPointersRequests = prometheus.NewCounterVec(
-	prometheus.CounterOpts{
-		Name: "gitaly_get_all_lfs_pointers_total",
-		Help: "Counter of go vs ruby implementation of GetAllLFSPointers",
-	},
-	[]string{"implementation"},
-)
-
-func init() {
-	prometheus.MustRegister(getAllLFSPointersRequests)
-}
-
 func (s *server) GetAllLFSPointers(in *gitalypb.GetAllLFSPointersRequest, stream gitalypb.BlobService_GetAllLFSPointersServer) error {
 	ctx := stream.Context()
 
@@ -136,18 +112,6 @@ func (s *server) GetAllLFSPointers(in *gitalypb.GetAllLFSPointersRequest, stream
 		return helper.ErrInvalidArgument(err)
 	}
 
-	if featureflag.IsEnabled(stream.Context(), featureflag.GetAllLFSPointersGo) {
-		getAllLFSPointersRequests.WithLabelValues("go").Inc()
-
-		if err := getAllLFSPointersRubyScript(in.GetRepository(), stream); err != nil {
-			return helper.ErrInternal(err)
-		}
-
-		return nil
-	}
-
-	getAllLFSPointersRequests.WithLabelValues("ruby").Inc()
-
 	client, err := s.ruby.BlobServiceClient(ctx)
 	if err != nil {
 		return err
@@ -181,130 +145,3 @@ func validateGetLfsPointersByRevisionRequest(in getLFSPointerByRevisionRequest)
 
 	return git.ValidateRevision(in.GetRevision())
 }
-
-type allLFSPointersSender struct {
-	stream      gitalypb.BlobService_GetAllLFSPointersServer
-	lfsPointers []*gitalypb.LFSPointer
-}
-
-func (s *allLFSPointersSender) Reset() { s.lfsPointers = nil }
-func (s *allLFSPointersSender) Append(it chunk.Item) {
-	s.lfsPointers = append(s.lfsPointers, it.(*gitalypb.LFSPointer))
-}
-
-func (s *allLFSPointersSender) Send() error {
-	return s.stream.Send(&gitalypb.GetAllLFSPointersResponse{LfsPointers: s.lfsPointers})
-}
-
-func getAllLFSPointersRubyScript(repository *gitalypb.Repository, stream gitalypb.BlobService_GetAllLFSPointersServer) error {
-	repoPath, err := helper.GetRepoPath(repository)
-	if err != nil {
-		return err
-	}
-
-	ctx := stream.Context()
-
-	cmd := exec.Command(
-		"ruby",
-		"--",
-		"-",
-		config.Config.Git.BinPath,
-		repoPath,
-		fmt.Sprintf("%d", LfsPointerMinSize),
-		fmt.Sprintf("%d", LfsPointerMaxSize),
-	)
-	cmd.Dir = config.Config.Ruby.Dir
-	ruby, err := command.New(ctx, cmd, strings.NewReader(rubyScript), nil, nil, os.Environ()...)
-	if err != nil {
-		return err
-	}
-
-	if err := parseCatfileOut(ruby, stream); err != nil {
-		return err
-	}
-
-	return ruby.Wait()
-}
-
-func parseCatfileOut(_r io.Reader, stream gitalypb.BlobService_GetAllLFSPointersServer) error {
-	chunker := chunk.New(&allLFSPointersSender{stream: stream, lfsPointers: nil})
-
-	r := bufio.NewReader(_r)
-	buf := &bytes.Buffer{}
-	for {
-		_, err := r.Peek(1)
-		if err == io.EOF {
-			break
-		}
-		if err != nil {
-			return err
-		}
-
-		info, err := catfile.ParseObjectInfo(r)
-		if err != nil {
-			return err
-		}
-
-		buf.Reset()
-		_, err = io.CopyN(buf, r, info.Size)
-		if err != nil {
-			return err
-		}
-		delim, err := r.ReadByte()
-		if err != nil {
-			return err
-		}
-		if delim != '\n' {
-			return fmt.Errorf("unexpected character %x", delim)
-		}
-
-		if !git.IsLFSPointer(buf.Bytes()) {
-			continue
-		}
-
-		b := make([]byte, buf.Len())
-		copy(b, buf.Bytes())
-		if err := chunker.Send(&gitalypb.LFSPointer{
-			Oid:  info.Oid,
-			Size: info.Size,
-			Data: b,
-		}); err != nil {
-			return err
-		}
-	}
-
-	return chunker.Flush()
-}
-
-var rubyScript = `
-
-def main(git_bin, git_dir, minSize, maxSize)
-  IO.popen(%W[#{git_bin} -C #{git_dir} rev-list --all --filter=blob:limit=#{maxSize+1} --in-commit-order --objects], 'r') do |rev_list|
-    # Loading bundler and rugged is slow. Let's do it while we wait for git rev-list.
-    require 'bundler/setup'
-    require 'rugged'
-
-    # disable encoding conversion: we want to read and write data as-is
-    rev_list.binmode
-    $stdout.binmode
-
-    repo = Rugged::Repository.new(git_dir)
-
-    rev_list.each_line do |line|
-      oid = line.split(' ', 2).first
-      abort "bad rev-list line #{line.inspect}" unless oid
-
-      header = repo.read_header(oid)
-      next unless header[:len] >= minSize && header[:len] <= maxSize && header[:type] == :blob
-
-      puts "#{oid} blob #{header[:len]}"
-      $stdout.write(repo.lookup(oid).content)
-      puts # newline separator, just like git cat-file
-    end
-  end
-
-  abort 'rev-list failed' unless $?.success?
-end
-
-main(ARGV[0], ARGV[1], Integer(ARGV[2]), Integer(ARGV[3]))
-`
diff --git a/internal/service/blob/lfs_pointers_test.go b/internal/service/blob/lfs_pointers_test.go
index 6b86ce7b1..5f40baa07 100644
--- a/internal/service/blob/lfs_pointers_test.go
+++ b/internal/service/blob/lfs_pointers_test.go
@@ -6,7 +6,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -406,13 +405,6 @@ func TestSuccessfulGetAllLFSPointersRequest(t *testing.T) {
 	require.NoError(t, err)
 
 	require.ElementsMatch(t, expectedLFSPointers, getAllPointers(t, c))
-
-	// test with go implementation
-	// TODO: remove once feature flag is removed
-	c, err = client.GetAllLFSPointers(featureflag.ContextWithFeatureFlag(ctx, featureflag.GetAllLFSPointersGo), request)
-	require.NoError(t, err)
-
-	require.ElementsMatch(t, expectedLFSPointers, getAllPointers(t, c))
 }
 
 func getAllPointers(t *testing.T, c gitalypb.BlobService_GetAllLFSPointersClient) []*gitalypb.LFSPointer {