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

test: create asok files in a temp directory under $TMPDIR #16445

Merged
merged 1 commit into from Jul 21, 2017

Conversation

Projects
None yet
4 participants
@tchaikov
Contributor

tchaikov commented Jul 20, 2017

to shorten the pathname of unix domain socket created for admin socket,
so it does not exceed the limit of 107 on GNU/Linux:

  • ceph-helper.sh: the temp directory is named ${TMPDIR:-/tmp}/ceph-asok.$$
  • vstart.sh: the temp directory is named mktemp -u -d "${TMPDIR:-/tmp}/ceph-asok.XXXXXX"

Fixes: http://tracker.ceph.com/issues/16895
Signed-off-by: Kefu Chai kchai@redhat.com

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 21, 2017

retest this please.

@tchaikov tchaikov requested review from , jdurgin and dmick Jul 21, 2017

@ghost

ghost approved these changes Jul 21, 2017

test: create asok files in a temp directory under $TMPDIR
to shorten the pathname of unix domain socket created for admin socket,
so it does not exceed the limit of 107 on GNU/Linux:

* ceph-helper.sh: the temp directory is named ${TMPDIR:-/tmp}/ceph-asok.$$
* vstart.sh: the temp directory is named `mktemp -u -d "${TMPDIR:-/tmp}/ceph-asok.XXXXXX"`

Fixes: http://tracker.ceph.com/issues/16895
Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 21, 2017

changelog

  • rebased against master
  • vstart.sh: rm -rf $asok_dir if $new

@tchaikov tchaikov merged commit 350feb8 into ceph:master Jul 21, 2017

2 of 4 checks passed

make check make check failed
Details
make check (arm64) running make check
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details

@tchaikov tchaikov deleted the tchaikov:wip-16895 branch Jul 21, 2017

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jul 22, 2017

@tchaikov

I'm now getting:
vstart.sh:350 and further

4: +++ /home/jenkins/workspace/ceph-master/build/bin/ceph-conf --show-config-value admin_socket
4: ++ dirname /var/run/ceph/ceph-client.admin.asok
4: + asok_dir=/var/run/ceph
4: + '[' -d /var/run/ceph ']'
4: + rm -f '/var/run/ceph/*'
4: + rmdir /var/run/ceph
4: rmdir: /var/run/ceph: Permission denied

While testing cephtool-test-mon.sh and some more tools.
So one way or another the admin_socket path isn't making it into the config of the tests.
And perhaps even because there is no config to begin with.

For a user ceph or jenkins is is not allowed to remove a dir from /var/run

Not that it is correctly set:
4: + CEPH_ASOK_DIR=/home/jenkins/workspace/ceph-master/build/src/test/td/t-7202/out

So I guess that asking for the ASOK_DIR is only usefull if it did not get set previously.
And also removing that directory only when it is already there, and perhaps use rm -rf <dir>.
And then even still if this is run on odd platform, perhaps it is not wise to plainly remove it, but just remove the content?

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jul 22, 2017

@tchaikov
Rewrote it to:

    if [ "$new" -ne 0 ]; then
        # only delete if -n
        if [ -z "$CEPH_ASOK_DIR" ] ; then
            asok_dir=`dirname $($CEPH_BIN/ceph-conf --show-config-value admin_socket)`
            if [ -d $asok_dir ]; then
                rm -f $asok_dir/*
                # could fail because the parent is not writable
                rmdir $asok_dir || true
            fi
            CEPH_ASOK_DIR=`mktemp -u -d "${TMPDIR:-/tmp}/ceph-asok.XXXXXX"`
        fi
        [ -e "$conf_fn" ] && rm -- "$conf_fn"
    else
        CEPH_ASOK_DIR=`dirname $($CEPH_BIN/ceph-conf --show-config-value admin_socket)`
        # -k is implied... (doesn't make sense otherwise)
        overwrite_conf=0
    fi
fi

But I fail to see how this would reduce the asok path length, unless the calling partner makes sure that
CEPH_ASOK_DIR does not exceed 85-90 chars.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 24, 2017

"${TMPDIR:-/tmp}/ceph-asok.XXXXXX"` is way shorter than the subdir in jenkins workspace.

@@ -21,7 +22,7 @@ shift 2
run_root=$script_root/run/$name
pidfile=$run_root/out/radosgw.${port}.pid
asokfile=$run_root/out/radosgw.${port}.asok
asokfile=$($ceph_bin/ceph-conf --show-config-value admin_socket --name radosgw.${port})

This comment has been minimized.

@cbodley

cbodley Jul 24, 2017

Contributor

@tchaikov why is it a problem for these .asok files to go under $run_root with the others? this script doesn't run as part of make check

this change causes ceph-conf to fail with:

error parsing 'radosgw.8000': expected string of the form TYPE.ID, valid types are: auth, mon, osd, mds, mgr, client

This comment has been minimized.

@cbodley

cbodley Jul 24, 2017

Contributor

see #16540

This comment has been minimized.

@tchaikov

tchaikov Jul 25, 2017

Contributor

@cbodley sorry for the inconvenience.

it was not a problem unless the $run_root is too long, so that the path name of asokfile exceeds the 107 limit. i was trying to find all the occurance of .asok, and to move them under $TMPDIR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment