ncm-spma: yum: Expose support for multiple reposdirs #1236
Conversation
jrha
changed the title
|
Locally I'm seeing the |
jrha
force-pushed
the
reposdir
branch
2 times, most recently
from
142edd1
to
e172d85
Compare
|
Found and fixed the source of the extra blank lines, also the tests which were actually checking for this strange behaviour. |
| @@ -1015,7 +1015,7 @@ sub configure_yum | |||
| $fh->add_or_replace_lines( | |||
| $name, | |||
| $name. q{\s*=\s*}.$valuereg.'$', | |||
| "\n$name=$value\n", | |||
| "$name=$value\n", | |||
There was a problem hiding this comment.
what if the file doesn't end with a newline? i'm not sure FileEditor tests this...
There was a problem hiding this comment.
seems it does by default: https://github.com/quattor/CAF/blob/master/src/main/perl/FileEditor.pm#L299 and https://github.com/quattor/CAF/blob/master/src/main/perl/FileEditor.pm#L349
can you modify the unittest below to check this?
There was a problem hiding this comment.
FileEditor should deal with that, not the components.
There was a problem hiding this comment.
The match should still work though...
There was a problem hiding this comment.
Done, the noaction test now starts with a file that has a missing newline at EOF.
|
@stdweird I've been trying to write an end-to-end test case that uses the new property in a profile and then checks that it exists in |
|
no end to end tests. but |
|
Also: any opinions on the name of the schema option? |
Start with file with missing newline at EOF.
| @@ -37,6 +37,7 @@ type component_spma_yum = { | |||
| "run" ? legacy_binary_affirmation_string # Run the SPMA after configuring it | |||
| "userpkgs_retry" : boolean = true | |||
| "userpkgs" ? legacy_binary_affirmation_string # Allow user packages | |||
| "extra_reposdirs" ? absolute_file_path[] # List of additional repo dirs to be included | |||
There was a problem hiding this comment.
add comment as annotation. also add that the priority: indentical repos in the extra dir precede (or not?) the quattor generated ones.
There was a problem hiding this comment.
wrt name: just reposdirs? or userreposdirs?
There was a problem hiding this comment.
Just reposdirs seems fine, I'll make sure the annotation explains what happens with regards to the managed repodir. As far as precendence... I might have to experiment to see how yum behaves.
There was a problem hiding this comment.
Given the yum.conf option is reposdir I think reposdirs makes sense. The only downside is this is additional config to be added to reposdir, there is no way to remove the default? OTOH that seems like such a niche use case that it's OK?
|
I've been playing around on a host with multiple All the repos are read and in the case of duplicate definitions, yum issues a warning and one of them (I haven't been able to predict which one) wins. |
jrha
force-pushed
the
reposdir
branch
3 times, most recently
from
1149969
to
d6d2733
Compare
|
@jrha We should be able to detect duplicates in PAN and error out before it's deployed? For the priorities I think you need yum-plugin-priorities. So the schema should allow the setting of priorities - does that mean the list needs to become a dict? |
|
@ned21 priorities are already supported in the individual repositories and work fine as is (we make heavy use of them). Unfortunately duplicates between managed and unmanaged repositories will be impossible to catch at build time, but that's the price you pay if you are want this functionality. |
|
This has been well tested at RAL and seems to be working well for us. Could someone re-review it? |
| @@ -37,6 +37,8 @@ type component_spma_yum = { | |||
| "run" ? legacy_binary_affirmation_string # Run the SPMA after configuring it | |||
| "userpkgs_retry" : boolean = true | |||
| "userpkgs" ? legacy_binary_affirmation_string # Allow user packages | |||
| @{ List of additional repo dirs to be included } | |||
There was a problem hiding this comment.
make it more clear that this are additional repository directories that are not managed by ncm-spma. maybe replacing additional with external?
ncm-spma/src/main/perl/spma/yum.pm
Outdated
| @@ -1168,6 +1168,9 @@ sub Configure | |||
| # TODO: for userpkgs, insert quattor unmanaged dir as first (so it takes priority) | |||
| # TODO: check that the first one wins | |||
| my $reposdir = [$quattor_managed_reposdir]; | |||
| if ($t->{reposdirs}) { | |||
| $reposdir = [@$reposdir, @{$t->{reposdirs}}]; | |||
There was a problem hiding this comment.
push(@$reposdir, @{$t->{reposdirs}})
|
@jrha tiny remarks |
|
@stdweird comments addressed. |
The underlying module code supports multiple reposdirs so expose this through the schema.
Potentially replaces #1075.