<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">
Hello,
<div><br>
</div>
<div>Thanks for bringing this up! I agree with all your points.</div>
<div><br>
</div>
<div>It would be great if you wanted to help get these goals completed and we could</div>
<div>work together on all that.</div>
<div><br>
</div>
<div>As Takashi said we’re having very few contributors these days and we appreciate</div>
<div>any help we can get, he has been doing an amazing job on cleaning up and doing</div>
<div>maintenance.</div>
<div><br>
</div>
<div>I agree with Takashi’s feedback below, starting with removing validate_legacy to get</div>
<div>us started seems like a good option.</div>
<div><br>
</div>
<div>We will be on the OpenInfra Summit next week and are going to meet up, if you are</div>
<div>there we would love to have you join us in a discussion/planning on moving forward.</div>
<div><br>
</div>
<div>Best regards</div>
<div>Tobias </div>
<div>
<div><br>
<blockquote type="cite">
<div>On 5 Jun 2023, at 03:33, Takashi Kajinami <tkajinam@redhat.com> wrote:</div>
<br class="Apple-interchange-newline">
<div>
<div dir="ltr">
<div dir="ltr">
<div>Hi Benedikt,</div>
<div><br>
</div>
<div>Thanks for bringing these items. I generally agree with the points you listed and will leave a few more</div>
<div>comments inline below.<br>
</div>
<div><br>
</div>
<div>Our puppet modules were developed initially some time ago(even long before I joined the team)</div>
<div>and we have many legacy implementations. It'd be nice if we can adapt to modern design patterns.</div>
<div><br>
</div>
<div>However at the same time we have limited resources in the project now so addressing all these topics</div>
<div>in a short term might be difficult. As we maintain number of modules in this project, I'd like to make sure</div>
<div>we have the consistent design pattern across all modules.</div>
<div>So we can probably determine the priorities of these works and also milestones(especially in which cycle</div>
<div>we implement each change).</div>
<div><br>
</div>
<div>Thank you,</div>
<div>Takashi<br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, May 30, 2023 at 9:37 PM Benedikt Trefzer <<a href="mailto:benedikt.trefzer@cirrax.com">benedikt.trefzer@cirrax.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Hi all<br>
<br>
I use the openstack puppet modules to deploy openstack. I'd like to <br>
suggest some improvements to the modules and like to know what the <br>
community thinks about:<br>
<br>
1.) use proper types for parameters<br>
Parameter validation is done with 'validate_legacy...' instead of <br>
defining the class/resource parameter with a proper type all over the code.<br>
I cannot imaging of any advantage not using proper type definitions.<br>
Instead using typed parameters would be more efficient an code <br>
readability would increase.<br>
</blockquote>
<div> </div>
<div>I think we have to prioritize this since puppetlabs-stdlib 9.0.0 deprecated the validate_legacy function</div>
<div>and emits large warnings.</div>
<div><br>
</div>
<div>The reason we(at least, I) hesitated implementing the strict type validation is that this is likely to break</div>
<div>existing manifests(in fact, even our modules or manifests have experienced breakage caused by new</div>
<div>validations added in dependent modules) and also can heavily affect TripleO (though the project has</div>
<div>been deprecated now).</div>
<div><br>
</div>
<div>We can start with just replacing all validate_legacy functions by typed parameters first and target this</div>
<div>work during this cycle then discuss how we improve coverage of type validations further.
<br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
2.) params.pp<br>
This is the legacy way to define parameter defaults for different OS's.<br>
In the modern puppet world a module specific hiera structure is used, <br>
which eliminates the need of params.pp class (with inheritance and include).<br>
The usage of hiera improves readability and flexibility (every parameter<br>
can be overwritten on request, eg. change of packages names etc.)<br>
This also eliminate the restriction that the modules can only be used by <br>
certain OS'es (osfamily 'RedHat' or 'Debian').<br>
</blockquote>
<div><br>
</div>
<div>+1. Though I'd like to understand which OS users would like to use with our modules so that we can</div>
<div>ideally implement CI coverage.<br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
3.) Eliminate "if OS=='bla' {" statements in code<br>
These statements make the code very inflexible. It cannot be overruled <br>
if necessary (eg. if I use custom packages to install and do not need <br>
the code provided in the if statement).<br>
Instead a parameter should be used with a default provided in hiera.<br>
</blockquote>
<div><br>
</div>
<div>+1 . We can partially cover this as part of 2 but might leave this for the last work among 3, I guess.<br>
</div>
<div> </div>
</div>
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
Since there is lot of code to change I do not expect this to be done in <br>
a single commit (per module) but in steps probably in more than one <br>
release cycle. But defining this as best practice for openstack puppet <br>
modules and start using above in new commits would bring the code forward.<br>
</blockquote>
<div><br>
</div>
<div>I tend to disagree that we start these new patterns in new commits. Having partial migration causes</div>
<div>difficulty in maintenance. I really want to see these are implemented consistently in a single moules</div>
<div>as well as among all modules, so that we are not confused when we are forced to implement global</div>
<div>changes in all of our modules.</div>
<div><br>
</div>
<div>We can probably start with a few "independent" modules such as puppet-vswitch(or p-o-i) and</div>
<div>once we agree with the pattern then we can schedule when we start implementing these changes<br>
in all modules in a single release cycle.<br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Finally: These are suggestions open for discussion. In no way I like to <br>
critic the current state of the puppet modules (which is quite good, but <br>
a bit legacy) or the people working on the modules. This is just a <br>
feedback/suggestion from an operator using the modules on a daily basis.<br>
<br>
Regards<br>
<br>
Benedikt Trefzer<br>
<br>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</body>
</html>