Skip to content

Commit

Permalink
Fix Panic in PRS due to a missing nil check (#14656)
Browse files Browse the repository at this point in the history
Signed-off-by: Manan Gupta <[email protected]>
  • Loading branch information
vitess-bot[bot] committed Dec 5, 2023
1 parent 0ec9d9d commit 0d78a29
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 7 deletions.
2 changes: 1 addition & 1 deletion go/vt/vtctl/grpcvtctldserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2319,7 +2319,7 @@ func (s *VtctldServer) PlannedReparentShard(ctx context.Context, req *vtctldatap
resp.Keyspace = ev.ShardInfo.Keyspace()
resp.Shard = ev.ShardInfo.ShardName()

if !topoproto.TabletAliasIsZero(ev.NewPrimary.Alias) {
if ev.NewPrimary != nil && !topoproto.TabletAliasIsZero(ev.NewPrimary.Alias) {
resp.PromotedPrimary = ev.NewPrimary.Alias
}
}
Expand Down
75 changes: 69 additions & 6 deletions go/vt/vtctl/grpcvtctldserver/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6200,7 +6200,7 @@ func TestPlannedReparentShard(t *testing.T) {
req *vtctldatapb.PlannedReparentShardRequest
expected *vtctldatapb.PlannedReparentShardResponse
expectEventsToOccur bool
shouldErr bool
expectedErr string
}{
{
name: "successful reparent",
Expand Down Expand Up @@ -6300,7 +6300,6 @@ func TestPlannedReparentShard(t *testing.T) {
},
},
expectEventsToOccur: true,
shouldErr: false,
},
{
// Note: this is testing the error-handling done in
Expand All @@ -6316,7 +6315,7 @@ func TestPlannedReparentShard(t *testing.T) {
Shard: "-",
},
expectEventsToOccur: false,
shouldErr: true,
expectedErr: "node doesn't exist: keyspaces/testkeyspace/shards/-",
},
{
name: "invalid WaitReplicasTimeout",
Expand All @@ -6326,7 +6325,71 @@ func TestPlannedReparentShard(t *testing.T) {
Nanos: 1,
},
},
shouldErr: true,
expectedErr: "duration: seconds:-1 nanos:1 is out of range for time.Duration",
},
{
name: "tablet unreachable",
ts: memorytopo.NewServer(ctx, "zone1"),

Check failure on line 6332 in go/vt/vtctl/grpcvtctldserver/server_test.go

View workflow job for this annotation

GitHub Actions / Unit Test (mysql57)

undefined: ctx

Check failure on line 6332 in go/vt/vtctl/grpcvtctldserver/server_test.go

View workflow job for this annotation

GitHub Actions / Unit Test (mysql80)

undefined: ctx

Check failure on line 6332 in go/vt/vtctl/grpcvtctldserver/server_test.go

View workflow job for this annotation

GitHub Actions / Static Code Checks Etc

undefined: ctx

Check failure on line 6332 in go/vt/vtctl/grpcvtctldserver/server_test.go

View workflow job for this annotation

GitHub Actions / Unit Test (Race)

undefined: ctx
tablets: []*topodatapb.Tablet{
{
Alias: &topodatapb.TabletAlias{
Cell: "zone1",
Uid: 100,
},
Type: topodatapb.TabletType_PRIMARY,
PrimaryTermStartTime: &vttime.Time{
Seconds: 100,
},
Keyspace: "testkeyspace",
Shard: "-",
},
{
Alias: &topodatapb.TabletAlias{
Cell: "zone1",
Uid: 200,
},
Type: topodatapb.TabletType_REPLICA,
Keyspace: "testkeyspace",
Shard: "-",
},
{
Alias: &topodatapb.TabletAlias{
Cell: "zone1",
Uid: 101,
},
Type: topodatapb.TabletType_RDONLY,
Keyspace: "testkeyspace",
Shard: "-",
},
},
tmc: &testutil.TabletManagerClient{
// This is only needed to verify reachability, so empty results are fine.
PrimaryStatusResults: map[string]struct {

Check failure on line 6367 in go/vt/vtctl/grpcvtctldserver/server_test.go

View workflow job for this annotation

GitHub Actions / Unit Test (mysql57)

unknown field PrimaryStatusResults in struct literal of type testutil.TabletManagerClient

Check failure on line 6367 in go/vt/vtctl/grpcvtctldserver/server_test.go

View workflow job for this annotation

GitHub Actions / Unit Test (mysql80)

unknown field PrimaryStatusResults in struct literal of type testutil.TabletManagerClient

Check failure on line 6367 in go/vt/vtctl/grpcvtctldserver/server_test.go

View workflow job for this annotation

GitHub Actions / Static Code Checks Etc

unknown field PrimaryStatusResults in struct literal of type testutil.TabletManagerClient (typecheck)

Check failure on line 6367 in go/vt/vtctl/grpcvtctldserver/server_test.go

View workflow job for this annotation

GitHub Actions / Unit Test (Race)

unknown field PrimaryStatusResults in struct literal of type testutil.TabletManagerClient
Status *replicationdatapb.PrimaryStatus
Error error
}{
"zone1-0000000200": {
Error: fmt.Errorf("primary status failed"),
},
"zone1-0000000101": {
Status: &replicationdatapb.PrimaryStatus{},
},
"zone1-0000000100": {
Status: &replicationdatapb.PrimaryStatus{},
},
},
},
req: &vtctldatapb.PlannedReparentShardRequest{
Keyspace: "testkeyspace",
Shard: "-",
NewPrimary: &topodatapb.TabletAlias{
Cell: "zone1",
Uid: 200,
},
WaitReplicasTimeout: protoutil.DurationToProto(time.Millisecond * 10),
},
expectEventsToOccur: true,
expectedErr: "primary status failed",
},
}

Expand Down Expand Up @@ -6362,8 +6425,8 @@ func TestPlannedReparentShard(t *testing.T) {
testutil.AssertLogutilEventsOccurred(t, resp, "expected events to occur during ERS")
}()

if tt.shouldErr {
assert.Error(t, err)
if tt.expectedErr != "" {
assert.EqualError(t, err, tt.expectedErr)

return
}
Expand Down

0 comments on commit 0d78a29

Please sign in to comment.