[PUP-958] PR (2137): (#23316) Updated yum package provider to support :holdable feature by - fatmcgav Created: 2013/12/09  Updated: 2019/04/04  Resolved: 2014/01/28

Status: Closed
Project: Puppet
Component/s: Types and Providers
Affects Version/s: None
Fix Version/s: PUP 3.5.0

Type: Task Priority: Normal
Reporter: gepetto-bot Assignee: Unassigned
Resolution: Incomplete Votes: 0
Labels: github, yum
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to PUP-1523 Add support for 'held' ensure value t... Closed
Template:

 Description   

(#23316) Updated yum package provider to support :holdable feature by

Pull Request Description


using the yum-versionlock feature.


(webhooks-id: 076bc809ef226e73fdf62b0955702f66)



 Comments   
Comment by gepetto-bot [ 2013/12/09 ]

puppetcla commented:

CLA signed by all contributors.

Comment by gepetto-bot [ 2013/12/11 ]

zaphod42 commented:

@fatmcgav Thanks for submitting this. We are looking into if there is a better way to do the check for the feature for the provider and will let you know if we find anything. The changes also need some unit tests to make sure that we don't regress on the functionality in the future.

Comment by gepetto-bot [ 2013/12/11 ]

fatmcgav commented:

@zaphod42 Cheers for the update.

Will see if I can get a working unit test done at some point soon...

Comment by gepetto-bot [ 2013/12/16 ]

fatmcgav commented:

@zaphod42 I've added a couple of initial test cases to check for method responses.

I'm not sure how best to stub the 'yum versionlock' command call to test for the ':holdable' feature.

Comment by gepetto-bot [ 2014/01/27 ]

Pull request (#23316) Updated yum package provider to support :holdable feature by has been closed.

Comment by gepetto-bot [ 2014/01/27 ]

adrienthebo commented:

Merged into master in a20b16b; this should be released in 3.5.0. Thanks for the contribution, and sorry about the delay in getting this merged - we've been having trouble with CI and that prevents us from merging things.

Comment by gepetto-bot [ 2014/01/27 ]

fatmcgav commented:

@adrienthebo No worries on the delay, just glad I could contribute

Cheers
Gav

Comment by gepetto-bot [ 2014/01/28 ]

adrienthebo commented:

Unfortunately this change introduced a number of problems that need to
be addressed before this code can be released.

1) The yum provider is always holdable

The package type detects the ability to hold packages based on the
presence of the #hold method. Even though there is a guarding clause
around the `has_feature :holdable` in the provider it's bypassed when it
fails since the provider defines the #hold method.

This ties into:

2) Holding a package without the yum-plugin-versionlock raises errors

Since the plugin isn't available but the provider looks like it's
holdable, applying a resource like:

package

{ 'bash': ensure => held, }

results in a backtrace since `yum versionlock` returns 1, and a
Puppet::ExecutionFailure is raised.

For instance:

<pre>
[root@localhost puppet]# bundle exec puppet resource package bash ensure=held
Error: Execution of '/usr/bin/yum versionlock bash' returned 1: Loaded plugins: fastestmirror
No such command: versionlock. Please use /usr/bin/yum --help
Error: /Package[bash]/ensure: change from 4.1.2-14.el6 to held failed: Execution of '/usr/bin/yum versionlock bash' returned 1: Loaded plugins: fastestmirror
No such command: versionlock. Please use /usr/bin/yum --help
Error: Execution of '/usr/bin/yum versionlock bash' returned 1: Loaded plugins: fastestmirror
No such command: versionlock. Please use /usr/bin/yum --help
Error: /Package[bash]/ensure: change from 4.1.2-14.el6 to held failed: Execution of '/usr/bin/yum versionlock bash' returned 1: Loaded plugins: fastestmirror
No such command: versionlock. Please use /usr/bin/yum --help
</pre>

3) Held packages are not idempotent

When the yum versionlock plugin is installed, the resulting resource is
not idempotent because the ensure value is always being changed from
$version to 'held':

[root@localhost puppet]# puppet resource package bash ensure=held
Notice: /Package[bash]/ensure: ensure changed '4.1.2-15.el6_4' to 'held'
package

{ 'bash': ensure => '4.1.2-15.el6_4', }

[root@localhost puppet]# puppet resource package bash ensure=held
Notice: /Package[bash]/ensure: ensure changed '4.1.2-15.el6_4' to 'held'
package

{ 'bash': ensure => '4.1.2-15.el6_4', }

I've reverted this commit in the mean time. If you would like to resubmit this pull request, you can run `git revert -m 1 b5a1a11fc10a4548411f3ebab2cd0603c1d96a9d` to restore your changes, and you should be able to continue from there.

If you would like to discuss this further, IRC would be one of the best ways to get ahold of me. I'm generally available on irc.freenode.net from 9:00 AM - 5:00 PM GMT -7 in #puppet and #puppet-dev, and my nickname is finch.

Comment by gepetto-bot [ 2014/01/29 ]

fatmcgav commented:

Adrien

Will take a look at the issues observed and see if I can come up with a
suitable update...

With regards to point 1, should this be considered a bug? Or is it
functionality that I've mis-interpreted?
As looking at the package type, I can see a mix'n'match of some features
using :methods and some not...

