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

qa/vstart_runner: update vstart_runner.LocalRemote.sh #33945

Merged

Conversation

rishabh-d-dave
Copy link
Contributor

Commit 9f6c764 replaces remote.run calls by remote.sh without
updating the definition of vstart_runner.LocalRemote.sh which breaks the
cephfs tests when executed locally.

Fixes: https://tracker.ceph.com/issues/44579

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@rishabh-d-dave
Copy link
Contributor Author

Tested this PR locally -

2020-03-19 22:44:20,073.073 INFO:__main__:test_issue_cephfs_shell_cmd_at_invocation (tasks.cephfs.test_cephfs_shell.TestMisc) ... ok
2020-03-19 22:44:20,074.074 INFO:__main__:Stopped test: test_issue_cephfs_shell_cmd_at_invocation (tasks.cephfs.test_cephfs_shell.TestMisc) in 17.912043s
2020-03-19 22:44:20,074.074 INFO:__main__:
2020-03-19 22:44:20,074.074 INFO:__main__:----------------------------------------------------------------------
2020-03-19 22:44:20,074.074 INFO:__main__:Ran 1 test in 17.912s
2020-03-19 22:44:20,075.075 INFO:__main__:
2020-03-19 22:44:20,075.075 INFO:__main__:OK

test_issue_cephfs_shell_cmd_at_invocation contains a call to remote.sh (see https://github.com/ceph/ceph/blob/master/qa/tasks/cephfs/test_cephfs_shell.py#L928)

@ukernel
Copy link
Contributor

ukernel commented Mar 20, 2020

Works for me

@rishabh-d-dave
Copy link
Contributor Author

@ukernel Can I please get an approval?

@gregsfortytwo
Copy link
Member

Needs a signed-off-by

@rishabh-d-dave rishabh-d-dave force-pushed the fs-qa-vstart_runner.LocalRemote.sh branch from 025ff1e to b088bd3 Compare March 24, 2020 04:24
@rishabh-d-dave
Copy link
Contributor Author

@gregsfortytwo Added.

@gregsfortytwo
Copy link
Member

http://pulpito.ceph.com/gregf-2020-03-26_16:03:14-fs-wip-greg-testing-326-distro-basic-smithi/

Hmm merge conflict with another one of your PRs, perhaps, @rishabh-d-dave?

Commit 9f6c764 replaces remote.run calls by remote.sh without
updating the definition of vstart_runner.LocalRemote.sh which breaks the
cephfs tests when executed locally.

Fixes: https://tracker.ceph.com/issues/44579
Signed-off-by: Rishabh Dave <ridave@redhat.com>
@gregsfortytwo
Copy link
Member

@rishabh-d-dave
Copy link
Contributor Author

In my understanding, this change should affect only how tests run locally. Therefore, I don't see the point in testing this patch with Teuthology.

@gregsfortytwo
Copy link
Member

In my understanding, this change should affect only how tests run locally. Therefore, I don't see the point in testing this patch with Teuthology.

Yes, that's technically true. But it doesn't hurt to run it through a QA suite to be sure and there are plenty of times we've had little change that "can't possibly break anything" cause an entire suite of tests to fail, so we like to be cautious unless there's a strong urgent reason not to be. :)

@rishabh-d-dave
Copy link
Contributor Author

the point in testing this patch with Teuthology.

Yes, that's technically true. But it doesn't hurt to run it through a QA suite to be sure and there are plenty of times we've had little change that "can't possibly break anything" cause an entire suite of tests to fail, so we like to be cautious unless there's a strong urgent reason not to be. :)

Cool. Thank you for the explanation.

@tchaikov
Copy link
Contributor

tchaikov commented Apr 7, 2020

jenkins test dashboard backend

@tchaikov
Copy link
Contributor

tchaikov commented Apr 7, 2020

In my understanding, this change should affect only how tests run locally. Therefore, I don't see the point in testing this patch with Teuthology.

FWIW, vstart-runner is also used by the "dashboard backend" jenkins job.

@rishabh-d-dave
Copy link
Contributor Author

In my understanding, this change should affect only how tests run locally. Therefore, I don't see the point in testing this patch with Teuthology.

FWIW, vstart-runner is also used by the "dashboard backend" jenkins job.

Oh, okay. Wasn't aware of that. Thanks!

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.

4 participants