Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

VReplication: refactor denied tables unit test, add couple more tests #15995

Merged
merged 3 commits into from
May 27, 2024
Merged

VReplication: refactor denied tables unit test, add couple more tests #15995

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b4e1dd
Refactor existing tests to make it easer to add more
rohit-nayak-ps May 22, 2024
d76d73c
Add more unit tests. Remove old tests kept previously in comments.
rohit-nayak-ps May 22, 2024
152a2ae
Address review comemnt
rohit-nayak-ps May 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 131 additions & 52 deletions go/vt/topo/shard_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,69 +140,148 @@ func TestUpdateSourcePrimaryDeniedTables(t *testing.T) {

func TestUpdateSourceDeniedTables(t *testing.T) {
si := NewShardInfo("ks", "sh", &topodatapb.Shard{}, nil)

// check we enforce the keyspace lock
ctx := context.Background()
if err := si.UpdateDeniedTables(ctx, topodatapb.TabletType_RDONLY, nil, false, nil); err == nil || err.Error() != "keyspace ks is not locked (no locksInfo)" {
t.Fatalf("unlocked keyspace produced wrong error: %v", err)
ctxWithLock := lockedKeyspaceContext("ks")

type testCase struct {
name string
ctx context.Context
tabletType topodatapb.TabletType
cells []string
remove bool
tables []string

wantError string
wantTabletControl *topodatapb.Shard_TabletControl
}
ctx = lockedKeyspaceContext("ks")

// add one cell
if err := si.UpdateDeniedTables(ctx, topodatapb.TabletType_RDONLY, []string{"first"}, false, []string{"t1", "t2"}); err != nil || !reflect.DeepEqual(si.TabletControls, []*topodatapb.Shard_TabletControl{
// These tests update the state of the shard tablet controls, so subsequent tests
// depend on the cumulative state from the previous tests.
testCases := []testCase{
{
TabletType: topodatapb.TabletType_RDONLY,
Cells: []string{"first"},
DeniedTables: []string{"t1", "t2"},
},
}) {
t.Fatalf("one cell add failed: %v", si)
}
name: "enforce keyspace lock",
ctx: ctx,
tabletType: topodatapb.TabletType_RDONLY,
cells: []string{"first"},

// remove that cell, going back
if err := si.UpdateDeniedTables(ctx, topodatapb.TabletType_RDONLY, []string{"first"}, true, nil); err != nil || len(si.TabletControls) != 0 {
t.Fatalf("going back should have remove the record: %v", si)
}

// re-add a cell, then another with different table list to
// make sure it fails
if err := si.UpdateDeniedTables(ctx, topodatapb.TabletType_RDONLY, []string{"first"}, false, []string{"t1", "t2"}); err != nil {
t.Fatalf("one cell add failed: %v", si)
}
if err := si.UpdateDeniedTables(ctx, topodatapb.TabletType_RDONLY, []string{"second"}, false, []string{"t2", "t3"}); err == nil || err.Error() != "trying to use two different sets of denied tables for shard ks/sh: [t1 t2] and [t2 t3]" {
t.Fatalf("different table list should fail: %v", err)
}
// add another cell, see the list grow
if err := si.UpdateDeniedTables(ctx, topodatapb.TabletType_RDONLY, []string{"second"}, false, []string{"t1", "t2"}); err != nil || !reflect.DeepEqual(si.TabletControls, []*topodatapb.Shard_TabletControl{
wantError: "keyspace ks is not locked (no locksInfo)",
},
{
TabletType: topodatapb.TabletType_RDONLY,
Cells: []string{"first", "second"},
DeniedTables: []string{"t1", "t2"},
name: "add one cell",
tabletType: topodatapb.TabletType_RDONLY,
cells: []string{"first"},
tables: []string{"t1", "t2"},
wantTabletControl: &topodatapb.Shard_TabletControl{
TabletType: topodatapb.TabletType_RDONLY,
Cells: []string{"first"},
DeniedTables: []string{"t1", "t2"},
},
},
}) {
t.Fatalf("second cell add failed: %v", si)
}

// add all cells, see the list grow to all
if err := si.UpdateDeniedTables(ctx, topodatapb.TabletType_RDONLY, []string{"first", "second", "third"}, false, []string{"t1", "t2"}); err != nil || !reflect.DeepEqual(si.TabletControls, []*topodatapb.Shard_TabletControl{
{
TabletType: topodatapb.TabletType_RDONLY,
Cells: []string{"first", "second", "third"},
DeniedTables: []string{"t1", "t2"},
name: "remove the only cell",
tabletType: topodatapb.TabletType_RDONLY,
cells: []string{"first"},
remove: true,
},
{
name: "re-add cell",
tabletType: topodatapb.TabletType_RDONLY,
cells: []string{"first"},
tables: []string{"t1", "t2"},
wantTabletControl: &topodatapb.Shard_TabletControl{
TabletType: topodatapb.TabletType_RDONLY,
Cells: []string{"first"},
DeniedTables: []string{"t1", "t2"},
},
},
}) {
t.Fatalf("all cells add failed: %v", si)
}

// remove one cell from the full list
if err := si.UpdateDeniedTables(ctx, topodatapb.TabletType_RDONLY, []string{"second"}, true, []string{"t1", "t2"}); err != nil || !reflect.DeepEqual(si.TabletControls, []*topodatapb.Shard_TabletControl{
{
TabletType: topodatapb.TabletType_RDONLY,
Cells: []string{"first", "third"},
DeniedTables: []string{"t1", "t2"},
name: "re-add existing cell, different tables, should fail",
tabletType: topodatapb.TabletType_RDONLY,
cells: []string{"first"},
tables: []string{"t3"},
wantError: "trying to use two different sets of denied tables for shard",
},
}) {
t.Fatalf("one cell removal from all failed: %v", si)
{
name: "add all cells, see cell list grow to all",
tabletType: topodatapb.TabletType_RDONLY,
cells: []string{"first", "second", "third"},
tables: []string{"t1", "t2"},
wantTabletControl: &topodatapb.Shard_TabletControl{
TabletType: topodatapb.TabletType_RDONLY,
Cells: []string{"first", "second", "third"},
DeniedTables: []string{"t1", "t2"},
},
},
{
name: "remove one cell",
tabletType: topodatapb.TabletType_RDONLY,
cells: []string{"second"},
remove: true,
tables: []string{"t1", "t2"},
wantTabletControl: &topodatapb.Shard_TabletControl{
TabletType: topodatapb.TabletType_RDONLY,
Cells: []string{"first", "third"},
DeniedTables: []string{"t1", "t2"},
},
},
{
name: "add replica tablet type",
tabletType: topodatapb.TabletType_REPLICA,
cells: []string{"first"},
tables: []string{"t1", "t2"},
wantTabletControl: &topodatapb.Shard_TabletControl{
TabletType: topodatapb.TabletType_REPLICA,
Cells: []string{"first"},
DeniedTables: []string{"t1", "t2"},
},
},
{
name: "confirm rdonly still stays the same, after replica was added",
tabletType: topodatapb.TabletType_RDONLY,
wantTabletControl: &topodatapb.Shard_TabletControl{
TabletType: topodatapb.TabletType_RDONLY,
Cells: []string{"first", "third"},
DeniedTables: []string{"t1", "t2"},
},
},
{
name: "remove rdonly entry",
tabletType: topodatapb.TabletType_RDONLY,
cells: []string{"first", "third"},
remove: true,
tables: []string{"t1", "t2"},
},
{
name: "remove replica entry",
tabletType: topodatapb.TabletType_REPLICA,
cells: []string{"first", "third"},
remove: true,
tables: []string{"t1", "t2"},
},
}

for _, tcase := range testCases {
t.Run(tcase.name, func(t *testing.T) {
if tcase.ctx == nil {
tcase.ctx = ctxWithLock
}
var err error
if tcase.tables != nil || tcase.cells != nil {
err = si.UpdateDeniedTables(tcase.ctx, tcase.tabletType, tcase.cells, tcase.remove, tcase.tables)
}
if tcase.wantError != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tcase.wantError)
Comment on lines +273 to +274
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use require.EqualError to replace both of these checks.

return
}
require.NoError(t, err)
if tcase.wantTabletControl == nil {
require.Nil(t, si.GetTabletControl(tcase.tabletType))
} else {
require.EqualValuesf(t, tcase.wantTabletControl, si.GetTabletControl(tcase.tabletType),
"want: %v, got: %v", tcase.wantTabletControl, si.GetTabletControl(tcase.tabletType))
}
})
}
}

Expand Down
Loading