Open Stack

Hi Benedikt,

Thanks for bringing these items. I generally agree with the points you
listed and will leave a few more
comments inline below.

Our puppet modules were developed initially some time ago(even long before
I joined the team)
and we have many legacy implementations. It'd be nice if we can adapt to
modern design patterns.

However at the same time we have limited resources in the project now so
addressing all these topics
in a short term might be difficult. As we maintain number of modules in
this project, I'd like to make sure
we have the consistent design pattern across all modules.
So we can probably determine the priorities of these works and also
milestones(especially in which cycle
we implement each change).

Thank you,
Takashi

On Tue, May 30, 2023 at 9:37 PM Benedikt Trefzer <
benedikt.trefzer at cirrax.com> wrote:

> Hi all
>
> I use the openstack puppet modules to deploy openstack. I'd like to
> suggest some improvements to the modules and like to know what the
> community thinks about:
>
> 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.


>
> 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.


>
> 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.


>
>
> 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.


>
> Finally: These are suggestions open for discussion. In no way I like to
> critic the current state of the puppet modules (which is quite good, but
> a bit legacy) or the people working on the modules. This is just a
> feedback/suggestion from an operator using the modules on a daily basis.
>
> Regards
>
> Benedikt Trefzer
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.openstack.org/pipermail/openstack-discuss/attachments/20230605/1d0f5eaa/attachment.htm>