Test for Worm Mode Introduced #277

sinhaashish · 2018-05-18T18:22:16Z

Test cases are added to check the worm mode of the Server

nitisht · 2018-05-22T08:49:46Z

this needs minio-go release for Travis/Mint build to work

balamurugana · 2018-05-28T10:06:54Z

run/core/worm/quick-worm-tests.go

+	startTime := time.Now()
+	object := "testObject"
+	function := "PutAndDelete"
+	bucket := randString(60, rand.NewSource(time.Now().UnixNano()), "worm-mode-test-")


You could have only one random source than creating every time.

code modified

balamurugana · 2018-05-28T10:28:38Z

run/core/worm/quick-worm-tests.go

+		remain--
+	}
+	return prefix + string(b[0:30-len(prefix)])
+}


Below code is much simpler than this

const charset = "abcdefghijklmnopqrstuvwxyz0123456789" var randSource *rand.Rand = rand.New(rand.NewSource(time.Now().UnixNano())) func randBucketName() string { const length = 55 b := make([]byte, length) for i := range b { b[i] = charset[randSource.Intn(len(charset))] } return "bucket-" + string(b) }

and it can be used as

bucketName := randBucketName()

Used the above given code.

balamurugana · 2018-05-28T10:34:43Z

run/core/worm/quick-worm-tests.go

