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.

Sign up for GitHub

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

Jump to bottom

qa/workunits: Remove redundant Xenial cmake3 requirements #35082

Merged
merged 2 commits into from May 22, 2020
Merged

qa/workunits: Remove redundant Xenial cmake3 requirements #35082

merged 2 commits into from May 22, 2020

Conversation

badone
Copy link
Contributor

@badone badone commented May 15, 2020 •
edited

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

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 dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@badone badone requested a review from tchaikov May 15, 2020 06:49
@badone
qa/workunits/rados/test_envlibrados_rocksdb: Remove Xenial cmake3 req…
9d2632a 
…uirements

It's no longer necessary to handle Xenial as a special case.

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

Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
@badone
qa/workunits/ceph-helpers-root: Remove unused function
88094c2 
Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
@badone badone force-pushed the wip-xenial-cmake-install-test_envlibrados_for_rocksdb branch from 64d5f32 to 88094c2 Compare May 15, 2020 06:50
@badone badone changed the title Wip xenial cmake install test envlibrados for rocksdb Remove redundant Xenial cmake3 requirements May 15, 2020
@tchaikov
Copy link
Contributor

@badone could you fill me with more context? we don't build nautilus on xenial anymore?

@badone
Copy link
Contributor Author

badone commented May 15, 2020

@badone could you fill me with more context? we don't build nautilus on xenial anymore?

The test still runs on Xenial. Is Nautilus supported on Xenial? If not I guess we could remove Xenail from this test instead but the code's redundant anyway right?

@tchaikov
Copy link
Contributor

tchaikov commented May 15, 2020 •
edited

@badone could you fill me with more context? we don't build nautilus on xenial anymore?

The test still runs on Xenial. Is Nautilus supported on Xenial? If not I guess we could remove Xenail from this test instead

i am not sure. you put "Fixes: https://tracker.ceph.com/issues/45561" in the commit message. and add "bug fix" label to this PR instead of "cleanup", that's why i am asking.

but the code's redundant anyway right?

that's why i approved this PR.

@badone
Copy link
Contributor Author

badone commented May 18, 2020

This forms part of the resolution for https://tracker.ceph.com/issues/44981

@tchaikov tchaikov changed the title Remove redundant Xenial cmake3 requirements qa/workunits: Remove redundant Xenial cmake3 requirements May 21, 2020
@tchaikov
Copy link
Contributor

http://pulpito.ceph.com/kchai-2020-05-21_10:34:02-rados-wip-kefu-testing-2020-05-21-1652-distro-basic-smithi/

the failures are either tracked or fixed by

@tchaikov tchaikov merged commit c927371 into ceph:master May 22, 2020
@tchaikov
Copy link
Contributor

i am leaving https://tracker.ceph.com/issues/45561 open. as i am not sure if nautilus on xenial is still being built or tested, if you could paste any link / document as a reference, that'd be great.

@badone badone deleted the wip-xenial-cmake-install-test_envlibrados_for_rocksdb branch May 22, 2020 22:21
@badone
Copy link
Contributor Author

badone commented May 22, 2020

https://shaman.ceph.com/builds/ceph/wip-nautilus-badone-testing/ built xenial 7 days ago.

@badone
Copy link
Contributor Author

badone commented May 22, 2020 •
edited

i am leaving https://tracker.ceph.com/issues/45561 open. as i am not sure if nautilus on xenial is still being built or tested, if you could paste any link / document as a reference, that'd be great.

@tchaikov https://shaman.ceph.com/builds/ceph/nautilus/fd80879112559f6509c42370d6cb41114d5e54d0/ is building on Xenial today.

(nautilus)$ find . -name ubuntu_16.04.yaml
./qa/distros/all/ubuntu_16.04.yaml
./qa/distros/supported-all-distro/ubuntu_16.04.yaml
./qa/distros/supported-random-distro$/ubuntu_16.04.yaml
./qa/suites/rados/thrash-old-clients/distro$/ubuntu_16.04.yaml
./qa/suites/upgrade/client-upgrade-nautilus/nautilus-client-x/basic/supported/ubuntu_16.04.yaml
./qa/suites/upgrade/client-upgrade-nautilus/nautilus-client-x/rbd/supported/ubuntu_16.04.yaml

@badone
Copy link
Contributor Author

badone commented May 22, 2020

http://pulpito.front.sepia.ceph.com/bhubbard-2020-05-22_22:46:42-rados-nautilus-distro-basic-smithi/

@badone
Copy link
Contributor Author

badone commented May 22, 2020

http://qa-proxy.ceph.com/teuthology/bhubbard-2020-05-22_22:46:42-rados-nautilus-distro-basic-smithi/5083578/teuthology.log <--- This is the specific failure in Nautilus HEAD this PR is designed to resolve.

@tchaikov
Copy link
Contributor

It's no longer necessary to handle Xenial as a special case.

so we still need to handle xenial, right?

@badone
Copy link
Contributor Author

badone commented May 23, 2020

It's no longer necessary to handle Xenial as a special case.

so we still need to handle xenial, right?

Right, but not as a special case.

@tchaikov
Copy link
Contributor

the commit message reads:

It's no longer necessary to handle Xenial as a special case.

and i did read it. you reply repeats it in a slightly different way.

the reason why we install cmake3 on xenial was we used cmake3 for compiling nautilus on xenial. and because we claimed to drop the xenial by then but was still using it. so i have to package cmake3 for xenial then.

i think it's a cleanup but not a fix, because we don't built the master branch on xenial anymore. please note i compiled cmake3 using the a recent toolchain offered by PPA, that's why "libstdc++6 (>= 6)" is required.

so far, i cannot find enough information connecting the issue with the fix.

@badone badone added cleanup and removed bug-fix labels May 24, 2020
@badone
Copy link
Contributor Author

badone commented May 24, 2020

I've removed 'bug fix' and added 'cleanup'. I think that's what you want? My argument is that it may be a clean up on master but I believe it's a bug fix on nautilus. My objective is to get the nautilus test to pass. This seemed like a reasonable way to achieve that. Please make or suggest any adjustments you deem necessary for this discussion to reach an end.

@tchaikov
Copy link
Contributor

this does not make difference anymore, as this PR has been merged. Brad, i was reviewing this change in hope to help you. what i can now, is to make sure the tracker issue is not closed because of a "maybe" fix. if i have enough bandwidth, i would make or suggest.

@badone
Copy link
Contributor Author

badone commented May 24, 2020

this does not make difference anymore, as this PR has been merged. Brad, i was reviewing this change in hope to help you. what i can now, is to make sure the tracker issue is not closed because of a "maybe" fix. if i have enough bandwidth, i would make or suggest.

I appreciate your help but am just not clear on why you consider it to be a "maybe" fix. Perhaps you could elaborate on what is required to convince you it is an actual fix?

@tchaikov
Copy link
Contributor

i tried to put my understanding of the issue in #35082 (comment) . if you think otherwise, i will give up on understanding the "why". if you could paste a link to pulpito of the passed fix on nautilus with backport of this change, that'd suffice.

@badone
Copy link
Contributor Author

badone commented May 24, 2020

https://paste.centos.org/view/d989151f

http://pulpito.front.sepia.ceph.com/bhubbard-2020-05-24_06:06:48-rados-wip-nautilus-badone-testing-distro-basic-smithi/

This was referenced May 25, 2020
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tchaikov tchaikov tchaikov approved these changes

Assignees
No one assigned
Labels
cleanup core needs-qa tests wip-kefu-testing
Projects
None yet
Milestone
No milestone
2 participants
@badone @tchaikov