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

tests: TEST_crush_reject_empty must not run a mon #5208

Merged
merged 4 commits into from Jul 11, 2015

Conversation

tchaikov
Copy link
Contributor

http://tracker.ceph.com/issues/12285 and http://tracker.ceph.com/issues/11975

this pull request adds some commits on top of #5195 to make osd-crush.sh tests happy.

After a make check fails, it shows a summary but not the output of the
failed tests although they contain information to diagnose the problem.

Set the VERBOSE=true automake variable which is documented to collect
and display the failed script output at the end of a run (the content of
the test-suite.log file (valid from automake-1.11 up).

http://www.gnu.org/software/automake/manual/automake.html#index-VERBOSE

Also remove the run-make-check.sh that did the same in a way that is not
compatible with automake-1.11.

Signed-off-by: Loic Dachary <ldachary@redhat.com>
(cherry picked from commit 3a55cb0)
To display the output in case the command did not fail with the expected
output.

Signed-off-by: Loic Dachary <ldachary@redhat.com>
(cherry picked from commit 5871781)
@ghost
Copy link

ghost commented Jul 11, 2015

@tchaikov can we maybe keep EPERM instead to minimize the extent of the change and avoid on non-cherry pick in the stable branch ?

@ghost
Copy link

ghost commented Jul 11, 2015

@tchaikov could you squash 558a192 and 4a66e35 together ? That will be a single non-cherry-pick to fix the current issue and probably easier to read afterwards.

@tchaikov
Copy link
Contributor Author

@tchaikov can we maybe keep EPERM instead to minimize the extent of the change and avoid on non-cherry pick in the stable branch ?

we could, but if we keep EPERM:

  1. ceph cli will return "Error EPERM: Failed to parse crushmap: error running crushmap through crushtool: (1) Operation not permitted": i am not sure if this is what we expect.
  2. and we need to change the test of TEST_crush_rename_bucket to check Error EPERM instead of Error EINVAL.

the first item is what i don't really like.

@tchaikov could you squash 558a192 and 4a66e35 together ? That will be a single non-cherry-pick to fix the current issue and probably easier to read afterwards.

will do.

ldachary and others added 2 commits July 12, 2015 01:32
* Back in Hammer, the osd-crush.sh individual tests did not run the
  monitor, it was taken care of by the run() function. An attempt to run
  another mon fails with:

  error: IO lock testdir/osd-crush/a/store.db/LOCK: Resource temporarily
  unavailable

  This problem was introduced by cc1cc03
  from ceph#4936
* replace test/mon/mon-test-helpers.sh with test/ceph-helpers.sh as
  we need run_osd() in this newly added test
* update the run-dir of commands: ceph-helpers.sh use the different
  convention for the run-dir of daemons.

http://tracker.ceph.com/issues/11975 Refs: ceph#11975

Signed-off-by: Loic Dachary <ldachary@redhat.com>
this backports a tiny part of ec02441, otherwise
CrushTester will return 1, and "ceph" cli will take it
as EPERM, which is miss leading, and fails
osd-crush.sh:TEST_crush_reject_empty.

Signed-off-by: Kefu Chai <kchai@redhat.com>
@ghost
Copy link

ghost commented Jul 11, 2015

@tchaikov you are right, EINVAL is needed.

Reviewed-by: Loic Dachary <ldachary@redhat.com>

tchaikov added a commit that referenced this pull request Jul 11, 2015
tests: TEST_crush_reject_empty must not run a mon

Reviewed-by: Loic Dachary <ldachary@redhat.com>
@tchaikov tchaikov merged commit 9a79e8e into ceph:hammer Jul 11, 2015
@tchaikov tchaikov deleted the wip-11975-hammer branch July 11, 2015 18:20
@tchaikov
Copy link
Contributor Author

thanks for your review @dachary , and i am very sorry for the noise in the backport pull requests. =(

@loic-bot
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants