diff options
author | John Cai <jcai@gitlab.com> | 2019-12-16 21:42:22 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2019-12-16 21:42:22 +0300 |
commit | 008843a1129b4657c38cb0bae770d34046f3ac38 (patch) | |
tree | 89043d83b7932c32ded7a11f69487da1af54800c | |
parent | ee122f80f96deabead45b1d8855303d7fb840708 (diff) | |
parent | 089fe4606a95eef03b7b8ea03d36d5165dc2a43d (diff) |
Merge branch 'ps-get-tag-messages-feature-flag-cleanup' into 'master'
Cleanup for the feature flag protection of the GetTagMessages method
See merge request gitlab-org/gitaly!1698
-rw-r--r-- | internal/metadata/featureflag/feature_flags.go | 2 | ||||
-rw-r--r-- | internal/service/ref/tag_messages.go | 54 | ||||
-rw-r--r-- | internal/service/ref/tag_messages_test.go | 29 |
3 files changed, 4 insertions, 81 deletions
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go index 3a4a33031..d64c00909 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" - // 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 FilterShasWithSignaturesGo = "filter_shas_with_signatures_go" // LinguistFileCountStats will invoke an additional git-linguist command to get the number of files per language diff --git a/internal/service/ref/tag_messages.go b/internal/service/ref/tag_messages.go index 88bf9b27d..e632dd7f8 100644 --- a/internal/service/ref/tag_messages.go +++ b/internal/service/ref/tag_messages.go @@ -5,30 +5,15 @@ import ( "fmt" "io" - "github.com/prometheus/client_golang/prometheus" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/helper" - "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" - "gitlab.com/gitlab-org/gitaly/internal/rubyserver" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/streamio" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) -var getTagMessagesRequests = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Name: "gitaly_get_tag_messages_total", - Help: "Counter of go vs ruby implementation of GetTagMessages", - }, - []string{"implementation"}, -) - -func init() { - prometheus.MustRegister(getTagMessagesRequests) -} - func (s *server) GetTagMessages(request *gitalypb.GetTagMessagesRequest, stream gitalypb.RefService_GetTagMessagesServer) error { if err := validateGetTagMessagesRequest(request); err != nil { return status.Errorf(codes.InvalidArgument, "GetTagMessages: %v", err) @@ -49,16 +34,6 @@ func validateGetTagMessagesRequest(request *gitalypb.GetTagMessagesRequest) erro } func (s *server) getAndStreamTagMessages(request *gitalypb.GetTagMessagesRequest, stream gitalypb.RefService_GetTagMessagesServer) error { - if featureflag.IsEnabled(stream.Context(), featureflag.GetTagMessagesGo) { - getTagMessagesRequests.WithLabelValues("go").Inc() - return getAndStreamTagMessagesGo(request, stream) - } - - getTagMessagesRequests.WithLabelValues("ruby").Inc() - return getAndStreamTagMessagesRuby(s.ruby, request, stream) -} - -func getAndStreamTagMessagesGo(request *gitalypb.GetTagMessagesRequest, stream gitalypb.RefService_GetTagMessagesServer) error { ctx := stream.Context() c, err := catfile.New(ctx, request.GetRepository()) @@ -87,32 +62,3 @@ func getAndStreamTagMessagesGo(request *gitalypb.GetTagMessagesRequest, stream g } return nil } - -func getAndStreamTagMessagesRuby(ruby *rubyserver.Server, request *gitalypb.GetTagMessagesRequest, stream gitalypb.RefService_GetTagMessagesServer) error { - ctx := stream.Context() - - client, err := ruby.RefServiceClient(ctx) - if err != nil { - return err - } - - clientCtx, err := rubyserver.SetHeaders(ctx, request.GetRepository()) - if err != nil { - return err - } - - rubyStream, err := client.GetTagMessages(clientCtx, request) - if err != nil { - return err - } - - return rubyserver.Proxy(func() error { - resp, err := rubyStream.Recv() - if err != nil { - md := rubyStream.Trailer() - stream.SetTrailer(md) - return err - } - return stream.Send(resp) - }) -} diff --git a/internal/service/ref/tag_messages_test.go b/internal/service/ref/tag_messages_test.go index d5b38af1f..fbd8ebac0 100644 --- a/internal/service/ref/tag_messages_test.go +++ b/internal/service/ref/tag_messages_test.go @@ -1,18 +1,15 @@ package ref import ( - "context" "io" "strings" "testing" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/helper" - "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" - "google.golang.org/grpc/metadata" ) func TestSuccessfulGetTagMessagesRequest(t *testing.T) { @@ -50,23 +47,11 @@ func TestSuccessfulGetTagMessagesRequest(t *testing.T) { }, } - for title, ctxModifier := range map[string]func(context.Context) context.Context{ - "enabled_feature_GetTagMessagesGo": func(ctx context.Context) context.Context { - return enableGetTagMessagesFeatureFlag(ctx) - }, - "disabled_feature_GetTagMessagesGo": func(ctx context.Context) context.Context { - return ctx - }, - } { - t.Run(title, func(t *testing.T) { - c, err := client.GetTagMessages(ctxModifier(ctx), request) - require.NoError(t, err) - - fetchedMessages := readAllMessagesFromClient(t, c) + c, err := client.GetTagMessages(ctx, request) + require.NoError(t, err) - require.Equal(t, expectedMessages, fetchedMessages) - }) - } + fetchedMessages := readAllMessagesFromClient(t, c) + require.Equal(t, expectedMessages, fetchedMessages) } func TestFailedGetTagMessagesRequest(t *testing.T) { @@ -131,9 +116,3 @@ func readAllMessagesFromClient(t *testing.T, c gitalypb.RefService_GetTagMessage return } - -func enableGetTagMessagesFeatureFlag(ctx context.Context) context.Context { - return metadata.NewOutgoingContext(ctx, metadata.New(map[string]string{ - featureflag.HeaderKey(featureflag.GetTagMessagesGo): "true", - })) -} |