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
Fixes #27686, #27730, #27731, #27732 - Version locking NG #278
Conversation
1 similar comment
9d4bf3b
to
36b12e2
Compare
@kgaikwad, @jameerpathan111, @upadhyeammit, could please take a look? |
|
|
+1, If we don't support it it then it would be better if we don't even lock packages for that Satellite version. |
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.
@mbacovsky,
Sorry for the delay. Added just a small inline comments.
Apart from comments by @upadhyeammit and @jameerpathan111, no functionality wise comments from my side.
Thanks for the feedback! All other comments should be addressed. Please give it another round. |
@kgaikwad thanks for review. Updated, please re-check. |
@upadhyeammit, @jameerpathan111 re 5/ I found out this is actually regression caused by fixing https://bugzilla.redhat.com/show_bug.cgi?id=1710305. I'm going to revert that fix and find better solution. |
@mbacovsky thanks for quickly working on issues. :)
File packages_locking_sat63_testing.txt contains the steps I did to reproduce this issue.
Ohh so that's how it was supposed to be used. It's ok then, we can keep it as it is.
Yeah, it would be great if we solve this.
Yes, it makes sense to not skip installer.
|
Unfortunately it didn't help me to reproduce. Could you sent me content of the
Changed to
Fixed.
The plugin reads the list from
Correct, is it okay or any changes required? |
Ok, I will send it.
yes, it's better now.
yeah, it seems to be fixed now.
Thanks for clarifying, it sure does work when I list package in whitelist file.
It's good, now we have warning message in it. |
Hello, Regarding
Before actually running the yum install or update command we can try querying if repository has the package for install or update,
For unavailable package I can see return code is 1,
I can see its working with wildcard, as it is common use case with yum,
Same goes with updates,
If I give another thought to this then having package available for install or update does not mean that installation or update will be successful, and if we really want to be sure if package got installed or updated then we can make use of yum history, but this will run after 'yum install/update package-name',
I can see that yum only adds transaction detail in history if transaction was successful, so maybe we can record the recent transaction number for package(s) and then check if it has been increased; this is simple one. Else we can even pull full transaction history of package and decide to run or not run installer again,
|
@mbacovsky @upadhyeammit
|
|
@mbacovsky Where can I see how many packages are excluded?
Only when user have enabled |
@upadhyeammit thanks for your input on yum operations. I'll take a look if it is something we can fit in. I need to test how it works if you install/update multiple packages at once. If we would need to test packages one by one it may have terrible performance. Also there is a complication with using patterns like 'install rubygem-*'. Perhaps we could leave it for separate PR. @jameerpathan111, thanks for extensive testing! It is much appreciated.
One last thing is the confirmation issue. Now with this patch we have back the double confirm message. I finally have patch and will include it in this PR as separate commit. Will update soon. |
yeah, it sure is risky to allow installing all dependencies.
+1
Martin I haven't seen this message on my Satellite so far. I had packages locked and ran
Yeah, it'll be good to have such message.
Awsome |
Updated.
TODO: message about unsupported locking on capsule. I included fix for RM#27072 for easier testing, it should go out in 0.4.7 to avoid the new regression introduced by 0.4.6. @kgaikwad, @upadhyeammit could you please take a look? |
Yes, I can see yum plugin message on Capsule but not on Satellite. |
Now the issue is fixed, this steps output is changed a little and it doesn't ask for confirmation twice.
|
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.
Checked these changes couple of times and I dont see any modifications. Its handled in better way than before.
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.
👍 Big thank you @mbacovsky. Almost all review comments are addressed 🎉
Different scenarios tested by @jameerpathan111 already.
No any additional comment from my side.
Thanks everybody involved! Let's merge this PR :-)
Original locking of foreman-related packages was replaced with new approach that locks all packages except some select exceptions. For better performance and usability we are introducing new yum plugin the will be used instead of the yum versionlock plugin. - the new yum plugin lives in extras/foreman_protector - the plugin excludes everything except for pkgs in whitelist - the plugin prints how many packages are excluded - the plugin prints hint about using f-m packages install/update - 'f-m packages install/update' install/update packages it unlocks them first, run the yum and run the installer --upgrade which locks packages again. User is informed and asked for confirmation - f-m packages status was extended to print if the locking is enabled If not, warning is printed. - packages commands can install and setup the new plugin
5e6ccc6
to
e730c52
Compare
Thank you all for your help! |
Thank you @mbacovsky! |
@mbacovsky is it possible to handle following scenario as well? :(
|
@jameerpathan111 that may be difficult to implement as it seems the local installs are handled differently by yum and our hook is ignored. Could you please create BZ to track this? We may give it a try later. |
Original locking of foreman-related packages was replaced
with new approach that locks all packages except some select
exceptions. For better performance and usability we are introducing
new yum plugin the will be used instead of the yum versionlock plugin.
it unlocks them first, run the yum and run the installer --upgrade
which locks packages again. User is informed and asked for confirmation
If not, warning is printed.