| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-01-29 | |||
| 16:03:38 | stephenfin | <dansmith> stephenfin: but I think it's legit to have a validation=no per-request, but that can return 403 either because of policy or config if it's documented when you add it, no? so later we could turn that off or default it off | |
| 16:03:44 | stephenfin | what did you mean by that? ^ | |
| 16:04:08 | stephenfin | I assumed you meant for the ability to set '?validation=<anything>' | |
| 16:04:49 | dansmith | stephenfin: I meant state that validation=no might be disabled (i.e. always required) in the docs, so that we can allow that to be turned off in config or policy in the future if the op wants to require all flavors be validated | |
| 16:05:05 | dansmith | I'm just arguing against having a client-controlled way to emit WARNING messages in the server-side logs | |
| 16:05:44 | sean-k-mooney | or i guess restircted to a group e.g. admins although it is an admin only api already by default | |
| 16:06:20 | stephenfin | I think WARNING might be a bit much, tbh. INFO seems appropriate if the user has explicitly requested no/limited validation | |
| 16:06:24 | sean-k-mooney | the policy.json seam like a better approch if we were to make it configurable | |
| 16:06:39 | stephenfin | If it was INFO, we'd be okay, right? | |
| 16:07:06 | stephenfin | since no operator will get paged for INFO logs | |
| 16:07:10 | sean-k-mooney | i think dansmith was concerned about ddos risks | |
| 16:07:13 | dansmith | I dunno, I just don't like it in general | |
| 16:07:14 | dansmith | right | |
| 16:07:18 | sean-k-mooney | infor does not really help with hat | |
| 16:07:21 | dansmith | but it's not a sticking point | |
| 16:07:21 | sean-k-mooney | *that | |
| 16:07:25 | dansmith | sean-k-mooney: correct | |
| 16:07:43 | stephenfin | they could already ddos things but just repeatedly hitting the API with working extra specs | |
| 16:07:50 | stephenfin | we log each request iirc | |
| 16:07:56 | stephenfin | *by just | |
| 16:08:05 | stephenfin | that's what API rate limiting is for | |
| 16:08:27 | sean-k-mooney | well i suspect the error could be long if you add a bunch of invalide extra specs so its a larger risk | |
| 16:08:42 | sean-k-mooney | but again its an admin only api by default | |
| 16:08:46 | dansmith | and logging something that is a warning as info doesn't make it info | |
| 16:08:58 | sean-k-mooney | so the request will get rejected by a normal user well before it hit your code | |
| 16:09:00 | dansmith | but anyway, like I say, not a sticking point | |
| 16:09:37 | stephenfin | Is it a warning though? Per above, info does seem appropriate if the user has explicitly requested no/limited validation | |
| 16:09:58 | stephenfin | It's a warning if they didn't, which they'll see in their HTTP 4xx response | |
| 16:10:07 | dansmith | seems like a warning to me :) | |
| 16:10:08 | dansmith | anyway | |
| 16:10:16 | dansmith | we needn't argue about it | |
| 16:10:27 | dansmith | I will just toldjaso if you get CVE paperwork over it :) | |
| 16:10:29 | stephenfin | fine by me :) | |
| 16:16:01 | bauzas | mmm, what the heck is this ? | |
| 16:16:04 | bauzas | stestr: error: unrecognized arguments: --test-path=./nova/tests/functional run test_nova_manage | |
| 16:16:09 | bauzas | holy shit | |
| 16:16:53 | bauzas | I probably need to upgrade stestr by hand | |
| 16:16:59 | bauzas | but I did recreated my venc | |
| 16:17:02 | bauzas | venv* | |
| 16:17:09 | stephenfin | tox -e function --recreate ? | |
| 16:17:17 | stephenfin | *al | |
| 16:18:32 | stephenfin | unless you're using stestr without tox, which is odd | |
| 16:18:57 | stephenfin | seeing as you can do stuff like 'tox -e functional -- -n path_to_functional_test.py::TestClass.test_method' | |
| 16:22:34 | bauzas | stephenfin: that's what I did | |
| 16:22:51 | bauzas | and I ended up with stestr==2.6.0 | |
| 16:23:24 | bauzas | I always asked to run a subset of tests by doing tox -efunc <my_regex> | |
| 16:23:27 | bauzas | and it worked | |
| 16:24:57 | stephenfin | bauzas: That's what I have too. Working fine here | |
| 16:25:17 | bauzas | checking with stestr==2.5.0 | |
| 16:25:38 | bauzas | functional runtests: commands[0] | stestr --test-path=./nova/tests/functional run test_nova_manage | |
| 16:25:42 | bauzas | mmmmm | |
| 16:26:21 | stephenfin | bauzas: '$ tox -e functional -- test_nova_manage' wfm | |
| 16:27:05 | bauzas | I never had to use the positional markers, but whatever, gonna try | |
| 16:27:41 | bauzas | crazy, still same issue | |
| 16:28:07 | stephenfin | want to dump the complete output to paste.o.o | |
| 16:28:19 | stephenfin | it's something simple, I'd suspect | |
| 16:30:06 | bauzas | yeah me too, but can't see the problem | |
| 16:30:22 | bauzas | it's just stestr which doesn't sound to accept --test-path | |
| 16:30:24 | bauzas | http://paste.openstack.org/show/788935/ | |
| 16:31:11 | bauzas | actually, nope | |
| 16:33:59 | bauzas | well, I'm puzzled | |
| 16:34:37 | stephenfin | ah, wait, have you added additional tests for nova-manage? | |
| 16:35:13 | stephenfin | There's a bug with oslo.config whereby the CLI parser is global'ish | |
| 16:35:49 | stephenfin | Yeah, '--remote_debug-port' is a nova-manage option. You're not mocking stuff properly | |
| 16:38:06 | stephenfin | bauzas: mriedem saw the issue pop up in a unit test at https://review.opendev.org/#/c/694806/2/ and it's currently causing an issue with glance and the latest version of cliff | |
| 16:40:19 | bauzas | stephenfin: interesting, if I use the functional-py36 target, it does work | |
| 16:40:49 | stephenfin | even if you rebuild the venv? | |
| 16:41:03 | bauzas | it was created, I never used the target yet | |
| 16:41:13 | stephenfin | gotcha | |
| 16:41:20 | stephenfin | weird | |
| 16:41:24 | stephenfin | then I'm not sure :( | |
| 16:41:25 | bauzas | to make it clear, the functional-py36 target works, but not the standard one | |
| 16:41:53 | bauzas | either way, I know what to do, but I have to look at the target differences in tox.ini | |
| 16:42:32 | efried | bauzas: would it be productive for me to read the current PS of the numa topo spec, or wait til you rev it? | |
| 16:43:06 | bauzas | efried: I'm wraping my head around the group_policy issue, but your thoughts could be helpful | |
| 16:43:59 | bauzas | efried: and we have a disagreement on the memory modeling with NUMA with sean-k-mooney, your opinion could help us finding a consensus | |
| 16:44:25 | bauzas | efried: to answer your question, yeah comments would be appreciated on the current rev | |
| 16:44:32 | efried | okay, I'll give it a read. My view on group_policy is that we should be ignoring it. It doesn't really have a place with granular groups and the other knobs we put in in recent microversions. | |
| 16:44:59 | efried | I at least proposed (though I don't remember if we actually pulled the trigger on this) making it no longer required in a recent microversion. | |
| 16:45:09 | sean-k-mooney | bauzas: sorry can we pick this up after the internal call | |
| 16:45:19 | sean-k-mooney | e.g. in 15 mins | |
| 16:45:29 | bauzas | sean-k-mooney: yeah, and I even wanted to discuss it during the internal call :D | |
| 16:45:42 | bauzas | (but I'm superseded by other topics :) ) | |
| 16:45:52 | sean-k-mooney | ya i saw | |
| 16:47:14 | efried | I won't be much help on the memory modeling, probably, as I really don't understand that level of detail of NUMA itself. But I'll give it a shot. | |
| 16:48:38 | bauzas | tbh, me too | |
| 16:49:13 | bauzas | the big question is should we iteratively model huge pages and have memory split now, or care about the whole now ? | |
| 16:51:02 | efried | I'll have to refresh my memory (heh) on whether we did things to support cross-provider accumulation of resources, e.g. so you could still land on a NUMA-modeled host with 128/128 if you asked for 256. | |
| 16:52:10 | efried | I seem to recall we tried to KISS by saying you land on a NUMA-modeled host by asking for NUMA-modeled resources, and the converse, but that may have only been a point in time in the discussion, not where we landed. | |
| 16:52:15 | efried | I'll have to swap this all back in. | |
| 16:53:05 | bauzas | efried: yeah that's what I recall too | |
| 17:03:35 | openstackgerrit | Vladyslav Drok proposed openstack/nova master: Fix volume attachment rollback https://review.opendev.org/704847 | |
| 17:07:39 | efried | nts: we did abandon "can_split" https://review.opendev.org/#/c/658510/ | |
| 17:08:03 | sean-k-mooney | yes we did | |
| 17:09:29 | stephenfin | sean-k-mooney: for that memory issue, is there a reason we need to continue to support floating memory when NUMA is used? | |
| 17:09:49 | stephenfin | do we do strict NUMA affinity when using a NUMA topology at the moment? | |
| 17:09:52 | sean-k-mooney | that depends. yes but the question is do we care | |
| 17:10:06 | stephenfin | for memory, that is | |
| 17:10:13 | sean-k-mooney | we would have to pin the memoyy based on the placemetn allocation of all vms | |
| 17:10:18 | stephenfin | I think we don't unless we explicitly request a pagesize, yeah? | |