Will try and jump on IRC later to discuss.

Cheers
Gavin

On 28 January 2014 23:07, Adrien Thebo <notifications@github.com> wrote:

> Unfortunately this change introduced a number of problems that need to
> be addressed before this code can be released.
>
> 1) The yum provider is always holdable
>
> The package type detects the ability to hold packages based on the
> presence of the #hold method. Even though there is a guarding clause
> around the has_feature :holdable in the provider it's bypassed when it
> fails since the provider defines the #hold method.
>
> This ties into:
>
> 2) Holding a package without the yum-plugin-versionlock raises errors
>
> Since the plugin isn't available but the provider looks like it's
> holdable, applying a resource like:
>
> package

{ 'bash': > ensure => held, > }

>
> results in a backtrace since yum versionlock returns 1, and a
> Puppet::ExecutionFailure is raised.
>
> For instance:
>
> [root@localhost puppet]# bundle exec puppet resource package bash ensure=held
> Error: Execution of '/usr/bin/yum versionlock bash' returned 1: Loaded plugins: fastestmirror
> No such command: versionlock. Please use /usr/bin/yum --help
> Error: /Package[bash]/ensure: change from 4.1.2-14.el6 to held failed: Execution of '/usr/bin/yum versionlock bash' returned 1: Loaded plugins: fastestmirror
> No such command: versionlock. Please use /usr/bin/yum --help
> Error: Execution of '/usr/bin/yum versionlock bash' returned 1: Loaded plugins: fastestmirror
> No such command: versionlock. Please use /usr/bin/yum --help
> Error: /Package[bash]/ensure: change from 4.1.2-14.el6 to held failed: Execution of '/usr/bin/yum versionlock bash' returned 1: Loaded plugins: fastestmirror
> No such command: versionlock. Please use /usr/bin/yum --help
>
> 3) Held packages are not idempotent
>
> When the yum versionlock plugin is installed, the resulting resource is
> not idempotent because the ensure value is always being changed from
> $version to 'held':
>
> [root@localhost puppet]# puppet resource package bash ensure=held
> Notice: /Package[bash]/ensure: ensure changed '4.1.2-15.el6_4' to 'held'
> package

{ 'bash': > ensure => '4.1.2-15.el6_4', > }

> [root@localhost puppet]# puppet resource package bash ensure=held
> Notice: /Package[bash]/ensure: ensure changed '4.1.2-15.el6_4' to 'held'
> package

{ 'bash': > ensure => '4.1.2-15.el6_4', > }

>
> I've reverted this commit in the mean time. If you would like to resubmit
> this pull request, you can run git revert -m 1
> b5a1a11fc10a4548411f3ebab2cd0603c1d96a9d to restore your changes, and you
> should be able to continue from there.
>
> If you would like to discuss this further, IRC would be one of the best
> ways to get ahold of me. I'm generally available on irc.freenode.net from
> 9:00 AM - 5:00 PM GMT -7 in #puppet and #puppet-dev, and my nickname is
> finch.
>
> –
> Reply to this email directly or view it on GitHub<https://github.com/puppetlabs/puppet/pull/2137#issuecomment-33538270>
> .
>

Comment by gepetto-bot [ 2014/01/29 ]

adrienthebo commented:

Issue 1 isn't necessarily a bug but it's an unexpected behavior. Features can be activated by checking for methods defined on the provider which can be a handy convenience, but they don't account for behavior where the functionality may not be there. This is the case that we're running into with yum-versionlock.

There are a couple of solutions for this.

The first one that I can think of is that we remove the methods check from the feature and require providers to explicitly declare themselves as holdable. This would mean that we would have to mark the existing holdable providers with `has_feature :holdable` if that isn't present. However this could be backwards incompatible as it would break providers outside of core that are relying on this behavior.

The other solution is to wrap the `#hold` method in a clause that checks if the yum versionlock plugin is actually installed, and fail at runtime. Unfortunately the yum provider would always look like it's holdable regardless of if the plugin is present and we would have to determine at runtime if the feature is available. We could take this route, deprecate the implicit behavior holdable in Puppet 3.5 or 3.6, and remove it in 4.0.

In addition there was another pull request filed that created a new package provider that inherited from the yum provider and added the holdable feature. At the time I was somewhat reluctant to merge that pull request as-is but in light of what we found here it might be a better approach. The pull request in question is https://github.com/puppetlabs/puppet/pull/1954, and it might be an option to reopen and merge that pull request.

Comment by gepetto-bot [ 2014/01/30 ]

fatmcgav commented:

@adrienthebo Cheers for your time yesterday to discuss this issue a bit more...

As discussed, I've been trying to put in some additional error handling to catch this error, however appear to have hit some other bug within Puppet around logging of exceptions.
I've raised PUP-1542 and PUP-1544 which details the 2 scenarios I've encountered...

I'm currently a bit out of my depth as to how to fix them, however will see what I can work out today...

Cheers again.
Gav

Generated at Fri Aug 07 09:19:21 PDT 2020 using Jira 8.5.2#805002-sha1:a66f9354b9e12ac788984e5d84669c903a370049.