Make it possible to define multiple server stanzas

[ZeroPointEnergy]

This change makes it possible to define multiple server stanzas via the "tsm::stanzas" hash. The change does not break compatibility, so the configuration of a single server with tsm class parameters is still possible.

The dms.sys template is now splitted up further into a header, a section for global setting and template for the tsm::config::stanza instances.

[ZeroPointEnergy]

Did you have any chance to look at the pull-request?

[tosmi]

i was just skimming through your changes and the code looks good. thanks for your hard work. the only thing that worries me is backward breakage. i'm currently not understanding the full set of changes because of -ENOTIME, but could you add a test so that we are sure that old configurations do not break?

i would like to deploy a test version of the module on our servers, but i have to be sure that nothing breaks :-)

thanks toni

[ZeroPointEnergy]

Hello Toni I expanded the already existing test cases to a full blown example. The way I implemented the backward compatibility is that if you don't define the stanzas hash it will just use the class parameters and construct a single stanza with it, which results in the same configuration file as before.

The tests then compare the example with fixtures so it should be easy to see what configuration gets generated (the relevant concat fragment without the header etc.):

Fragment created with class parameters: https://github.com/nosolutions/puppet-tsm/pull/51/files#diff-f8aa3992ff270c73952ce726bba37e24

Fragments created with stanzas: https://github.com/nosolutions/puppet-tsm/pull/51/files#diff-a55890474c00f9cf83eda99c3ee2aaff https://github.com/nosolutions/puppet-tsm/pull/51/files#diff-098be2f47714bb11ad3096106c195b54

I hope this covers your concerns and makes this whole thing a bit less scary 😄. Let me know if you need more test cases.

Regards Andreas

[tosmi]

thanks again. i will try to review the changes until end of next week or over the holidays latest.

thanks for your patience

toni

[ruriky]

Thanks for the stanzas @ZeroPointEnergy ! I took this PR for a spin and the multi-server configuration seem to be created correctly. Some notes though:

1. The dsmsched service was restarted before the InclExcl.hash file was created and the service was complaining about the missing file. Restarting the service helped.

   ANS0297E Error opening specified file.
   The complete entry: '   INCLExcl              /opt/tivoli/tsm/client/ba/bin/InclExcl.hash'
   at line number: 92
   ANS1036S The option 'INCLEXCL' or the value supplied for it is not valid. It was found in options file '/opt/tivoli/tsm/client/ba/bin/dsm.sys'

2. This is only a cosmetic change but currently in the master branch the dms.sys.erb template sorts the configuration parameters alphabetically. This PR removes the sorting.

3. In master branch the inclexcl_hash parameter is included in the dsm.sys.erb template only if its value is not an empty hash. In this PR the if-clause is a bit different and causes the parameter to be added even if the value is an empty hash.

<%- if @inclexcl_hash -%> vs <%- if scope['::tsm::inclexcl_hash'] != {} -%>

-- Rurik

[tosmi]

finally started working on integrating the pull request (yeah!). i think i understand the changes now and it looks good. just one remark regarding the service restart issue @ruriky reported:

in ::tsm::config we just include the full_template and stanzas classes. so there is no order. shouldn't we change that to a require? imho this would prevent the service restart before all files a created.

toni

[ZeroPointEnergy]

@tosmi I tried to address the points @ruriky raised with some additional commits. The ordering problem was probably caused by me separating the config into subclasses. This should now be fixed by using contain instead of include for those subclasses, so the top-level ordering in the init.pp works for them as well.

[tosmi]

regarding dsm.sys_stanza.erb:

• i would change @inclexcl_hash if statement to the original one, so

  if @inclexcl_hash != {}

• are we sure the template produces the same output if the config_values are arrays? i can't remember why i implemented the array check. are there config values of type array?

[tosmi]

@ZeroPointEnergy, you are right contain is the correct way to solve this, thanks.

[ZeroPointEnergy]

@tosmi Yes, I already changed to the old check for inclexcl_hash.

The array was because it can be possible that you want to have multiple entries with the same key. It would not be possible to specify that in a hash, since the key has to be unique. At least that is what I guessed this was for 😄

[config_values].flatten

This is just a trick to make sure we always have an array, even if config_value was a simple value so we can then handle both cases equally.

irb(main):001:0> a = "foo" => "foo" irb(main):002:0> [a].flatten => ["foo"] irb(main):003:0> a = ["foo", "bar"] => ["foo", "bar"] irb(main):004:0> [a].flatten => ["foo", "bar"]

[tosmi]

@ZeroPointEnergy, thanks for the hint, i struggle with understanding my own code :-)

• i will merge the pull request now
• the next step are some internal tests with our own tsm clients
• if nothing breaks i'm going to release a 2.0 version. there are no backward incompatibilities, but because of the number of changes this feels more like a new version...
• i can't promise when the release will exactly happen, hopefully within next week.

thanks you guys for your hard work and being that patient

toni