Skip to content

vstart_runner: let LocalRemote inherit Remote#37667

Merged
vshankar merged 1 commit intoceph:masterfrom
rishabh-d-dave:vr-inherit-Remote
May 10, 2022
Merged

vstart_runner: let LocalRemote inherit Remote#37667
vshankar merged 1 commit intoceph:masterfrom
rishabh-d-dave:vr-inherit-Remote

Conversation

@rishabh-d-dave
Copy link
Contributor

@rishabh-d-dave rishabh-d-dave commented Oct 14, 2020 •
edited
Loading

And get rid of methods duplicated from
teuthology.orchestra.remote.Remote.

Depends on #37655, #38443 and ceph/teuthology#1626.

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 classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@rishabh-d-dave rishabh-d-dave requested review from a team, batrick and lxbsz October 14, 2020 11:45
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Oct 14, 2020

Test and copied the output here - https://gist.github.com/rishabh-d-dave/10b39a234c9f03e752a7253db55cb4df

tchaikov
tchaikov reviewed Oct 14, 2020
View reviewed changes
batrick
batrick previously requested changes Oct 19, 2020
View reviewed changes
lxbsz
lxbsz approved these changes Oct 21, 2020
View reviewed changes
Copy link
Member

@lxbsz lxbsz left a comment

Choose a reason for hiding this comment

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

LGTM.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Nov 9, 2020

The persistent error on Ceph API tests (which is same as the one here https://jenkins.ceph.com/job/ceph-api/6273/) is due to a bug in mgr_test_case.py. Here's the fix for that bug - #37992.

This PR needs to wait for that PR as well.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Nov 9, 2020

The fix for dashboard QA gets the CI green - https://jenkins.ceph.com/job/ceph-api/6373/

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Nov 19, 2020

jenkins test api

lxbsz
lxbsz approved these changes Nov 24, 2020
View reviewed changes
Copy link
Member

@lxbsz lxbsz left a comment

Choose a reason for hiding this comment

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

LGTM.

@lxbsz
Copy link
Member

lxbsz commented Nov 24, 2020

jenkins test api

@github-actions github-actions bot added the tests label Dec 2, 2020
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Dec 3, 2020

jenkins test api

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Dec 3, 2020

jenkins test make check

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Dec 4, 2020

Without commit "vstart_runner: make "shell" as default argument", ceph API tests ran successfully - https://jenkins.ceph.com/job/ceph-api/8102/.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Dec 7, 2020

Removed a commit since there's a separate PR for it now; PR #38443. Now this PR also depends on #38443.

@rishabh-d-dave rishabh-d-dave mentioned this pull request Dec 7, 2020
Merged
3 tasks
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Sep 8, 2021

https://readthedocs.org/projects/ceph/builds/14662065/

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Sep 8, 2021

jenkins test docs

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Sep 8, 2021

jenkins render docs

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Nov 11, 2021

jenkins test make check

@rishabh-d-dave rishabh-d-dave force-pushed the vr-inherit-Remote branch 3 times, most recently from cdc1835 to 156484f Compare January 24, 2022 08:28
@github-actions
Copy link

github-actions bot commented May 4, 2022

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@rishabh-d-dave
vstart_runner: make LocalRemote inherit RemoteShell
04209a1 
And therefore get rid of methods duplicated in LocalRemote and add a
call to empty constructor of RemoteShell in LocalRemote.__init__().

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 5, 2022

Rebased.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 5, 2022

All required PRs have been merged. Removing DNM now...

@rishabh-d-dave rishabh-d-dave removed the DNM label May 5, 2022
@rishabh-d-dave rishabh-d-dave added the cephfs Ceph File System label May 5, 2022
lxbsz
lxbsz approved these changes May 5, 2022
View reviewed changes
Copy link
Member

@lxbsz lxbsz left a comment

Choose a reason for hiding this comment

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

LGTM.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 5, 2022

@lxbsz Thanks!

vshankar
vshankar approved these changes May 9, 2022
View reviewed changes
Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

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

Please update the PR with a sample test run w/ vstart_runner.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 10, 2022

mktemp(), read_file() and write_file() from Reomte were exercised by this testing - https://gist.github.com/rishabh-d-dave/cdb4f64e6251ec2cc663f9e3d5ab4788

@vshankar vshankar dismissed stale reviews from tchaikov and batrick May 10, 2022 04:59

comments addressed.

@vshankar vshankar merged commit f005520 into ceph:master May 10, 2022
@rishabh-d-dave rishabh-d-dave deleted the vr-inherit-Remote branch May 10, 2022 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vshankar vshankar vshankar approved these changes

@lxbsz lxbsz lxbsz approved these changes

@batrick batrick Awaiting requested review from batrick

@tchaikov tchaikov Awaiting requested review from tchaikov

Assignees

No one assigned

Labels

cephfs Ceph File System cleanup needs-review tests vstart_runner

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rishabh-d-dave @lxbsz @tchaikov @batrick @vshankar