Skip to content

Commit 6820742

Browse files
Fix Panic in PRS due to a missing nil check (#14656)
Signed-off-by: Manan Gupta <manan@planetscale.com>
1 parent 8520049 commit 6820742

File tree

2 files changed

+70
-7
lines changed

2 files changed

+70
-7
lines changed

go/vt/vtctl/grpcvtctldserver/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2752,7 +2752,7 @@ func (s *VtctldServer) PlannedReparentShard(ctx context.Context, req *vtctldatap
27522752
resp.Keyspace = ev.ShardInfo.Keyspace()
27532753
resp.Shard = ev.ShardInfo.ShardName()
27542754

2755-
if !topoproto.TabletAliasIsZero(ev.NewPrimary.Alias) {
2755+
if ev.NewPrimary != nil && !topoproto.TabletAliasIsZero(ev.NewPrimary.Alias) {
27562756
resp.PromotedPrimary = ev.NewPrimary.Alias
27572757
}
27582758
}

go/vt/vtctl/grpcvtctldserver/server_test.go

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7439,7 +7439,7 @@ func TestPlannedReparentShard(t *testing.T) {
74397439
req *vtctldatapb.PlannedReparentShardRequest
74407440
expected *vtctldatapb.PlannedReparentShardResponse
74417441
expectEventsToOccur bool
7442-
shouldErr bool
7442+
expectedErr string
74437443
}{
74447444
{
74457445
name: "successful reparent",
@@ -7554,7 +7554,6 @@ func TestPlannedReparentShard(t *testing.T) {
75547554
},
75557555
},
75567556
expectEventsToOccur: true,
7557-
shouldErr: false,
75587557
},
75597558
{
75607559
// Note: this is testing the error-handling done in
@@ -7570,7 +7569,7 @@ func TestPlannedReparentShard(t *testing.T) {
75707569
Shard: "-",
75717570
},
75727571
expectEventsToOccur: false,
7573-
shouldErr: true,
7572+
expectedErr: "node doesn't exist: keyspaces/testkeyspace/shards/-",
75747573
},
75757574
{
75767575
name: "invalid WaitReplicasTimeout",
@@ -7580,7 +7579,71 @@ func TestPlannedReparentShard(t *testing.T) {
75807579
Nanos: 1,
75817580
},
75827581
},
7583-
shouldErr: true,
7582+
expectedErr: "duration: seconds:-1 nanos:1 is out of range for time.Duration",
7583+
},
7584+
{
7585+
name: "tablet unreachable",
7586+
ts: memorytopo.NewServer(ctx, "zone1"),
7587+
tablets: []*topodatapb.Tablet{
7588+
{
7589+
Alias: &topodatapb.TabletAlias{
7590+
Cell: "zone1",
7591+
Uid: 100,
7592+
},
7593+
Type: topodatapb.TabletType_PRIMARY,
7594+
PrimaryTermStartTime: &vttime.Time{
7595+
Seconds: 100,
7596+
},
7597+
Keyspace: "testkeyspace",
7598+
Shard: "-",
7599+
},
7600+
{
7601+
Alias: &topodatapb.TabletAlias{
7602+
Cell: "zone1",
7603+
Uid: 200,
7604+
},
7605+
Type: topodatapb.TabletType_REPLICA,
7606+
Keyspace: "testkeyspace",
7607+
Shard: "-",
7608+
},
7609+
{
7610+
Alias: &topodatapb.TabletAlias{
7611+
Cell: "zone1",
7612+
Uid: 101,
7613+
},
7614+
Type: topodatapb.TabletType_RDONLY,
7615+
Keyspace: "testkeyspace",
7616+
Shard: "-",
7617+
},
7618+
},
7619+
tmc: &testutil.TabletManagerClient{
7620+
// This is only needed to verify reachability, so empty results are fine.
7621+
PrimaryStatusResults: map[string]struct {
7622+
Status *replicationdatapb.PrimaryStatus
7623+
Error error
7624+
}{
7625+
"zone1-0000000200": {
7626+
Error: fmt.Errorf("primary status failed"),
7627+
},
7628+
"zone1-0000000101": {
7629+
Status: &replicationdatapb.PrimaryStatus{},
7630+
},
7631+
"zone1-0000000100": {
7632+
Status: &replicationdatapb.PrimaryStatus{},
7633+
},
7634+
},
7635+
},
7636+
req: &vtctldatapb.PlannedReparentShardRequest{
7637+
Keyspace: "testkeyspace",
7638+
Shard: "-",
7639+
NewPrimary: &topodatapb.TabletAlias{
7640+
Cell: "zone1",
7641+
Uid: 200,
7642+
},
7643+
WaitReplicasTimeout: protoutil.DurationToProto(time.Millisecond * 10),
7644+
},
7645+
expectEventsToOccur: true,
7646+
expectedErr: "primary status failed",
75847647
},
75857648
}
75867649

@@ -7610,8 +7673,8 @@ func TestPlannedReparentShard(t *testing.T) {
76107673
testutil.AssertLogutilEventsOccurred(t, resp, "expected events to occur during ERS")
76117674
}()
76127675

7613-
if tt.shouldErr {
7614-
assert.Error(t, err)
7676+
if tt.expectedErr != "" {
7677+
assert.EqualError(t, err, tt.expectedErr)
76157678

76167679
return
76177680
}

0 commit comments

Comments
 (0)