User:Giuseppe Lavagetto/PuppetAndScope

The problem

We need to transform some of the construct we depended upon in our code in order to make the puppet 2.7 -> 3.4.3 transition. The biggest problem is that until puppet 2.7 the dynamic scope lookup rules will give you a de-facto set of global variables accessible and modifiable from everywhere. In puppet 3 this behaviour, that probably generated a lot of bugs for many people, has been completely phased-out.

To give some context, we actually have the $cluster variable that gets modified happily in a few places in the code. A reduced logical version of what we do today could be summarized as follows:

$cluster = 'misc'

class role::somerole (){
    $cluster = 'appserver'
    # do a lot of other things
    include standard
}

class role::somerole::children () {
    $cluster = 'imagescalers'
    include standard
}

class standard {
    include class_that_blindly_use_globals
}

class class_that_blindly_use_globals {
    notice($cluster)
}

node /foo/ {
    include role::somerole
    include role::somerole::children
    # Actually this will print 'appserver' on puppet 2.7 because of resource ordering;
    # In puppet 3, this prints 'misc'
}

node /baz/ {
    $cluster = 'lvs'
    include standard
    # prints 'lvs' correctly on both puppet 2.7 and puppet 3.x
}

Solutions

Take all the logic for cluster at the node level

We could just move cluster declaration at node level instead than inside roles (where it is about 50% of the times anyway) like this:

$cluster = 'misc'

class role::somerole (){
    # do a lot of other things
    include standard
}

class role::somerole::children () {
    include standard
}

class standard {
    include class_that_blindly_use_globals
}

class class_that_blindly_use_globals {
    notice($cluster)
}

node /foo/ {
    $cluster = 'appserver'
    include role::somerole
    include role::somerole::children
}

node /baz/ {
    $cluster = 'lvs'
    include standard
}

The globals class (with factory)

In this case we create a globals class and we declare the main role (in the case of node /foo/, role::somerole with the factory class role::main that also instantiates the globals class with the correct values. The upside of this approach is we'd need to write less code (we should just replace a few variables here and there, and that's it). The downside is that's an horrible hack that forces puppet to do things it's not exactly intended to do (naming the class ganglia::params will only make it appear as we're not mimicking a global namespace).

class globals($cluster = 'misc'){
    #noop
}

class role::main (
    $role = 'misc'
    ) {
    # Logic goes here!
    case $role {
        'somerole': { $cluster = 'appserver' }
        'somerole::children': { $cluster = 'imagescalers' }
        default: {$cluster = $role}
    }
    class {'::globals':
        cluster => $cluster
    }
    include "role::${role}"
}



class role::somerole (){
    # do a lot of other things
    include standard
}

class role::somerole::children () {
    include standard
}

class standard {
    if (!defined(Class['globals'])) {
        # If not declared upper in the include chain, do it here.
        include globals
    }
    include class_that_blindly_use_globals
}

class class_that_blindly_use_globals {
    notice($globals::cluster)
}

node /foo/ {
    class {'role::main':
        role => 'somerole'
    }
    include role::somerole::children
}

node /baz/ {
    class {'::globals':
        cluster => 'lvs'
    }'
    include standard
}

Using a define instead (NOT working)

A much better way to do this would be to use a defined resource here and include it in each role, which will make code less complex and less clunky; unfortunately, this does not seem to work as the 'before' statement does not seem to have any effect here. Does anyone see how to fix this?

class globals($cluster = 'misc'){
    #noop
}

define setglobals() {
    if (!defined(Class['globals'])) {
        notice('Defining globals')
        class {'::globals':
            cluster => $title
        }
    }
}

class role::somerole (){
    setglobals{'appserver':
        before => Class['standard']
    }
    # do a lot of other things
    include standard
}

class role::somerole::children () {
    setglobals{'imagescalers':
        before => Class['standard']
    }
    include standard
}

class standard {
    if (!defined(Class['globals'])) {
        # If not declared upper in the include chain, do it here.
        include globals
    }
    include class_that_blindly_use_globals
}

class class_that_blindly_use_globals {
    notice($globals::cluster)
}
node /foo/ {
    # prints 'misc' on puppet 2.7 because setglobals() gets called AFTER the inclusion of standard
    include role::somerole
    include role::somerole::children
}

node /baz/ {
    setglobals{'lvs': }
    include standard
}

Using an ENC

If we used an External Node Classifier, it will surely know what roles are associated with the single node; in that context it would be easy to assign the correct values to the top-scope variables that the ENC will provide to puppet. This way, we can virtually declare $cluster via the ENC in every node. This is obviously be much more elegant than the other solutions but requires quite an effort, which may still be worth it anyway.