Open Stack

Hello,

Thanks for bringing this up! I agree with all your points.

It would be great if you wanted to help get these goals completed and we could
work together on all that.

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.

I agree with Takashi’s feedback below, starting with removing validate_legacy to get
us started seems like a good option.

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.

Best regards
Tobias

On 5 Jun 2023, at 03:33, Takashi Kajinami <tkajinam at redhat.com> wrote:

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<mailto: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/87c6d464/attachment-0001.htm>