(PUP-2916) yumrepo fails to honour custom reposdir #3227
Conversation
|
Waiting for CLA signature by @dnlsng @dnlsng - We require a Contributor License Agreement (CLA) for people who contribute to Puppet, but we have an easy click-through license with instructions, which is available at https://cla.puppetlabs.com/ Note: if your contribution is trivial and you think it may be exempt from the CLA, please post a short reply to this comment with details. http://docs.puppetlabs.com/community/trivial_patch_exemption.html |
|
CLA signed by all contributors. |
dnlsng
changed the title
| Dir.glob("#{dir}/*.repo").each do |file| | ||
| files << file | ||
| end | ||
| Dir.glob("#{reposdir.last}/*.repo").each do |file| |
There was a problem hiding this comment.
A comment is needed here that explains the behavior - the code raises the question why "grabbing the resposdir.last" is the correct thing to do.
|
Triage Notes:
|
|
Opened PUP-3667. Not sure if this will help the situation with this particular issue, but it will make the tests more worthwhile. |
|
@dnlsng we had some concerns about why |
|
The handling of From the I suspect the desired feature here is to be able to specify the directory in which the yumrepo resource is flushed? In that scenario, conflicting repo declarations that are active |
|
Note that repos declared directly in |
|
Apologies for not getting back to these comments sooner as I haven't had a chance to look at this again until now. I was not aware that the reposdir setting could be a list of directories and it looks like that wouldn't even work with the current implementation. Currently, self.reposdir returns an array with the value of reposdir (if it exists) and the default repo directory (/etc/yum.repos.d or /etc/yum/repos.d). In reality, the directory set in yum.conf for reposdir should replace the default reposdir. This patch just got around that by picking the last entry in the array, but I see that it is probably better to fix self.reposdir instead of changing self.repofiles. I will have a look at this now and will update the PR. |
|
PR triage notes: waiting for update from contributor for feedback made in previous triage. @dnlsng Thanks again for your contribution. Have there been any updates to this PR? |
|
PR triage notes: no changes noted. |
|
@dnlsng Thank you for your contribution. I was wondering if you've had a change this review and update this PR? |
dnlsng
force-pushed
the
bug/stable/PUP-2916-fix-custom-reposdir
branch
from
1255f83
to
59724c3
Compare
|
I finally had some time today to get around and updated this PR. Rather than appending the reposdir directories to the default directories, it should replace them. |
|
CLA signed by all contributors. |
|
Hi @dnlsng - it looks like there are some unrelated commits in this PR that need to be rebased? |
|
In particular, you seem to have based your branch off stable and be targetting it against master. |
dnlsng
force-pushed
the
bug/stable/PUP-2916-fix-custom-reposdir
branch
from
59724c3
to
622bf0d
Compare
Previously, find_conf_value was always returning nil, therefore the custom reposdir setting was never appended onto the reposdir array.
…d of appending When a custom reposdir is present in the yum.conf file, yum will only read files from that directory/directories (plus yum.conf). Previously, the yumrepo provider was reading all files from all directories in the reposdir array including the default directory, even though they would have been ignored by yum. This was an issue if the same repo existed in both the default directory and the custom directory. This change just replaces the default directories instead of appending to them if reposdir is set.
dnlsng
force-pushed
the
bug/stable/PUP-2916-fix-custom-reposdir
branch
from
622bf0d
to
c461f98
Compare
|
Oops, how did I not notice that? I actually wanted to target stable as I would prefer that this fix be in place in 3.x but no matter, I have rebased it to master now. |
|
Alright, I can give this my 👍 |
|
This is working well for us. |
…eposdir (PUP-2916) yumrepo fails to honour custom reposdir
|
@dnlsng Since this bug was reported as a regression, it should go into the stable branch as well. I'll create a separate PR for that. |
|
PR-3585 has been created to get this fix into stable. |
The yum.conf file wasn't being parsed for the reposdir which was causing the yumrepo provider to ignore the setting and just place the repo files into the default directory. Yum is then not seeing these repos as it has been configured to read files from the custom directory.
Also, self.repofiles was enumerating files from all directories that exist, i.e. the defaults directory and the custom directory. This created issues for puppet when the same repo existed in both directories. In reality, yum only parses the files in the custom directory if specified and ignores any files in the default directory.