Fixes #30120 - Added check-update option in foreman-maintain

[patilsuraj767]

$ yum check-update
Loaded plugins: foreman-protector, product-id, search-disabled-repos, subscription-manager

WARNING: Excluding 12529 packages due to foreman-protector. 
Use foreman-maintain packages install/update <package> 
to safely install packages without restrictions.
Use foreman-maintain upgrade run for full upgrade.

$ ./bin/foreman-maintain packages check-update
Running Check for available package updates
================================================================================
Check for available package updates: 
Loaded plugins: product-id, search-disabled-repos, subscription-manager

microcode_ctl.x86_64                 2:2.1-61.6.el7_8         rhel-7-server-rpms
tomcat.noarch                        7.0.76-12.el7_8          rhel-7-server-rpms
tomcat-el-2.2-api.noarch             7.0.76-12.el7_8          rhel-7-server-rpms
tomcat-jsp-2.2-api.noarch            7.0.76-12.el7_8          rhel-7-server-rpms
tomcat-lib.noarch                    7.0.76-12.el7_8          rhel-7-server-rpms
tomcat-servlet-3.0-api.noarch        7.0.76-12.el7_8          rhel-7-server-rpms
                                                                      [OK]
--------------------------------------------------------------------------------

[theforeman-bot]

Issues: #30120

[kgaikwad]

Thank you @patilsuraj767
Looks good to me! Added small inline comment.

[kgaikwad]

Will this option be only for check_update?

[patilsuraj767]

No, this option will for all the yum related task.
With "--disableplugin=foreman-protector" in place there is no need to unlock package while performing yum related task.
We will need to revisit all the commands/scenarios where we are doing yum related task and will need to remove the step Procedures::Packages::UnlockVersions For example - While installing package - code.
I think we should do above refactoring in separate PR.

[kgaikwad]

I think we should do above refactoring in separate PR.

Could you please create one refactoring issue for tracking this change?

[patilsuraj767]

I have opened the issue for refactoring - #30393

[upadhyeammit]

It works as expected. I am not liking the idea of hardcoding --disableplugin=foreman-protector in yum_options. I still prefer the method to unlock packages and lock it again; as this feels like hack.
Maybe if we want to save the extra logic of disable and enable plugin which has written in setup_protector ? you can override yum_options in setup_protector itself ?
With this we should change the description of check-update command to,

check-update    Check for available package updates in unlocked session

[patilsuraj767]

I am not sure how we can override the yum_options in setup_protector, because currently yum_option variable is scoped for def yum_action.

Let me tell you why I think '--disableplugin=foreman-protector' is better option.

a) I did not found any scenario where we do yum action without unlocking packages(disabling foreman-protector) plugin.
b) In the current code, I think there are two scenarios where foreman-maintain can leave packages in the unlocked state.

1. If you see the implementation here -

   foreman_maintain/definitions/scenarios/packages.rb

   Line 67 in c8be149

   add_step_with_context(Procedures::Packages::UnlockVersions)

   (or any other scenario). So we are unlocking the packages but there can be the times that foreman-maintain fails or user can force terminate the process (ctrl +c). In this case, foreman-maintain will leave packages in unlocked state.
2. We are not explicitly locking the packages in any scenario, rather we are relying on foreman-installer, thinking that it will do the job of locking the packages but what if the installer fails in between? then the package will still stay in the unlocked state.

Using the --disableplugin=foreman-protector will make sure that the package will not be in unlock state ever.

[patilsuraj767]

Also, with this fix, we can get rid of this kind of issues - #30414.

[kgaikwad]

No additional comments from my side.
@upadhyeammit, do you have any additional comments?
@jameerpathan111, it would be good to have testing around this new command.

[kgaikwad]

As per offline discussion, I have tested by passing invalid plugin name.
Example - yum install mlocate --disableplugin=xyz
It gives output with a message No plugin match for: xyz. But command returns exit code based on package action,

[foreman_maintain]# yum install mlocate --disableplugin=xyz
Loaded plugins: product-id, search-disabled-repos, subscription-manager
No plugin match for: xyz
Package mlocate-0.26-8.el7.x86_64 already installed and latest version
Nothing to do
[foreman_maintain]# echo $?
0

[foreman_maintain]# yum install fhfh --disableplugin=xyz
Loaded plugins: product-id, search-disabled-repos, subscription-manager
No plugin match for: xyz
No package fhfh available.
Error: Nothing to do
[foreman_maintain]# echo $?
1

[upadhyeammit]

@jameerpathan111 or @vsedmik can you help to test this change ? Most important is, with this change packages wont be unlocked while doing packages actions from foreman-maintain.

[jameerpathan111]

@jameerpathan111 or @vsedmik can you help to test this change ?

yeah, sure. will take a look at it this week.

[upadhyeammit]

Tested on latest versions and works without any problem. Merging!