+	maxRetries  = 1
+)
+
+type ErrorResponse struct {


Is this type used anywhere?

balamurugana · 2018-05-28T10:36:19Z

run/core/worm/quick-worm-tests.go

@@ -0,0 +1,434 @@
+/*
+*
+*  Mint, (C) 2017 Minio, Inc.


removed the blank space.

balamurugana · 2018-05-28T10:39:38Z

mint.sh

-        exit 1
-    fi
+
+     if [ "$MINT_MODE" = "worm" ];then


I think its not required to print mode specific here because user runs with worm mode, not by default.

removed worm specific output

balamurugana · 2018-05-28T10:40:16Z

mint.sh

@@ -136,14 +162,25 @@ function main()
            break
        fi
    done
+    fi


you could need to format this script properly

balamurugana · 2018-05-28T10:43:26Z

mint.sh

+          fi
+    else   
+        # Remove worm test case       
+        for i in "${!run_list[@]}"; do


You could keep run_list populated for worm mode and non-worm mode properly. There is no need to run specifically with if block.

balamurugana · 2018-05-28T10:46:48Z

build/worm/install.sh

+go get -u github.com/aws/aws-sdk-go/aws/...
+go get -u github.com/aws/aws-sdk-go/aws/credentials/...
+go get -u github.com/aws/aws-sdk-go/aws/session/...
+go get -u github.com/aws/aws-sdk-go/service/s3/...


What package is required here? go get -u downloads dependencies automatically.

Point here is to have fewer (mostly one) go get -u .... Could you figure out and fix accordingly?

@balamurugana I will try go get -u github.com/aws/aws-sdk-go/... and confirm if it works.

I am not sure whether the test requires everything from github.com/aws/aws-sdk-go. I hope you checked that. I think https://talks.golang.org/2014/organizeio.slide#1 may be helpful.

I found that all the dependencies are already present in install.sh of awscli thus i have removed it from worm. Please suggest if any modifications is needed for this.

kannappanr · 2018-05-29T16:58:57Z

@sinhaashish Travis build is failing. Can you please take a look?

nitisht · 2018-05-29T17:10:19Z

@kannappanr we need to release minio-go for Travis build fix

harshavardhana · 2018-05-30T00:05:58Z

@kannappanr we need to release minio-go for Travis build fix

minio-go released @nitisht @sinhaashish

nitisht · 2018-05-30T04:18:10Z

@sinhaashish build works now, can you please check the review comments

sinhaashish · 2018-06-02T11:52:38Z

@balamurugana @nitisht please validate. Modified the mint.sh for creating scenario based runlist.

balamurugana · 2018-06-03T01:32:21Z

run/core/worm/quick-worm-tests.go

+}
+
+func randBucketName() string {
+	const length = 55


IMO this const is not required because its used in only one place

balamurugana · 2018-06-03T01:34:46Z

run/core/worm/quick-worm-tests.go

@@ -0,0 +1,410 @@
+/*
+*  Mint, (C) 2017 Minio, Inc.


As this is new file, you would need to set copyright to 2018

balamurugana · 2018-06-03T01:36:43Z

Dockerfile.dev

 COPY remove-packages.list /mint
 COPY postinstall.sh /mint
 RUN /mint/postinstall.sh

 COPY mint.sh /mint/mint.sh
 COPY entrypoint.sh /mint/entrypoint.sh
-ENTRYPOINT ["/mint/entrypoint.sh"]
+ENTRYPOINT ["/mint/entrypoint.sh"]


Add newline

balamurugana · 2018-06-03T01:40:55Z

README.md

+```sh
+$ docker run -e SERVER_ENDPOINT=play.minio.io:9000 -e ACCESS_KEY=Q3AM3UQ867SPQQA43P2F \
+             -e SECRET_KEY=zuf+tfteSlswRu7BJ86wekitnifILbZam1KYY3TG -e ENABLE_HTTPS=1 -e MINT_MODE=worm minio/mint
+```


As below table updated with mint mode specific info, this is not required for specific about WORM mode run IMO

removed this.

balamurugana · 2018-06-03T01:45:41Z

README.md

+$ docker run -e SERVER_ENDPOINT=`Server End Point` -e ACCESS_KEY=`Your Access Key` \
+             -e SECRET_KEY=`Your Secret Key`\
+             -e ENABLE_HTTPS=1 -e MINT_MODE=worm  minio/mint:latest
+```


IMO this section of change is also not required. Developer knows how to run with various modes by looking into the above table

balamurugana · 2018-06-03T01:47:11Z

mint.sh

@@ -60,9 +61,10 @@ function run_test()
    start=$(date +%s)

    mkdir -p "$BASE_LOG_DIR/$sdk_name"
-
+    echo "BASE_LOG_DIR :$BASE_LOG_DIR"


Overall you would need to remove your debug prints/commands.

format the code to the standard and remove unnecessary newlines (double newlines)

balamurugana · 2018-06-03T02:59:30Z

mint.sh

@@ -111,21 +114,39 @@ function main()
    echo "To get logs, run 'docker cp ${CONTAINER_ID}:/mint/log /tmp/mint-logs'"

    declare -a run_list
+    declare -a run_list_excluding_worm


You would need to replace code block between lines 114 and 122 with below code and there is no other change required in this file.

if [ "$MINT_MODE" == "worm" ]; then if [ "$#" -gt 1 ]; then echo "No argument is accepted for worm mode" exit 1 fi run_list=( "$TEST_DIR/worm" ) else sdks=( "$@" ) ## populate all sdks except worm when no argument is given. if [ "$#" -eq 1 ]; then sdks=( $(ls -I worm "$TESTS_DIR") ) fi for sdk in "${sdks[@]}"; do if [ "$sdk" == "worm" ]; then echo "worm test cannot be run without worm mode" exit 1 fi run_list=( "${run_list[@]}" "$TESTS_DIR/$sdk" ) done fi

This is untested code.

Sorry for pasting the code. I am unable to explain the logic without the code.

Thanks for guiding. Please correct if i am wrong.
if condition should compare with 0 in the below code

## populate all sdks except worm when no argument is given. if [ "$#" -eq 1 ]; then sdks=( $(ls -I worm "$TESTS_DIR") ) fi

Yep. "$#" comes without $0

nitisht · 2018-06-07T07:04:10Z

Dockerfile.dev

 COPY remove-packages.list /mint
 COPY postinstall.sh /mint
 RUN /mint/postinstall.sh

 COPY mint.sh /mint/mint.sh
 COPY entrypoint.sh /mint/entrypoint.sh
-ENTRYPOINT ["/mint/entrypoint.sh"]
+
+ENTRYPOINT ["/mint/entrypoint.sh"]


add new line

nitisht · 2018-06-09T09:04:25Z

run/core/worm/quick-worm-tests.go

+	}
+	_, err = s3Client.PutObject(putInput2)
+	if err == nil {
+		failureLog(function, args, startTime, "", fmt.Sprintf("WORM_MODE ON Put is expected to fail, but it Success %v", nil), nil).Fatal()


Error message can be WORM_MODE ON - Put is expected to fail, but it passed %v in here and all the other occurrences.

nitisht

Tested locally, works fine, one small comment pending

nitisht

LGTM & Tested

Introducing new test cases for testing server in worm mode

sinhaashish force-pushed the onetest branch 3 times, most recently from 127f510 to a3c64f4 Compare May 18, 2018 18:50

deekoder requested review from balamurugana and nitisht May 18, 2018 19:10

sinhaashish force-pushed the onetest branch 3 times, most recently from 3f6ba90 to 88858e4 Compare May 19, 2018 02:21

sinhaashish force-pushed the onetest branch 2 times, most recently from 6717c22 to 294c24c Compare May 23, 2018 02:54

balamurugana requested changes May 28, 2018

View reviewed changes

sinhaashish force-pushed the onetest branch from 294c24c to 92e1099 Compare June 2, 2018 11:35

balamurugana reviewed Jun 3, 2018

View reviewed changes

sinhaashish force-pushed the onetest branch 5 times, most recently from 98bea2e to 8749b78 Compare June 6, 2018 05:10

nitisht reviewed Jun 7, 2018

View reviewed changes

sinhaashish force-pushed the onetest branch from 8749b78 to 497b227 Compare June 9, 2018 06:55

nitisht suggested changes Jun 9, 2018

View reviewed changes

nitisht reviewed Jun 9, 2018

View reviewed changes

sinhaashish force-pushed the onetest branch from 497b227 to f7023c7 Compare June 11, 2018 03:57

nitisht approved these changes Jun 11, 2018

View reviewed changes

Adding Test for Worm Mode

c13c78f

Introducing new test cases for testing server in worm mode

sinhaashish force-pushed the onetest branch from f7023c7 to c13c78f Compare June 13, 2018 04:00

balamurugana approved these changes Jun 14, 2018

View reviewed changes

nitisht merged commit a05cf19 into minio:master Jun 14, 2018

@@ @@ -136,14 +162,25 @@ function main() @@
                           break
                       fi
                   done
+                  fi

Test for Worm Mode Introduced #277

Test for Worm Mode Introduced #277

Conversation

sinhaashish commented May 18, 2018

nitisht commented May 22, 2018

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

sinhaashish Jun 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kannappanr commented May 29, 2018

nitisht commented May 29, 2018

harshavardhana commented May 30, 2018

nitisht commented May 30, 2018

sinhaashish commented Jun 2, 2018

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

balamurugana Jun 3, 2018 • edited Loading

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

nitisht left a comment

Choose a reason for hiding this comment

nitisht left a comment

Choose a reason for hiding this comment

sinhaashish Jun 12, 2018 •

edited

Loading

balamurugana Jun 3, 2018 •

edited

Loading