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.
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
quincy: mgr/prometheus: use vendored "packaging" instead #49698
Conversation
jenkins test make check |
b3876e4
to
53ba162
Compare
please use git cherry-pick -x to perform backport. |
e298339
to
09634e9
Compare
@@ -93,6 +93,7 @@ Build-Depends: automake, | |||
tox <pkg.ceph.check>, | |||
python3-coverage <pkg.ceph.check>, | |||
python3-dateutil <pkg.ceph.check>, | |||
python3-pkg-resources <pkg.ceph.check>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conflict (vs debian/ceph-mgr-modules-core.requires
file in main) needs to be documented in the commit message, in particular explaining the reason why this dependency is a pkg.ceph.check
dependency instead of regular one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you help me understand why is it a conflict?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it's not a straight cherry-pick, is it? The original commit in main adds a line to debian/ceph-mgr-modules-core.requires
file, here you are adding a different line to debian/control
file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, the original commit couldn't be cherry-picked as is since we introduced some changes in the organization of the debian
dir (ceph-mgr-modules-core.requires
doesn't exist yet).
All the packages listed in main's ceph-mgr-modules-core.requires
are marked as pkg.ceph.check
in P/Q debian/control
.
The changes here are the same as in Pacific's backport #49695.
I can add this explanation to the commit's message if this was your intent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add this explanation to the commit's message if this was your intent.
Yes -- this should be done as a rule, not just upon request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added, thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the packages listed in main's
ceph-mgr-modules-core.requires
are marked aspkg.ceph.check
in P/Qdebian/control
.
Unresolving as this statement appears to be false for 2 out of 6 packages:
88 python3-cherrypy3,
89 python3-natsort,
91 python3-pecan <pkg.ceph.check>,
95 python3-dateutil <pkg.ceph.check>,
98 python3-requests <pkg.ceph.check>,
103 python3-werkzeug <pkg.ceph.check>,
Am I looking in the wrong place?
Therefore, python3-pkg-resources is listed as <pkg.ceph.check> in debian/control
All of this begs the question: are these backports adding this line to the right section to begin with? I think we need to clarify whether python3-pkg-resources
package is:
- a general build dependency for Ceph
- a build dependency for Ceph that is needed only for
make check
- a runtime dependency for
ceph-mgr-modules-core
package - some combination of 1-3?
The changes here are the same as in Pacific's backport #49695.
After @yuriw picked up this PR into his integration branch, the previous
errors expectedly disappeared but the rerun is full of SELinux denials now (pasting just a couple):
@b-ranto Could you please take a look? |
Are we sure these denials are related to this PR? Looking at the denials a lot of them do not seem to be related to ceph at all (more to setroubleshoot and sssd). The one that actually seems to be related might be a known issue -- it complains about a log context mismatch. IIRC, we have been ignoring/filtering this one in teuthology as it was a test case issue where it stored the log in home dir. |
No, we aren't. Yesterday I suggested that @yuriw did a quincy baseline run with no PRs included. |
@idryomov It looks to me like the filters for denials are broken. IIRC, we used to filter out these denials in teuthology runs. |
To confirm, this run on quincy https://pulpito.ceph.com/gabrioux-2023-01-25_19:12:38-orch:cephadm-wip-guits-testing-2-2023-01-25-1520-pacific-distro-default-smithi/ did not have this PR and still saw a bunch of selinux denials. I feel like this PR is probably good and we can just tackle the denials separately. |
I tend to agree but @Matan-B needs to add an explanation for how the conflict was addressed to the commit message first. |
* Note: The cherry-pick is altered, the original commit couldn't be cherry-picked as is since we introduced some changes in the organization of the debian dir (ceph-mgr-modules-core.requires doesn't exist yet). All the packages listed in main's ceph-mgr-modules-core.requires are marked as pkg.ceph.check in P/Q debian/control. Therefore, python3-pkg-resources is listed as <pkg.ceph.check> in debian/control. instead of using the top-level "packaging" module, use the one vendored by setuptools. packaging python module provides versioning defined by PEP-440. but python3-packaging is provided by CentOS8 powertools repo, which is not enabled by default. and in CentOS9, this package is provided by AppStream instead of BaseOS. as prometheus mgr module is included by ceph-mgr-module-core, it would be desirable if our user can install ceph-mgr-module-core without enabling powertools or AppStream repo on a CentOS or its derivative distros. fortunately, setuptools vendors packaging module. and both CentOS8 and CentOS9 provide python3-setuptools in their BaseOS repos. in this change, instead of using "packging" module, we use the venderored one, which is in turn embedded in pkg_resources. this python module is provided by python3-setuptools on CentOS distros, and python3-pkg-resources on Debian and its derivatives the packaging recipes are updated accordingly to reflect the new runtime dependency. Signed-off-by: Kefu Chai <tchaikov@gmail.com> (cherry picked from commit cf60892) Signed-off-by: Matan Breizman <mbreizma@redhat.com>
09634e9
to
853b6d7
Compare
Explanation is added. |
Now tracked here: https://tracker.ceph.com/issues/58610 |
https://pulpito.ceph.com/?branch=wip-yuri7-testing-2023-01-30-1510-quincy No failures related to this PR. Merging |
@ljflores I'd like to highlight the concern that both pacific and quincy backports here may be incorrect that seems to have fallen through the cracks: #49698 (comment). In particular, if |
@idryomov thanks for bringing this to my attention, you're right that the comment was lost. I approved this PR initially to unblock failures in the rados suite. This patch has fixed those failures, so it was good in my eyes. However, @tchaikov and/or @Matan-B, would either of you be able to address Ilya's concern about |
@adk3798 as well? ^ |
Backport of: #49712
Fixes: https://tracker.ceph.com/issues/58416
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. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
Checklist
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