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

Fix Windows build #45272

Merged
merged 2 commits into from Mar 9, 2022
Merged

Fix Windows build #45272

merged 2 commits into from Mar 9, 2022

Conversation

petrutlucian94
Copy link
Contributor

@petrutlucian94 petrutlucian94 commented Mar 7, 2022 •
edited

Fix Windows build

The Windows build is currently failing because of the following issues:

  • missing "dlfcn.h" - we'll define the file and include Ceph's "dlfcn_compat.h"
  • libceph-common linking issues - we'll temporarily revert this [1], at least for Windows builds

[1] 22fefb2

Fixes: https://tracker.ceph.com/issues/54453
Signed-off-by: Lucian Petrut lpetrut@cloudbasesolutions.com

@petrutlucian94
include: Define dlfcn.h on Windows
8b7432b 
"dlfcn.h" is not available on Windows, so Ceph provides a drop-in
replacement through "dlfcn_compat.h".

The issue is that directly importing "dlfcn.h" fails at the moment,
for which reason we'll simply add a file called "dlfcn.h" that
includes "dlfcn_compat.h".

Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
@petrutlucian94
build: avoid Windows linking issues
c11455f 
A recent commit [1] changed the way in which libceph-common is built.
Previously, it was always built statically but now it uses the
"CEPH_SHARED" flag.

Static Windows builds are now failing. For now, we'll just revert the
previous behavior and always build libceph-common dinamically on
Windows.

[1] 22fefb2

Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
@ionutbalutoiu
Copy link
Contributor

/cc @djgalloway to raise awareness about the recent Windows build failures in the CI.

Could you please help us ping the appropriate people to have this reviewed and merged ?

@petrutlucian94
Copy link
Contributor Author

jenkins test docs

@ljflores
Copy link
Contributor

ljflores commented Mar 7, 2022

Hello @petrutlucian94, is this the failure that this PR is fixing? https://tracker.ceph.com/issues/54453 If so, can you link the tracker in your commit?

@petrutlucian94
Copy link
Contributor Author

Hello @petrutlucian94, is this the failure that this PR is fixing? https://tracker.ceph.com/issues/54453 If so, can you link the tracker in your commit?

Indeed, thanks for mentioning the tracker issue.

@ljflores ljflores requested a review from tchaikov March 7, 2022 16:06
@djgalloway
Copy link

@ljflores Forgive my ignorance but how can one fulfill the needs-qa tag? teuthology run? Any specific suite?

@ljflores
Copy link
Contributor

ljflores commented Mar 8, 2022 •
edited

@ljflores Forgive my ignorance but how can one fulfill the needs-qa tag? teuthology run? Any specific suite?

@djgalloway thanks for asking. It's tricky because there's not a specific suite that covers this, but I see that the commit this PR reverts went through testing in the rados suite: #44321 (comment). I think it would be wise to test this one in the rados suite as well to ensure the same results.

@djgalloway
Copy link

Okay. I'm building packages now. https://shaman.ceph.com/builds/ceph/wip-45272/

@djgalloway
Copy link

Only one job failure http://qa-proxy.ceph.com/teuthology/dgalloway-2022-03-09_02:34:58-rados-wip-45272-distro-basic-smithi/6727560/teuthology.log

Could it be related to this change?

@petrutlucian94
Copy link
Contributor Author

Only one job failure http://qa-proxy.ceph.com/teuthology/dgalloway-2022-03-09_02:34:58-rados-wip-45272-distro-basic-smithi/6727560/teuthology.log

Could it be related to this change?

One of the OSDs is down but it seems unrelated, in fact this change shouldn't affect Linux builds at all.

2022-03-09T03:36:25.794 DEBUG:teuthology.misc:2 of 3 OSDs are up
2022-03-09T03:36:25.794 ERROR:teuthology.contextutil:Saw exception from nested tasks
Traceback (most recent call last):
  File "/home/teuthworker/src/git.ceph.com_git_teuthology_eeaeee38264fad8a01fd1ce912d8908676ed64bc/teuthology/contextutil.py", line 31, in nested
    vars.append(enter())
  File "/usr/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "/home/teuthworker/src/github.com_ceph_ceph_e338e1f7d2e42256305bc55fe03d20b6f3b402e5/qa/tasks/ceph.py", line 403, in create_rbd_pool
    ceph_cluster=cluster_name,
  File "/home/teuthworker/src/git.ceph.com_git_teuthology_eeaeee38264fad8a01fd1ce912d8908676ed64bc/teuthology/misc.py", line 858, in wait_until_osds_up
    while proceed():
  File "/home/teuthworker/src/git.ceph.com_git_teuthology_eeaeee38264fad8a01fd1ce912d8908676ed64bc/teuthology/contextutil.py", line 133, in __call__
    raise MaxWhileTries(error_msg)
teuthology.exceptions.MaxWhileTries: reached maximum tries (90) after waiting for 540 seconds

@ljflores
Copy link
Contributor

ljflores commented Mar 9, 2022

Only one job failure http://qa-proxy.ceph.com/teuthology/dgalloway-2022-03-09_02:34:58-rados-wip-45272-distro-basic-smithi/6727560/teuthology.log

Could it be related to this change?

I'll take a look

@idryomov
Copy link
Contributor

idryomov commented Mar 9, 2022

Yeah, definitely not related.

@djgalloway djgalloway merged commit 6917edc into ceph:master Mar 9, 2022
@ljflores
Copy link
Contributor

ljflores commented Mar 9, 2022

@djgalloway I see this was already merged, I agree it's not related. Here is a summary of the other tracked failures:

https://tracker.ceph.com/issues/45423
https://tracker.ceph.com/issues/53680

@djgalloway
Copy link

@djgalloway I see this was already merged, I agree it's not related. Here is a summary of the other tracked failures:

https://tracker.ceph.com/issues/45423 https://tracker.ceph.com/issues/53680

You rock. Thank you.

@kamoltat
Copy link
Member

Hi,
not sure if anyone notices, but quincy backport PRs are being blocked by the ceph windows test getting stuck https://tracker.ceph.com/issues/54552

I wondered if this PR should be backported to quincy to possibly solve the issue

@idryomov
Copy link
Contributor

Definitely! The check was made required for both master and quincy. I'll do it.

@kamoltat
Copy link
Member

Definitely! The check was made required for both master and quincy. I'll do it.

cheers! @idryomov

@idryomov idryomov mentioned this pull request Mar 14, 2022
Merged
@petrutlucian94 petrutlucian94 mentioned this pull request Oct 13, 2022
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@idryomov idryomov idryomov approved these changes

@ljflores ljflores ljflores approved these changes

@djgalloway djgalloway Awaiting requested review from djgalloway

@tchaikov tchaikov Awaiting requested review from tchaikov

Assignees
No one assigned
Labels
build/ops needs-qa
Projects
None yet
Milestone
No milestone
6 participants
@petrutlucian94 @ionutbalutoiu @ljflores @djgalloway @idryomov @kamoltat