| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-21 | |||
| 15:20:09 | efried | cdent I'm with you. Though TBH I'm not sure the amount of code will be more one way or the other. We either collate on the server side or the client side. | |
| 15:21:06 | cdent | I’m biased to doing collation on the client side because I want the server to be as simple as possible, and will consistently resist adding new stuff, until overwhelmed and then blithely acquiese | |
| 15:25:49 | openstackgerrit | Merged openstack/nova master: placement: Document request headers in api-ref https://review.openstack.org/521563 | |
| 15:26:00 | openstackgerrit | Merged openstack/nova master: placement: Document `in:` prefix for ?member_of= https://review.openstack.org/521216 | |
| 15:26:10 | artom | dansmith, mriedem, is it worth it to "argue" about which filters should be policy at this point? | |
| 15:26:34 | dansmith | artom: if you see something glaringly wrong, point it out | |
| 15:26:40 | artom | I see there are many comments to that effect already, and I haven't read all of them yet | |
| 15:27:18 | artom | dansmith, I wouldn't say glaringly. To me it's "does policy == things can change through a rebuild"? | |
| 15:27:26 | artom | Or rather, is that what we want? | |
| 15:27:51 | artom | Or are we going with the "real" semantic definition of policy, even it it means running some filters needlessly? | |
| 15:28:11 | efried | cdent Are you aware of a bug complaining that GET / of the placement API should be unauthenticated? | |
| 15:28:43 | dansmith | artom: well, as we discussed on that review, it's hard to draw a neat box around that | |
| 15:28:53 | efried | cdent AFAIK, we want every service to permit version discovery (which is what's at that URI) for free. <== mordred True? | |
| 15:29:19 | artom | dansmith, yeah, which is why I'm asking here before leaving a bunch of somments | |
| 15:29:21 | artom | *comments | |
| 15:29:21 | dansmith | artom: I want it to be "affects _where_ a thing should land" vs. "affects whether a host is "allowed" or "can" land" or something | |
| 15:29:34 | dansmith | artom: but clearly for the purposes of this backport, it's mostly just "does this break rebuild" | |
| 15:29:46 | artom | For instance, the AZ filter... the AZ can't change with a rebuild, can it? Do we need to run it? | |
| 15:30:03 | dansmith | artom: and I'm not running it on rebuild right? | |
| 15:30:18 | dansmith | oh I am | |
| 15:30:46 | dansmith | that might be a mistake.. I was thinking that certain aggregate lines could have changed such that the operator wants to block the rebuild, | |
| 15:30:51 | dansmith | but az is probably not right | |
| 15:31:14 | dansmith | artom: I was thinking https://review.openstack.org/#/c/521186/5/nova/scheduler/filters/aggregate_instance_extra_specs.py so I might have confused the az one | |
| 15:31:14 | mriedem | heh, maybe the variable should just be, RUN_ON_REBUILD | |
| 15:31:18 | mordred | efried: yes - we want all the things to be able to do version discovery - lemme read more scrollback though | |
| 15:31:25 | artom | The aggregate stuff as well... | |
| 15:31:38 | efried | mordred Just the comment two up from that one | |
| 15:31:44 | artom | I grok that aggregate metadata can be changed by an operator | |
| 15:31:54 | efried | mordred Today placement's GET / requires auth | |
| 15:32:08 | artom | But nothing about the instance can change to affect its aggregate placement, I think? | |
| 15:32:16 | dansmith | artom: so we have a test that makes sure that rebuild fails if the operator has manually migrated one of your anti-affinity brothers to your same host.. I figure that means that late violation of such a policy should make rebuild fail | |
| 15:32:23 | mriedem | artom: the image can change on rebuild | |
| 15:32:39 | mriedem | so the image meta can change the host aggregates you can be in | |
| 15:32:43 | mriedem | which was the original bug | |
| 15:32:48 | artom | mriedem, ah, does that specify aggregate stuff? OK, that's ignorance on my part then | |
| 15:33:55 | mriedem | efried: didn't you have a devstack patch at one point that depended on https://review.openstack.org/#/c/490057/ and removed the [glance] auth options from nova.conf to see it passing? | |
| 15:34:04 | efried | mriedem one sec... | |
| 15:34:06 | mriedem | artom: good luck | |
| 15:34:24 | mriedem | oh wait, nevermind, we codify image meta | |
| 15:34:30 | mriedem | nova.objects.image_meta | |
| 15:34:34 | dansmith | yeah it's in there | |
| 15:34:45 | mriedem | was thinking extra specs | |
| 15:35:06 | artom | Those are flavour though... (right?) | |
| 15:35:07 | efried | mriedem https://review.openstack.org/#/c/490031/ (It was sdague's) | |
| 15:35:08 | mordred | efried: yes. please make to all version discovery unauthenticated | |
| 15:35:10 | dansmith | however, I think the point is that you can put extra_specs on the aggregate that looks at image properties to keep things in place | |
| 15:35:13 | artom | And flavour can't change with a rebuild... | |
| 15:35:25 | mriedem | artom: correct, | |
| 15:35:28 | mriedem | flavor can only change on resize | |
| 15:35:33 | mriedem | and we run through the scheduler on resize already | |
| 15:35:34 | dansmith | artom: that's the point of the original bug, you might have landed on a linux host, rebuild to a windows image, and blow a license for the operator | |
| 15:37:10 | sdague | efried: those test results current? if so we can get that moved in | |
| 15:37:26 | efried | sdague They're ~6w old. I'll recheck. | |
| 15:37:34 | sdague | efried: cool | |
| 15:39:42 | artom | dansmith, going back to anti-affinity... we never checked the scheduler previously, so if during an instance's lifetime an anti-affinity brother migrated to the same host, the rebuild would not fail | |
| 15:39:50 | artom | Whereas now it would | |
| 15:40:03 | mriedem | efried: i'm a bit confused in that we don't even fill in the auth options for [glance] in nova.conf in our dsvm ci runs today, | |
| 15:40:11 | dansmith | artom: we already have a test for it | |
| 15:40:19 | mriedem | does nova really not talk to glance anywhere where it needs a token that doesn't come from the user context? | |
| 15:40:28 | sdague | mriedem: correct | |
| 15:40:38 | artom | dansmith, ah right, I was reading scrollback backwards | |
| 15:40:39 | mriedem | same for cinder | |
| 15:40:43 | sdague | mriedem: yes | |
| 15:40:56 | efried | what he said ^ | |
| 15:41:01 | dansmith | artom: it's in functional.test_server_group somewhere | |
| 15:41:01 | artom | This is a minefield:S | |
| 15:41:02 | sdague | neutron runs mixed mode, some times user token, some times system | |
| 15:41:16 | sdague | and ironic is system level, because nova is it's multi tenancy | |
| 15:41:37 | mriedem | the other thing i remembered i need to look at for this is the xenapi plugins for glance | |
| 15:41:56 | efried | sdague Neutron some of each? Hum, I may have screwed up https://review.openstack.org/509892 | |
| 15:42:25 | sdague | efried: yeh, there is a background network info cache refresh iirc | |
| 15:43:00 | efried | sdague Actually, I didn't change how the auth plugin is retrieved for the neutron patch. So whatever it was doing before, it's still doing now. | |
| 15:43:10 | sdague | and, there are times we "escalate" privs because neutron privs for users aren't always sufficient to do server builds | |
| 15:43:17 | sdague | efried: cool | |
| 15:43:24 | sdague | yeh, the neutron one is messier | |
| 15:44:12 | efried | sdague This business - unchanged: https://review.openstack.org/#/c/509892/6/nova/network/neutronv2/api.py@150 | |
| 15:44:18 | mriedem | sdague: do you remember any of the xenapi glance plugin whackiness? https://github.com/openstack/os-xenapi/blob/master/os_xenapi/dom0/etc/xapi.d/plugins/glance.py | |
| 15:44:43 | mriedem | i can't tell if any of that would be affected by this | |
| 15:45:01 | sdague | I do, I can look | |
| 15:46:48 | sdague | oh, ug, that's in a different tree now, one sec | |
| 15:47:37 | mriedem | yeah as far as i can tell, | |
| 15:47:39 | mriedem | https://github.com/openstack/os-xenapi/blob/master/os_xenapi/dom0/etc/xapi.d/plugins/glance.py#L630 | |
| 15:47:41 | efried | That sucker is hardcoding glance URIs. And I can't even see how it's doing authentication. | |
| 15:47:42 | mriedem | it's not getting a token, | |
| 15:47:50 | mriedem | so extra_headers would have to include the token | |
| 15:47:54 | sdague | right | |
| 15:48:03 | mriedem | jianghuaw: still around? | |
| 15:49:23 | sdague | https://github.com/openstack/nova/blob/25afc31b202d66c294a8ac84895e2876cdd6681b/nova/virt/xenapi/image/glance.py#L40 | |
| 15:49:31 | sdague | I think that's where it all comes together | |
| 15:49:35 | jianghuaw | mriedem, yes. | |
| 15:50:25 | sdague | https://github.com/openstack/nova/blob/25afc31b202d66c294a8ac84895e2876cdd6681b/nova/image/glance.py#L99-L106 yeh | |
| 15:50:27 | mriedem | ok, so xenapi just gets the glance identity headers and passes those through to the plugin for upload/download | |
| 15:50:31 | sdague | yep | |
| 15:51:00 | sdague | so it's the same basic model, it just does custom serialization | |
| 15:51:10 | mriedem | ok | |
| 15:51:31 | mriedem | jianghuaw: nevermind, we were just discussing https://review.openstack.org/#/c/490057/ and whether or not it would affect the xenapi glance plugin in os-xenapi | |
| 15:51:39 | mriedem | but it looks like the user token is passed through | |
| 15:51:59 | jianghuaw | mriedem, ok:-) | |
| 15:52:21 | mriedem | xenserver ci did fail on that patch | |
| 15:52:27 | mriedem | but it looks like due to devstack setup failing | |