chore(test): Integration tests for advanced backup/restore scenarios #8643

SiddeshUbale · 2023-02-01T10:27:22Z

This PR contains Integration tests for:

1] Illegal namespace test (more than 127 namespaces).

Backup restore with a dgraph setup that has more than 127 namespaces

CLAassistant · 2023-02-01T10:27:27Z

All committers have signed the CLA.

coveralls · 2023-02-01T15:42:28Z

Coverage: 67.267% (+0.2%) from 67.078% when pulling d06322b on siddesh/127-namespace into 0f82499 on main.

systest/backup/advanced-scenarios/127-Namespace/acl-secret

systest/backup/advanced-scenarios/127-Namespace/backup_test.go

systest/backup/common/utils.go

graphql/e2e/common/common.go

systest/backup/advanced-scenarios/127-Namespace/backup_test.go

systest/backup/common/utils.go

graphql/e2e/common/common.go

systest/backup/advanced-scenarios/127-Namespace/backup_test.go

systest/backup/common/utils.go

mangalaman93

Looks much better, just two more comments:

can we use shared volume instead of .gitkeep files
can we refactor code from other backup test and use the functions that you have added?

mangalaman93 · 2023-02-08T09:46:39Z

systest/backup/advanced-scenarios/127-Namespace/backup_test.go

