Fix to make sure all keyrange comparisons go through the key.Package

* We ran into some issues related to padding. This is another form of the bug
fixed in here: https://github.com/vitessio/vitess/pull/8296

Signed-off-by: Rafael Chacon <rafael@slack-corp.com>

go/vt/key/key.go

@@ -242,6 +242,14 @@ func KeyRangeStartEqual(left, right *topodatapb.KeyRange) bool {
 	return bytes.Equal(addPadding(left.Start), addPadding(right.Start))
 }
 
+// KeyRangeIsContiguous returns true if left.End equals right.End (i.e they are contigious)
+func KeyRangeIsContiguous(left, right *topodatapb.KeyRange) bool {
+	if left == nil || right == nil {
+		return false
+	}
+	return bytes.Equal(addPadding(left.End), addPadding(right.Start))
+}
+
 // KeyRangeEndEqual returns true if both key ranges have the same end
 func KeyRangeEndEqual(left, right *topodatapb.KeyRange) bool {
 	if left == nil {

go/vt/key/key_test.go

@@ -212,20 +212,6 @@ func TestKeyRangeAdd(t *testing.T) {
 		out:    "40-c0",
 		ok:     true,
 	}}
-	stringToKeyRange := func(spec string) *topodatapb.KeyRange {
-		if spec == "" {
-			return nil
-		}
-		parts := strings.Split(spec, "-")
-		if len(parts) != 2 {
-			panic("invalid spec")
-		}
-		kr, err := ParseKeyRangeParts(parts[0], parts[1])
-		if err != nil {
-			panic(err)
-		}
-		return kr
-	}
 	keyRangeToString := func(kr *topodatapb.KeyRange) string {
 		if kr == nil {
 			return ""
@@ -271,20 +257,6 @@ func TestKeyRangeEndEqual(t *testing.T) {
 		second: "-8000",
 		out:    true,
 	}}
-	stringToKeyRange := func(spec string) *topodatapb.KeyRange {
-		if spec == "" {
-			return nil
-		}
-		parts := strings.Split(spec, "-")
-		if len(parts) != 2 {
-			panic("invalid spec")
-		}
-		kr, err := ParseKeyRangeParts(parts[0], parts[1])
-		if err != nil {
-			panic(err)
-		}
-		return kr
-	}
 
 	for _, tcase := range testcases {
 		first := stringToKeyRange(tcase.first)
@@ -326,20 +298,6 @@ func TestKeyRangeStartEqual(t *testing.T) {
 		second: "-8000",
 		out:    true,
 	}}
-	stringToKeyRange := func(spec string) *topodatapb.KeyRange {
-		if spec == "" {
-			return nil
-		}
-		parts := strings.Split(spec, "-")
-		if len(parts) != 2 {
-			panic("invalid spec")
-		}
-		kr, err := ParseKeyRangeParts(parts[0], parts[1])
-		if err != nil {
-			panic(err)
-		}
-		return kr
-	}
 
 	for _, tcase := range testcases {
 		first := stringToKeyRange(tcase.first)
@@ -377,20 +335,6 @@ func TestKeyRangeEqual(t *testing.T) {
 		second: "-8000",
 		out:    true,
 	}}
-	stringToKeyRange := func(spec string) *topodatapb.KeyRange {
-		if spec == "" {
-			return nil
-		}
-		parts := strings.Split(spec, "-")
-		if len(parts) != 2 {
-			panic("invalid spec")
-		}
-		kr, err := ParseKeyRangeParts(parts[0], parts[1])
-		if err != nil {
-			panic(err)
-		}
-		return kr
-	}
 
 	for _, tcase := range testcases {
 		first := stringToKeyRange(tcase.first)
@@ -402,6 +346,51 @@ func TestKeyRangeEqual(t *testing.T) {
 	}
 }
 
+func TestKeyRangeIsContiguous(t *testing.T) {
+	testcases := []struct {
+		first  string
+		second string
+		out    bool
+	}{{
+		first:  "-40",
+		second: "40-80",
+		out:    true,
+	}, {
+		first:  "40-80",
+		second: "-40",
+		out:    false,
+	}, {
+		first:  "-",
+		second: "-40",
+		out:    true,
+	}, {
+		first:  "40-80",
+		second: "c0-",
+		out:    false,
+	}, {
+		first:  "40-80",
+		second: "80-c0",
+		out:    true,
+	}, {
+		first:  "40-80",
+		second: "8000000000000000-c000000000000000",
+		out:    true,
+	}, {
+		first:  "4000000000000000-8000000000000000",
+		second: "80-c0",
+		out:    true,
+	}}
+
+	for _, tcase := range testcases {
+		first := stringToKeyRange(tcase.first)
+		second := stringToKeyRange(tcase.second)
+		out := KeyRangeIsContiguous(first, second)
+		if out != tcase.out {
+			t.Fatalf("KeyRangeEqual(%q, %q) expected %t, got %t", tcase.first, tcase.second, tcase.out, out)
+		}
+	}
+}
+
 func TestEvenShardsKeyRange_Error(t *testing.T) {
 	testCases := []struct {
 		i, n      int
@@ -815,3 +804,18 @@ func TestShardCalculatorForShardsGreaterThan512(t *testing.T) {
 
 	assert.Equal(t, want, got[511], "Invalid mapping for a 512-shard keyspace. Expected %v, got %v", want, got[511])
 }
+
+func stringToKeyRange(spec string) *topodatapb.KeyRange {
+	if spec == "" {
+		return nil
+	}
+	parts := strings.Split(spec, "-")
+	if len(parts) != 2 {
+		panic("invalid spec")
+	}
+	kr, err := ParseKeyRangeParts(parts[0], parts[1])
+	if err != nil {
+		panic(err)
+	}
+	return kr
+}

go/vt/topo/srv_keyspace.go

@@ -17,7 +17,6 @@ limitations under the License.
 package topo
 
 import (
-	"bytes"
 	"encoding/hex"
 	"fmt"
 	"path"
@@ -681,7 +680,7 @@ func OrderAndCheckPartitions(cell string, srvKeyspace *topodatapb.SrvKeyspace) e
 				// this is the custom sharding case, all KeyRanges must be nil
 				continue
 			}
-			if !bytes.Equal(currShard.KeyRange.End, nextShard.KeyRange.Start) {
+			if !key.KeyRangeIsContiguous(currShard.KeyRange, nextShard.KeyRange) {
 				return fmt.Errorf("non-contiguous KeyRange values for %v in cell %v at shard %v to %v: %v != %v", tabletType, cell, i, i+1, hex.EncodeToString(currShard.KeyRange.End), hex.EncodeToString(nextShard.KeyRange.Start))
 			}
 		}