voxpupuli / puppet-gluster

Create and manage Gluster pools, volumes, and mounts
https://forge.puppet.com/puppet/gluster
MIT License
16 stars 72 forks source link

Repo release and version logic broken? #187

Open jacksgt opened 5 years ago

jacksgt commented 5 years ago

Hello,

maybe I'm just not getting the logic of this module (if so please correct me!), but I think there is an error in the way this module handles the repository version (at least for Debian systems).

Starting in init.pp, I can specify two parameters concerning the package version number to be installed: release and version. (BTW: at the top of the file only one of them is documented) https://github.com/voxpupuli/puppet-gluster/blob/72a80cd94de36f5b4fb4df3fa3983bda1a3ee50e/manifests/init.pp#L44

But init.pp itself passes only version onto ::gluster::install:

class { '::gluster::install':
    server         => $server,
    server_package => $server_package,
    client         => $client,
    client_package => $client_package,
    version        => $version,
    repo           => $repo,
}

Because install.pp can only handle version, but not release. This makes sense as ::gluster::install deals with installing the packages, but not setting up the repositories.

However, the availability of the release parameter in init.pp lead me to assume I can configure both the packages and the repository through the main package (because enabling or disabling the repository is also possible through init.pp), like so:

  class { '::gluster':
    server                 => true,
    client                 => true,
    repo                   => true,
    release                => '4.1',
    version                => '4.1',
  }

This will simply lead to the creation of the file /etc/apt/sources.list.d/glusterfs-4.1.list:

# This file is managed by Puppet. DO NOT EDIT.
# glusterfs-4.1
deb [arch=amd64] http://download.gluster.org/pub/gluster/glusterfs/3.12/LATEST/Debian/stretch/amd64/apt/ stretch main

Which is obviously not what I wanted (the URL points to gluster 3.12).

This happens because install.pp includes repo.pp (if that has not already been done), but only passes the version parameter onto repo.pp.

  if $repo {
    # install the correct repo
    if ! defined ( Class['::gluster::repo'] ) {
      class { '::gluster::repo':
        version => $version,
      }
    }
}

Since repo.pp does then not know about the release parameter I specified in init.pp, it simply uses the default from params.pp: https://github.com/voxpupuli/puppet-gluster/blob/72a80cd94de36f5b4fb4df3fa3983bda1a3ee50e/manifests/params.pp#L27


Can someone please confirm or deny this observation?

I'd be happy to discuss and submit a PR.

jacksgt commented 5 years ago

Also I don't understand why gluster::repo only passes on version to Debian repositories, how are you supposed to select the release number then?

https://github.com/voxpupuli/puppet-gluster/blob/master/manifests/repo.pp

deric commented 5 years ago

Yes, it's weird :confused:

For Debian repositories you need to explicitly set the release

gluster::repo::apt::release: '5'

as the variable gluster::repo::release is not passed to class gluster::repo::apt