Skip to content

qa: disable metrics on kernel client during upgrade#43974

Merged
batrick merged 1 commit intoceph:masterfrom
batrick:i53293
Nov 19, 2021
Merged

qa: disable metrics on kernel client during upgrade#43974
batrick merged 1 commit intoceph:masterfrom
batrick:i53293

Conversation

@batrick
Copy link
Member

@batrick batrick commented Nov 16, 2021 •
edited
Loading

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)
  • 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)
  • Teuthology
    • Completed teuthology run
    • No teuthology test necessary (e.g., documentation)
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

@batrick batrick changed the title qa: use ceph-fuse to avoid via new messages from kernel qa: use ceph-fuse to avoid new messages from kernel Nov 16, 2021
@batrick batrick force-pushed the i53293 branch 2 times, most recently from 725b524 to 5024594 Compare November 17, 2021 14:29
batrick added a commit to batrick/ceph that referenced this pull request Nov 17, 2021
@batrick
Merge PR ceph#43974 into wip-pdonnell-testing-20211116.201517
ff361bd 
* refs/pull/43974/head:
	qa: use ceph-fuse to avoid new messages from kernel
@djgalloway
Copy link
Contributor

djgalloway commented Nov 17, 2021

Does this also fix https://tracker.ceph.com/issues/53288?

@batrick
Copy link
Member Author

batrick commented Nov 18, 2021

Does this also fix https://tracker.ceph.com/issues/53288?

I assume 53288 is for fixing teuthology/qa so this type of failure does not result in a 12 hour dead job.

@batrick
Copy link
Member Author

batrick commented Nov 18, 2021

Currently trying to figure out why ceph-fuse is not unmounting: https://pulpito.ceph.com/pdonnell-2021-11-18_02:55:54-fs:upgrade-wip-pdonnell-testing-20211116.201517-distro-basic-smithi/

@sebastian-philipp sebastian-philipp mentioned this pull request Nov 18, 2021
Merged
15 tasks
@batrick batrick force-pushed the i53293 branch 2 times, most recently from bccc5e6 to 2b67927 Compare November 18, 2021 15:19
batrick added a commit to batrick/ceph that referenced this pull request Nov 19, 2021
@batrick
Merge PR ceph#43974 into wip-pdonnell-testing-20211118.151903
8974755 
* refs/pull/43974/head:
	qa: use ceph-fuse to avoid new messages from kernel
	client: note major unmount log event early
@lxbsz lxbsz mentioned this pull request Nov 19, 2021
Merged
3 tasks
@lxbsz lxbsz requested review from lxbsz and vshankar November 19, 2021 02:05
lxbsz
lxbsz approved these changes Nov 19, 2021
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.

@batrick @vshankar @jtlayton

BTW, will it also make sense in MDS add one flag or a version flag about which metrics has been supported, then in kclient it will only send the metrics the MDS supported ?

@lxbsz lxbsz requested a review from jtlayton November 19, 2021 02:09
@jtlayton
Copy link
Contributor

jtlayton commented Nov 19, 2021 •
edited
Loading

Honestly, this all seems like a lot of workaround for something that's just a bug in the MDS. It should have never asserted on unknown messages. Why not just fix the MDS and move on? I know it's already fixed in later versions, so why are we testing something that we know is broken? Maybe it'd be easier to just avoid testing metrics with an MDS this old?

@batrick
Copy link
Member Author

batrick commented Nov 19, 2021

Honestly, this all seems like a lot of workaround for something that's just a bug in the MDS. It should have never asserted on unknown messages. Why not just fix the MDS and move on? I know it's already fixed in later versions, so why are we testing something that we know is broken? Maybe it'd be easier to just avoid testing metrics with an MDS this old?

Yes, that's the crux of the problem. The reason I'm testing v16.2.4 is that the upgrade procedure changed in v16.2.5 due to changes in the MDSMap. I need to verify v16.2.4 (and older) can still upgrade.

@batrick batrick force-pushed the i53293 branch 3 times, most recently from a7c061e to 7c1f0a7 Compare November 19, 2021 14:16
@batrick batrick changed the title qa: use ceph-fuse to avoid new messages from kernel qa: disable metrics on kernel client during upgrade Nov 19, 2021
@batrick
Copy link
Member Author

batrick commented Nov 19, 2021 •
edited
Loading

https://pulpito.ceph.com/pdonnell-2021-11-19_16:32:48-fs:upgrade-wip-pdonnell-testing-20211119.143151-distro-basic-smithi/

Just one transient cephadm failure.

@batrick
Copy link
Member Author

batrick commented Nov 19, 2021

jenkins test api

@batrick
qa: disable metrics on kernel client during upgrade
dcda5cb 
v16.2.4 MDS triggers an assert from these messages.

Also: add latest pacific for extra coverage.

Fixes: https://tracker.ceph.com/issues/53293
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
@batrick
Copy link
Member Author

batrick commented Nov 19, 2021

Made a small change by adding a comment: https://github.com/ceph/ceph/pull/43974/files#diff-2b38621f9773a577e0bb07983016de246cb8ec0c936b75e7135e9218632ac20dR6

this is ready to merge IMO when tests pass

@batrick batrick merged commit 860518b into ceph:master Nov 19, 2021
@batrick batrick deleted the i53293 branch November 19, 2021 23:44
@lxbsz lxbsz mentioned this pull request Mar 15, 2022
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lxbsz lxbsz lxbsz approved these changes

@vshankar vshankar Awaiting requested review from vshankar

+1 more reviewer

@jtlayton jtlayton jtlayton approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug-fix cephfs Ceph File System needs-review wip-pdonnell-testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@batrick @djgalloway @jtlayton @lxbsz