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: ceph osd stat out has changed, fix tests for that #16403

Merged
merged 1 commit into from Jul 20, 2017

Conversation

Projects
None yet
2 participants
@wjwithagen
Contributor

wjwithagen commented Jul 18, 2017

The output of ceph osd stat has changed,
It printed:

cluster b370a29d-9287-4ca3-ab57-3d824f65e339
 health HEALTH_OK
 monmap e1: 1 mons at {ceph1=10.0.0.8:6789/0}, election epoch 2, quorum 0 ceph1
 osdmap e63: 2 osds: 2 up, 2 in
  pgmap v41338: 952 pgs, 20 pools, 17130 MB data, 2199 objects
        115 GB used, 167 GB / 297 GB avail
             952 active+clean

but now the osdmap line has gone and thus this no longer works:
qa/workunits/cephtool/test.sh:1944:
old_pgs=$(ceph osd pool get $TEST_POOL_GETSET pg_num | sed -e 's/pg_num: //')
new_pgs=$(($old_pgs+$(ceph osd stat | grep osdmap | awk '{print $3}')*32))

4: qa/workunits/cephtool/test.sh: line 1945: 10+*32: syntax errotoken is "*32")

Signed-off-by: Willem Jan Withagen wjw@digiware.nl

ceph osd pool set $TEST_POOL_GETSET pg_num $new_pgs
ceph osd pool set $TEST_POOL_GETSET pgp_num $new_pgs
wait_for_clean
old_pgs=$(ceph osd pool get $TEST_POOL_GETSET pg_num | sed -e 's/pg_num: //')
new_pgs=$(($old_pgs+$(ceph osd stat | grep osdmap | awk '{print $3}')*32+1))
new_pgs=$(($old_pgs+$(ceph osd stat | awk '{print $1}')*32+1))

This comment has been minimized.

@tchaikov

tchaikov Jul 19, 2017

Contributor

i'd suggest use the json output instead of paring the plain text. also jq allows us to do simple math in its filters.

diff --git a/qa/workunits/cephtool/test.sh b/qa/workunits/cephtool/test.sh
index 9586d81c0d..b23ec28be4 100755
--- a/qa/workunits/cephtool/test.sh
+++ b/qa/workunits/cephtool/test.sh
@@ -1942,12 +1942,12 @@ function test_mon_osd_pool_set()
   ceph osd pool set $TEST_POOL_GETSET pgp_num 10

   old_pgs=$(ceph osd pool get $TEST_POOL_GETSET pg_num | sed -e 's/pg_num: //')
-  new_pgs=$(($old_pgs+$(ceph osd stat | grep osdmap | awk '{print $3}')*32))
+  new_pgs=$(($old_pgs+$(ceph osd stat --format json | jq '.num_osds * 32')))
   ceph osd pool set $TEST_POOL_GETSET pg_num $new_pgs
   ceph osd pool set $TEST_POOL_GETSET pgp_num $new_pgs
   wait_for_clean
   old_pgs=$(ceph osd pool get $TEST_POOL_GETSET pg_num | sed -e 's/pg_num: //')
-  new_pgs=$(($old_pgs+$(ceph osd stat | grep osdmap | awk '{print $3}')*32+1))
+  new_pgs=$(($old_pgs+$(ceph osd stat --format json | jq '.num_osds * 32 + 1')))
   expect_false ceph osd pool set $TEST_POOL_GETSET pg_num $new_pgs

   ceph osd pool set $TEST_POOL_GETSET nosizechange 1

This comment has been minimized.

@wjwithagen

wjwithagen Jul 19, 2017

Contributor

@tchaikov
That would be the smart thing to do.
Uptill now I've tried avoiding to lean yet another language/paradigm.
I'll take a look.

This comment has been minimized.

@wjwithagen

wjwithagen Jul 19, 2017

Contributor

@tchaikov hat was relatively easy.

@tchaikov

not what i imagined. but a lot better. and good catch !

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 19, 2017

lgtm once "make check" is green.

@wjwithagen wjwithagen closed this Jul 19, 2017

@wjwithagen wjwithagen reopened this Jul 19, 2017

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jul 19, 2017

@tchaikov
Yes, I think a lot of trivial awk/grep/sed could go away by using json/jq.
But it is a LOT of cleaning.

And I need to make less typos. :(

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 19, 2017

could you prefix the title of your commit message with the subcomponent your are changing ? see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#3-describe-your-changes

the title of your commit message is not very descriptive:

The output of ceph osd stat has changed,

could you improve it a little bit?

qa/workunits/cephtool/test.sh: ceph osd stat out has changed, fix tes…
…ts for that

The output of ceph osd stat has changed,
It printed:

cluster b370a29d-9287-4ca3-ab57-3d824f65e339
 health HEALTH_OK
 monmap e1: 1 mons at {ceph1=10.0.0.8:6789/0}, election epoch 2, quorum 0 ceph1
 osdmap e63: 2 osds: 2 up, 2 in
  pgmap v41338: 952 pgs, 20 pools, 17130 MB data, 2199 objects
        115 GB used, 167 GB / 297 GB avail
             952 active+clean

but now the osdmap line has gone and thus this no longer works:
qa/workunits/cephtool/test.sh:1944:
old_pgs=$(ceph osd pool get $TEST_POOL_GETSET pg_num | sed -e 's/pg_num: //')
new_pgs=$(($old_pgs+$(ceph osd stat | grep osdmap | awk '{print $3}')*32))

4: qa/workunits/cephtool/test.sh: line 1945: 10+*32: syntax errotoken is "*32")

 - And parse the output in json , with jq, for better reliability

Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jul 19, 2017

@tchaikov That got lost in a rebase.
Is fixed

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 20, 2017

The following tests FAILED:
	  5 - cephtool-test-mon.sh (Timeout)

retest this please

@tchaikov tchaikov self-assigned this Jul 20, 2017

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jul 20, 2017

@tchaikov
I have that timeout regularly in both master and my wip.
Locally and on the Ceph jenkins.
Have not yet figured out what it is, but I suspect that OSDs are shutdown by other testprograms, due to reuse of the same infra.

@tchaikov tchaikov merged commit aea471d into ceph:master Jul 20, 2017

4 checks passed

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

This comment has been minimized.

Contributor

wjwithagen commented Jul 20, 2017

@tchaikov
Looks like this PR has sort reverted itself???
And it now just removes a quote.

I'll redo this PR

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 20, 2017

d'oh ! i thought you just updated the commit message!

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 20, 2017

lemme do this.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jul 20, 2017

@tchaikov
The PR has a history of small adds and amends. Including me deleting everything in between.
So could be a mess to fix.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 20, 2017

#16444 addresses this

@wjwithagen wjwithagen deleted the wjwithagen:bug-wjw-ceph-osd-stat branch Feb 11, 2018

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