Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-06
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
22:38:28 _ix Thanks. I'll give that a shot.
22:48:07 _ix I'm trying to force the build on a single host... but it says it can't find it.
22:49:38 openstackgerrit Eric Fried proposed openstack/nova master: Move aggregates from report client to ProviderTree https://review.openstack.org/521685
22:49:38 openstackgerrit Eric Fried proposed openstack/nova master: Traits ops on ProviderTree https://review.openstack.org/521605
22:49:39 openstackgerrit Eric Fried proposed openstack/nova master: Aggregate ops on ProviderTree https://review.openstack.org/526216
22:49:39 openstackgerrit Eric Fried proposed openstack/nova master: Track provider traits in report client https://review.openstack.org/521686

Earlier   Later