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.

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

RPM: remove user_namespace for c9s in qm.spec #93

Merged
merged 1 commit into from
Jun 22, 2023

Conversation

lsm5
Copy link
Member

@lsm5 lsm5 commented Jun 15, 2023

Copr builds occur in a mock chroot on a Fedora host. So Makefile checks
for /sys/fs/selinux/class/user_namespace may suffice for local builds on
c9s, but won't work alone for Copr builds.

This patch will remove user_namespace from qm.if for all c9s/rhel
environments.

@lsm5
Copy link
Member Author

lsm5 commented Jun 15, 2023

@dougsland builds passed here. PTAL

@lsm5 lsm5 changed the title test RPM: remove user_namespace for c9s in qm.spec Jun 15, 2023
Copr builds occur in a mock chroot on a Fedora host. So Makefile checks
for /sys/fs/selinux/class/user_namespace may suffice for local builds on
c9s, but won't work alone for Copr builds.

This patch will remove `user_namespace` from qm.if for all c9s/rhel
environments.

Signed-off-by: Lokesh Mandvekar <[email protected]>
@lsm5 lsm5 mentioned this pull request Jun 15, 2023
Closed
@lsm5
Copy link
Member Author

lsm5 commented Jun 15, 2023

we could either do it this way or perhaps check in the Makefile for /etc/redhat-release as well

@lsm5 lsm5 marked this pull request as ready for review June 15, 2023 16:40
@lsm5 lsm5 requested review from rhatdan and dougsland as code owners June 15, 2023 16:40
@lsm5
Copy link
Member Author

lsm5 commented Jun 15, 2023

@Yarboa @rhatdan PTAL

@lsm5
Copy link
Member Author

lsm5 commented Jun 16, 2023

@dougsland @Yarboa feel free to merge this to get things unblocked. Only affects rpms on c9s (if at all). @rhatdan might be occupied with devconf.cz .

@dougsland
Copy link
Collaborator

@lsm5 could you please confirm if this helps keep the logic in the makefile:

 @if grep -q "CentOS Stream 9" /etc/os-release; then \
                echo "Host is CentOS Stream 9"; \
        else \
                echo "Host is not CentOS Stream 9"; \
        fi

@dougsland
Copy link
Collaborator

@lsm5 could you please confirm if this helps keep the logic in the makefile:

 @if grep -q "CentOS Stream 9" /etc/os-release; then \
                echo "Host is CentOS Stream 9"; \
        else \
                echo "Host is not CentOS Stream 9"; \
        fi

Of course, using TAB as space. It's bad indentation here.

@lsm5
Copy link
Member Author

lsm5 commented Jun 16, 2023

@lsm5 could you please confirm if this helps keep the logic in the makefile:

 @if grep -q "CentOS Stream 9" /etc/os-release; then \
                echo "Host is CentOS Stream 9"; \
        else \
                echo "Host is not CentOS Stream 9"; \
        fi

Of course, using TAB as space. It's bad indentation here.

RE: Makefile, do we see this condition hitting environments other than CI / copr / koji environments? I got no strong opinion on this btw.

grepping for that string works, but may want to grep for Red Hat Enterprise Linux 9 if you want it to be more generic.

@dougsland
Copy link
Collaborator

@lsm5 could you please confirm if this helps keep the logic in the makefile:

 @if grep -q "CentOS Stream 9" /etc/os-release; then \
                echo "Host is CentOS Stream 9"; \
        else \
                echo "Host is not CentOS Stream 9"; \
        fi

Of course, using TAB as space. It's bad indentation here.

RE: Makefile, do we see this condition hitting environments other than CI / copr / koji environments? I got no strong opinion on this btw.

grepping for that string works, but may want to grep for Red Hat Enterprise Linux 9 if you want it to be more generic.

sure

@dougsland
Copy link
Collaborator

@lsm5 @Yarboa @rhatdan finally made via Makefile too: #94
I am okay to merge both, have the spec don't hurt but should not be required.

@rhatdan
Copy link
Member

rhatdan commented Jun 22, 2023

LGTM

@rhatdan rhatdan merged commit f0b2123 into containers:main Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants