[PFS-10] Fix Misleading Version Check Logic

src/client/admin.go

@@ -2,15 +2,16 @@ package client
 
 import (
 	"github.com/gogo/protobuf/types"
+
 	"github.com/pachyderm/pachyderm/v2/src/admin"
-	"github.com/pachyderm/pachyderm/v2/src/internal/grpcutil"
+	"github.com/pachyderm/pachyderm/v2/src/internal/errors"
 )
 
 // InspectCluster retrieves cluster state
 func (c APIClient) InspectCluster() (*admin.ClusterInfo, error) {
 	clusterInfo, err := c.AdminAPIClient.InspectCluster(c.Ctx(), &types.Empty{})
 	if err != nil {
-		return nil, grpcutil.ScrubGRPC(err)
+		return nil, errors.Wrap(err, "failed to inspect cluster")
 	}
 	return clusterInfo, nil
 }

src/client/client.go

@@ -9,13 +9,16 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/health/grpc_health_v1"
+	"google.golang.org/grpc/status"
 
 	// Import registers the grpc GZIP encoder
 	_ "google.golang.org/grpc/encoding/gzip"
@@ -40,6 +43,7 @@ import (
 	"github.com/pachyderm/pachyderm/v2/src/pps"
 	"github.com/pachyderm/pachyderm/v2/src/proxy"
 	"github.com/pachyderm/pachyderm/v2/src/transaction"
+	"github.com/pachyderm/pachyderm/v2/src/version"
 	"github.com/pachyderm/pachyderm/v2/src/version/versionpb"
 )
 
@@ -606,11 +610,21 @@ func newOnUserMachine(cfg *config.Config, context *config.Context, contextName,
 	// Verify cluster deployment ID
 	clusterInfo, err := client.InspectCluster()
 	if err != nil {
-		if strings.Contains("unknown service admin_v2.API", err.Error()) {
-			return nil, errors.Errorf("this client is for pachyderm 2.x, but the server has a different version - please install the correct client for your server")
-
+		scrubbedErr := grpcutil.ScrubGRPC(err)
+		if status.Code(err) == codes.Unimplemented {
+			pachdVersion, versErr := client.Version()
+			if err != nil {
+				return nil, errors.Wrap(scrubbedErr, errors.Wrap(versErr, "could not determine pachd version").Error())
+			}
+			pachdMajVersion, convErr := strconv.Atoi(strings.Split(pachdVersion, ".")[0])
+			if convErr != nil {
+				return nil, errors.Wrap(scrubbedErr, errors.Wrap(convErr, "could not parse pachd major version").Error())
+			}
+			if pachdMajVersion != int(version.Version.Major) {
+				return nil, errors.Errorf("this client is for pachyderm %d.x, but the server has a version %d.x - please install the correct client for your server", version.Version.Major, pachdMajVersion)
+			}
 		}
-		return nil, errors.Wrap(err, "could not get cluster ID")
+		return nil, errors.Wrap(scrubbedErr, "could not get cluster ID")
 	}
 	if context.ClusterDeploymentID != clusterInfo.DeploymentID {
 		if context.ClusterDeploymentID == "" {

src/internal/minikubetestenv/deploy.go

@@ -19,15 +19,16 @@ import (
 	"github.com/gruntwork-io/terratest/modules/helm"
 	"github.com/gruntwork-io/terratest/modules/k8s"
 	terraTest "github.com/gruntwork-io/terratest/modules/testing"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	kube "k8s.io/client-go/kubernetes"
+
 	"github.com/pachyderm/pachyderm/v2/src/client"
 	"github.com/pachyderm/pachyderm/v2/src/internal/backoff"
 	"github.com/pachyderm/pachyderm/v2/src/internal/errors"
 	"github.com/pachyderm/pachyderm/v2/src/internal/grpcutil"
 	"github.com/pachyderm/pachyderm/v2/src/internal/require"
 	"github.com/pachyderm/pachyderm/v2/src/internal/testutil"
-	v1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	kube "k8s.io/client-go/kubernetes"
 )
 
 const (
@@ -423,8 +424,9 @@ func pachClient(t testing.TB, pachAddress *grpcutil.PachdAddress, authUser, name
 		}
 		// Ensure that pachd is really ready to receive requests.
 		if _, err := c.InspectCluster(); err != nil {
-			t.Logf("retryable: failed to inspect cluster on port %v: %v", pachAddress.Port, err)
-			return errors.Wrapf(err, "failed to inspect cluster on port %v", pachAddress.Port)
+			scrubbedErr := grpcutil.ScrubGRPC(err)
+			t.Logf("retryable: failed to inspect cluster on port %v: %v", pachAddress.Port, scrubbedErr)
+			return errors.Wrapf(scrubbedErr, "failed to inspect cluster on port %v", pachAddress.Port)
 		}
 		return nil
 	}, backoff.RetryEvery(time.Second).For(50*time.Second)))

src/internal/pachd/builder.go

@@ -176,13 +176,6 @@ func (b *builder) initExternalServer(ctx context.Context) error {
 	b.daemon.external, err = grpcutil.NewServer(
 		ctx,
 		true,
-		// Add an UnknownServiceHandler to catch the case where the user has a client with the wrong major version.
-		// Weirdly, GRPC seems to run the interceptor stack before the UnknownServiceHandler, so this is never called
-		// (because the version_middleware interceptor throws an error, or the auth interceptor does).
-		grpc.UnknownServiceHandler(func(srv interface{}, stream grpc.ServerStream) error {
-			method, _ := grpc.MethodFromServerStream(stream)
-			return errors.Errorf("unknown service %v", method)
-		}),
 		grpc.ChainUnaryInterceptor(
 			errorsmw.UnaryServerInterceptor,
 			version_middleware.UnaryServerInterceptor,

src/server/admin/cmds/cmds.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/pachyderm/pachyderm/v2/src/client"
 	"github.com/pachyderm/pachyderm/v2/src/internal/cmdutil"
+	"github.com/pachyderm/pachyderm/v2/src/internal/grpcutil"
 
 	"github.com/spf13/cobra"
 )
@@ -24,7 +25,7 @@ func Cmds() []*cobra.Command {
 			defer c.Close()
 			ci, err := c.InspectCluster()
 			if err != nil {
-				return err
+				return grpcutil.ScrubGRPC(err)
 			}
 			fmt.Println(ci.ID)
 			return nil