Quash my.cnf.epp template's extra-newline issues

[greatflyingsteve]

Summary

The my.cnf.epp template was not converted from ERB to EPP very well, with seemingly little attention paid to the template bracketry's newline suppression. It inserts tons of random newlines into the body of the config file in weird random places, causing unnecessary diffs when upgrading from prior module versions and resulting in a weirdly sparse config layout that's more difficult to read.

The template logic was also obtuse and not especially easy to read or reason about, nor was it particularly efficient, producing a lot of values that would have been immediately discarded, prefering obtuse approaches, and failing to embrace the side-effects through which template rendering inherently works. These should be somewhat better now. Ruby and Puppet are related languages, but not identical; what makes sense in one will not always make sense in the other.

Also remove a few leftover TODO comments that all cite the aforementioned conversion as a thing that still needs to be done, despite being done already.

Additional Context

The issue is that while the original ERB template did the sensible thing and added a newlines after every line it added to the output, the translated EPP template for some reason tries to add newlines at loop exit instead of at line output. Because not all iterations through a loop add template output, and exiting multiple loops at once (end of a section, end of all sections, etc.), will add newlines for each exited loop, this yields predictably abysmal results.

Upgrading from a pre-v16.0.0 version of this module (prior to the template's conversion to EPP) to v16.3.0 of this module, with no other changes of any kind, with an override_options hash that included the following:

mysqld => {
  'sql_mode' => 'NO_UNSIGNED_SUBTRACTION',
  'ssl'      => undef,
  'ssl-ca'   => "/etc/mysql_ssl/cool_cert_bundle.crt",
  'ssl-cert' => "/etc/mysql_ssl/cool_certificate.crt",
  'ssl-key'  => "/etc/mysql_ssl/cool_certificate.key",
'mysqld-5.5'  => {
  'query_cache_limit' => undef,
  'query_cache_size'  => undef,
},
'mysqld-5.6'  => {
  'query_cache_limit' => undef,
  'query_cache_size'  => undef,
},

...resulted in diffs that looked like this:

@@ -70,8 +74,10 @@
 slow_query_log_file = /data/log/mysql/slow-queries.log
 socket = /data/mysql.sock
 sql_mode = NO_UNSIGNED_SUBTRACTION
+
 ssl-ca = /etc/mysql_ssl/cool_cert_bundle.crt
 ssl-cert = /etc/mysql_ssl/cool_certificate.crt
+
 ssl-key = /etc/mysql_ssl/cool_certificate.key
 sync_binlog = 1
 sysdate_is_now = 1
@@ -83,26 +89,39 @@
 tmpdir = /data/tmp
 user = mysql
 
+
 [mysqld-5.0]
 myisam-recover = BACKUP
 
+
 [mysqld-5.1]
 myisam-recover = BACKUP
 
+
 [mysqld-5.5]
 myisam-recover = BACKUP
 
+
+
+
 [mysqld-5.6]
 myisam-recover-options = BACKUP
 
+
+
+

And to walk through this a little bit, this was:

• line 77: 'ssl' => false prints nothing, still adds a newline when we exit that iteration
• line 80: 'ssl-disable' => false (from default_options in params.pp) prints nothing, still adds a newline when we exit that iteration
• line 92: (end of mysqld section): post-v16.x, ending any section adds one more newline
• line 96: (end of mysqld-5.0 section): post-v16.x, ending any section adds a second newline
• line 100: (end of mysqld-5.1 section): post-v16.x, ending any section adds a second newline
• line 104: 'query_cache_limit' => undef prints nothing, still adds a newline when we exit that iteration
• line 105: 'query_cache_size' => undef prints nothing, still adds a newline when we exit that iteration
• line 106: (end of mysqld-5.5 section): post-v16.x, ending any section adds a second newline
• line 111: 'query_cache_limit' => undef prints nothing, still adds a newline when we exit that iteration
• line 112: 'query_cache_size' => undef prints nothing, still adds a newline when we exit that iteration
• line 113: (end of mysqld-5.6 section): post-v16.x, ending any section adds a second newline

This approach to newlines is fundamentally broken. Newlines should be added whenever we've just printed a line, and should not be added unless we've added a line or done something significant like end a section (though in that case, ideally, we just get one extra, as previously). This change returns to exactly that approach, exactly like the ERB template used to do; and in addition, adds unit tests to catch this if anyone screws it up again.

Checklist

• 🟢 Spec tests.
• 🟢 Acceptance tests.
• Manually verified. (For example puppet apply)

[greatflyingsteve]

In real testing on the same host on which I was seeing the diffs shown above, all the extra newlines are removed. At the end of all sections, before !includedir would potientially appear at the bottom of the file, there are two newlines, which was exactly the outcome I was looking for.