[Starlingx-discuss] Notes: Weekly StarlingX non-OpenStack Distro meeting, 12/12
Agenda & Notes for 12/12 meeting: 1. minor kernel version upgrade to 3.10.0.957 (Shuicheng/Martin) storyboard: https://storyboard.openstack.org/#!/story/2004521 installer update: https://storyboard.openstack.org/#!/story/2004516 (this is depends on 2004521) Martin is currently working on kernel upgrade. Patch rebase for both std and rt kernel will be finished by this week. Will do testing on bare metal deployment. Will ask GDC for testing before merge. 2. preperation for CentOS 7.6 upgrade status (Shuicheng) storyboard: https://storyboard.openstack.org/#!/story/2004522 49 sRPM to be upgraded (+ 2 kernels). hundreds RPMs needs to be upgraded. out-of-tree kernel drivers are not included yet - need to check and see how many drivers need to be upgraded. Saul comments: Dean mentioned yesterday, general Openstack enablement in CentOS 7.6 with RPMs. no validated CentOS with Openstack yet. May need to verify if CentOS 7.6 + Vanilla Openstack work or not, before we port any additional patch. 3. Ceph upgrade status (Vivian/Dehao/Changcheng) Dean merged all the build process patche (on staging stx-ceph and Changcheng finish rebasing all patches according to latest stx-ceph (stx/v13.2.2). totally 17 PRs New PR: https://github.com/starlingx-staging/stx-ceph/pull/18 pending for review. We could build out ISO with below patch list patch list: 1. https://github.com/starlingx-staging/stx-ceph/pull/18 2. https://review.openstack.org/#/c/619460/11 3. https://review.openstack.org/#/c/619463/8 4. https://review.openstack.org/#/c/619465/7 5. https://review.openstack.org/#/c/620449/ 6. https://review.openstack.org/#/c/624085/ 5 & 6 are used to be base version and make ceph patches effect in build (Please use ceph_13_2_2.xml as the manifest file) Submitted 2 PR to staging. Dean reviewed them and merged stx/v13.2.2 branch. Those PR has no impact to other stx module. Current base build is still using the master branch. Other 4 patches are in openStack gerrit review. Image has been build based on the current patch porting. Still cannot work as expected. still working w/ WR to debug the problem under StarlingX system. Ovidiu is working w/ Changcheng to debug the issue. @Changcheng, Please continue work with Ovidiu to debug the issues. Changcheng/Dehao: 1st priority will need to make dedicated storage working w/ new Ceph. Once Ceph is working w/ dedicated storage on controller-config, Frank would like to try the containers. all-in-one simplex already working, dedicated storage is working but not stable. @Mingyuan will help to try Simplex first and then dedicated storage w/ old Ceph. 4. Python2to3 status, flocks and OS packages (Austin) focus on flock service upgrade, still working on stx-discloud; stx-distcloud-client, stx-nfv, stx-integ and stx-config. for other Os packages, out of 436 packages, we still find 73 packages do not have Python3 transition activities. 5. Qemu 3.0 branch switch (Ghada/Jim) Code merged as of Dec 10 https://review.openstack.org/#/c/623045/ https://review.openstack.org/#/c/622583/ no pending patches. eveything is in! Congratulation!!!! 6. Opens (all) -----Original Appointment----- From: Xie, Cindy Sent: Monday, November 5, 2018 2:27 PM To: Xie, Cindy; Wold, Saul; Jones, Bruce E; Troyer, Dean; Lin, Shuicheng; Zhu, Vivian; Shang, Dehao; Liu, ZhipengS; Hu, Yong; Sun, Austin; 'Rowsell, Brent'; 'Khalil, Ghada'; Waheed, Numan; Somerville, Jim; starlingx-discuss@lists.starlingx.io Cc: Perez Carranza, Jose; Armstrong, Robert H; Perez Rodriguez, Humberto I; Martinez Landa, Hayde; Martinez Monroy, Elio; Hu, Wei W; Gomez, Juan P; Lara, Cesar; Arce Moreno, Abraham; Cobbley, David A; Hernandez Gonzalez, Fernando; 'Hellmann, Gil'; 'Waines, Greg'; 'Chen, Jacky'; 'Seiler, Glenn'; 'Eslimi, Dariush'; 'Young, Ken' Subject: Weekly StarlingX non-OpenStack Distro meeting When: Wednesday, December 12, 2018 6:00 AM-7:00 AM (UTC-08:00) Pacific Time (US & Canada). Where: https://zoom.us/j/342730236 . Cadence and time slot: o Wednesday 9AM Winter EDT (10PM China time, US PDT Winter time 6AM) . Call Details: o Zoom link: https://zoom.us/j/342730236 o Dialing in from phone: o Dial(for higher quality, dial a number based on your current location): US: +1 669 900 6833 or +1 646 876 9923 o Meeting ID: 342 730 236 o International numbers available: https://zoom.us/u/ed95sU7aQ . Meeting Agenda and Minutes: o https://etherpad.openstack.org/p/stx-distro-other
On Wed, Dec 12, 2018 at 8:47 AM Xie, Cindy <cindy.xie@intel.com> wrote:
3. Ceph upgrade status (Vivian/Dehao/Changcheng) Dean merged all the build process patche (on staging stx-ceph and Changcheng finish rebasing all patches according to latest stx-ceph (stx/v13.2.2). totally 17 PRs New PR: https://github.com/starlingx-staging/stx-ceph/pull/18 pending for review.
I had asked for the relevant information to be included in the individual commit messages and I still do not see that being done. We are losing valuable information and traceability for why we are making these changes to upstream. Let's look at an example: In [0] we have the following commit message: ---------- Port: RevertMe: Use user root to run ceph services Avoid debugging file permission issues when upgrading to Jewel. This is done to provide the same setup as Hammer in StarlingX. This commit should be reverted when we decide to enable the ceph user. Port From: Ceph Rebase: Disable ceph user/group for Hammer equivalence.patch 0001____src_ceph-disk_ceph_disk_main.py.patch 0002____src_init-ceph.in.patch 0003____wrs_ceph.conf.patch Signed-off-by: Robert Church <robert.church@windriver.com> Signed-off-by: Daniel Badea <daniel.badea@windriver.com> Signed-off-by: Changcheng Liu <changcheng.liu@intel.com> Signed-off-by: Dehao Shang <dehao.shang@intel.com> ---------- This appears to correspond to the original R5 commit c87de31f that has the following commit message: ---------- Ceph Rebase: Disable ceph user/group for Hammer equivalence Use default (root) user to run ceph services instead of dedicated (ceph) user and group to avoid debugging file permission issues while upgrading to Jewel. This is done to provide the same setup as Hammer in TiS. This commit should be reverted when we decide to enable the ceph user. ---------- Notice how the second paragraph of the original message is missing from the new commit. Also, the references to the original commit are not available externally, I have no idea what "0001____src_ceph-disk_ceph_disk_main.py.patch" refers to. So even for someone with access to the original commit I have to do text string searches to attempt to locate it in the R5 repo. It also seems like it would be easier to review and merge these in smaller batches. One big PR with 35 commits takes time to review, and when a single change needs to be made we have to re-review looking for the changes. There is also no reference in either the commit messages or the PR description to a Storyboard story or task or any further documentation to why this work is being done. Think of what you have available while doing this rebase/upgrade and imagine what the next person doing the next rebase/upgrade will want to see and make sure all of that is present in the commit messages. The GitHub PR may or may not be available at that time, only the git commit messages are guaranteed to stay with the code changes. dt [0] https://github.com/starlingx-staging/stx-ceph/pull/18/commits/552736f77f3989... -- Dean Troyer dtroyer@gmail.com
Hi Dean, 1. [Dean] Notice how the second paragraph of the original message is missing from the new commit. [Changcheng] The second paragraph isn't missed in the new patch. You could find: " RevertMe: " & " This commit should be reverted when we decide to enable the ceph user." 2. [Dean] the references to the original commit are not available externally [Changcheng] Yes. The reference should be removed from commit message at last. Originally, I want both Intel & WindRiver engineers could find where the patches are ported from which place at the initial porting stage. 3. [Dean] It also seems like it would be easier to review and merge these in smaller batches. [Changcheng] Yes. I’m syncing with WindRiver engineers to check whether we could merge some patches firstly to avoid times of rebase and review. 4. [Dean] There is also no reference in either the commit messages or the PR description to a Storyboard story or task or any further documentation to why this work is being done. [Changcheng] I’ll add related information in PR message if we agree with merge part of patches firstly. 5. [Dean] only the git commit messages are guaranteed to stay with the code changes. [Changcheng] We’ll give document about stx-ceph upgrade once it’s been upgraded successfully. 6. [Dean] I had asked for the relevant information to be included in the individual commit messages and I still do not see that being done. We are losing valuable information and traceability for why we are making these changes to upstream. [Changcheng] Personally, I think I’ve kept most part of original commit message in the new ported patches. Some huge patch is divided into small patches(If you look the original patch, it’s merged by several patches. It’s hard to be maintained). For PR info, we could give more detail info according to your requirement. B.R. Changcheng [X] -----Original Message----- From: Dean Troyer [mailto:dtroyer@gmail.com] Sent: Thursday, December 13, 2018 12:20 AM To: Xie, Cindy <cindy.xie@intel.com> Cc: starlingx-discuss@lists.starlingx.io Subject: Re: [Starlingx-discuss] Notes: Weekly StarlingX non-OpenStack Distro meeting, 12/12 On Wed, Dec 12, 2018 at 8:47 AM Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> wrote:
3. Ceph upgrade status (Vivian/Dehao/Changcheng) Dean merged all the build process patche (on staging stx-ceph and Changcheng finish rebasing all patches according to latest stx-ceph (stx/v13.2.2). totally 17 PRs New PR: https://github.com/starlingx-staging/stx-ceph/pull/18 pending for review.
I had asked for the relevant information to be included in the individual commit messages and I still do not see that being done. We are losing valuable information and traceability for why we are making these changes to upstream. Let's look at an example: In [0] we have the following commit message: ---------- Port: RevertMe: Use user root to run ceph services Avoid debugging file permission issues when upgrading to Jewel. This is done to provide the same setup as Hammer in StarlingX. This commit should be reverted when we decide to enable the ceph user. Port From: Ceph Rebase: Disable ceph user/group for Hammer equivalence.patch 0001____src_ceph-disk_ceph_disk_main.py.patch 0002____src_init-ceph.in.patch 0003____wrs_ceph.conf.patch Signed-off-by: Robert Church <robert.church@windriver.com<mailto:robert.church@windriver.com>> Signed-off-by: Daniel Badea <daniel.badea@windriver.com<mailto:daniel.badea@windriver.com>> Signed-off-by: Changcheng Liu <changcheng.liu@intel.com<mailto:changcheng.liu@intel.com>> Signed-off-by: Dehao Shang <dehao.shang@intel.com<mailto:dehao.shang@intel.com>> ---------- This appears to correspond to the original R5 commit c87de31f that has the following commit message: ---------- Ceph Rebase: Disable ceph user/group for Hammer equivalence Use default (root) user to run ceph services instead of dedicated (ceph) user and group to avoid debugging file permission issues while upgrading to Jewel. This is done to provide the same setup as Hammer in TiS. This commit should be reverted when we decide to enable the ceph user. ---------- Notice how the second paragraph of the original message is missing from the new commit. Also, the references to the original commit are not available externally, I have no idea what "0001____src_ceph-disk_ceph_disk_main.py.patch" refers to. So even for someone with access to the original commit I have to do text string searches to attempt to locate it in the R5 repo. It also seems like it would be easier to review and merge these in smaller batches. One big PR with 35 commits takes time to review, and when a single change needs to be made we have to re-review looking for the changes. There is also no reference in either the commit messages or the PR description to a Storyboard story or task or any further documentation to why this work is being done. Think of what you have available while doing this rebase/upgrade and imagine what the next person doing the next rebase/upgrade will want to see and make sure all of that is present in the commit messages. The GitHub PR may or may not be available at that time, only the git commit messages are guaranteed to stay with the code changes. dt [0] https://github.com/starlingx-staging/stx-ceph/pull/18/commits/552736f77f3989... -- Dean Troyer dtroyer@gmail.com<mailto:dtroyer@gmail.com> _______________________________________________ Starlingx-discuss mailing list Starlingx-discuss@lists.starlingx.io<mailto:Starlingx-discuss@lists.starlingx.io> http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
On Wed, Dec 12, 2018 at 8:06 PM Liu, Changcheng <changcheng.liu@intel.com> wrote:
2. *[Dean] **the references to the original commit are not available externally* [Changcheng] Yes. The reference should be removed from commit message at last. Originally, I want both Intel & WindRiver engineers could find where the patches are ported from which place at the initial porting stage.
[dt] The reference should NOT be removed. If a patch is being rebased the reference to the original patch must be preserved.
3. *[Dean] **It also seems like it would be easier to review and merge these in smaller batches.* [Changcheng] Yes. I’m syncing with WindRiver engineers to check whether we could merge some patches firstly to avoid times of rebase and review.
[dt] I am not talking about merging commits, I am talking about splitting the existing commits into multiple Github PRs. Please do not merge commits that are not directly related to each other..
4. *[Dean] **There is also no reference in either the commit messages or the PR description to a Storyboard story or task or any further documentation to why this work is being done.* [Changcheng] I’ll add related information in PR message if we agree with merge part of patches firstly.
Please put it into the commit messages. The PR text is not part of the git repo and is lost github is unavailable.
5. *[Dean] **only the git commit messages are guaranteed to stay with the code changes.* [Changcheng] We’ll give document about stx-ceph upgrade once it’s been upgraded successfully.
That does not address the need to put good information into the individual commit messages.
6. *[**Dean**]* *I had asked for the relevant information to be included in the individual commit messages and I still do not see that being done. We are losing valuable information and traceability for why we are making these changes to upstream.* [Changcheng] Personally, I think I’ve kept most part of original commit message in the new ported patches. Some huge patch is divided into small patches(If you look the original patch, it’s merged by several patches. It’s hard to be maintained). For PR info, we could give more detail info according to your requirement.
Thank you for splitting up previously squashed patches. Please do not confuse PR information (the text that is part of a Github PR) with a commit message (the text that is part of a git commit). Github PRs are not the place of record for us. Information that does not fit into a git commit message should be in Storyboard or Launchpad, the two places we keep track of those things. But more importantly, things that the next team that looks at this code will want to have without access to Github or Storyboard or Launchpad needs to be in the commit message. This is exactly the problem we have with the existing patches agains upstream code where we have commitmessages sometimes with only a link to a ticketing system that we do not have access to. I do not want that to continue. dt -- Dean Troyer dtroyer@gmail.com
Hi Dean, Please check my below interleaved reply. Thanks for your suggestion. B.R. Changcheng From: Dean Troyer [mailto:dtroyer@gmail.com] Sent: Friday, December 14, 2018 12:36 AM To: Liu, Changcheng <changcheng.liu@intel.com> Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io Subject: Re: [Starlingx-discuss] Notes: Weekly StarlingX non-OpenStack Distro meeting, 12/12 On Wed, Dec 12, 2018 at 8:06 PM Liu, Changcheng <changcheng.liu@intel.com<mailto:changcheng.liu@intel.com>> wrote: 2. [Dean] the references to the original commit are not available externally [Changcheng] Yes. The reference should be removed from commit message at last. Originally, I want both Intel & WindRiver engineers could find where the patches are ported from which place at the initial porting stage. [dt] The reference should NOT be removed. If a patch is being rebased the reference to the original patch must be preserved. [Changcheng]: I didn’t get your point. Commit message should be kept there. For the reference(how to find the original patch), I’ll keep them in the initial porting stage. Once we verified these ported patches works in the end, I’ll remove the reference since they can’t be accessed by external Intel/WindRiver engineers. Do you mean that we should keep “url link” in the commit message directly? Intel internal github can’t be accessed by external engineers. 3. [Dean] It also seems like it would be easier to review and merge these in smaller batches. [Changcheng] Yes. I’m syncing with WindRiver engineers to check whether we could merge some patches firstly to avoid times of rebase and review. [dt] I am not talking about merging commits, I am talking about splitting the existing commits into multiple Github PRs. Please do not merge commits that are not directly related to each other.. [Changcheng] Some patches are depended on previous patches. We could try to extract patches into new PR later. Currently, we need make sure ceph works locally. The PR could be accessed by WindRiver engineers who’re working with us to debug the problems. 4. [Dean] There is also no reference in either the commit messages or the PR description to a Storyboard story or task or any further documentation to why this work is being done. [Changcheng] I’ll add related information in PR message if we agree with merge part of patches firstly. [dt] Please put it into the commit messages. The PR text is not part of the git repo and is lost github is unavailable. [Changcheng] Yes. Right commit message should be kept there. If you find some commit message is lost, please tell give comment in the patch. Currently, I have kept all the commit message in the right place. For PR message, we could refine them to meet with your requirement. 5. [Dean] only the git commit messages are guaranteed to stay with the code changes. [Changcheng] We’ll give document about stx-ceph upgrade once it’s been upgraded successfully. [dt]That does not address the need to put good information into the individual commit messages. [Changcheng] As I’ve said previously “If you find some commit message is lost, please tell give comment in the patch. Currently, I have kept all the commit message in the right place.” 6. [Dean] I had asked for the relevant information to be included in the individual commit messages and I still do not see that being done. We are losing valuable information and traceability for why we are making these changes to upstream. [Changcheng] Personally, I think I’ve kept most part of original commit message in the new ported patches. Some huge patch is divided into small patches(If you look the original patch, it’s merged by several patches. It’s hard to be maintained). For PR info, we could give more detail info according to your requirement. [dt]Thank you for splitting up previously squashed patches. Please do not confuse PR information (the text that is part of a Github PR) with a commit message (the text that is part of a git commit). Github PRs are not the place of record for us. Information that does not fit into a git commit message should be in Storyboard or Launchpad, the two places we keep track of those things. But more importantly, things that the next team that looks at this code will want to have without access to Github or Storyboard or Launchpad needs to be in the commit message. This is exactly the problem we have with the existing patches agains upstream code where we have commitmessages sometimes with only a link to a ticketing system that we do not have access to. I do not want that to continue. [Changcheng] We could avoid much effort if we didn’t squash several patches into one big patch in stx-ceph/v10.2.2. Thanks for your reminding about the distinguish between PR info and patch commit message. For PR info, we’ll refine them with your requirement. For patch commit message, I haven’t found any serious problem. If you find something wrong, give comment in the patch directly. dt -- Dean Troyer dtroyer@gmail.com<mailto:dtroyer@gmail.com>
participants (3)
-
Dean Troyer
-
Liu, Changcheng
-
Xie, Cindy