cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Will Stevens <wstev...@cloudops.com>
Subject Re: 4.10.0 release
Date Thu, 04 Aug 2016 18:39:00 GMT
This sounds good.  I agree that not every PR needs full CI on hardware, but
I do think that every PR should have some form of verification that the
code does what it is intended to do.  So for a UI change, it should include
a screenshot (for example).

The main point of this is that we maintain stability.  We have come a long
way since 4.6.  Then we had to be a bit more draconian about this because
we were breaking the norm in order to stabilize master.  Now, with a more
ingrained Github workflow and some better CI tools, I think we can afford
to start giving back some control to the committing community.  This is of
course assuming the RMs are comfortable with that.  At the end of the day,
they are the ones who are directly impacted by this decision.

*Will STEVENS*
Lead Developer

*CloudOps* *| *Cloud Solutions Experts
420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6
w cloudops.com *|* tw @CloudOps_

On Thu, Aug 4, 2016 at 2:11 PM, John Burwell <john.burwell@shapeblue.com>
wrote:

> Will and Rajani,
>
> I recall the motivations for the release principles well.  Their primary
> intention was to improve the testing and codify review requirements for
> commits to a release branch.  In my view, this goal does not require that
> only a few people perform merges to release branches.  We should let any
> committer merge a PR that has the requisite LGTMs.  I also believe we need
> to be a bit nuanced about the requirement to test every PR against
> hardware.  While most PRs require this level of testing, we have plenty of
> PRs that do not impact hardware (e.g. UI changes, code cleanups, etc).  I
> am in favor of refining the release principles to say that the test LGTM
> for any PR that impacts provisioning/management of hardware must include
> hardware tests.  As we monitor merges to master, if we find any PRs that do
> impact these functions that have not been tested against hardware then we
> will roll them back and work with the author to complete that testing.
>
> Rohit is currently creating a Jenkins build pipeline that builds a PR,
> creates a test environment using Trillian, and runs the tests.  He is
> nearly finished.  When he is done, I get him to push docs to the Trillian
> repo and open a discussion on dev@.
>
> Thanks,
> -John
>
> >
> john.burwell@shapeblue.com
> www.shapeblue.com
> 53 Chandos Place, Covent Garden, London VA WC2N 4HSUK
> @shapeblue
>
>
>
> On Aug 4, 2016, at 1:29 PM, Will Stevens <wstevens@cloudops.com> wrote:
> >
> > Rajani, you have very valid points and they echo my concerns.
> >
> > I also agree that the CI (actual testing of PRs against real hardware
> > environments) is required in order for a PR to be committed.  This is
> where
> > ALL the work is for the RM in the current process.  As RM, I ended up
> with
> > 10 CI environments running in parallel in order to get testing done to at
> > least a basic level.  This type of testing MUST continue or we are at
> risk
> > of falling into old pitfalls around stability.
> >
> > John and Paul, how feasible would it be for me to setup an automated
> > process for pulling and testing PRs with Trillian?  I have environments
> > that I can test on, but I don't have the time to manage 8=10 environments
> > testing in parallel by hand.  We need get to the point where PRs are
> queued
> > and the testing of these PRs on real hardware is automated.  The manual
> > approach is not viable long term (trust me).  :)
> >
> > *Will STEVENS*
> > Lead Developer
> >
> > *CloudOps* *| *Cloud Solutions Experts
> > 420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6
> > w cloudops.com *|* tw @CloudOps_
> >
> > On Thu, Aug 4, 2016 at 1:19 PM, Rajani Karuturi <rajani@apache.org>
> wrote:
> >
> >> John,
> >> We understand the bylaws. But, practically we saw master in an unusable
> >> state for many months. This is not just about trust. CloudStack is so
> vast
> >> that its difficult for a single person to test all the affected areas.
> Many
> >> a times, people does not even have enough hardware/knowledge to run
> >> integration tests. Bugs which are found at a later point of time have
> very
> >> high cost. Finding that commit which broke something is very hard task.
> >> Rolling back is the easiest part of it.
> >>
> >> In an ideal world, we could just allow everyone to commit (maybe we
> could
> >> even allow non committers to commit and increase the velocity). But, it
> did
> >> not workout. which is why we have all these rules based on previous
> >> learning. For 4.6, we did not start with RM only merges. But, after many
> >> failed attempts we came to that conclusion.
> >>
> >> Watching the branches and policing is more difficult than allowing only
> a
> >> few people to commit.
> >>
> >> I do understand Rohit's concern about RM being bottleneck. The initial
> idea
> >> was we will have more than one RM(may be around 5) so that one of them
> >> would be available always. But, given the amount of time it takes to do
> the
> >> RM job, we did not get enough volunteers.
> >>
> >> Even now, I dont think its the RM that is the bottleneck. But, its the
> >> number of people with real hardware who are willing to run integration
> >> tests for the PRs. Most of the efforts of RM goes in setting up test
> >> environments, running tests, bringing the results back to PR.
> >>
> >> Maybe, we could follow a middle ground like this.
> >> We as a community can authorize few more people to merge PRs in
> addition to
> >> RMs.
> >>
> >> How do we select these additional volunteers?
> >> Any committer can just come forward and declare(in a mail to dev@)
> that he
> >> read the release principles[1] and would like to volunteer to commit.
> >>
> >> [1]
> >> https://cwiki.apache.org/confluence/display/CLOUDSTACK/
> >> Release+principles+for+Apache+CloudStack+4.6+and+up
> >>
> >> ~Rajani
> >> http://cloudplatform.accelerite.com/
> >>
> >> On Thu, Aug 4, 2016 at 8:28 PM, John Burwell <
> john.burwell@shapeblue.com>
> >> wrote:
> >>
> >>> Rajani and Will,
> >>>
> >>> It is actually not up to the release managers to make such a
> >>> determination.  Our bylaws state that anyone with a commit bit can
> commit
> >>> (or, if necessary, rollback a commit).  By granting someone a commit
> bit,
> >>> we have imparted a trust that an individual will protect the integrity
> of
> >>> the codebase and work in the best interest of the community.  If
> someone
> >>> makes a mistake (which has happened), we can easily rollback a commit.
> >> We
> >>> have even decided to rollback commits after getting 2 LGTMs on a PR.
> >> Under
> >>> the Apache Way, committers are encouraged to propel a project forward.
> >>> Restricting merges to RMs not only restricts velocity and it also
> limits
> >>> the energy/contributions of our large committer base.
> >>>
> >>> In terms of the rules below, we have, by consensus, accepted a set of
> >>> release principles [1] that specify how PRs should be reviewed and
> >> accepted
> >>> for merge to master.   If the guidelines outlined in that document are
> >> not
> >>> followed, we have accepted that another committer may rollback a
> commit.
> >>> Rajani and I will be watching the release branches.  If we find commits
> >>> that do not conform to our accepted merge rules, we will roll them back
> >> and
> >>> work with the committer to fix the issues.
> >>>
> >>> Thanks,
> >>> -John
> >>>
> >>> [1]: https://cwiki.apache.org/confluence/display/CLOUDSTACK/
> >>> Release+principles+for+Apache+CloudStack+4.6+and+up
> >>>
> >>>>
> >>> john.burwell@shapeblue.com
> >>> www.shapeblue.com
> >>> 53 Chandos Place, Covent Garden, London VA WC2N 4HSUK
> >>> @shapeblue
> >>>
> >>>
> >>>
> >>> On Aug 4, 2016, at 10:17 AM, Will Stevens <wstevens@cloudops.com>
> wrote:
> >>>>
> >>>> I will let the RMs for this release weigh in on this, but here are my
> >>>> thoughts.
> >>>>
> >>>> If we let anyone commit, I think the following rules MUST be followed:
> >>>> - No commits directly to the repo, which are not a merge of a GitHub
> >> Pull
> >>>> Request.  So every change to the repo should be through `git pr ####`
> >>> using
> >>>> this tool [1].  This ensures everything that gets committed goes
> >> through
> >>>> our CI pipeline and is verified before commit.  It also makes it
> easier
> >>> for
> >>>> us to be able to script the generation of release notes and correlate
> >> the
> >>>> commit history with other sources (GitHub, Jira, etc).  I will submit
> a
> >>> PR
> >>>> with tools for generating release notes based on merged GitHub PRs
> >>> soon...
> >>>> - Every PR merged into a previous branch must be forward merged to
> >> later
> >>>> branches.  This is done using this tool [2] in order to make sure the
> >>>> commit hashes are consistent across all branches.  This is for
> >>> auditability
> >>>> and comparing what exists in one branch vs another.
> >>>>
> >>>> [1] https://github.com/apache/cloudstack/blob/master/tools/git/git-pr
> >>>> [2] https://github.com/apache/cloudstack/blob/master/tools/
> >>> git/git-fwd-merge
> >>>>
> >>>> This is my two cents anyway...
> >>>>
> >>>> *Will STEVENS*
> >>>> Lead Developer
> >>>>
> >>>> *CloudOps* *| *Cloud Solutions Experts
> >>>> 420 rue Guy *|* Montreal *|* Quebec *|* H3J 1S6
> >>>> w cloudops.com *|* tw @CloudOps_
> >>>>
> >>>> On Thu, Aug 4, 2016 at 3:43 AM, Rohit Yadav <
> rohit.yadav@shapeblue.com
> >>>
> >>>> wrote:
> >>>>
> >>>>> I disagree with having only RMs to merge PRs when we're not in
> freeze.
> >>> In
> >>>>> general we've implicitly honoured this behaviour but it was never
> >> voted.
> >>>>> Our RMs may not be as active as we want them to be, while they are
> >>>>> historically good at writing policies but it's hard to put them
in
> >>> practice
> >>>>> and further it's understandable that they may not be able to
> volunteer
> >>>>> enough time and effort to get the PRs sorted.
> >>>>>
> >>>>>
> >>>>> Over past months this and similar practices have killed our commit
> and
> >>>>> development momentum, and I think it's not a healthy practice for
our
> >>>>> community to engage in further. Instead, we can have committers
(and
> >> in
> >>>>> future maybe bots) to merge a PR if they have 2 LGTMs, no objections
> >> and
> >>>>> test results from both Travis (simulator) and Bubble/BVT/Trillian
> >> (tests
> >>>>> against at least one and ideally all three hypervisors - KVM, Xen
and
> >>>>> VMware).
> >>>>>
> >>>>>
> >>>>> Regards.
> >>>>>
> >>>>> ________________________________
> >>>>> From: Rajani Karuturi <rajani@apache.org>
> >>>>> Sent: 03 August 2016 13:43:54
> >>>>> To: dev@cloudstack.apache.org
> >>>>> Subject: Re: 4.10.0 release
> >>>>>
> >>>>> ouch.. looks like my email client stripped all the new lines.
> >>>>> Re-sending from webmail
> >>>>>
> >>>>> Hi All,
> >>>>> These are the proposed dates for 4.10 release (copied from another
> >>> thread
> >>>>> by John Burwell)
> >>>>> * Development (master open to features and defect fixes): 1 August
> >> 2016
> >>> -
> >>>>> 11 September 2016
> >>>>> * Testing: 12 - 18 September 2016
> >>>>> * RC Voting: 19 - 25 September 2016
> >>>>> * Release: 26 September 2016
> >>>>>
> >>>>> master is open for 4.10.0.
> >>>>> It still means that only PRs will be merged and they will be merged
> >>> only by
> >>>>> RMs ( For 4.10.0, its John Burwell and Rajani Karuturi)
> >>>>> Every PR should have a JIRA bug ID, 1 code review and 1 test review.
> >>>>> It would help in reviewing if the contributor could put information
> >>> about
> >>>>> the feature/bug and how its tested.
> >>>>> Also, please rebase any pending PRs you have to the latest master
or
> >> the
> >>>>> 4.9 release branch.
> >>>>>
> >>>>> Finally, anyone in the community can review and test PRs. We
> currently
> >>> have
> >>>>> huge backlog. We need everyones help in getting them
> merged(especially
> >>>>> running the tests)
> >>>>> Looking forward for your help in merging PRs.
> >>>>> Happy PR bashing!!
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>>
> >>>>>
> >>>>> ~Rajani
> >>>>>
> >>>>>
> >>>>> rohit.yadav@shapeblue.com
> >>>>> www.shapeblue.com
> >>>>> 53 Chandos Place, Covent Garden, London  WC2N 4HSUK
> >>>>> @shapeblue
> >>>>>
> >>>>>
> >>>>>
> >>>>> On Wed, Aug 3, 2016 at 1:19 PM, Erik Weber <terbolous@gmail.com>
> >> wrote:
> >>>>>
> >>>>>> A newline or two wouldn't hurt, this is pretty hard to read
tbh.
> >>>>>>
> >>>>>> --
> >>>>>> Erik
> >>>>>>
> >>>>>> On Wed, Aug 3, 2016 at 9:27 AM, Rajani Karuturi <rajani@apache.org>
> >>>>> wrote:
> >>>>>>
> >>>>>>> Hi All,These are the proposed dates for 4.10 release (copied
from
> >>>>>>> another thread by John Burwell)* Development (master open
to
> >>>>>>> features and defect fixes): 1 August 2016 - 11 September
2016*
> >>>>>>> Testing: 12 - 18 September 2016* RC Voting: 19 - 25 September
> >>>>>>> 2016* Release: 26 September 2016
> >>>>>>> master is open for 4.10.0. It still means that only PRs
will be
> >>>>>>> merged and they will be merged only by RMs ( For 4.10.0,
its John
> >>>>>>> Burwell and Rajani Karuturi)Every PR should have a JIRA
bug ID, 1
> >>>>>>> code review and 1 test review.It would help in reviewing
if the
> >>>>>>> contributor could put information about the feature/bug
and how
> >>>>>>> its tested.Also, please rebase any pending PRs you have
to the
> >>>>>>> latest master or the 4.9 release branch.
> >>>>>>> Finally, anyone in the community can review and test PRs.
We
> >>>>>>> currently have huge backlog. We need everyones help in getting
> >>>>>>> them merged(especially running the tests)Looking forward
for your
> >>>>>>> help in merging PRs. Happy PR bashing!!
> >>>>>>> Thanks,~ Rajanihttp://cloudplatform.accelerite.com/
> >>>>>>
> >>>>>
> >>>
> >>>
> >>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message