Add versionlock support

[msurato]

Adding versionlock plugin class test. This is a port of the versionlock
class in the yum module. The purpose of this is to prevent zypper from
updating a specified package under a normal update run.

Pull Request (PR) description

This is a port of the yum::versionlock to zypper.

This Pull Request (PR) fixes the following issues

n/a

[Dan33l]

I am neither a user of the module, nor a user of the tool. So my review is just with Puppet point of view.

[Dan33l]

How was decided the lower bound ?

[msurato]

The lower bound was copied from the Yum module. As the code is for the most part copied directly I also copied the version requirements from that module. In your opinion should these be different?

[Dan33l]

@msurato thank you for the update.
I do not understand why using a parameter ensure without using it in an standard way

[Dan33l]

Why this parameter without using it ?
IMO this parameter should be simply removed.
Otherwise it may suggest that it's possible to make sure it's not there.

[msurato]

Because of how concat is used ensure=>absent will indeed remove the lock. It can be argued that we can remove the statement as this is the same as ensure=>absent. However, will that also not break user's expectations (i.e. to ensure this is absent add ensure=>absent)?

[Dan33l]

Removing a statement does not imply the absence. It just means that we do not describe the desired state of this. So this means that it is not handled by Puppet.
Arguing that remove a statement is same that ensure => absent is misunderstanding how Puppet works.

[msurato]

So I double checked my test just to be certain.

Due to how concat is setup, it will remove any line that is not specifically defined by the module. This is unlike augeas that will remove specific lines. I can, of course, remove the ensure parameter, but would this break user's expectations?

[dhollinger]

So, I think a better option would be to make a parameter that sets the value of concat::replace to false.

class zypprepo::plugin::versionlock (
  Boolean $replace = true,
  Stdlib::Absolutepath $path = '/etc/zypp/locks',
) {

  concat { $path:
    mode   => '0644',
    owner  => 'root',
    group   => 'root',
    replace => $replace,
  }

  concat::fragment { 'versionlock_header':
    target  => $path,
    content => '# File managed by puppet\n',
    order   => '01',
  }
}

[dhollinger]

That said, the point of Puppet is to set system state. It is never a good idea to have something both managed by Puppet and manually modifiable.

[msurato]

@dhollinger and @Dan33l , how about this as a plan?

1. I remove the ensure block as only present does anything. And
2. I add a notice to the documentation giving the warning that once any locks are defined by the module, all locks must be defined by the module or they will be removed.

Does this work?

[Dan33l]

Same here.
IMO this parameter should be simply removed.
Otherwise it may suggest that it's possible to make sure it's not there.

[msurato]

It is possible to ensure that it is not there. Only statements that are defined will be locked and running "zypper al package" will be reset at the next Puppet run.

[dhollinger]

I'm in agreement with @Dan33l here. The proper use here would be to port any existing locks you have into Puppet code or Hieradata and let Puppet manage the entire thing. Puppet doesn't do much good as a tool for managing system state if the stuff it manages can also be touched from other sources as well.

[bastelfreak]

sweet, a nice type with good documentation! 👍