| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-01-25 | |||
| 09:02:54 | gibi | zigo: better to call from nova with format='json' to limit the change to that single call. But I see you arrived to that solution anyhow | |
| 09:03:18 | zigo | Yeah ! | |
| 09:03:31 | zigo | Hopefully, I can just take these patches and do -> stein -> rocky, and then I'm good ! :) | |
| 09:05:31 | zigo | Contrary to what I wrote yesterday, only backporting https://review.opendev.org/c/openstack/nova/+/706897 was enough to get the VMDK check work in Train (plus that oslo.utils patch...). | |
| 09:15:28 | zigo | Shit, other failures ... :/ | |
| 09:18:38 | johnthetubaguy | For security fixes, does the usual branch ordering of merging the fixss apply, I don't remember? i.e. do we just merge each stable patch as it goes green, or we do them in order? | |
| 09:18:59 | johnthetubaguy | (i.e. xena seems ready to go now) | |
| 09:20:31 | zigo | I may need all of https://review.opendev.org/c/openstack/nova/+/711679 after all ... | |
| 09:21:22 | gibi | johnthetubaguy: I don't know about any exception from the stable policy for sec patches but maybe elodilles knows | |
| 09:22:11 | johnthetubaguy | I remember we can't do anything other than +2, by policy, as the review was on the security bug ticket (well for the maintained branches anyways). | |
| 09:30:09 | bauzas | what's the problem with the proposed fix ? | |
| 09:30:51 | johnthetubaguy | bauzas: So zigo has issues in train I think (which I am interested in, sadly), but I am more curious if we can merge stable branches out of order for a security fix like this? | |
| 09:31:25 | zigo | bauzas: Parts of nova changed between train and ussuri, but I think I can manage. | |
| 09:31:33 | zigo | Let me finish the backporting ... :) | |
| 09:31:51 | bauzas | johnthetubaguy: I'm rushing to review the stable branches | |
| 09:32:24 | bauzas | johnthetubaguy: technically, we have a CI job that prevents merging a patch on a stable branch if the parent isn't merged | |
| 09:32:47 | johnthetubaguy | ah, I didn't know that. | |
| 09:33:03 | bauzas | fwiw, ChatGPT is unable to find the typo https://review.opendev.org/c/openstack/oslo.utils/+/706880/4/oslo_utils/imageutils.py despite me giving him clues | |
| 09:33:09 | johnthetubaguy | I see arguments both ways for sure, but I wanted to check. | |
| 09:36:46 | bauzas | so, master is merged | |
| 09:36:59 | bauzas | zed was running on the gate but we got a failure | |
| 09:37:08 | johnthetubaguy | ack | |
| 09:37:09 | bauzas | and I just +2d yoga | |
| 09:37:29 | johnthetubaguy | OK, that was my question really, is that allowed? | |
| 09:37:51 | johnthetubaguy | I guess we just wait for the +W? | |
| 09:38:06 | bauzas | to merge some stable N-x patch before the parents ? | |
| 09:38:45 | bauzas | https://docs.openstack.org/project-team-guide/stable-branches.html#appropriate-fixes | |
| 09:38:53 | bauzas | " Whether the fix is already on master and all consequent stable branches: a change must be a backport of a change already merged onto master, unless the change simply does not make sense on master. Same applies to N-2 releases, where N is master, in which case both N-1 and N branches should have the patch merged and so on." | |
| 09:39:03 | gibi | nova-tox-validate-backport job is voting in the gate queue and I that will prevent merging an older branch before the patch on newer branch lands | |
| 09:39:04 | bauzas | but, there is an exception rule | |
| 09:39:13 | bauzas | " Some patches may get exception from rule 4 above. These are patches that do not touch production code, like test-only patches, or tox.ini changes that fix major gate breakage, etc.; or security patches that should not take much time to merge once the patches are published. In those cases, stable patches may be pushed into gate without waiting for all consequent branches to be fixed." | |
| 09:39:25 | bauzas | gibi: that's what I told to johnthetubaguy | |
| 09:39:42 | johnthetubaguy | yeah, I think we should just merge them all ASAP: "or security patches that should not take much time to merge once the patches are published" | |
| 09:39:44 | gibi | bauzas: so even though the policy allow secu patches to land our tooling did not | |
| 09:39:55 | bauzas | gibi: I was about to write this | |
| 09:39:59 | johnthetubaguy | ah, got you | |
| 09:40:05 | bauzas | somehow we are strictier than the policy | |
| 09:40:45 | johnthetubaguy | so what is the quickest path to merge? in parallel drop that CI job? | |
| 09:40:49 | bauzas | johnthetubaguy: either way, you know we'll need to publish releases | |
| 09:41:07 | bauzas | upstream isn't generally a quick path for remediation | |
| 09:41:14 | bauzas | but we can try to rush | |
| 09:41:21 | johnthetubaguy | its meant to be though, in this particular case. | |
| 09:41:40 | bauzas | johnthetubaguy: that's why distros got notified one week before the disclosure | |
| 09:41:46 | johnthetubaguy | granted the patches are published, which is the important and crucial bit. | |
| 09:42:18 | bauzas | I don't disagree and the security hole is quite dirty | |
| 09:42:41 | bauzas | I would recommend our operators to exceptionnally patch directly | |
| 09:43:00 | bauzas | the patch itself is just about enforcing | |
| 09:46:23 | gibi | one way to rush this is to add [stable-only] to the commit message to silence the backport job | |
| 09:47:41 | bauzas | that's debatable | |
| 09:47:49 | johnthetubaguy | gibi: I like your thinking! | |
| 09:48:11 | bauzas | gibi: your help may be needed on https://review.opendev.org/c/openstack/nova/+/871624 | |
| 09:48:46 | bauzas | johnthetubaguy: which release do you target to ship the security fix ? | |
| 09:49:23 | elodilles | o/ | |
| 09:49:37 | gibi | bauzas: I can approve that but the job will kick it out from the gate | |
| 09:49:44 | elodilles | let me know if any patch needs review | |
| 09:49:50 | gibi | elodilles: it is more like a process thing | |
| 09:49:53 | bauzas | elodilles: we're on the CVE backports | |
| 09:50:03 | gibi | elodilles: the stable policy allows parallel merge of secu stable fixes | |
| 09:50:03 | elodilles | (i'm not aware of any special handling of security bug fixes) | |
| 09:50:06 | bauzas | elodilles: the zed one got kicked out due to some gate failure | |
| 09:50:09 | gibi | elodilles: but the backport job does not | |
| 09:50:43 | gibi | elodilles: do you have a magic want to turn off the job or merge the patches against that job? | |
| 09:50:43 | elodilles | ok, then [stable-only] is right | |
| 09:50:46 | bauzas | gibi: I'd say that if the stable policy agrees on that, we could refer to it if we respin the patch to add the [stable-only] tag | |
| 09:50:56 | gibi | OK then we have a consensus | |
| 09:51:04 | elodilles | if you want them to merge not in the known order | |
| 09:51:08 | gibi | bauzas: will you add the [stable-only] tags? | |
| 09:51:10 | bauzas | ideally, I'd rather prefer to have a specific tag but we're a bit on a rush | |
| 09:51:12 | johnthetubaguy | bauzas: we have a zoo of people on different things, I am not worried about them, thats all happening, I am more worried in general about it not landing in the stable branches. | |
| 09:51:32 | gibi | bauzas: then elodilles and me can approve qucikly | |
| 09:51:42 | johnthetubaguy | what about [stable-only][actually-a-cve] ? | |
| 09:52:05 | gibi | johnthetubaguy: adding a sentence why we add stable-only make sense, yes | |
| 09:52:11 | elodilles | :] | |
| 09:52:35 | gibi | and as a follow up we should extend the job to accept [cve] as a tag to silence the landing order check | |
| 09:52:45 | johnthetubaguy | gibi: +1 | |
| 09:53:00 | bauzas | johnthetubaguy: as a reminder, train requires some oslo.utils bump due to https://review.opendev.org/c/openstack/oslo.utils/+/706880 missing | |
| 09:53:34 | johnthetubaguy | bauzas: appreciated, thanks, I saw zigo is working through that. | |
| 09:56:04 | zigo | Either oslo bump, or backport of https://review.opendev.org/c/openstack/oslo.utils/+/706880 | |
| 09:56:07 | zigo | I did the later ... | |
| 09:56:32 | zigo | YEAH !!! | |
| 09:56:38 | zigo | Got something that worked ! \o/ | |
| 09:56:45 | bauzas | gibi: if we start parallelizing the backports, then we'll break the sha1 information on the commit msgs | |
| 09:57:11 | gibi | bauzas: ignore the sha (it does not have it now either) as we disable the check anyhow | |
| 09:57:11 | bauzas | since there is no guarantee that the commit in the gerrit branch will have the same sha1 once merged | |
| 09:57:29 | bauzas | gibi: I'll then mention the gerrit links | |
| 09:57:50 | gibi | bauzas: the bug ref is there to make a connection, but yes if you want you can add gerrit change id | |
| 09:57:53 | gibi | or link | |
| 09:58:09 | bauzas | unfortunately, we can't also track all commits easily as they're on different branches | |
| 09:58:55 | zigo | https://salsa.debian.org/openstack-team/services/nova/-/blob/debian/train/debian/patches/images_Make_JSON_the_default_output_format_of_calls_to_qemu-img_info.patch | |
| 09:58:55 | zigo | https://salsa.debian.org/openstack-team/services/nova/-/blob/debian/train/debian/patches/images_Move_qemu-img_info_calls_into_privsep.patch | |
| 09:58:55 | zigo | https://salsa.debian.org/openstack-team/services/nova/-/blob/debian/train/debian/patches/cve-2022-47951-nova-stable-train.patch | |
| 09:58:55 | zigo | So, I did: | |
| 09:58:56 | zigo | With these 3 patches, all unit tests are passing. | |
| 09:59:58 | zigo | Is this acceptable (from your Nova upstream point of view) to backport these 3 patches on the train branch? | |
| 10:01:39 | bauzas | zigo: propose the backports | |
| 10:01:54 | bauzas | Train is EM | |
| 10:02:08 | zigo | bauzas: The only issue is that I apply them in the wrong order, so patch 1 and 2 may have failures ... | |
| 10:02:22 | zigo | Can I just merge the 3 patches then? | |
| 10:02:37 | johnthetubaguy | zigo: +1 that looks like a nice solution to me. You can submit them as a patch chain in gerrit, that should work OK I think? | |
| 10:02:39 | bauzas | then I need to review those patches | |