Vladimir,
can you please elaborate on "such invasive changes"?

There was a plan developed, including risk mitigation, etc. - like do 'grep
-r' to check, and revert the change all together right away if we see
regression. So far, everyone was aligned with the plan. It was discussed
yesterday during IRC meeting [1]. Again, no one had objections.

Please provide your concerns, explain your opinion in more details. I'd
like other core reviewers to jump in here and reply. If you need details of
the approach, please jump in a call with Alex Schultz.

Thank you,

[1]
http://eavesdrop.openstack.org/meetings/fuel/2015/fuel.2015-07-30-16.00.html

On Fri, Jul 31, 2015 at 5:52 AM Vladimir Kuklin <vkuk...@mirantis.com>
wrote:

> Folks
>
> I do actively support our initiative to use librarian to be as close as
> possible to upstream, but let's not merge such invasive changes until we
> announce Hard Code Freeze and create stable/7.0 branch. So far I put my -2
> onto the first commit in the chain. Let's get through 7.0 and then land
> this code in the master as early as possible after HCF.
>
>
> On Fri, Jul 31, 2015 at 3:21 PM, Aleksandra Fedorova <
> afedor...@mirantis.com> wrote:
>
>> > So far CI has been successful on all of these
>> changes, and bvt is currently running.
>>
>> Small update - BVT test passed.
>>
>> On Fri, Jul 31, 2015 at 6:27 AM, Alex Schultz <aschu...@mirantis.com>
>> wrote:
>> > Hey everyone,
>> >
>> > During on the fuel meeting today we discussed the librarian changes and
>> > their status.
>> > As part of this work, the wiki page was updated and a first attempt at
>> > migrating the
>> > following modules has been completed pending merge:
>> >
>> > stdlib
>> > concat
>> > inifile
>> > ssh
>> > ntp
>> > apache
>> > firewall
>> > xinetd
>> > cinder
>> > apt*
>> >
>> > It should be noted that apt is currently blocked by the lack of a
>> mirror so
>> > while it has
>> > been prepared, it should not be merged at this time.
>> >
>> > As part of this migration we are doing two things. The first is an
>> update to
>> > the build
>> > process that is included as part of the initial librarian[0] patch.  The
>> > other patches
>> > consist of the actual module code changes.
>> >
>> > Here is the list of the diffs for each change so that it can be
>> reviewed and
>> > people can
>> > raise concerns if there are any with this change. As part of the
>> migration,
>> > I inspected
>> > the code and file differences for each module to determine how much
>> impact
>> > they might
>> > have.  I chose the list of modules based on their minimal differences
>> from
>> > the upstream
>> > or if they already had our forked differences rolled into a newer
>> version of
>> > the module.
>> > For this list, I took the current stable iso (#110) and rebased the
>> changes
>> > on top of this
>> > to create a custom iso with just the librarian changes. We have kicked
>> off a
>> > bvt_2 test for
>> > the custom iso as well. From this iso I have extracted the fuel-library
>> > package from both
>> > of these isos and exploded the fuel-library folder structure to do the
>> > diffs.
>> >
>> > Code Changes:
>> >
>> > For stdlib, the only differences are related to git, travis or
>> fixtures[1].
>> > There are no
>> > puppet code changes as part of the librarian migration.
>> >
>> > For concat, the only differences were a git folder and in a custom
>> change to
>> > the spec tests[2].
>> > The test difference[3], was a change we made because it was failing our
>> > syntax checker.
>> > This change has been included in a newer version of concat (1.2.4) but
>> are
>> > not necessary
>> > when the module gets moved to be included via librarian.
>> >
>> > For inifile, the only difference is the addition of git and metadata
>> > files[4].
>> >
>> > For ssh, the only difference is a single line to have the config notify
>> > service[5]. This
>> > difference is already covered by another file and is not needed[6].
>> >
>> > For ntp, this change introduces more code changes[7] because we are
>> updating
>> > the module
>> > to the 4.0.0 version because of previous extending of functionality
>> that is
>> > now covered by
>> > 4.0.0 vs 3.3.0[8]. The changes in our fork were upstreamed and are
>> include
>> > in 4.0.0.
>> >
>> > For apache, this change includes an upgrade from 1.2.0 to 1.3.0[9][10].
>> Our
>> > fork had a
>> > customization made which was contributed upstream.
>> > (apache::mod::proxy_connect)
>> >
>> > For firewall, this change also includes an upgrade from 1.0.2 to
>> 1.2.0[11]
>> > as our fork had
>> > mac supported added[12] in which is now covered upstream.
>> >
>> > For xinetd, the only change was the addition of a .git folder and a
>> > .gitignore with librarian.
>> >
>> > For cinder, the only change was the addition of .git, .gitignore, and
>> > .gitreview.
>> >
>> > Once we can get the apt mirror created, the only change for that is
>> also the
>> > addition of
>> > .git.
>> >
>> >
>> > If there are any of these upgrades/changes that we do not want to tackle
>> > right now, I can
>> > adjust the review order such that it can be skipped for now.  Please
>> take
>> > some time to
>> > review these changes and raise concerns.  So far CI has been successful
>> on
>> > all of these
>> > changes, and bvt is currently running.
>> >
>> > Also please take some time to review the changes themselves:
>> >
>> https://review.openstack.org/#/q/status:open+project:stackforge/fuel-library+branch:master+topic:bp/fuel-puppet-librarian,n,z
>> >
>> > Please raise any concerns as quickly as possible as this is the last
>> call
>> > for objections
>> > for these reviews.  This has been talked about extensively and these
>> reviews
>> > have
>> > been available for several weeks now.
>> >
>> > Thanks,
>> > -Alex
>> >
>> >
>> > [0] https://review.openstack.org/#/c/202763/
>> > [1] http://paste.openstack.org/show/406523/
>> > [2] http://paste.openstack.org/show/406524/
>> > [3] http://paste.openstack.org/show/406525/
>> > [4] http://paste.openstack.org/show/406526/
>> > [5] http://paste.openstack.org/show/406527/
>> > [6]
>> >
>> https://github.com/saz/puppet-ssh/blob/v2.4.0/manifests/server/config.pp#L9
>> > [7] http://paste.openstack.org/show/406536/
>> > [8] https://github.com/puppetlabs/puppetlabs-ntp/compare/3.3.0...4.0.0
>> > [9] http://paste.openstack.org/show/406538/
>> > [10]
>> https://github.com/puppetlabs/puppetlabs-apache/compare/1.2.0...1.3.0
>> > [11]
>> https://github.com/puppetlabs/puppetlabs-firewall/compare/1.0.2...1.2.0
>> > [12] https://review.openstack.org/#/c/92167/
>> >
>> >
>> __________________________________________________________________________
>> > OpenStack Development Mailing List (not for usage questions)
>> > Unsubscribe:
>> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>> >
>>
>>
>>
>> --
>> Aleksandra Fedorova
>> Fuel CI Engineer
>> bookwar
>>
>> __________________________________________________________________________
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe:
>> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>
>
>
> --
> Yours Faithfully,
> Vladimir Kuklin,
> Fuel Library Tech Lead,
> Mirantis, Inc.
> +7 (495) 640-49-04
> +7 (926) 702-39-68
> Skype kuklinvv
> 35bk3, Vorontsovskaya Str.
> Moscow, Russia,
> www.mirantis.com <http://www.mirantis.ru/>
> www.mirantis.ru
> vkuk...@mirantis.com
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-- 
Mike Scherbakov
#mihgen
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to