+)
+
+func TestDeletedNamespaceID(t *testing.T) {
+	defer utilsCommon.DirCleanup(t)


why are we only doing cleanup? Where do we create the dir that we are cleaning up here?

backupTemp folder gets created when the code takes a backup(backupdest is /data/backups/backupTemp).

Now the test is using shared volume and I have removed the DirCleanup() call from the test.

all-seeing-code · 2023-02-09T10:56:05Z

graphql/e2e/common/common.go

-func CreateNamespace(t *testing.T, headers http.Header) uint64 {
+type CreateNamespaceParams struct {
+	CustomGraphAdminURLs string
+	NamespaceQuant       int


NamespaceQuant -> just Count

all-seeing-code · 2023-02-09T11:22:54Z

systest/backup/advanced-scenarios/127-Namespace/backup_test.go

+	_ = e2eCommon.CreateNamespace(t, headerAlpha1, e2eCommon.CreateNamespaceParams{CustomGraphAdminURLs: "", NamespaceQuant: 20})
+	utilsCommon.AddSchema(t, headerAlpha1, "alpha1")
+	e2eCommon.AssertGetGQLSchema(t, testutil.ContainerAddr("alpha1", 8080), headerAlpha1)
+	utilsCommon.AddData(t, 1, 10, jwtTokenAlpha1, "alpha1")
+	utilsCommon.CheckDataExists(t, 10, jwtTokenAlpha1, "alpha1")
+	_ = e2eCommon.CreateNamespace(t, headerAlpha1, e2eCommon.CreateNamespaceParams{CustomGraphAdminURLs: "", NamespaceQuant: 20})
+	utilsCommon.AddData(t, 11, 20, jwtTokenAlpha1, "alpha1")
+	utilsCommon.CheckDataExists(t, 20, jwtTokenAlpha1, "alpha1")
+	_ = e2eCommon.CreateNamespace(t, headerAlpha1, e2eCommon.CreateNamespaceParams{CustomGraphAdminURLs: "", NamespaceQuant: 20})
+	utilsCommon.AddData(t, 21, 30, jwtTokenAlpha1, "alpha1")
+	utilsCommon.CheckDataExists(t, 30, jwtTokenAlpha1, "alpha1")
+	_ = e2eCommon.CreateNamespace(t, headerAlpha1, e2eCommon.CreateNamespaceParams{CustomGraphAdminURLs: "", NamespaceQuant: 20})
+	utilsCommon.AddData(t, 31, 40, jwtTokenAlpha1, "alpha1")
+	utilsCommon.CheckDataExists(t, 40, jwtTokenAlpha1, "alpha1")
+	_ = e2eCommon.CreateNamespace(t, headerAlpha1, e2eCommon.CreateNamespaceParams{CustomGraphAdminURLs: "", NamespaceQuant: 20})
+	utilsCommon.AddData(t, 41, 50, jwtTokenAlpha1, "alpha1")
+	utilsCommon.CheckDataExists(t, 50, jwtTokenAlpha1, "alpha1")
+	_ = e2eCommon.CreateNamespace(t, headerAlpha1, e2eCommon.CreateNamespaceParams{CustomGraphAdminURLs: "", NamespaceQuant: 20})
+	utilsCommon.AddData(t, 51, 60, jwtTokenAlpha1, "alpha1")
+	utilsCommon.CheckDataExists(t, 60, jwtTokenAlpha1, "alpha1")
+	_ = e2eCommon.CreateNamespace(t, headerAlpha1, e2eCommon.CreateNamespaceParams{CustomGraphAdminURLs: "", NamespaceQuant: 30})
+	utilsCommon.AddData(t, 61, 70, jwtTokenAlpha1, "alpha1")
+	utilsCommon.CheckDataExists(t, 70, jwtTokenAlpha1, "alpha1")
+	utilsCommon.TakeBackup(t, jwtTokenAlpha1, backupDst, "alpha1")
+	utilsCommon.RunRestore(t, jwtTokenAlpha2, restoreLocation, "alpha2")
+	dg := testutil.DgClientWithLogin(t, "groot", "password", x.GalaxyNamespace)
+	testutil.WaitForRestore(t, dg, testutil.ContainerAddr("alpha2", 8080))
+	e2eCommon.AssertGetGQLSchema(t, testutil.ContainerAddr("alpha2", 8080), headerAlpha2)
+	utilsCommon.CheckDataExists(t, 70, jwtTokenAlpha2, "alpha2")


I don't think we need so many steps, what would more useful would be distributing data across different namespaces as well.

I'd structure it like this:

Step 1, Create 50 namespaces, add data to namespace 0, take backup and restore and check if data is present. (in the restored backend)

Step 2. Create another 50 namespaced, add data to namespace 51, take backup and restore and check if the data is present in 0 and 51

Step 3 Create another 50 namespaces, add data to namespace 130,, take backup and restore and check if data is present in 0, 51 and 130.

With this we check iteratively adding data to new namespaces preserves data in older namespaces across restores. We also check the namespace increment is monotonic, that is after first restore we get the namespace 51 created.

Resolved
Now the test has the following flow:-

Create 1-50 Namespaces Add data to Namespace 0 Backup on Alpha1 Restore on Alpha2 Log into Namespace 0 on Alpha2 CheckDataExist Create 51-100 Namespaces Add data to 51 Namespace Backup on Alpha1 Restore on Alpha 2 Log into Namespace 0 on Alpha2 CheckDataExist Log into Namespace51 on Alpha2 CheckDataExist Create 101-130 Namespaces Add data to Namespace 130 Backup on Alpha1 Restore on Alpha 2 Log into Namespace 0 on Alpha2 CheckDataExist Log into Namespace51 on Alpha2 CheckDataExist Log into Namespace130 on Alpha2 CheckDataExist

all-seeing-code · 2023-02-10T12:46:20Z

systest/backup/advanced-scenarios/127-Namespace/backup_test.go

+	headerAlpha1Np0.Set("Content-Type", "application/json")
+
+	jwtTokenAlpha2Np0 := testutil.GrootHttpLoginNamespace("http://"+testutil.ContainerAddr("alpha2", 8080)+"/admin", 0).AccessJwt
+	headerAlpha2Np0.Set(accessJwtHeader, jwtTokenAlpha2Np0)


a lot of this code can be wrapped inside a function and can be re-used.

mangalaman93 · 2023-02-10T12:53:53Z

graphql/e2e/common/common.go

@@ -70,6 +70,9 @@ var (
 		"indicating that the GraphQL layer didn't get the updated schema even after 10" +
 		" retries. The most probable cause is the new GraphQL schema is same as the old" +
 		" GraphQL schema."
+
+	customAdminURL string
+	namespaceCount = 1


you can define this inside the test/function itself

mangalaman93 · 2023-02-10T12:54:07Z

graphql/e2e/common/common.go

@@ -329,7 +332,12 @@ func containsRetryableCreateNamespaceError(resp *GraphQLResponse) bool {
 	return false
 }

-func CreateNamespace(t *testing.T, headers http.Header) uint64 {
+type CreateNamespaceParams struct {
+	CustomGraphAdminURLs string


can it be just GraphAdminURLs?

mangalaman93 · 2023-02-13T12:09:55Z

graphql/e2e/common/common.go

+
+func CreateNamespace(t *testing.T, headers http.Header, cnp ...CreateNamespaceParams) uint64 {
+	var customAdminURL string
+	var namespaceCount = 1


this needs to be

const namespaceCount = 1

mangalaman93 · 2023-02-13T12:28:01Z

graphql/e2e/common/common.go

@@ -360,7 +379,7 @@ func CreateNamespace(t *testing.T, headers http.Header) uint64 {
 	return resp.AddNamespace.NamespaceId
 }

-func DeleteNamespace(t *testing.T, id uint64, header http.Header) {
+func DeleteNamespace(t *testing.T, id uint64, header http.Header, whichAlpha ...string) {


We should change this function instead.

Resolved

The same fix is applied to CreateNamespace()

mangalaman93 · 2023-02-13T12:28:42Z

graphql/e2e/common/common.go

+	Count          int
+}
+
+func CreateNamespace(t *testing.T, headers http.Header, cnp ...CreateNamespaceParams) uint64 {


we need to have two functions here CreateNamespace and CreateNamespaces.

mangalaman93

a few minor comments, otherwise looks good

mangalaman93 · 2023-02-14T17:47:07Z

graphql/e2e/common/common.go

@@ -344,7 +353,8 @@ func CreateNamespace(t *testing.T, headers http.Header) uint64 {
 	// keep retrying as long as we get a retryable error
 	var gqlResponse *GraphQLResponse
 	for {
-		gqlResponse = createNamespace.ExecuteAsPost(t, GraphqlAdminURL)
+		// gqlResponse = createNamespace.ExecuteAsPost(t, GraphqlAdminURL)


remove this comment?

mangalaman93 · 2023-02-16T02:54:47Z

systest/backup/advanced-scenarios/127-Namespace/docker-compose.yml

+      source: $GOPATH/bin
+      target: /gobin
+      read_only: true
+    - data-volume:/data/backups/


can you leave a space after colon just like we do everywhere else?

I reconfirmed online. Shared volumes are written like this.

mangalaman93 · 2023-02-16T02:55:44Z

systest/backup/common/utils.go

@@ -91,6 +101,179 @@ func CopyToLocalFs(t *testing.T) {
 	require.NoError(t, testutil.DockerCp(srcPath, copyBackupDir))
 }

+func AddSchema(t *testing.T, header http.Header, whichAlpha string) {


decided against changing the name as we discussed?

…es)'

… out of range [0] with length 0'

…usNamespaces().

1] Resolved review comments 2] Updated CreateNamespace() to accept whichAlpha string value on which it runs mutation 3] Added CreateNamespaces() to iterate CreateNamespace() function to create multiple namespaces. 4] Updated DeleteNamespace() to accept whichAlpha string value on which it runs mutation 5] Updated the CreateNamespace() & DeleteNamespace() function calls from multi_tenancy_test.go & 127-Namespace/backup_test.go

SiddeshUbale requested a review from mangalaman93 February 1, 2023 10:27

SiddeshUbale requested review from akon-dey, MichelDiz, matthewmcneely, skrdgraph, darkn3rd, meghalims and billprovince as code owners February 1, 2023 10:27

github-actions bot added area/graphql Issues related to GraphQL support on Dgraph. area/testing Testing related issues labels Feb 1, 2023

mangalaman93 requested changes Feb 2, 2023

View reviewed changes

SiddeshUbale force-pushed the siddesh/127-namespace branch from b777af2 to 78de1d8 Compare February 3, 2023 10:19

SiddeshUbale requested a review from all-seeing-code February 3, 2023 12:49

mangalaman93 requested changes Feb 5, 2023

View reviewed changes

SiddeshUbale force-pushed the siddesh/127-namespace branch from 78de1d8 to 25f1b3f Compare February 7, 2023 06:51

mangalaman93 requested changes Feb 7, 2023

View reviewed changes

SiddeshUbale force-pushed the siddesh/127-namespace branch from 25f1b3f to 856507b Compare February 8, 2023 09:24

mangalaman93 requested changes Feb 8, 2023

View reviewed changes

mangalaman93 reviewed Feb 8, 2023

View reviewed changes

SiddeshUbale force-pushed the siddesh/127-namespace branch 2 times, most recently from d8a3adc to 9db7a5c Compare February 9, 2023 09:25

mangalaman93 previously approved these changes Feb 9, 2023

View reviewed changes

all-seeing-code reviewed Feb 9, 2023

View reviewed changes

SiddeshUbale dismissed mangalaman93’s stale review via 6619f68 February 10, 2023 12:33

SiddeshUbale force-pushed the siddesh/127-namespace branch from 9db7a5c to 6619f68 Compare February 10, 2023 12:33

all-seeing-code reviewed Feb 10, 2023

View reviewed changes

mangalaman93 requested changes Feb 10, 2023

View reviewed changes

all-seeing-code previously approved these changes Feb 13, 2023

View reviewed changes

mangalaman93 requested changes Feb 13, 2023

View reviewed changes

SiddeshUbale dismissed all-seeing-code’s stale review via 2a8a2d7 February 14, 2023 11:02

SiddeshUbale force-pushed the siddesh/127-namespace branch from 20ac6d3 to 2a8a2d7 Compare February 14, 2023 11:02

all-seeing-code previously approved these changes Feb 14, 2023

View reviewed changes

mangalaman93 requested changes Feb 16, 2023

View reviewed changes

SiddeshUbale dismissed all-seeing-code’s stale review via 61a8d70 February 16, 2023 06:57

SiddeshUbale force-pushed the siddesh/127-namespace branch from 2a8a2d7 to 61a8d70 Compare February 16, 2023 06:57

mangalaman93 previously approved these changes Feb 16, 2023

View reviewed changes

all-seeing-code previously approved these changes Feb 16, 2023

View reviewed changes

SiddeshUbale added 12 commits February 17, 2023 11:17

Integration tests for 'Illegal namespace test (more than 127 namespac…

90f02c5

…es)'

Resolved review comments

7f09cf1

Resolved review comments

e0cd92f

Resolved review comments

35982ba

Added shared volume.

4f2a89a

Fixed CreateNamespace() and DeleteNamespace() which is causing 'index…

6140f63

… out of range [0] with length 0'

Altered Test function name from TestDeletedNamespaceID() to Test127Pl…

dcddf1c

…usNamespaces().

Resolved Review comments

dc305a6

Resolved review comments

5e64f01

Resolved review comments

af4ec85

Fixed golangci-lint error.

d06322b

SiddeshUbale dismissed stale reviews from all-seeing-code and mangalaman93 via d06322b February 17, 2023 06:14

SiddeshUbale force-pushed the siddesh/127-namespace branch from 61a8d70 to d06322b Compare February 17, 2023 06:14

mangalaman93 approved these changes Feb 17, 2023

View reviewed changes

all-seeing-code approved these changes Feb 17, 2023

View reviewed changes

mangalaman93 merged commit a8558ab into main Feb 17, 2023

mangalaman93 deleted the siddesh/127-namespace branch February 17, 2023 10:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(test): Integration tests for advanced backup/restore scenarios #8643

chore(test): Integration tests for advanced backup/restore scenarios #8643

SiddeshUbale commented Feb 1, 2023

CLAassistant commented Feb 1, 2023 •

edited

Loading

coveralls commented Feb 1, 2023 •

edited

Loading

mangalaman93 left a comment

mangalaman93 Feb 8, 2023

SiddeshUbale Feb 8, 2023

all-seeing-code Feb 9, 2023

SiddeshUbale Feb 10, 2023

all-seeing-code Feb 9, 2023

SiddeshUbale Feb 10, 2023

all-seeing-code Feb 10, 2023

SiddeshUbale Feb 13, 2023

mangalaman93 Feb 10, 2023

SiddeshUbale Feb 13, 2023

mangalaman93 Feb 10, 2023

SiddeshUbale Feb 13, 2023

mangalaman93 Feb 13, 2023

SiddeshUbale Feb 14, 2023

mangalaman93 Feb 13, 2023

SiddeshUbale Feb 14, 2023 •

edited

Loading

mangalaman93 Feb 13, 2023

SiddeshUbale Feb 14, 2023

mangalaman93 left a comment

mangalaman93 Feb 14, 2023

SiddeshUbale Feb 16, 2023

mangalaman93 Feb 16, 2023

SiddeshUbale Feb 16, 2023

mangalaman93 Feb 16, 2023

SiddeshUbale Feb 16, 2023

chore(test): Integration tests for advanced backup/restore scenarios #8643

chore(test): Integration tests for advanced backup/restore scenarios #8643

Conversation

SiddeshUbale commented Feb 1, 2023

CLAassistant commented Feb 1, 2023 • edited Loading

coveralls commented Feb 1, 2023 • edited Loading

mangalaman93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SiddeshUbale Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mangalaman93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Feb 1, 2023 •

edited

Loading

coveralls commented Feb 1, 2023 •

edited

Loading

SiddeshUbale Feb 14, 2023 •

edited

Loading