<div dir="ltr"><div><div><div><div><div>Hi,<br><br></div>let me try to rephrase this a bit and Bogdan will correct me if I'm wrong or missing something.<br><br>We have a set of top-scope manifests (called Fuel puppet tasks) that we use for OpenStack deployment. We execute those tasks with "puppet apply". Each task supposed to bring target system into some desired state, so puppet compiles a catalog and applies it. So basically, puppet catalog = desired system state.<br></div><br>So we can compile* catalogs for all top-scope manifests in master branch and store those compiled* catalogs in fuel-library repo. Then for each proposed patch CI will compare new catalogs with stored ones and print out the difference if any. This will pretty much show what is going to be changed in system configuration by proposed patch.<br><br>We were discussing such checks before several times, iirc, but we did not have right tools to implement such thing before. Well, now we do :) I think it could be quite useful even in non-voting mode.<br><br></div>* By saying compiled catalogs I don't mean actual/real puppet catalogs, I mean sorted lists of all classes/resources with all parameters that we find during puppet-rspec tests in our noop test framework, something like standard puppet-rspec coverage. See example [0] for networks.pp task [1].<br><br></div>Regards,<br></div>Alex<br><br>[0] <a href="http://paste.openstack.org/show/477839/">http://paste.openstack.org/show/477839/</a><br>[1] <a href="https://github.com/openstack/fuel-library/blob/master/deployment/puppet/osnailyfacter/modular/openstack-network/networks.pp">https://github.com/openstack/fuel-library/blob/master/deployment/puppet/osnailyfacter/modular/openstack-network/networks.pp</a><br><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 2, 2015 at 5:35 PM, Bogdan Dobrelya <span dir="ltr"><<a href="mailto:bdobrelia@mirantis.com" target="_blank">bdobrelia@mirantis.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Here is a docs update [0] for the patch [1] - which is rather a<br>
framework - being discussed here.<br>
Note, that the tool fuel_noop_tests.rb Dmitry Ilyin wrote became a Noop<br>
testing framework, which is Fuel specific. But the same approach may be<br>
used for any set of puppet modules and a composition layer manifests<br>
with a dataset of deployment parameters you may want it to be tracked<br>
against potential regressions.<br>
<br>
I believe we should think about how that Noop testing framework (and<br>
the deployment data checks under discussion as well) might benefit the<br>
puppet community.<br>
<br>
[1] <a href="https://review.openstack.org/240901" rel="noreferrer" target="_blank">https://review.openstack.org/240901</a><br>
[2] <a href="https://review.openstack.org/240015" rel="noreferrer" target="_blank">https://review.openstack.org/240015</a><br>
<span class="im HOEnZb"><br>
On 29.10.2015 15:24, Bogdan Dobrelya wrote:<br>
</span><div class="HOEnZb"><div class="h5">> Hello.<br>
> There are few types of a deployment regressions possible. When changing<br>
> a module version to be used from upstream (or internal module repo), for<br>
> example from Liberty to Mitaka. Or when changing the composition layer<br>
> (modular tasks in Fuel). Specifically, adding/removing/changing classes<br>
> and a class parameters.<br>
><br>
> An example regression for swift deployment data [0]. Something was<br>
> changed unnoticed by existing noop tests and as a result<br>
> the swift data became being stored in root partition.<br>
><br>
> Suggested per-commit based regressions detection [1] for deployment data<br>
> assumes to automatically detect if a class in a noop catalog run has<br>
> gained or lost a parameter or if it has been updated to another value by<br>
> a patch under test. Later, this check could even replace existing noop<br>
> tests, everything will be checked automatically, unless every deployment<br>
> scenario is covered by a corresponding template, which are represented<br>
> as YAML files [2] in Fuel.<br>
> Note: The tool [3] can help to get all deployment cases (-Y) and all<br>
> deployment tasks (-S) as well.<br>
><br>
> I propose to review the patch [1], understand how it works (see tl;dr<br>
> section below) and to start using it ASAP. The earlier we commit the<br>
> "initial" data layer state, less regressions would pop up.<br>
><br>
> (tl;dr)<br>
> The check should be done for every modular component (aka deployment<br>
> task). Data generated in the noop catalog run for all classes and<br>
> defines of a given deployment task should be verified against its<br>
> "acknowledged" (committed) state.<br>
> And fail the test gate, if changes has been found, like new parameter<br>
> with a defined value, removed a parameter, changed a parameter's value.<br>
><br>
> In order to remove a regression, a patch author will have to add (and<br>
> reviewers should acknowledge) detected changes in the committed state of<br>
> the deployment data. This may be done manually, with a tool like [3] or<br>
> by a pre-commit hook, or even at the CI side!<br>
> The regression check should show the diff between committed state and a<br>
> new state proposed in a patch. Changed state should be *reviewed* and<br>
> accepted with a patch, to became a committed one. So the deployment data<br>
> will evolve with *only* approved changes. And those changes would be<br>
> very easy to be discovered for each patch under review process!<br>
> No more regressions, everyone happy.<br>
><br>
> Examples:<br>
><br>
> - A. A patch author removed the mpm_module parameter from the<br>
> composition layer (apache modular task). The test should fail with a<br>
><br>
> Diff:<br>
>       @@ -90,7 +90,7 @@<br>
>          manage_user            => 'true',<br>
>          max_keepalive_requests => '100',<br>
>          mod_dir                => '/etc/httpd/conf.d',<br>
>       -  mpm_module             => 'false',<br>
>       +  mpm_module             => 'prefork',<br>
>          name                   => 'Apache',<br>
>          package_ensure         => 'installed',<br>
>          ports_file             => '/etc/httpd/conf/ports.conf',<br>
><br>
> It illustrates that the mpm_module's committed value was "false".<br>
> But the new one came as the 'prefork', likely from the apache class<br>
> defaults.<br>
> The solution:<br>
> Follow the failed build link and see for detected changes (a diff).<br>
> Acknowledge the changes and include rebuilt templates in the patch as a<br>
> new revision. The tool [3] (use -h for help) example command:<br>
> ./utils/jenkins/fuel_noop_tests.rb -q -b -s api-proxy/api-proxy_spec.rb<br>
><br>
> Or edit the committed templates manually and include data changes in the<br>
> patch as well.<br>
><br>
> -B. An upstream module author added the new parameter mpm_mode with a<br>
> default '123'. The test should fail with a<br>
><br>
> Diff:<br>
>        @@ -90,6 +90,7 @@<br>
>           manage_user            => 'true',<br>
>           max_keepalive_requests => '100',<br>
>           mod_dir                => '/etc/httpd/conf.d',<br>
>        +  mpm_mode               => '123',<br>
>           mpm_module             => 'false',<br>
>           name                   => 'Apache',<br>
>           package_ensure         => 'installed',<br>
><br>
> It illustrates that the composition layer is not consistent with the<br>
> upstream module data schema, and that could be a potential regression in<br>
> deployment (new parameter added upstream and goes with defaults, being<br>
> ignored by the composition manifest).<br>
> The solution is the same as for the case A.<br>
><br>
> [0] <a href="https://bugs.launchpad.net/fuel/+bug/1508482" rel="noreferrer" target="_blank">https://bugs.launchpad.net/fuel/+bug/1508482</a><br>
> [1] <a href="https://review.openstack.org/240015" rel="noreferrer" target="_blank">https://review.openstack.org/240015</a><br>
> [2]<br>
> <a href="https://github.com/openstack/fuel-library/tree/master/tests/noop/astute.yaml" rel="noreferrer" target="_blank">https://github.com/openstack/fuel-library/tree/master/tests/noop/astute.yaml</a><br>
> [3]<br>
> <a href="https://review.openstack.org/#/c/240015/7/utils/jenkins/fuel_noop_tests.rb" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/240015/7/utils/jenkins/fuel_noop_tests.rb</a><br>
><br>
<br>
<br>
--<br>
Best regards,<br>
Bogdan Dobrelya,<br>
Irc #bogdando<br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br></div>