Ceph backports workflow update

Ceph backports workflow update

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 4507 bytes --]

Hi Ceph,

A month ago the following workflow was posted and I began to implement it.

> 0. Developer follows normal process to land PR to master. Once complete and ticket is marked Pending Backport this process initiates.

There were a few inconsistencies but they were easy to fix. When the tag is missing I update it manually (the redmine API is broken http://tracker.ceph.com/issues/10727 otherwise I would probably have written a script to semi-manually do that).

> 1. I periodically polls Redmine to look for tickets in Pending Backport state and focus on the ones that are left unattended for too long

I focused on giant and dumpling and was able to help with a few backports. However, I've not yet visited the majority of the issues that need attention ( see http://workbench.dachary.org/ceph/ceph-backports/wikis/dumpling#issues-that-need-backporting for dumpling and http://workbench.dachary.org/ceph/ceph-backports/wikis/giant#issues-that-need-backporting for giant ).

> 1a. Under the supervision of the author of the original patch, I find the commits associated with the Redmine ticket and Cherry Pick to the backport integration branch off of the desired maintenance branch (Dumping, Firefly, etc).
> 1b. I resolve any merge conflicts with the cherry-picked commit

It turns out that finding the relevant commits in almost all backports is made possible by the cross references between pull requests, commits and issues. I was able to backport commits that are trivial. Most of the other backports were done by the original author of the patch because I did not understand enough of the context to be helpful. 

> 2. I merge all backports for a given branch in an integration branch

It is done with something like

git merge --strategy octopus backports/pull/3439/head backports/pull/3552/head backports/pull/3489/head

and there currently are two integration branches:

* pull requests in dumpling-backports http://workbench.dachary.org/ceph/ceph-backports/wikis/dumpling#included-and-tested-in-integration-branch
* pull requests in giant-backports http://workbench.dachary.org/ceph/ceph-backports/wikis/giant#included-and-tested-in-integration-branch

Only once did I face a merge conflict. It was trivial and I resolved it. I should go back to the author of the patch and warn him about this conflict so that it does not create difficulties when all branches are finally merged.

> 3. I ask the leads of each project to review the integration

I did not do that. If I had been able to backport the non trivial commits, it may have been necessary. But I've been in contact with the leads regarding the individual backports and reviewing the set of commits being integrated seemed redundant. 

> 4. Once satisfied with group of backported commits to integration branch, I notify QE.

Running rbd, rados, rgw and is the bulkd of the work. The progress of the test runs and analysis are:

* dumpling http://tracker.ceph.com/issues/10560
* giant http://tracker.ceph.com/issues/10501

This is usually done as a mail thread in the ceph-qa mailing list but I did not want to disturb the list with the backports. In addition I felt the need to see the past analysis on a single page to remember where I was after a few days doing something else. The mail thread format did not provide that and I prefered to create a ticket for that purpose. It's proven both useful and cumbersome, I'll try to figure out something that shows the same information without so much manual maintenance.

> 5. QE tests backport integration branch against appropriate suites
> 6a. If QE is satisfied with test results, they merge backport integration branch.
> 6b. If QE is NOT satisfied with the test results, they indicate backport integration branch is NOT ready to merge and return to me to work with original Developer to resolve issue and return to steps 2/3
> 7. Ticket is moved to Resolved once backport integration branch containing cherry-picked backport is merged to the desired mainteance branch(es)

I've not yet reached this point, to be continued :-)

Cheers

P.S. Manually investigating each backport proved to be extremely tedious and repetitive. Fortunately there are patterns that allowed me to grow a script that creates an inventory for each branch to be backported ( http://workbench.dachary.org/ceph/ceph-backports/wikis/dumpling etc. ) that I use as my landing page when working on backports.

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Ceph backports workflow update

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 10708 bytes --]

Hi Ceph,

Yesterday the dumpling & giant backport integration branches were approved by Yehuda, Sam and Josh and were handed over to QE. An interesting discussion followed and it revealed that my understanding of the intended workflow was significantly wrong. It took me a while to figure out something that makes sense. There is a good chance that I'm wrong again, please speak up :-)

The workflow proposed a month ago was the following:

0. Developer follows normal process to land PR to master. Once complete and ticket is marked Pending Backport this process initiates.
1. I periodically polls Redmine to look for tickets in Pending Backport state and focus on the ones that are left unattended for too long
1a. Under the supervision of the author of the original patch, I find the commits associated with the Redmine ticket and Cherry Pick to the backport integration branch off of the desired maintenance branch (Dumping, Firefly, etc).
1b. I resolve any merge conflicts with the cherry-picked commit
2. I merge all backports for a given branch in an integration branch
3. I ask the leads of each project to review the integration
4. Once satisfied with group of backported commits to integration branch, I notify QE.
5. QE tests backport integration branch against appropriate suites
6a. If QE is satisfied with test results, they merge backport integration branch.
6b. If QE is NOT satisfied with the test results, they indicate backport integration branch is NOT ready to merge and return to me to work with original Developer to resolve issue and return to steps 2/3
7. Ticket is moved to Resolved once backport integration branch containing cherry-picked backport is merged to the desired mainteance branch(es)-- 

I think it should be modified to something like the following: (I have the role of "the backporter"):

0. Developer follows normal process to land PR to master. Once complete and ticket is marked Pending Backport this process initiates.
1. A cron job updates an inventory of the backports for each branch (see http://workbench.dachary.org/ceph/ceph-backports/wikis/firefly for instance)
1a. The PR does not require integration (e.g. it changes the qa directory), it is merged in the release branch (dumpling, firefly, etc.) by the reviewer
1b. The PR requires integration and the label "needs-qa" is added to it by the reviewer.
2. If the inventory page shows PR that are pending integration
2a. The backporter merges them in the dumpling-backports, etc. integration branch
2b. The backporter runs the rados, rgw, rbd etc. suites on the integration branch (if no rgw backports are present, rgw suite can be skipped etc.)
2c. The backporter sorts and analyzes the test results, asking the developer if necessary
2d. If a problem is found on a PR, the backporter removes the "needs-qa" label from the PR and makes sure the developer is aware that integration failed and why
2e. The PR successfully integrated are merged into the dumpling, firelfy, etc. branch
3. A cron job runs various suites on the dumpling, firefly, etc. branches (AKA "the nightlies")
3a. The nightlies send reports to the ceph-qa list when a suite completes 
3b. The leads of each component are responsible for analyzing and updating tracker.ceph.com accordingly
4. The backporter periodically asks each lead if they think it is ready for the next point release
4a. If it is not ready, the backporter offers his help to make progress so that PR resolving the remaining issues are eventually merged in dumpling, firefly etc.  and restart at step 0.
4b. If it is ready the backporter records the SHA of the commit for the given component
5. The backporter loops back to stp 0 until all leads think the same SHA is ready for the next point release
6. Given the SHA agreed on by all leads, QE tests the dumpling, firefly, etc. branch against the appropriate suites
6a. If QE is NOT satisfied with the test results, they update the tracker.ceph.com issues accordingly and asks the backporter to make sure they are addressed
6b. If QE is satisfied with test results, the SHA is provided to the person responsible for publishing the next release

The main difference is that QE approval is on a SHA instead of being the action of merging from a branch to another. In the backport development cycle there are multiple integration branches and QE is not involved at this point. The continuous analysis and fixes of each dumpling, firefly, etc. branch currently depends on the fact that PR are merged on a regular basis and the results of the nightlies also analyzed on a regular basis via the reports sent to the ceph-qa list. This workflow is in place and works. If QE was to merge an integration branch into the release branch (dumpling, firefly, etc.) that would disrupt this workflow. What really matters is to figure out exactly which SHA has been properly tested according to QE so that the release is made on this SHA and not another. 

The other difference is about updating the tracker to change the tickets from Pending Backport to Resolved. In this workflow the developer and backporter are responsible for changing the state, as they do when working on master. The inventory of the backport effort provides a list of all issues addressed by the PR merged into the dumpling, firefly, etc. branch. Instead of batch updating the tickets, QE can use the inventory to check if all issues have been addressed. The former workflow required discipline by developers and if a ticket was accidentally set to Resolved, QE would have a difficulties finding it. The inventory (e.g. http://workbench.dachary.org/ceph/ceph-backports/wikis/firefly) automates the issue discovery process and QE will see all issues related to the pending release, regarless of their status.

I think this process is actually simpler than the former one, requires less changes in the habits of everyone invovled, improves the visibility of the ongoing backport effort and I'm happy about it.

Feel free to criticize bluntly, now is the time :-)

On 03/02/2015 14:32, Loic Dachary wrote:
> Hi Ceph,
> 
> A month ago the following workflow was posted and I began to implement it.
> 
>> 0. Developer follows normal process to land PR to master. Once complete and ticket is marked Pending Backport this process initiates.
> 
> There were a few inconsistencies but they were easy to fix. When the tag is missing I update it manually (the redmine API is broken http://tracker.ceph.com/issues/10727 otherwise I would probably have written a script to semi-manually do that).
> 
>> 1. I periodically polls Redmine to look for tickets in Pending Backport state and focus on the ones that are left unattended for too long
> 
> I focused on giant and dumpling and was able to help with a few backports. However, I've not yet visited the majority of the issues that need attention ( see http://workbench.dachary.org/ceph/ceph-backports/wikis/dumpling#issues-that-need-backporting for dumpling and http://workbench.dachary.org/ceph/ceph-backports/wikis/giant#issues-that-need-backporting for giant ).
> 
>> 1a. Under the supervision of the author of the original patch, I find the commits associated with the Redmine ticket and Cherry Pick to the backport integration branch off of the desired maintenance branch (Dumping, Firefly, etc).
>> 1b. I resolve any merge conflicts with the cherry-picked commit
> 
> It turns out that finding the relevant commits in almost all backports is made possible by the cross references between pull requests, commits and issues. I was able to backport commits that are trivial. Most of the other backports were done by the original author of the patch because I did not understand enough of the context to be helpful. 
> 
>> 2. I merge all backports for a given branch in an integration branch
> 
> It is done with something like
> 
> git merge --strategy octopus backports/pull/3439/head backports/pull/3552/head backports/pull/3489/head
> 
> and there currently are two integration branches:
> 
> * pull requests in dumpling-backports http://workbench.dachary.org/ceph/ceph-backports/wikis/dumpling#included-and-tested-in-integration-branch
> * pull requests in giant-backports http://workbench.dachary.org/ceph/ceph-backports/wikis/giant#included-and-tested-in-integration-branch
> 
> Only once did I face a merge conflict. It was trivial and I resolved it. I should go back to the author of the patch and warn him about this conflict so that it does not create difficulties when all branches are finally merged.
> 
>> 3. I ask the leads of each project to review the integration
> 
> I did not do that. If I had been able to backport the non trivial commits, it may have been necessary. But I've been in contact with the leads regarding the individual backports and reviewing the set of commits being integrated seemed redundant. 
> 
>> 4. Once satisfied with group of backported commits to integration branch, I notify QE.
> 
> Running rbd, rados, rgw and is the bulkd of the work. The progress of the test runs and analysis are:
> 
> * dumpling http://tracker.ceph.com/issues/10560
> * giant http://tracker.ceph.com/issues/10501
> 
> This is usually done as a mail thread in the ceph-qa mailing list but I did not want to disturb the list with the backports. In addition I felt the need to see the past analysis on a single page to remember where I was after a few days doing something else. The mail thread format did not provide that and I prefered to create a ticket for that purpose. It's proven both useful and cumbersome, I'll try to figure out something that shows the same information without so much manual maintenance.
> 
>> 5. QE tests backport integration branch against appropriate suites
>> 6a. If QE is satisfied with test results, they merge backport integration branch.
>> 6b. If QE is NOT satisfied with the test results, they indicate backport integration branch is NOT ready to merge and return to me to work with original Developer to resolve issue and return to steps 2/3
>> 7. Ticket is moved to Resolved once backport integration branch containing cherry-picked backport is merged to the desired mainteance branch(es)
> 
> I've not yet reached this point, to be continued :-)
> 
> Cheers
> 
> P.S. Manually investigating each backport proved to be extremely tedious and repetitive. Fortunately there are patterns that allowed me to grow a script that creates an inventory for each branch to be backported ( http://workbench.dachary.org/ceph/ceph-backports/wikis/dumpling etc. ) that I use as my landing page when working on backports.
> 

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Ceph backports workflow update

[Gregory Farnum]

On Wed, Feb 11, 2015 at 8:42 AM, Loic Dachary <loic@dachary.org> wrote:
> Hi Ceph,
>
> Yesterday the dumpling & giant backport integration branches were approved by Yehuda, Sam and Josh and were handed over to QE. An interesting discussion followed and it revealed that my understanding of the intended workflow was significantly wrong. It took me a while to figure out something that makes sense. There is a good chance that I'm wrong again, please speak up :-)
>
> The workflow proposed a month ago was the following:
>
> 0. Developer follows normal process to land PR to master. Once complete and ticket is marked Pending Backport this process initiates.
> 1. I periodically polls Redmine to look for tickets in Pending Backport state and focus on the ones that are left unattended for too long
> 1a. Under the supervision of the author of the original patch, I find the commits associated with the Redmine ticket and Cherry Pick to the backport integration branch off of the desired maintenance branch (Dumping, Firefly, etc).
> 1b. I resolve any merge conflicts with the cherry-picked commit
> 2. I merge all backports for a given branch in an integration branch
> 3. I ask the leads of each project to review the integration
> 4. Once satisfied with group of backported commits to integration branch, I notify QE.
> 5. QE tests backport integration branch against appropriate suites
> 6a. If QE is satisfied with test results, they merge backport integration branch.
> 6b. If QE is NOT satisfied with the test results, they indicate backport integration branch is NOT ready to merge and return to me to work with original Developer to resolve issue and return to steps 2/3
> 7. Ticket is moved to Resolved once backport integration branch containing cherry-picked backport is merged to the desired mainteance branch(es)--
>
> I think it should be modified to something like the following: (I have the role of "the backporter"):
>
> 0. Developer follows normal process to land PR to master. Once complete and ticket is marked Pending Backport this process initiates.
> 1. A cron job updates an inventory of the backports for each branch (see http://workbench.dachary.org/ceph/ceph-backports/wikis/firefly for instance)

How's it updating that inventory?

> 1a. The PR does not require integration (e.g. it changes the qa directory), it is merged in the release branch (dumpling, firefly, etc.) by the reviewer

Who are you expecting to create backport PRs? Way back when I thought
that you were assembling branches and PRs based on tickets in the
"Pending Backport" state, but now that looks not to be the case.

> 1b. The PR requires integration and the label "needs-qa" is added to it by the reviewer.

I thought the needs-qa label was a flag that it should go into a
master integration branch (wip-sam-testing, wip-sage-testing, w/e);
aren't the milestone LTS labels sufficient to indicate they should be
tested in integration?

> 2. If the inventory page shows PR that are pending integration
> 2a. The backporter merges them in the dumpling-backports, etc. integration branch
> 2b. The backporter runs the rados, rgw, rbd etc. suites on the integration branch (if no rgw backports are present, rgw suite can be skipped etc.)
> 2c. The backporter sorts and analyzes the test results, asking the developer if necessary
> 2d. If a problem is found on a PR, the backporter removes the "needs-qa" label from the PR and makes sure the developer is aware that integration failed and why
> 2e. The PR successfully integrated are merged into the dumpling, firelfy, etc. branch
> 3. A cron job runs various suites on the dumpling, firefly, etc. branches (AKA "the nightlies")
> 3a. The nightlies send reports to the ceph-qa list when a suite completes
> 3b. The leads of each component are responsible for analyzing and updating tracker.ceph.com accordingly

Mmm. I'm happy to look at suites that get run this way but I'm
unlikely to notice them go by on the list if I'm not poked about them
— I generally filter out anything that has a person's name in it or
whatever.
-Greg
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Ceph backports workflow update

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 6045 bytes --]



On 11/02/2015 18:27, Gregory Farnum wrote:
> On Wed, Feb 11, 2015 at 8:42 AM, Loic Dachary <loic@dachary.org> wrote:
>> Hi Ceph,
>>
>> Yesterday the dumpling & giant backport integration branches were approved by Yehuda, Sam and Josh and were handed over to QE. An interesting discussion followed and it revealed that my understanding of the intended workflow was significantly wrong. It took me a while to figure out something that makes sense. There is a good chance that I'm wrong again, please speak up :-)
>>
>> The workflow proposed a month ago was the following:
>>
>> 0. Developer follows normal process to land PR to master. Once complete and ticket is marked Pending Backport this process initiates.
>> 1. I periodically polls Redmine to look for tickets in Pending Backport state and focus on the ones that are left unattended for too long
>> 1a. Under the supervision of the author of the original patch, I find the commits associated with the Redmine ticket and Cherry Pick to the backport integration branch off of the desired maintenance branch (Dumping, Firefly, etc).
>> 1b. I resolve any merge conflicts with the cherry-picked commit
>> 2. I merge all backports for a given branch in an integration branch
>> 3. I ask the leads of each project to review the integration
>> 4. Once satisfied with group of backported commits to integration branch, I notify QE.
>> 5. QE tests backport integration branch against appropriate suites
>> 6a. If QE is satisfied with test results, they merge backport integration branch.
>> 6b. If QE is NOT satisfied with the test results, they indicate backport integration branch is NOT ready to merge and return to me to work with original Developer to resolve issue and return to steps 2/3
>> 7. Ticket is moved to Resolved once backport integration branch containing cherry-picked backport is merged to the desired mainteance branch(es)--
>>
>> I think it should be modified to something like the following: (I have the role of "the backporter"):
>>
>> 0. Developer follows normal process to land PR to master. Once complete and ticket is marked Pending Backport this process initiates.
>> 1. A cron job updates an inventory of the backports for each branch (see http://workbench.dachary.org/ceph/ceph-backports/wikis/firefly for instance)
> 
> How's it updating that inventory?

I wrote a script that does cross referencing and writes this page. It's been a precious tool for me to not get lost ;-) It is still in a state of flux http://workbench.dachary.org/dachary/ceph_workbench and will solidify each time I use / update it.

> 
>> 1a. The PR does not require integration (e.g. it changes the qa directory), it is merged in the release branch (dumpling, firefly, etc.) by the reviewer
> 
> Who are you expecting to create backport PRs? Way back when I thought
> that you were assembling branches and PRs based on tickets in the
> "Pending Backport" state, but now that looks not to be the case.

Sometime the developer does spontaneously. Sometime the developer marks the "backport" field in the ticket and forgets about it. Then I look at the backport, try to do it myself and if I can't I ask the developer (kindly ;-) if he has time to do it. Sometime the developer will ask me "that needs backporting" and I'll update the tickets and try to backport successfully and only report back to the developer when the test results come back green or red (that happened with ~15 backports for rgw). Sometime the lead tells me : we can't release this, we need to backport this issue (that happened with rbd and Jason quickly backported the missing issue because it was the last piece of the puzzle). 

>> 1b. The PR requires integration and the label "needs-qa" is added to it by the reviewer.
> 
> I thought the needs-qa label was a flag that it should go into a
> master integration branch (wip-sam-testing, wip-sage-testing, w/e);
> aren't the milestone LTS labels sufficient to indicate they should be
> tested in integration?

The "needs-qa" has no meaning when it comes to stable branch releases, that's why I proposed to use it. The purpose is essentially the same. There is a need to distinguish between PR that needs integration and the others. For instance the tests suite patches that you merged in dumpling do not need qa and you merged them directly. If there is no label to distinguish between the two, there will be confusion and tools cannot be written to pick the "needs-qa" PR.

> 
>> 2. If the inventory page shows PR that are pending integration
>> 2a. The backporter merges them in the dumpling-backports, etc. integration branch
>> 2b. The backporter runs the rados, rgw, rbd etc. suites on the integration branch (if no rgw backports are present, rgw suite can be skipped etc.)
>> 2c. The backporter sorts and analyzes the test results, asking the developer if necessary
>> 2d. If a problem is found on a PR, the backporter removes the "needs-qa" label from the PR and makes sure the developer is aware that integration failed and why
>> 2e. The PR successfully integrated are merged into the dumpling, firelfy, etc. branch
>> 3. A cron job runs various suites on the dumpling, firefly, etc. branches (AKA "the nightlies")
>> 3a. The nightlies send reports to the ceph-qa list when a suite completes
>> 3b. The leads of each component are responsible for analyzing and updating tracker.ceph.com accordingly
> 
> Mmm. I'm happy to look at suites that get run this way but I'm
> unlikely to notice them go by on the list if I'm not poked about them
> — I generally filter out anything that has a person's name in it or
> whatever.

Ok. Do you look into the results for fs produced by the nightlies for the stable branches ? 

Cheers

> -Greg
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Ceph backports workflow update

[Gregory Farnum]

On Wed, Feb 11, 2015 at 10:19 AM, Loic Dachary <loic@dachary.org> wrote:
> On 11/02/2015 18:27, Gregory Farnum wrote:
>> Mmm. I'm happy to look at suites that get run this way but I'm
>> unlikely to notice them go by on the list if I'm not poked about them
>> — I generally filter out anything that has a person's name in it or
>> whatever.
>
> Ok. Do you look into the results for fs produced by the nightlies for the stable branches ?

Yes.
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html