Skip to content

cephfs_mirror: update peer status for invalid metadata in remote snapshot#56816

Merged
vshankar merged 3 commits intoceph:mainfrom
joscollin:wip-B65317-B65226-peer-status-invalid-metadata
Aug 22, 2024
Merged

cephfs_mirror: update peer status for invalid metadata in remote snapshot#56816
vshankar merged 3 commits intoceph:mainfrom
joscollin:wip-B65317-B65226-peer-status-invalid-metadata

Conversation

@joscollin
Copy link
Member

@joscollin joscollin commented Apr 10, 2024

Fixes: https://tracker.ceph.com/issues/65317
Fixes: https://tracker.ceph.com/issues/65226

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
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 dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

@vshankar
Copy link
Contributor

@joscollin Why close #56698 in favour of this. That PR could have been updated o_O

@joscollin
Copy link
Member Author

@joscollin Why close #56698 in favour of this. That PR could have been updated o_O

I needed a different branch altogether. It's difficult for me to track without a branch id.

@joscollin joscollin force-pushed the wip-B65317-B65226-peer-status-invalid-metadata branch from d516767 to 8e43500 Compare April 10, 2024 12:38
Copy link
Contributor

@leonid-s-usov leonid-s-usov left a comment

Choose a reason for hiding this comment

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

Shouldn't we set the last_failed_reason also at other error exits of the function, e.g. around lines 513 and 530?

@joscollin joscollin added the DNM label Apr 10, 2024
@joscollin
Copy link
Member Author

Shouldn't we set the last_failed_reason also at other error exits of the function, e.g. around lines 513 and 530?

It is out of scope of this PR. Please read the requirement.
Another thing is this PR is Draft, not ready for review yet.

@joscollin joscollin force-pushed the wip-B65317-B65226-peer-status-invalid-metadata branch from 8e43500 to 61531d5 Compare April 11, 2024 09:33
@joscollin
Copy link
Member Author

@joscollin joscollin marked this pull request as ready for review April 15, 2024 12:47
@joscollin joscollin removed the DNM label Apr 15, 2024
@ceph ceph deleted a comment from leonid-s-usov Apr 22, 2024
@vshankar
Copy link
Contributor

@joscollin what's the status of this change?

@joscollin
Copy link
Member Author

@joscollin what's the status of this change?

I've completed my work on it. @vshankar It's been waiting for your review.

@joscollin
Copy link
Member Author

jenkins test windows

@joscollin
Copy link
Member Author

jenkins test make check arm64

@joscollin joscollin force-pushed the wip-B65317-B65226-peer-status-invalid-metadata branch from 61531d5 to 7667a25 Compare July 9, 2024 11:37
@joscollin
Copy link
Member Author

rebased

@joscollin
Copy link
Member Author

jenkins test api

@joscollin
Copy link
Member Author

jenkins test api

@joscollin
Copy link
Member Author

jenkins test windows

@joscollin
Copy link
Member Author

jenkins test make check

@joscollin
Copy link
Member Author

jenkins test make check arm64

@vshankar
Copy link
Contributor

vshankar commented Aug 2, 2024

This PR is under test in https://tracker.ceph.com/issues/67318.

@joscollin
Copy link
Member Author

jenkins test api

@joscollin
Copy link
Member Author

jenkins test make check

joscollin pushed a commit to joscollin/ceph that referenced this pull request Aug 7, 2024
* refs/pull/56816/head:
	doc: mention the peer status failed when snapshot created on the remote filesystem.
	qa: add test_cephfs_mirror_remote_snap_corrupt_fails_synced_snapshot
	cephfs_mirror: update peer status for invalid metadata in remote snapshot

Reviewed-by: Venky Shankar <vshankar@redhat.com>
@joscollin joscollin force-pushed the wip-B65317-B65226-peer-status-invalid-metadata branch from 0495910 to 4fc49b5 Compare August 10, 2024 03:18
joscollin added a commit to joscollin/ceph that referenced this pull request Aug 12, 2024
* refs/pull/56816/head:
	qa: fixed rwps
	doc: mention the peer status failed when snapshot created on the remote filesystem.
	qa: add test_cephfs_mirror_remote_snap_corrupt_fails_synced_snapshot
	cephfs_mirror: update peer status for invalid metadata in remote snapshot

Reviewed-by: Anthony D Atri <anthony.datri@gmail.com>
Reviewed-by: Venky Shankar <vshankar@redhat.com>
…te filesystem.

* Update the docs for the failed status, when the snapshot is created on the remote fs.
* Mention the read-only restrictions of the remote fs.

Fixes: https://tracker.ceph.com/issues/65317
Signed-off-by: Jos Collin <jcollin@redhat.com>
@joscollin joscollin force-pushed the wip-B65317-B65226-peer-status-invalid-metadata branch from 4fc49b5 to ce10e5e Compare August 12, 2024 06:21
@joscollin
Copy link
Member Author

@vshankar
The failure found in your current QA run is fixed. The test is passed:
https://pulpito.ceph.com/jcollin-2024-08-12_04:38:52-fs:mirror-wip-jcollin-testing-20240810.032732-main-distro-default-smithi/

Please do another QA for this PR.

@joscollin
Copy link
Member Author

jenkins test make check

@vshankar
Copy link
Contributor

@vshankar The failure found in your current QA run is fixed. The test is passed: https://pulpito.ceph.com/jcollin-2024-08-12_04:38:52-fs:mirror-wip-jcollin-testing-20240810.032732-main-distro-default-smithi/

Please do another QA for this PR.

I will add it to the test batch. Could you explain the failure and the fix please?

@joscollin
Copy link
Member Author

@vshankar The failure found in your current QA run is fixed. The test is passed: https://pulpito.ceph.com/jcollin-2024-08-12_04:38:52-fs:mirror-wip-jcollin-testing-20240810.032732-main-distro-default-smithi/
Please do another QA for this PR.

I will add it to the test batch. Could you explain the failure and the fix please?

@vshankar
rwps seems necessary here: https://github.com/ceph/ceph/pull/56816/files#diff-807e4dcab2eedecf94ad73511d8816cb7f328f7b3b2f99c746e75e265c2dff53R1522. I've ran it two times and the test passes. Please QA again.

@joscollin
Copy link
Member Author

jenkins test make check

vshankar added a commit to vshankar/ceph that referenced this pull request Aug 20, 2024
* refs/pull/56816/head:
	doc: mention the peer status failed when snapshot created on the remote filesystem.
	qa: add test_cephfs_mirror_remote_snap_corrupt_fails_synced_snapshot
	cephfs_mirror: update peer status for invalid metadata in remote snapshot

Reviewed-by: Anthony D Atri <anthony.datri@gmail.com>
Reviewed-by: Venky Shankar <vshankar@redhat.com>
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.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants