| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-06 | |||
| 20:46:51 | efried | jaypipes I seem to have flubbed https://review.openstack.org/#/c/521605/ which was supposed to be two separate commits (one to add agg stuff to ProviderTree, one to add traits stuff). | |
| 20:47:05 | efried | jaypipes I was about to split it up, but thought I should ask you whether it's worth bothering | |
| 20:47:53 | efried | jaypipes i.e. should I just do both in one change set? | |
| 21:03:46 | mriedem | ildikov: ok, final comments in https://review.openstack.org/#/c/330285/173 | |
| 21:03:54 | mriedem | ildikov: easy things to address and then i'm ready to go on that | |
| 21:04:03 | ildikov | mriedem: ok | |
| 21:04:05 | mriedem | we'll have to decide if we hold it for the shared_targets microversion in cinder | |
| 21:04:32 | ildikov | mriedem: let me ask Jay about the client | |
| 21:05:00 | ildikov | mriedem: as for the new attach patch, did you mean I should take over on those? | |
| 21:05:18 | mriedem | ildikov: i was thinking, in the api change to support multiattach, if/when we check the volume to see if it's multiattach=True, couldn't we just check the cinder microversion at that point if the volume already has >0 attachments? | |
| 21:05:43 | ildikov | mriedem: I haven't touched the API patch in almost years now | |
| 21:05:49 | mriedem | basically, nova api can't attach a multiattach volume to >1 instance until we are sure we can get shared_targets info | |
| 21:05:55 | ildikov | mriedem: so I will take a look shortly and figure out | |
| 21:05:58 | mriedem | ok | |
| 21:06:06 | ildikov | I didn't want to keep those checks, those are from the ancient times | |
| 21:06:14 | mriedem | i can update https://review.openstack.org/#/c/330285/173 quick | |
| 21:06:18 | mriedem | and then i'm done with it i think | |
| 21:06:27 | ildikov | another question is whether it's ok to do the lock for detach in a separate patch? | |
| 21:06:42 | ildikov | so we could land the libvirt change and then the lock and then the API | |
| 21:06:48 | mriedem | if it's a separate patch, it has to come before we start multiattaching things | |
| 21:06:53 | mriedem | oh, | |
| 21:06:54 | ildikov | also, we will bump a microversion for this, right? | |
| 21:07:04 | mriedem | as long as we don't turn on multiattach until the parts in the compute are ready | |
| 21:07:06 | mriedem | i think we can do that | |
| 21:07:20 | mriedem | i think we will bump a microversion as an indication of multiattach yes | |
| 21:07:31 | mriedem | i thought we updated the spec about that | |
| 21:07:55 | ildikov | ok, so we will need an API level check whether the volume is attached already or not independently from Cinder | |
| 21:07:57 | mriedem | oh it's a todo https://specs.openstack.org/openstack/nova-specs/specs/queens/approved/cinder-volume-multi-attach.html#rest-api-impact | |
| 21:08:06 | mriedem | "Whether or not a new compute API microversion is needed will be determined during implementation and code review. API users will need some way to discover if they can perform volume multiattach and a microversion might be the signal, but it is unclear if Nova would block those requests on a lower microversion, e.g. 2.1. It probably makes sense to do a microversion like 2.49 for tagged attach capabilities." | |
| 21:08:48 | ildikov | yeah, we were a bit hesitant | |
| 21:09:24 | ildikov | and it'll kinda require another check_attach too as basically that's what turns multi-attach on on the Nova side | |
| 21:09:28 | ildikov | every other check is in CInder | |
| 21:10:18 | ildikov | the libvirt change is supposed to be harmless as it plays with things only for multi-attach volumes on intention, but it doesn't do any attaching related stuff | |
| 21:10:31 | ildikov | just turns on the shareable flag and adds the capabilities info | |
| 21:25:54 | ildikov | mriedem: so will you upload any more versions, or it's on me now? I got lost on that one. | |
| 21:28:29 | mriedem | i'm doing it | |
| 21:29:07 | ildikov | mriedem: cool, tnx | |
| 21:29:20 | ildikov | mriedem: replied to your comments | |
| 21:30:51 | ildikov | mriedem: as we're adding a new API microversion I think we can land this and deal with the new Cinder microversion in the multi-attach change | |
| 21:31:20 | ildikov | mriedem: as we will need to do some extra checks for that anyway therefore we might as well add the new Cinder microversion to that one | |
| 21:31:39 | mriedem | i tend to agree, | |
| 21:31:47 | ildikov | mriedem: we can do a version check now independently from the attachment_create call so that's not an issue either | |
| 21:31:51 | mriedem | especially since the point of microversions is that the client can opt into whatever they want when it's available (or not) | |
| 21:31:54 | mriedem | and in this case, nova is the client | |
| 21:32:21 | mriedem | so we just won't support a multiattach volume to >1 instance if the new cinder microversion for shared_targets isn't htere | |
| 21:32:26 | mriedem | because we won't be able to safely detach | |
| 21:32:32 | ildikov | yeah, true, I just wanted to avoid the check_attach-like checks as much as possible, but the API microversion in Nova will require that anyway | |
| 21:32:55 | ildikov | yep | |
| 21:33:03 | mriedem | well, the compute microversoin is mainly just a signal | |
| 21:33:24 | ildikov | I mean the Nova API microversion | |
| 21:33:25 | mriedem | 'i want to attach this multiattach volume to >1 instance and i'm opting into that, but i know it might not work too' | |
| 21:34:00 | mriedem | might not work b/c (1) old cinder (2) old computes (3) policy (4) virt driver doesn't support it (5) volume type doens't support it, etc | |
| 21:34:17 | mriedem | (6) uranus is not aligned with pluto | |
| 21:34:37 | ildikov | yeah, if Cinder is mid-old then they will not be able to create a multi-attach volume on the first place | |
| 21:34:45 | ildikov | lol | |
| 21:34:55 | ildikov | I hope I | |
| 21:35:06 | ildikov | 'll never need to debug (6) :) | |
| 21:36:35 | ildikov | anyway, so if we don't want to hold the new attach patch now we can just check whether the volume is already attached and continue if the Cinder microversion is high enough | |
| 21:38:43 | ildikov | however the new flow currently doesn't allow multi-attach in general so we might not need that check | |
| 21:39:27 | ildikov | I need to think about it, but the point is still that I think we all had enough of this patch by now... | |
| 21:41:04 | mriedem | incoming | |
| 21:41:35 | mriedem | or not | |
| 21:41:39 | mriedem | anyway, new patches are up | |
| 21:41:42 | mriedem | https://review.openstack.org/#/c/330285/174/releasenotes/notes/bp-cinder-new-attach-apis-eca854e27a255e3e.yaml is the reno i added | |
| 21:43:21 | ildikov | mriedem: looks good to me | |
| 21:43:43 | ildikov | not that I would've worded anything so far good enough to judge, but still :) | |
| 21:45:15 | mriedem | +2 on the new attach flow patch | |
| 21:45:41 | mriedem | i think johnthetubaguy is on paternity leave | |
| 21:45:51 | mriedem | so we'll likely need gibi and/or melwitt to go through this | |
| 21:46:09 | ildikov | mriedem: wow, we haven't even reach 200... :) | |
| 21:46:38 | edleafe | so it looks like I need to rebase my series. Will that kill the patches that are in the gate? | |
| 21:46:49 | mriedem | edleafe: i can re-approve | |
| 21:47:08 | ildikov | mriedem: gibi is familiar with the patch, but he might not familiar with the whole Cinder-side new flow to +A this | |
| 21:47:33 | mriedem | ildikov: i don't think anyone from nova besides john will be | |
| 21:47:43 | ildikov | mriedem: can check with him tomorrow or look at melwitt with cat eyes from Shrek :) | |
| 21:48:03 | ildikov | mriedem: I know, I'm here if anyone wants to ask questions | |
| 21:48:18 | mriedem | at this point i think i can answer any questions on the nova patches | |
| 21:48:26 | edleafe | mriedem: sure, but won't that mean restarting the whole CI process? They are mostly through the gate now. | |
| 21:48:33 | mriedem | edleafe: yes if they are rebased | |
| 21:49:19 | edleafe | ok, then I'll wait | |
| 21:50:13 | ildikov | mriedem: if I could have just 1/10 of your brain :) | |
| 21:55:32 | melwitt | ildikov: if gibi is already familiar with the patch, check with him first. if he won't be able to look, let me know and I'll look at it | |
| 21:55:54 | mriedem | melwitt: the bottom 2 patches in the series are non-destructive. | |
| 21:56:07 | mriedem | i.e. they lay some groundwork but aren't run through until the 3rd in the series | |
| 21:56:09 | mriedem | turns it all on | |
| 21:56:38 | mriedem | the first patch is a straight up simple refactor, no functional change | |
| 21:56:50 | melwitt | ah, okay | |
| 21:56:59 | ildikov | melwitt: I will check with him tomorrow on the Implement new attach Cinder flow patch | |
| 21:57:28 | ildikov | melwitt: if you can look into the two under it that mriedem mentioned that would be pretty great | |
| 21:57:51 | melwitt | k, can do | |
| 21:59:18 | melwitt | grrr something weird is going on in the api sample tests I'm trying to do for noop quotas. when I set the noop driver in a derived test class, it seems to leak into the parent class tests where I didn't set it | |
| 22:01:47 | ildikov | melwitt: I had that with a mocked value earlier... | |
| 22:02:21 | ildikov | not 100% sure on the reason still, setting up the mock in a different way helped there | |
| 22:35:06 | _ix | Hello again. I'm having some trouble creating a larger node on my new pike deployment. | |
| 22:35:53 | _ix | Any tips on where I ought to look to get around this dreadful 'No valid host was found. There are not enough hosts available' error? | |
| 22:36:43 | _ix | It's a 10vcpu 112gb 20gb root and 3000gb machine. I've definitely got the resources. | |
| 22:36:45 | efried | _ix The compute log is where I would start. | |
| 22:37:05 | _ix | The request doesn't seem to be making it to the compute log. | |
| 22:37:25 | efried | okay, have you checked the sched or conductor logs? | |
| 22:37:48 | melwitt | in that case, I suggest turning on log level debug on the scheduler node, restart scheduler, try it again and check the scheduler log to see which filter is blocking the request out | |