puppet agent include the APT class locking it and will make other .pp fail if they include apt

[aqueos]

Describe the Bug

hi,

This module include the class apt but if it is called elsewhere with parameters this will conflict.

If included elsewhere on a node it will lead to :

Error while evaluating a Resource Statement, Duplicate declaration: Class[Apt] is already declared;

Expected Behavior

It should not think important class like the APT one is used only in the module as it can be used elsewhere with parameters and other things. a quick test can help i think ?

+      if !defined(Class['apt']) {
+          include apt
+      }

this seems enough to prevent this :)

Steps to Reproduce

Steps to reproduce the behavior:
use in a node the apt class, then use the puppet agent module.

Environment

• Version [e.g. 1.27.0]├── puppetlabs-puppet_agent (v4.20.1)
• Platform [e.g. Ubuntu 18.04] debian 11

Additional Context

here is the patch i used:

diff --git a/modules/puppet_agent/manifests/osfamily/debian.pp b/modules/puppet_agent/manifests/osfamily/debian.pp
index 7cbd9d3e..e434f8fd 100644
--- a/modules/puppet_agent/manifests/osfamily/debian.pp
+++ b/modules/puppet_agent/manifests/osfamily/debian.pp
@@ -13,7 +13,9 @@ class puppet_agent::osfamily::debian {
     contain puppet_agent::prepare::package
   } else {
     if getvar('::puppet_agent::manage_repo') == true {
-      include apt
+      if !defined(Class['apt']) {
+          include apt
+      }
       if ($puppet_agent::is_pe and (!$puppet_agent::use_alternate_sources)) {
         $pe_server_version = pe_build_version()
         if $puppet_agent::source {

[joshcooper]

@aqueos can you provide a complete example manifest to reproduce the error?

[aqueos]

hi,

yes, for exemple this code 👍 , sorry the firts one was stupid here is the good one

# enfin on installe l'agent
class {'::puppet_agent':
                package_version => 'latest',
                collection      => $version,
                service_names   => [],
                manage_repo     => true,
                is_pe           => false,
}


class { 'apt':
  update => {
    frequency => 'daily',
    timeout   => 300,
  },
}

lead to

Error: Evaluation Error: Error while evaluating a Resource Statement, Duplicate declaration: Class[Apt] is already declared; cannot redeclare (file: /root/test.pp, line: 15) (file: /root/test.pp, line: 15, column: 1) on node xxxxx

best regards,
Ghislain.

[joshcooper]

ah, I can reproduce too, thanks!

# cat manifest.pp
class { 'puppet_agent':
  package_version => 'latest',
  collection      => 'puppet8',
  service_names   => [],
  manage_repo     => true,
  is_pe           => false,
}

class { 'apt':
  update => {
    frequency => 'daily',
    timeout   => 300,
  },
}
# /opt/puppetlabs/puppet/bin/puppet apply --modulepath /tmp/modules/ manifest.pp
Error: Evaluation Error: Error while evaluating a Resource Statement, Duplicate declaration: Class[Apt] is already declared; cannot redeclare (file: /tmp/modules/puppet_agent/manifest.pp, line: 9) (file: /tmp/modules/puppet_agent/manifest.pp, line: 9, column: 1) on node da13fead8f56

However, this diff below doesn't solve the problem, because puppet evaluates the manifest in the order specified. So when the puppet_agent class is evaluated, the apt class hasn't been included yet. And then later you declare the apt class.

-      include apt
+      if !defined(Class['apt']) {
+          include apt
+      }

@kenyon @bastelfreak is there a pattern for avoiding this problem?

[kenyon]

Yes, I think using hiera is typically how to supply parameters to puppetlabs-apt.

[bastelfreak]

IMO if !defined(Class['apt']) { include apt } isn't the right fix. puppetlabs/puppet_agent does include apt and that's correct. If you call a class with parameters, you need to ensure the ordering by yourself, usually in a profile, or you move the data to Hiera.

[joshcooper]

Thank you @kenyon and @bastelfreak, that makes sense. I'm going to close this.