| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-30 | |||
| 15:48:21 | mriedem | we just need prelude | |
| 15:48:38 | mriedem | we might also want some things in the placement devref about the scheduler integration and config required for comptues in ocata? | |
| 15:48:43 | mriedem | cdent: ^ | |
| 15:49:22 | cdent | mriedem: you mean placement.rst or placement_dev.rst? | |
| 15:49:34 | cdent | (or both) | |
| 15:49:38 | bauzas | mriedem: yup, I was thinking of a prelude section | |
| 15:49:46 | mriedem | cdent: the former | |
| 15:49:49 | bauzas | mriedem: that's something I can write | |
| 15:50:25 | mriedem | cdent: more like notes for operators, i.e. nova-compute won't start with ocata code unless placement is configured in nova.conf, | |
| 15:51:08 | mriedem | and maybe just a general note that the filter scheduler will not use placement in ocata while there are <ocata computes, but that's going to be a hard fail in pike | |
| 15:52:36 | mdbooth | mriedem: I just sent this to the operators ML: http://lists.openstack.org/pipermail/openstack-operators/2017-January/012540.html | |
| 15:52:53 | mdbooth | Would that require a BP or similar? Or would a link to the ML post be sufficient? | |
| 15:54:46 | mriedem | mdbooth: bp to remove a broken feature? | |
| 15:54:52 | mriedem | mdbooth: i think we can just bug fix it in pike | |
| 15:55:06 | mdbooth | mriedem: That's what I hoped you'd say :) | |
| 15:55:07 | mriedem | mdbooth: i think cfriesen and the windriver people are using lvm | |
| 15:55:10 | mriedem | so might want to ask them about it | |
| 15:55:19 | mdbooth | cfriesen: Sparse lvm? | |
| 15:55:21 | mdbooth | mriedem: Thanks | |
| 15:56:03 | mriedem | cdent: L17 here if you want to sign up https://etherpad.openstack.org/p/nova-ocata-rc1-todos | |
| 15:57:01 | purplerbot | <bauzas> mriedem: that's something I can write [2017-01-30 15:49:49] [n 387] | |
| 15:57:01 | cdent | I thought bauzas said he'd take it: [t 387] ? | |
| 15:57:17 | mdbooth | Incidentally, the issue with it seems to be that it relies on a thinpool being created implicitly, which is created as 64MB on a default setup. This means that as soon as you write more than 64MB of data to your instance you start getting io errors. Which is nice. | |
| 15:57:41 | mdbooth | So a cirros instance works on initial boot, but not if you leave it for a bit. | |
| 15:57:44 | cdent | (happy to do it, just don't want to duplicate the effort) | |
| 15:57:45 | ildikov | johnthetubaguy: mriedem TBH I don't think that patch is that risky | |
| 15:59:39 | mriedem | cdent: i thought bauzas was going to do the release note prelude section | |
| 15:59:41 | mriedem | but either way | |
| 16:00:29 | cdent | mriedem: if it's not clear when I finish what I'm doing right now, I'll take a stab | |
| 16:01:26 | mdbooth | mriedem: Is this an rc1 potential bug as in your etherpad: https://review.openstack.org/#/c/383859/13 | |
| 16:01:27 | bauzas | mriedem: cdent: I can do both, I don't care that much | |
| 16:01:40 | bauzas | mriedem: cdent: I was mentioning the prelude section but anyway | |
| 16:01:53 | mriedem | mdbooth: it's not a regression right? | |
| 16:02:06 | mdbooth | mriedem: Nope, but it's a bug. | |
| 16:02:14 | mriedem | mdbooth: sure, but we have lots of bugs | |
| 16:02:23 | mriedem | mdbooth: having said all of that, i was planning on trying to get that one in yet | |
| 16:02:23 | mdbooth | mriedem: Right, but this one has a well-tested fix. | |
| 16:02:29 | mdbooth | Thanks | |
| 16:07:56 | ildikov | johnthetubaguy: mriedem: so just to ensure we're on the same page, the plan now is to get that remove check_attach patch landed, when master opens for Pike? | |
| 16:08:21 | mriedem | ildikov: that's what i would prefer | |
| 16:08:27 | mriedem | since we're 4 days from rc1 | |
| 16:09:01 | ildikov | I guess as the new Cinder API changes will be tied to a version etc. we don't necessarily have to land it a cycle earlier | |
| 16:09:17 | ildikov | as it makes those changes easier to do and follow | |
| 16:10:12 | ildikov | mriedem: so I can annoy you to hell with this, when master opens then? | |
| 16:10:27 | ildikov | mriedem: as it's been two cycles now... :/ | |
| 16:10:41 | ildikov | mriedem: and it's really just a bugfix on its own | |
| 16:11:26 | mriedem | yeah, but to be fair, it's also super latent behavior | |
| 16:12:37 | ildikov | mriedem: I see this as one more reason to remove :) | |
| 16:13:04 | artom | mriedem, so I wrote the spec for the device role tagging bug: https://review.openstack.org/#/c/426030/ | |
| 16:13:40 | artom | I poked johnthetubaguy to review it, dunno how much bandwidth he has to spare - it'd be awesome if we can fix this for ocata to avoid a cycle without device tagging (yeah, the client the can version pinned, I know) | |
| 16:13:51 | artom | dansmith ^^ same to you :) | |
| 16:13:59 | artom | Many thanks in advance, gentlemen :) | |
| 16:14:16 | openstackgerrit | Dan Smith proposed openstack/nova: Allow scheduler to run cell host discovery periodically https://review.openstack.org/426826 | |
| 16:14:51 | mriedem | artom: i know, i have it open in a tab | |
| 16:16:08 | dansmith | do we really have to add another microversion to bring that back in? | |
| 16:16:56 | mriedem | jordanP: left a question in https://review.openstack.org/#/c/426259/ - i don't really see how that doesn't break the case where the admin is listing instances for a specific tenant | |
| 16:17:41 | mriedem | dansmith: i wrestled with that | |
| 16:18:01 | mriedem | it is an api change though | |
| 16:18:33 | mriedem | i.e. trying to tag bdms on server boot with 2.33 in newton wouldn't work but if we bug fix it in ocata, then it will work | |
| 16:19:02 | dansmith | it's pretty much the suck though | |
| 16:20:12 | jordanP | mriedem, I replied. Not sure I understood the question though. Sorry if I misunderstood | |
| 16:20:13 | mriedem | http://lists.openstack.org/pipermail/openstack-dev/2017-January/110798.html | |
| 16:20:13 | mriedem | yes i know | |
| 16:20:33 | mriedem | jordanP: before your change, an admin is listing instances for a specific tenant, right? | |
| 16:20:46 | jordanP | only if all_tenants=1 is passed | |
| 16:20:53 | jordanP | in addition to tenant_id=XXX | |
| 16:21:49 | mriedem | but in your patch, you're removing tenant_id from search_opts if all_tenants is False | |
| 16:22:04 | mriedem | so as an admin, i want to specifically list instances for a single tenant, not all tenants | |
| 16:22:51 | jordanP | yeah.. I know it sucks, but that's how the current api behaves | |
| 16:23:27 | openstack | Launchpad bug 1185290 in OpenStack Compute (nova) "nova list's --tenant flag also requires --all-tenants" [Undecided,Fix released] - Assigned to Ghanshyam Mann (ghanshyammann) | |
| 16:23:27 | jordanP | it's "documented" here: https://bugs.launchpad.net/nova/+bug/1185290 | |
| 16:25:06 | mriedem | should be documented here http://developer.openstack.org/api-ref/compute/?expanded=list-servers-detail#id4 | |
| 16:26:28 | mriedem | jordanP: your reply in the patch still doesn't make it clear to me *why* all_tenants has to be true | |
| 16:26:36 | mriedem | is it because the if all_tenants block elevates the context? | |
| 16:27:10 | mriedem | because if an admin is making the request, we shouldn't need to elevate the contexzt | |
| 16:27:27 | mriedem | is it something in compute_api.get_all()? or something in the DB API? | |
| 16:29:56 | jordanP | I don't understand the question. All I am enforcing is that tenant_id option is dropped if all_tenants doesn"t evaluate to "true" | |
| 16:30:19 | jordanP | why, the API was designed this way, I don"t know | |
| 16:31:31 | mriedem | jordanP: i'd like to understand why filtering by tenant_id does not work unless all_tenants=1 | |
| 16:31:46 | jordanP | ah ok | |
| 16:32:42 | jordanP | I think it should work but then in the get_all() method | |
| 16:33:16 | jordanP | we have this filter_mapping dict that maps tenant_id to project_id | |
| 16:33:50 | mriedem | right, which could overwrite project_id in filters to point at the tenant_id, rather than context.project_id | |
| 16:34:04 | jordanP | and if we pass both tenant_id and project_id (project_id is automatically added (search_opts['project_id'] = context.project_id), then it becomes unpredictable | |
| 16:34:21 | jordanP | yeah exactly | |
| 16:35:01 | jordanP | maybe there's a better fix for the issue | |
| 16:36:15 | mriedem | why not just sort search_opts.items() in get_all() if it's a random order issue? | |
| 16:36:42 | mriedem | i don't really enjoy hacking stuff in the REST API because of some behavior in the compute API code, | |
| 16:36:46 | mriedem | making it all totally tightly coupled | |
| 16:38:18 | jordanP | so the get_all() method would have these search_opts : {'tenant_id': XX, 'project_id': YY}. This is conflicting | |
| 16:39:37 | jordanP | sorting would work because we would be able to control, for sure, whether tenant_id takes precedence over project_id, but the overwritting would still be here and unexpected | |
| 16:40:19 | mriedem | so if tenant_id is handled first, it maps filters['project_id'] to XX | |
| 16:40:38 | mriedem | and then handles project_id and sets filters['project_id'] to YY | |
| 16:41:20 | jordanP | something like this, the logic inside the "for opt, value in search_opts.items():" is not trivial to say the least | |
| 16:41:40 | mriedem | so if both tenant_id and project_id are in search_opts, you want to handle project_id first | |
| 16:41:57 | mriedem | so that the final value is whatever was specified for tenant_id in the request, XX in your example | |
| 16:42:29 | mriedem | sorting search_opts.items() would fix that right? | |
| 16:42:38 | jordanP | yes | |
| 16:43:20 | jordanP | but that's more than a hack to me, no one will understand that the order on which items are iterated upon is important | |
| 16:43:55 | jordanP | we shouldn't be able to call get_all() with both tenant_id and project_id to begin with | |