Avoid conflicts with an already declared curl package

[adepretis]

Curl is a very common package that will be most likely declared by custom modules too. This PR avoids conflicts with a curl package already declared by other modules (most likely custom modules).

[freibuis]

isn't this what ensure_packages() packages do any way?

[adepretis]

nope :-(

[garethr]

@adepretis as mentioned by @freibuis this is the specific intent with ensure_packages. https://github.com/puppetlabs/puppetlabs-stdlib/blob/dce8d7b194e3f6d9bb24e076c6e835aeb671099a/lib/puppet/parser/functions/ensure_resource.rb

So you may have hit a bug either in it's usage here or in the stdlib function. Could you provide details of how your triggering the issue, along with the logs from the agent run?

[adepretis]

It seems there is a problem with ensure_packages and package resources defined as array. When using

package { 'curl': ensure => present }

It works, no errors. As soon as it's done this way:

$latest = hiera_array('packages::latest', [])
package { $latest:
  ensure => latest,
}

The following error occurs:

Debug: Resource package[curl] was not determined to be defined
Debug: Create new resource package[curl] with params {"ensure"=>"present"}
Error: Evaluation Error: Error while evaluating a Function Call, Duplicate declaration: Package[curl] is already declared in file /etc/puppetlabs/code/environments/global/modules/twentyfifth/manifests/packages.pp:16; cannot redeclare at /etc/puppetlabs/code/environments/global/modules/docker/manifests/compose.pp:31 at /etc/puppetlabs/code/environments/global/modules/docker/manifests/compose.pp:31:7 on node ...

This error also occurs when using

create_resources('package', hiera_hash('packages::latest_hash', {}))

So it seems, as long as the conflicting package is used explicitly, without passing an array or using create_resources, the ensure_packages function does what it should do. Otherwise it does not.

I don't know if it that is intended. My solution for know is to use ensure_packages instead of packages in our module.

[adepretis]

Hm, ok, my solution doesn't work neither:

ensure_packages(hiera_array('packages::latest', []), { ensure => latest })

results in the same error, only explicitly using

ensure_packages(['curl'])

doesn't conflict. Which leaves no working solution of defining an array of packages to be installed (using hiera or not) that does not conflict with ensure_packages([packagename]).

[garethr]

@adepretis mmm, interesting. Could you open an issue against puppetlabs-stdlib at https://tickets.puppetlabs.com/browse/MODULES/. Might be something obvious I'm missing. @DavidS for reference.

[adepretis]

As a side note: when I remove curl from my own package list, and therefore solely rely on the docker module to require/install it, I get:

Error: Failed to apply catalog: Found 1 dependency cycle:
(Package[curl] => Class[Docker::Compose] => Package[curl])

By the way, I totally forget so metadata:

• Ubuntu 16.04 LTS using puppet apply
• Puppet Server 2.7.2
• Puppet Agent 1.8.2
• garethr-docker: 5.3.0
• puppetlabs-stdlib: 4.14.0

[adepretis]

Ok, I did: https://tickets.puppetlabs.com/browse/MODULES-4259

[adepretis]

But now I'm totally confused. I did a little test-setup:

node 'mynode` {
  include ::twentyfifth::test_this
  include ::twentyfifth::test_that
}

class twentyfifth::test_this {
  package { 'ncftp': ensure => latest }
}

class twentyfifth::test_that {
  ensure_packages(['ncftp'])
}

and it results in an error

Error: Evaluation Error: Error while evaluating a Function Call, Duplicate declaration: Package[ncftp] is already declared in file /etc/puppetlabs/code/environments/global/modules/twentyfifth/manifests/test_this.pp:2; cannot redeclare at /etc/puppetlabs/code/environments/global/modules/twentyfifth/manifests/test_that.pp:2 at /etc/puppetlabs/code/environments/global/modules/twentyfifth/manifests/test_that.pp:2:3

[EmersonPrado]

ensure_packages requires all declarations to have the same value for ensure. So:
ensure_packages(['ncftp'])
Declares the package with ensure => present (default value) and conflicts with former declaration.

Either change it to:
ensure_packages(['ncftp'], {'ensure' => 'latest'})
Or the former declaration to:
package { 'ncftp': ensure => present }

[esalberg]

What about adding a parameter that allows people to disable the ensure_packages for curl if it's causing issues (since in that case, they're managing it elsewhere in their code)? At least that doesn't use both defined and ensure_packages, which may not work in all cases anyway.

So just something basic like:

if $manage_curl {
}

with $manage_curl defaulting to true (matches the existing $manage_service parameter). I can put in a PR if that would help (didn't want to do so with this open one until I got feedback).

FWIW, we were able to resolve the duplicate definition by changing to the following in our code (the ensure => is not required, but this is a part of an iteration in reality, so the ensure is set explicitly):

    ensure_packages(['curl'], {ensure => 'installed'})

Note that 'installed' is the default, not 'present'. I had issues until I switched to using both 'installed' / only default values and the array syntax.

[EmersonPrado]

Note that 'installed' is the default, not 'present'.

I got the information that 'present' is the default from the function code itself.

I had issues until I switched to using both 'installed' / only default values and the array syntax.

This seems weird for me. From my tests, I noticed that declaring the same package with the same parameters would always work. BTW, pls check out this ticket and the related PR about the conflict between 'present' and 'installed'.

[esalberg]

Fair enough on the installed versus present - I was just going on the Puppet docs and didn't check the function:
https://docs.puppet.com/puppet/latest/types/package.html#package-attribute-ensure

I went ahead and retested with the following results:
Duplicate against ensure_packages([curl]):
ensure_packages({'curl' => { ensure => 'installed'}})

No Duplicate:
ensure_packages({'curl' => { ensure => 'present' }})
ensure_packages(['curl'], { ensure => 'present' })
ensure_packages(['curl'], { ensure => 'installed' })

This was a little surprising for me (I tested twice to make sure). I'll add a note to the stdlib ticket you mentioned.

Also, for the previous person looking to add multiple packages from Hiera, if you're still looking for a Hiera-enabled solution, this is working for us:

  $packages_linux.each |$package_linux| {
    ensure_packages(
      [$package_linux[0]],
      {
        ensure   => $package_linux[1],
      }
    )
  }

Then $packages_linux can be a hash in Hiera (theoretically - we actually do our package list in code with overrides).
$packages_linux:
package1: present
package2: absent