common: provide empty mounts.conf for non-RHEL systems#842
Conversation
|
LGTM |
|
@lsm5 PTAL |
| %if %{defined rhel} | ||
| install -m0644 common/contrib/redhat/rhel-mounts.conf %{buildroot}%{_datadir}/containers/mounts.conf | ||
| %else | ||
| install -m0644 common/pkg/subscriptions/mounts.conf %{buildroot}%{_datadir}/containers/mounts.conf |
There was a problem hiding this comment.
that is not right I think, the path should be shipped on all our distros, fedora, centos, etc...
IIRC correctly this is used to leak the subscription manager secrets/repos into the RHEL containers so if the fedora host has subscription manager and you build a RHEL image it should work
There was a problem hiding this comment.
Got it, is there an exhaustive list or a particular macro I should use?
There was a problem hiding this comment.
I dunno, are you using this spec file on suse? Normally I would have said do not use any condition.
I don't think so far anyone has considered this spec to get build outside of fedora/centos/rhel
There was a problem hiding this comment.
I dunno, are you using this spec file on suse?
Not really, and I'm also unaware of anyone else using or not using this spec file as is in other distros. If this is exclusively RHEL, I could simply drop the vanilla mounts.conf entry from this spec file, that would, perhaps, be the cleanest.
But if someone else is using this downstream, they run into the same problem. Since we're not breaking backwards compat with this change, I guess we should be fine.
There was a problem hiding this comment.
But if someone else is using this downstream, they run into the same problem. Since we're not breaking backwards compat with this change, I guess we should be fine.
Well I would argue downstreams expect that, if they base of fedora they want the fedora special cases as well. Anyhow we can wait for @lsm5 to confirm.
There was a problem hiding this comment.
@danishprakash At the very least, we want it shipped on all of Fedora, RHEL and CentOS Stream. So, ideally the existing install line should switch the source file path to contrib/redhat .. .
If we are absolutely certain that other rpm distros do NOT want any subscription-manager support or anything else that depends on RHEL secrets, then your conditional should be changed to: %if %{defined fedora} || %{defined rhel} . That should work on all fedora, rhel and centos.
There was a problem hiding this comment.
If we are absolutely certain that other rpm distros do NOT want any subscription-manager support or anything else that depends on RHEL secrets
I don't think I can answer that for any distro apart from (open)SUSE; we don't use that.
That being said, I agree with the idea of keeping this spec clean and tailored towards RHEL, Fedora, and CentOS as it has been so far. Distros not wanting to depend on RHEL secrets can now use the vanilla mounts.conf.
Provide an empty mounts.conf for non-RHEL systems and an additional rhel-mounts.conf for RHEL systems. This makes packaging easier for distros where RHEL-specific mount entries don't apply. It should be noted that the spec file in this repo is tailored towards RHEL, Fedora and CentOS and unconditionally applies the RHEL-specific mounts.conf. Fixes: containers#807 Signed-off-by: Danish Prakash <contact@danishpraka.sh>
65a4557 to
116ca44
Compare
Provide an empty mounts.conf for non-RHEL systems and an additional rhel-mounts.conf for RHEL systems. This makes packaging easier for distros where RHEL-specific mount entries don't apply.
Fixes: #807