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.

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

Fix mc logging and exit on first failure #159

Merged
merged 1 commit into from
Sep 29, 2017

Conversation

nitisht
Copy link
Contributor

@nitisht nitisht commented Sep 27, 2017

Fixes #154

@balamurugana
Copy link
Member

PR #139 is pulling functional test of mc which is taken care in that.

@nitisht
Copy link
Contributor Author

nitisht commented Sep 27, 2017

@balamurugana we need to release mint soon. Do you think #139 should be merged and be the part of mint first release?

@balamurugana
Copy link
Member

@nitisht I am not saying we need to take/wait for #139 for the release

Copy link
Collaborator

@kannappanr kannappanr left a comment

Choose a reason for hiding this comment

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

Value for error is missing double quotes

function log_failure() {
    function=$(python -c 'import sys,json; print(json.dumps(sys.stdin.read()))' <<<"$2")
    printf '{"name": "mc", "duration": "%d", "function": %s, "status": "FAIL", "error": %s}\n' "$1" "$function" "$3"
}

Within test_mirror_list_objects function, around line 362

if [ $rv -eq 0 ]; then 
        if  diff -bB <(ls "$MINT_DATA_DIR") <(./mc ls --json "${SERVER_ALIAS}/${bucket_name}" | jq -r .key ) > /dev/null; then
            function="delete_bucket"
            out=$(delete_bucket "$bucket_name") 
            rv=$?
        else
            rv=1
        fi
    fi

I see that the diff check is failing in my case. You will have to add sort to both the ls and mc command output and also redirect the output to /dev/null

Something like this

 if  diff -bB <(ls "$MINT_DATA_DIR" | sort) <(./mc ls --json "${SERVER_ALIAS}/${bucket_name}" | jq -r .key | sort ) > /dev/null; then

Also, I see that the buckets and objects are getting deleted only in the case of failure. Since, every test is creating a bucket, it is important to properly cleanup the buckets and objects.

@nitisht
Copy link
Contributor Author

nitisht commented Sep 28, 2017

updated the PR @kannappanr

Copy link
Collaborator

@kannappanr kannappanr left a comment

Choose a reason for hiding this comment

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

Similar to AWSCLI, we will modify the function name and args in a different PR. Otherwise LGTM

@deekoder
Copy link
Contributor

is travis expected to fail?

@kannappanr
Copy link
Collaborator

Shellcheck generates a warning on ls and sort in the following line of code

if  diff -bB <(ls "$MINT_DATA_DIR" | sort) <(./mc ls --json "${SERVER_ALIAS}/${bucket_name}" | jq -r .key | sort) > /dev/null; then

this can be replaced by find like this

if  diff -bB <(find "$MINT_DATA_DIR" -type f -printf "%f\n" | sort) <(./mc ls --json "${SERVER_ALIAS}/${bucket_name}" | jq -r .key | sort) > /dev/null; then

@nitisht
Copy link
Contributor Author

nitisht commented Sep 29, 2017

Updated the PR @kannappanr @deekoder

@nitisht nitisht removed the blocked label Sep 29, 2017
@nitisht nitisht merged commit 89f46f0 into minio:master Sep 29, 2017
@nitisht nitisht deleted the mc-console-fix branch September 29, 2017 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants