User:Giuseppe Lavagetto/PuppetFutureParser

From Wikitech

This page summarizes some data on how to migrate manifests to use the future parser.

How to move a set of nodes to the future parser

$ pcc --future 367876 mw1261.eqiad.wmnet
Compiling 367876 on node(s) mw1261.eqiad.wmnet...
Your build URL is https://integration.wikimedia.org/ci/job/operations-puppet-catalog-compiler/7165
Started by user Giuseppe Lavagetto
Building remotely on compiler02.puppet3-diffs.eqiad.wmflabs (puppet-compiler-node) in workspace /mnt/jenkins-workspace/workspace/operations-puppet-catalog-compiler
[operations-puppet-catalog-compiler] $ /bin/bash -xe /tmp/hudson5993008488523454240.sh
+ NUM_THREADS=2
+ MODE=change,future
+ CHANGE=367876
+ NODES=mw1261.eqiad.wmnet
+ puppet-compiler
[ 2017-07-26T10:54:39 ] INFO: Working on change 367876
[ 2017-07-26T10:54:39 ] INFO: Refreshing the common repos from upstream if needed 
[ 2017-07-26T10:54:48 ] INFO: Creating directories under /mnt/jenkins-workspace/puppet-compiler     
Applying: role::mediawiki::canary_appserver: move to the future parser
[ 2017-07-26T10:55:06 ] INFO: Starting run in mode future
[ 2017-07-26T10:55:06 ] INFO: Compiling host mw1261.eqiad.wmnet (change)    
[ 2017-07-26T10:55:22 ] INFO: Compiling host mw1261.eqiad.wmnet (future)  
[ 2017-07-26T10:55:30 ] INFO: Compilation failed for hostname mw1261.eqiad.wmnet  in environment future.
[ 2017-07-26T10:55:30 ] INFO: Compilation exited with code 30
[ 2017-07-26T10:55:30 ] INFO: [future] Nodes: 1 ERROR 
[ 2017-07-26T10:55:30 ] INFO: Run finished for mode future; see your results at https://puppet-compiler.wmflabs.org/compiler02/7165/index-future.html
[ 2017-07-26T10:55:30 ] INFO: Starting run in mode change
[ 2017-07-26T10:55:30 ] INFO: Compiling host mw1261.eqiad.wmnet (prod)     
[ 2017-07-26T10:55:45 ] INFO: Calculating diffs for mw1261.eqiad.wmnet
Error: Add --debug for realtime output, add --render-as {json,yaml} for parsed output  
[ 2017-07-26T10:55:54 ] INFO: [change] Nodes: 1 DIFF 
[ 2017-07-26T10:55:54 ] INFO: Run finished for mode change; see your results at https://puppet-compiler.wmflabs.org/compiler02/7165/
  • In this case, the compiler reported that the catalog can't be compiled with the future parser. So we have to check what is broken, fix that, resubmit the change, repeat  the steps above until all errors are fixed
  • Carefully inspect the differences once the future catalog does compile. Most of those are spurious (integer parameters are now integers, not strings) but any significant difference might have spurred by real issues (see below).
  • Once you're ok with the results, merge the change, and run puppet twice (the first time you'll just switch environment, the second you will actually run with the future parser)

Known issues to fix

Variable scope is now respected in templates

Say you have a class that calls a define which calls template() Until now, you could access /any/ variable in the caller class scope with `@variable`. This is not possible anymore.

So for instance most templates we define in order to use them with base::service_unit reference variables in the caller class via @variable, while they should use fully-qualified variables

So, say you have modules/foo/manifests/init.pp

class foo {
    $cmd = 'bar'
    base::service_unit{ 'foo':
         systemd => true,
    }
}

and in modules/foo/templates/initscripts/foo.systemd.erb

Exec=/usr/bin/<%= @cmd %>

This will NOT work with the new parser!!! The fix is, luckily, quite easy: you should instead pass the content of the systemd unit to base::service_unit directly, as follows:

class foo {
    $cmd = 'bar'
    base::service_unit { 'foo':
        systemd => template('foo/initscripts/foo.systemd.erb'),
    }
}

in this case, the lookup via @bar will work as the template will be evaluated within the local scope of class foo.

Finally, if your base::service_unit serves just systems that run systemd, you are strongly urged to convert your manifests to use systemd::service or systemd::unit, which are more specialized and cleaner.

Integers in string operations

Since puppet 4 has real types, you can't compare strings and integers or make string operations on an integer without casting, so

$a = 123
if $a !~ /\d+/ { 
    fail()
}

This will error out with "Evaluation Error: Left match operand must result in a String value. Got an Integer.

If this is not in a base module, you can just avoid doing such validation, as your declaration guarantees that value is an integer. If it is, you can this https://github.com/wikimedia/puppet/commit/46369ea4a88c1b22716f1f4b9e61c78e8932f663

please note the # TODO/puppet4 comment since validate_* functions from stdlib are deprecated in puppet4 (yes, you read that right!).

Double slashes in single quoted strings

This is particularly sad - in puppet 4 (and the future parser), $a = 'a\\b' will actually assign 'a\b' to $a, breaking compatibility with the past.

In this case, convert your string to be double-quoted $a = "a\\\\b" (will result in a\\b) that will work with both parsers

The empty string evaluates to true in boolean context

The following snippet

$foo = ''
if $foo {
    ...
}

will descend in the if conditional on puppet 4, and will not do that in puppet 3.

Whenever possible, use $foo=undef, which will evaluate to false in both.

Known diff that are NOOP

Priorities for files in conf.d directories

Those are now integers and it's possible to see diffs like:

-	     priority => 05
+	     priority => 5

Given that we then print the priority with %02d the result is actually the same on disk, but they are shown as diff.