Hi
As Takashi said we’re having very few contributors these days and we appreciate any help we can get, he has been doing an amazing job on cleaning up and doing maintenance.
Ack and fully agree.
We will be on the OpenInfra Summit next week and are going to meet up, if you are there we would love to have you join us in a discussion/planning on moving forward.
I will not be there, but willing to contribute. It would be nice if you could define a hiera.yaml file to use for all youre modules at the summit. Think this is a work that can better be done in a meetup than on reviews.
1.) use proper types for parameters Parameter validation is done with 'validate_legacy...' instead of defining the class/resource parameter with a proper type all over the code. I cannot imaging of any advantage not using proper type definitions. Instead using typed parameters would be more efficient an code readability would increase.
I think we have to prioritize this since puppetlabs-stdlib 9.0.0 deprecated the validate_legacy function and emits large warnings.
The reason we(at least, I) hesitated implementing the strict type validation is that this is likely to break existing manifests(in fact, even our modules or manifests have experienced breakage caused by new validations added in dependent modules) and also can heavily affect TripleO (though the project has been deprecated now).
We can start with just replacing all validate_legacy functions by typed parameters first and target this work during this cycle then discuss how we improve coverage of type validations further.
Agree, an like to add: "And ensure for all modules that new parameters introduced are properly typed."
2.) params.pp This is the legacy way to define parameter defaults for different OS's. In the modern puppet world a module specific hiera structure is used, which eliminates the need of params.pp class (with inheritance and include). The usage of hiera improves readability and flexibility (every parameter can be overwritten on request, eg. change of packages names etc.) This also eliminate the restriction that the modules can only be used by certain OS'es (osfamily 'RedHat' or 'Debian').
+1. Though I'd like to understand which OS users would like to use with our modules so that we can ideally implement CI coverage.
Ack. We use Debian. But with self brewed openstack packages. Which means, we maintain a patchset for eliminating if "OS=='Debian' " statements (in code and in params.pp) for many of the puppet modules.
3.) Eliminate "if OS=='bla' {" statements in code These statements make the code very inflexible. It cannot be overruled if necessary (eg. if I use custom packages to install and do not need the code provided in the if statement). Instead a parameter should be used with a default provided in hiera.
+1 . We can partially cover this as part of 2 but might leave this for the last work among 3, I guess.
Although this is the most hurting part for us, I agree that for proper solutions point 2 is needed.
Since there is lot of code to change I do not expect this to be done in a single commit (per module) but in steps probably in more than one release cycle. But defining this as best practice for openstack puppet modules and start using above in new commits would bring the code forward.
I tend to disagree that we start these new patterns in new commits. Having partial migration causes difficulty in maintenance. I really want to see these are implemented consistently in a single moules as well as among all modules, so that we are not confused when we are forced to implement global changes in all of our modules.
We can probably start with a few "independent" modules such as puppet-vswitch(or p-o-i) and once we agree with the pattern then we can schedule when we start implementing these changes in all modules in a single release cycle.
Probably although a good thing to discuss at Vancouver. Would be nice if somebody could communicate after Vancouver what the outcome was for the further development. Thanks Benedikt