| sean-k-mooney | placement is broken by this however as are a few other i have seen so far | 00:00 |
|---|---|---|
| sean-k-mooney | oh actully no playment has the fix | 00:00 |
| smcginnis | sean-k-mooney: What is the issue with the codesearch and upper constraints? | 00:02 |
| smcginnis | Nearly every repo should have that. | 00:02 |
| sean-k-mooney | smcginnis: yes but if you have that and you have a lower-constrats tox env and dont overwrive the install_command the the lower-constratis env uses upper-constratits | 00:03 |
| sean-k-mooney | so novas lower-constraints job is broken as is congresses git.openstack.org/cgit/openstack/congress/tree/tox.ini | 00:04 |
| sean-k-mooney | and tripleo-validations | 00:05 |
| sean-k-mooney | there are not that many project that actully have a lower-constraints tox env | 00:05 |
| smcginnis | Really? Most should have that. It was one of those big blasts out to update the world. | 00:06 |
| smcginnis | https://review.openstack.org/#/q/topic:requirements-stop-syncing+(status:open+OR+status:merged) | 00:06 |
| smcginnis | Congress does. At the bottom of tox.ini | 00:06 |
| sean-k-mooney | yes i know | 00:07 |
| *** wolverineav has joined #openstack-nova | 00:07 | |
| smcginnis | https://opendev.org/openstack/congress/src/branch/master/tox.ini#L103-L108 | 00:07 |
| sean-k-mooney | yes but its broken | 00:07 |
| sean-k-mooney | https://opendev.org/openstack/congress/src/branch/master/tox.ini#L9 | 00:07 |
| sean-k-mooney | that install command means its lower-constrtis is actully installing the upper-constrats instead | 00:08 |
| *** liuyulong has quit IRC | 00:08 | |
| smcginnis | I believe with the deps set there, that makes sure it also uses the lower-constraints.txt | 00:09 |
| sean-k-mooney | smcginnis: it does not | 00:10 |
| smcginnis | At least it has been working that way for nearly a year now. | 00:10 |
| smcginnis | Where do you see it not? | 00:10 |
| sean-k-mooney | in the output of the gate logs | 00:10 |
| sean-k-mooney | give me a sec an ill grab some | 00:10 |
| *** gyee has quit IRC | 00:11 | |
| sean-k-mooney | so this is a random nova change | 00:12 |
| sean-k-mooney | http://logs.openstack.org/52/626952/12/check/openstack-tox-lower-constraints/7bc660b/tox/lower-constraints-1.log | 00:12 |
| *** mriedem_afk is now known as mriedem | 00:12 | |
| sean-k-mooney | if you scroll to the end you will see its installing zVMCloudConnector-1.4.0 | 00:12 |
| sean-k-mooney | the lower-constrating is 1.1.1 | 00:12 |
| sean-k-mooney | the lower constrait of psycopg2 is 2.6.8 its using psycopg2-2.7.7 | 00:14 |
| sean-k-mooney | doing the same with congress | 00:16 |
| sean-k-mooney | http://logs.openstack.org/13/643113/1/check/openstack-tox-lower-constraints/b00ac73/tox/lower-constraints-1.log | 00:16 |
| sean-k-mooney | that job used testtools-2.3.0 | 00:17 |
| sean-k-mooney | there lower-constrating is 2.2.0 https://github.com/openstack/congress/blob/master/lower-constraints.txt#L139 | 00:17 |
| *** irclogbot_1 has joined #openstack-nova | 00:17 | |
| smcginnis | Hmm, this had been working fine. I wonder if a tox upgrade or something like that has caused a change in behavior. | 00:17 |
| sean-k-mooney | smcginnis: im not sure if it has been working fine or not | 00:18 |
| sean-k-mooney | the issue is we are pasing two constratit files to pip | 00:18 |
| sean-k-mooney | cmdargs: '/home/zuul/src/git.openstack.org/openstack/congress/.tox/lower-constraints/bin/pip install -c/home/zuul/src/git.openstack.org/openstack/requirements/upper-constraints.txt -U -c/home/zuul/src/git.openstack.org/openstack/congress/lower-constraints.txt -r/home/zuul/src/git.openstack.org/openstack/congress/test-requirements.txt | 00:18 |
| sean-k-mooney | -r/home/zuul/src/git.openstack.org/openstack/congress/requirements.txt' | 00:18 |
| sean-k-mooney | i dont think that is actully allowed | 00:19 |
| smcginnis | It had been working. | 00:19 |
| melwitt | sean-k-mooney: doesn't lower constraint mean the lower bound? how can you tell it's not working if it installs a version newer than the lower bound? | 00:19 |
| sean-k-mooney | melwitt: the lower-constrating job is ment to install the oldest version that we say should work | 00:20 |
| melwitt | oh, the job. I see | 00:21 |
| sean-k-mooney | right so the job has not been running using our actual lower bound so we dont actully know the lower bound work. | 00:22 |
| melwitt | gotcha. thanks | 00:22 |
| sean-k-mooney | and in nova case i know our mocking is broken on the lower bound as we are actully trying to connect to rabbitmq in some of the test and its not mocked | 00:22 |
| *** wolverineav has quit IRC | 00:23 | |
| sean-k-mooney | if i bumped the version of oslo.messaging we have in lower bound our tests would pass with the fix to the tox.ini but i dont think we actully need the newer version is just broken tests | 00:24 |
| gmann | it seems few deps are taken from lower-constraints.txt like hacking | 00:25 |
| sean-k-mooney | smcginnis: anyway this is either a change in pips behavior or this was always broken unfortunetly | 00:25 |
| sean-k-mooney | gmann: i think what is happening is any dep that is not n upper-consrtraitng but is in lower-constratits is being honered | 00:26 |
| sean-k-mooney | but i think ip is takeing the constraint form the first file it finds it in | 00:26 |
| sean-k-mooney | that is jsut a guess | 00:26 |
| gmann | seems so. and that is why we have upper-constraints.txt also in dep list but picking order might have changed on tox/pip side | 00:28 |
| sean-k-mooney | well tox just runs pip so tox is not really invoved | 00:29 |
| sean-k-mooney | but for this env specifcially we should not include upper-constraints.txt or at least if we did it should be after lower | 00:30 |
| sean-k-mooney | looking at the pip change log i dont imediatly see something related to this | 00:31 |
| sean-k-mooney | the -U flag proably isnt helping much either in this case | 00:32 |
| gmann | sean-k-mooney: we might need upper-constraints.txt pkg not in lower-constraints.txt, in case latest pip version of them would create issue. | 00:41 |
| sean-k-mooney | gmann: we may yes but we need to validate the ordering we pass them | 00:42 |
| gmann | yeah ordering is imp | 00:42 |
| sean-k-mooney | the docs for pip dont state anything about orderign so im going to look at the code tomorow | 00:43 |
| sean-k-mooney | that said its alreay tomorow i.e. its after mindnight so i should proable leave this untill after i sleep for selveral hours | 00:44 |
| sean-k-mooney | gmann: ok im actully going now but i think this is the isue. https://github.com/pypa/pip/blob/d4217f0cc30ea2012911be1cb22a74d8cc4a365a/src/pip/_internal/req/req_set.py#L133-L136 | 00:54 |
| sean-k-mooney | the way i processses constratits if a constraint is repeated the first one is used | 00:55 |
| *** ileixe has joined #openstack-nova | 00:58 | |
| sean-k-mooney | looking at git blame the logic seam to be unchange bar minor change for at least 4 years | 00:59 |
| sean-k-mooney | https://github.com/pypa/pip/blame/0bc7a974fac35ebc0b38beeba4de59017166b1bc/src/pip/_internal/req/req_set.py#L109-L130 | 00:59 |
| *** igordc has quit IRC | 01:01 | |
| gmann | humm, i think added upper-constraints.txt dep in tox before lower-constraints.txt things happened. which means it never worked as expected | 01:02 |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP: Make os-services policies granular https://review.openstack.org/645427 | 01:16 |
| openstackgerrit | melanie witt proposed openstack/nova master: DNM Re-enable testing of console with TLS in nova-next job https://review.openstack.org/645432 | 01:29 |
| *** awalende has joined #openstack-nova | 01:31 | |
| *** itlinux has joined #openstack-nova | 01:34 | |
| *** awalende has quit IRC | 01:36 | |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP: Make os-services policies granular https://review.openstack.org/645427 | 01:45 |
| *** tbachman has quit IRC | 01:55 | |
| *** hongbin has joined #openstack-nova | 02:22 | |
| *** wolverineav has joined #openstack-nova | 02:24 | |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP: Make os-services policies granular https://review.openstack.org/645427 | 02:28 |
| *** wolverineav has quit IRC | 02:28 | |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP: Make os-services policies granular https://review.openstack.org/645427 | 02:29 |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Introduce scope_types in os-services, lock policies https://review.openstack.org/645452 | 02:46 |
| *** psachin has joined #openstack-nova | 02:48 | |
| openstackgerrit | Merged openstack/nova master: Require python-ironicclient>=2.7.0 https://review.openstack.org/642863 | 02:55 |
| *** udesale has joined #openstack-nova | 03:21 | |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP:Introduce scope_types in os-services, lock policies https://review.openstack.org/645452 | 03:21 |
| *** marst has joined #openstack-nova | 03:21 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Stop running tempest-multinode-full https://review.openstack.org/645456 | 03:24 |
| mriedem | keeel it ^ | 03:24 |
| *** mriedem has quit IRC | 03:29 | |
| openstackgerrit | Boxiang Zhu proposed openstack/nova-specs master: Scheduler filters evaluated even forced host https://review.openstack.org/645458 | 03:30 |
| *** marst has quit IRC | 03:37 | |
| *** whoami-rajat has joined #openstack-nova | 03:41 | |
| *** marst has joined #openstack-nova | 03:43 | |
| openstackgerrit | Boxiang Zhu proposed openstack/nova-specs master: Scheduler filters evaluated even forced host https://review.openstack.org/645458 | 03:46 |
| openstackgerrit | OpenStack Release Bot proposed openstack/nova stable/stein: Update .gitreview for stable/stein https://review.openstack.org/645461 | 03:50 |
| openstackgerrit | OpenStack Release Bot proposed openstack/nova stable/stein: Update UPPER_CONSTRAINTS_FILE for stable/stein https://review.openstack.org/645462 | 03:50 |
| openstackgerrit | OpenStack Release Bot proposed openstack/nova master: Update master for stable/stein https://review.openstack.org/645463 | 03:50 |
| *** nicolasbock has quit IRC | 04:03 | |
| *** hongbin has quit IRC | 04:12 | |
| *** erlon has quit IRC | 04:13 | |
| *** zhubx007 has quit IRC | 04:17 | |
| *** ileixe has quit IRC | 04:18 | |
| *** zhubx007 has joined #openstack-nova | 04:18 | |
| *** mlavalle has quit IRC | 04:23 | |
| *** hamzy_ has joined #openstack-nova | 04:31 | |
| *** TheJulia has quit IRC | 04:31 | |
| *** wxy-xiyuan has quit IRC | 04:31 | |
| *** coreycb has quit IRC | 04:31 | |
| *** wxy-xiyuan has joined #openstack-nova | 04:31 | |
| *** TheJulia has joined #openstack-nova | 04:31 | |
| *** fyx has quit IRC | 04:31 | |
| *** dustinc has quit IRC | 04:31 | |
| *** spsurya has quit IRC | 04:31 | |
| *** lxkong has quit IRC | 04:32 | |
| *** yonglihe has quit IRC | 04:32 | |
| *** yonglihe has joined #openstack-nova | 04:32 | |
| *** awestin1 has quit IRC | 04:32 | |
| *** adrianreza has quit IRC | 04:32 | |
| *** kmalloc has quit IRC | 04:32 | |
| *** hogepodge has quit IRC | 04:32 | |
| *** johnsom has quit IRC | 04:32 | |
| *** jbryce has quit IRC | 04:33 | |
| *** jbernard has quit IRC | 04:33 | |
| *** hamzy has quit IRC | 04:33 | |
| *** jbernard has joined #openstack-nova | 04:33 | |
| *** coreycb has joined #openstack-nova | 04:34 | |
| *** spsurya has joined #openstack-nova | 04:34 | |
| *** fyx has joined #openstack-nova | 04:34 | |
| *** adrianreza has joined #openstack-nova | 04:34 | |
| *** lxkong has joined #openstack-nova | 04:34 | |
| *** awestin1 has joined #openstack-nova | 04:34 | |
| *** hogepodge has joined #openstack-nova | 04:34 | |
| *** johnsom has joined #openstack-nova | 04:35 | |
| *** jbryce has joined #openstack-nova | 04:38 | |
| *** dustinc has joined #openstack-nova | 04:39 | |
| *** kmalloc has joined #openstack-nova | 04:39 | |
| *** udesale has quit IRC | 04:47 | |
| *** udesale has joined #openstack-nova | 04:47 | |
| *** lpetrut has joined #openstack-nova | 04:52 | |
| *** yankcrime has joined #openstack-nova | 04:54 | |
| *** ileixe has joined #openstack-nova | 05:03 | |
| *** janki has joined #openstack-nova | 05:04 | |
| *** marst has quit IRC | 05:08 | |
| *** lpetrut has quit IRC | 05:25 | |
| *** sridharg has joined #openstack-nova | 05:43 | |
| *** dustinc has quit IRC | 05:50 | |
| *** tkajinam has quit IRC | 05:59 | |
| *** tkajinam has joined #openstack-nova | 05:59 | |
| *** takashin has joined #openstack-nova | 06:10 | |
| *** lpetrut has joined #openstack-nova | 06:17 | |
| *** cfriesen has quit IRC | 06:18 | |
| *** psachin has quit IRC | 06:20 | |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove deprecated 'default_flavor' config option https://review.openstack.org/645476 | 06:22 |
| *** wolverineav has joined #openstack-nova | 06:24 | |
| *** sapd1_x has joined #openstack-nova | 06:25 | |
| *** psachin has joined #openstack-nova | 06:25 | |
| openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata https://review.openstack.org/645478 | 06:26 |
| *** david-lyle has joined #openstack-nova | 06:27 | |
| *** dklyle has quit IRC | 06:27 | |
| *** david-lyle has quit IRC | 06:29 | |
| *** dklyle has joined #openstack-nova | 06:29 | |
| *** wolverineav has quit IRC | 06:29 | |
| *** ivve has joined #openstack-nova | 06:29 | |
| *** lpetrut has quit IRC | 06:30 | |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Add description about sort order in API ref guideline https://review.openstack.org/627282 | 06:33 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Add description about sort order in API ref guideline https://review.openstack.org/627282 | 06:33 |
| *** dims has quit IRC | 06:39 | |
| *** dims has joined #openstack-nova | 06:41 | |
| *** Luzi has joined #openstack-nova | 06:49 | |
| *** brault has quit IRC | 06:50 | |
| *** rcernin has quit IRC | 07:07 | |
| *** whoami-rajat has quit IRC | 07:11 | |
| *** lpetrut has joined #openstack-nova | 07:11 | |
| *** vrushali_kamde has joined #openstack-nova | 07:14 | |
| *** vrushali_kamde_ has joined #openstack-nova | 07:16 | |
| *** vrushali_kamde_ has left #openstack-nova | 07:18 | |
| *** vrushali_kamde_ has joined #openstack-nova | 07:19 | |
| *** vrushali_kamde has left #openstack-nova | 07:20 | |
| *** sapd1_x has quit IRC | 07:20 | |
| *** vrushali_kamde_ has left #openstack-nova | 07:20 | |
| *** vrushali_kamde has joined #openstack-nova | 07:21 | |
| *** dpawlik_ is now known as dpawlik | 07:21 | |
| *** pcaruana has joined #openstack-nova | 07:27 | |
| *** whoami-rajat has joined #openstack-nova | 07:30 | |
| *** takashin has left #openstack-nova | 07:30 | |
| *** rpittau|afk is now known as rpittau | 07:33 | |
| *** phasespace has quit IRC | 07:35 | |
| *** luksky has joined #openstack-nova | 07:49 | |
| *** tosky has joined #openstack-nova | 07:53 | |
| *** lpetrut has quit IRC | 08:04 | |
| *** xek_ has joined #openstack-nova | 08:08 | |
| *** ttsiouts has joined #openstack-nova | 08:13 | |
| *** helenaAM has joined #openstack-nova | 08:17 | |
| *** tesseract has joined #openstack-nova | 08:25 | |
| *** ttsiouts has quit IRC | 08:26 | |
| *** ttsiouts has joined #openstack-nova | 08:27 | |
| *** phasespace has joined #openstack-nova | 08:27 | |
| *** ttsiouts has quit IRC | 08:31 | |
| *** dtantsur|afk is now known as dtantsur | 08:33 | |
| *** ttsiouts has joined #openstack-nova | 08:33 | |
| *** tkajinam has quit IRC | 08:34 | |
| *** ttsiouts has quit IRC | 08:45 | |
| *** ttsiouts has joined #openstack-nova | 08:46 | |
| *** ttsiouts has quit IRC | 08:50 | |
| *** ralonsoh has joined #openstack-nova | 08:59 | |
| *** ttsiouts has joined #openstack-nova | 09:04 | |
| *** tssurya has joined #openstack-nova | 09:04 | |
| *** tobias-urdin has joined #openstack-nova | 09:15 | |
| *** ttsiouts has quit IRC | 09:21 | |
| *** ttsiouts has joined #openstack-nova | 09:22 | |
| *** ttsiouts has quit IRC | 09:26 | |
| openstackgerrit | Matthew Booth proposed openstack/nova master: Eventlet monkey patching should be as early as possible https://review.openstack.org/626952 | 09:27 |
| openstackgerrit | Lee Yarwood proposed openstack/nova master: Use migration_status during volume migrating and retyping https://review.openstack.org/637224 | 09:28 |
| *** IvensZambrano has joined #openstack-nova | 09:37 | |
| *** derekh has joined #openstack-nova | 09:42 | |
| *** ttsiouts has joined #openstack-nova | 09:44 | |
| openstackgerrit | Boxiang Zhu proposed openstack/nova master: [WIP] Scheduler filters evaluated even forced host https://review.openstack.org/645520 | 09:45 |
| kashyap | mdbooth_: Nice work on that eventlet patch (although I'm not fully qualified to comment on it). | 09:50 |
| kashyap | "I started with what seemed like the simplest and most obvious change, and moved on as I discovered more interactions which broke." | 09:50 |
| kashyap | :D | 09:50 |
| kashyap | (Also kudos for the "commit message mini essay") | 09:51 |
| *** mdbooth_ is now known as mdbooth | 09:51 | |
| *** cdent has joined #openstack-nova | 09:51 | |
| mdbooth | kashyap: Heh, thanks. The commit message has also grown over time :) | 09:51 |
| kashyap | Yes, I can see. I'm sure you spent many hours fine-tuning it as you discovered things. | 09:51 |
| mdbooth | I feel that enormous comments are an indicator of code smell, though, and nova.monkey_patch is almost all comment. | 09:53 |
| mdbooth | Most code should not require that much explanation. | 09:53 |
| openstackgerrit | Merged openstack/nova master: Cleanup comments around claim_resources method https://review.openstack.org/644576 | 09:56 |
| openstackgerrit | Merged openstack/nova master: Address old TODO in claim_resources_on_destination https://review.openstack.org/644596 | 09:56 |
| *** IvensZambrano has quit IRC | 09:57 | |
| *** lpetrut has joined #openstack-nova | 10:04 | |
| *** jangutter has joined #openstack-nova | 10:19 | |
| *** IvensZambrano has joined #openstack-nova | 10:19 | |
| *** lpetrut has quit IRC | 10:23 | |
| *** dtantsur is now known as dtantsur|brb | 10:28 | |
| lyarwood | mdbooth: re https://review.openstack.org/#/c/637224/ - good news, I can't reproduce the cinder weirdness locally after rebasing and recloning everything this morning. Just pushed https://review.openstack.org/#/c/637527/ again, hopefully this also passes in the check gate now. | 10:30 |
| lyarwood | s/gate/queue/g | 10:30 |
| kashyap | mdbooth: Yeah, was about to remark on that, too. THe large commentary section. | 10:33 |
| *** nicolasbock has joined #openstack-nova | 10:40 | |
| *** ttsiouts has quit IRC | 10:42 | |
| *** ttsiouts has joined #openstack-nova | 10:43 | |
| *** ttsiouts_ has joined #openstack-nova | 10:48 | |
| *** ttsiouts has quit IRC | 10:48 | |
| *** johnsom has quit IRC | 10:59 | |
| *** adrianreza has quit IRC | 10:59 | |
| *** johnsom has joined #openstack-nova | 10:59 | |
| *** whoami-rajat has quit IRC | 10:59 | |
| *** spsurya has quit IRC | 10:59 | |
| *** yonglihe has quit IRC | 10:59 | |
| *** yonglihe has joined #openstack-nova | 11:00 | |
| *** awestin1 has quit IRC | 11:00 | |
| *** spsurya has joined #openstack-nova | 11:00 | |
| *** adrianreza has joined #openstack-nova | 11:00 | |
| *** wolverineav has joined #openstack-nova | 11:00 | |
| *** whoami-rajat has joined #openstack-nova | 11:01 | |
| *** awestin1 has joined #openstack-nova | 11:02 | |
| *** phasespace has quit IRC | 11:04 | |
| *** wolverineav has quit IRC | 11:05 | |
| *** kaisers has quit IRC | 11:12 | |
| *** kaisers has joined #openstack-nova | 11:13 | |
| *** ileixe has quit IRC | 11:21 | |
| *** IvensZambrano has quit IRC | 11:27 | |
| *** IvensZambrano has joined #openstack-nova | 11:29 | |
| *** luksky has quit IRC | 11:32 | |
| *** pcaruana has quit IRC | 11:53 | |
| openstackgerrit | Matthew Booth proposed openstack/nova master: Fix incomplete instance data returned after build failure https://review.openstack.org/645546 | 11:53 |
| *** ttsiouts_ has quit IRC | 11:54 | |
| *** ttsiouts has joined #openstack-nova | 11:55 | |
| *** ttsiouts has quit IRC | 11:59 | |
| *** luksky has joined #openstack-nova | 12:05 | |
| *** dtantsur|brb is now known as dtantsur | 12:05 | |
| *** arne_wiebalck_ has joined #openstack-nova | 12:06 | |
| *** csatari_ has joined #openstack-nova | 12:07 | |
| *** Cardoe_ has joined #openstack-nova | 12:07 | |
| *** Guest12731 has joined #openstack-nova | 12:08 | |
| *** ttsiouts has joined #openstack-nova | 12:09 | |
| *** eharney has quit IRC | 12:14 | |
| *** logan- has quit IRC | 12:14 | |
| *** arne_wiebalck has quit IRC | 12:14 | |
| *** ab-a has quit IRC | 12:14 | |
| *** csatari has quit IRC | 12:14 | |
| *** med_ has quit IRC | 12:14 | |
| *** Cardoe has quit IRC | 12:14 | |
| *** johnthetubaguy has quit IRC | 12:14 | |
| *** Guest12731 is now known as logan- | 12:14 | |
| *** Cardoe_ is now known as Cardoe | 12:14 | |
| *** csatari_ is now known as csatari | 12:14 | |
| *** arne_wiebalck_ is now known as arne_wiebalck | 12:14 | |
| *** johnthetubaguy has joined #openstack-nova | 12:15 | |
| *** weshay is now known as weshay|rover | 12:17 | |
| *** eharney has joined #openstack-nova | 12:23 | |
| *** erlon has joined #openstack-nova | 12:28 | |
| *** udesale has quit IRC | 12:47 | |
| *** udesale has joined #openstack-nova | 12:48 | |
| *** pcaruana has joined #openstack-nova | 12:54 | |
| kaisers | mdbooth: Hi! If possible, could you revisit the driver bugfix at https://review.openstack.org/#/c/554195/ ? Needs a second +2 and you already looked into it some time ago. :) | 12:55 |
| mdbooth | kaisers: I don't have that mojo, but I can take a look. | 12:55 |
| kaisers | mdbooth: still fine, thanks | 12:56 |
| *** altlogbot_0 has quit IRC | 13:01 | |
| *** irclogbot_1 has quit IRC | 13:01 | |
| *** yonglihe has quit IRC | 13:02 | |
| *** irclogbot_2 has joined #openstack-nova | 13:02 | |
| *** altlogbot_3 has joined #openstack-nova | 13:02 | |
| openstackgerrit | Merged openstack/nova master: Imported Translations from Zanata https://review.openstack.org/645478 | 13:04 |
| *** dklyle has quit IRC | 13:07 | |
| *** erlon has quit IRC | 13:08 | |
| *** lbragstad has joined #openstack-nova | 13:10 | |
| openstackgerrit | Surya Seetharaman proposed openstack/nova-specs master: Support adding the reason behind a server lock https://review.openstack.org/638629 | 13:11 |
| *** dklyle has joined #openstack-nova | 13:12 | |
| tssurya | cdent, edleaf, mriedem: thanks for the spec review ^ I have updated it with respect to your comments. | 13:13 |
| cdent | tssurya: great, thank you. will look again soon. | 13:13 |
| tssurya | edleafe* | 13:13 |
| edleafe | tssurya: :) | 13:13 |
| tssurya | mriedem: I have a question regarding the notification change: https://review.openstack.org/#/c/638629/1/specs/train/approved/add-locked-reason.rst@168 not particularly what/why it should change | 13:15 |
| gibi | tssurya: I think it is not mandatory to add the new fields to the notification payload but it would be nice to add | 13:17 |
| gibi | tssurya: if we are not adding now, then we can add it later | 13:17 |
| *** jmlowe has quit IRC | 13:18 | |
| tssurya | gibi: ah ok, yea I can add them in this spec itself, np, I wasn't sure of the workflow since I haven't dealt with notification changes before | 13:18 |
| tssurya | so that means I need to add the fields into the InstancePayload | 13:19 |
| tssurya | and then send that via the notifications event right ? | 13:19 |
| *** lbragstad is now known as elbragstad | 13:19 | |
| *** eharney has quit IRC | 13:20 | |
| gibi | tssurya: yes, you extend the IntancePayload and with the new fields and make sure that the value of the new fields are populated from the instance | 13:20 |
| tssurya | https://github.com/openstack/nova/blob/b33fa1c054ba4b7d4e789aa51250ad5c8325da2d/nova/compute/utils.py#L447 somewhere there ? | 13:20 |
| *** ttsiouts has quit IRC | 13:20 | |
| tssurya | oh ok | 13:20 |
| gibi | tssurya: there is a schema def in the InstancePayload ovo that defined how to copy data from the Instance ovo so if you fill the schema then the copy is automatic | 13:21 |
| *** ttsiouts has joined #openstack-nova | 13:21 | |
| *** ivve has quit IRC | 13:21 | |
| gibi | https://github.com/openstack/nova/blob/master/nova/notifications/objects/instance.py#L59 | 13:21 |
| tssurya | gibi: ah okay that's great | 13:22 |
| tssurya | sure then I will add this change too to the spec | 13:22 |
| tssurya | thanks gibi! :) | 13:22 |
| gibi | tssurya: here is a patch from the past that extended the InstancePayload https://github.com/openstack/nova/commit/2bca6431e69bf2c6e657736b7fe11f5a2fbb9433 | 13:22 |
| *** ttsiouts has quit IRC | 13:26 | |
| *** mrch_ has quit IRC | 13:26 | |
| tssurya | gibi: nice, I can follow that workflow :) | 13:26 |
| gibi | tssurya: and you can invite me to review the patch when it is up. :) | 13:27 |
| openstackgerrit | Mohammed Naser proposed openstack/nova master: bdm: store empty object as connection_info by default https://review.openstack.org/645352 | 13:27 |
| *** marst has joined #openstack-nova | 13:27 | |
| tssurya | gibi: absolutely! :D thank you | 13:27 |
| gibi | np | 13:27 |
| mnaser | i added a note to this bug. it's been around for a few cycles ^ | 13:27 |
| smcginnis | sean-k-mooney: I see now what you were saying with the lower-constraints issue yesterday. | 13:32 |
| smcginnis | sean-k-mooney: Cinder has this: https://opendev.org/openstack/cinder/src/branch/master/tox.ini#L15-L18 | 13:32 |
| smcginnis | sean-k-mooney: That's what you were saying the nova tox.ini needed to be changed to, right? | 13:32 |
| *** ivve has joined #openstack-nova | 13:34 | |
| cdent | smcginnis , sean-k-mooney I have a patch pending to fix lower constraints in nova for months and months https://review.openstack.org/#/c/622972/ | 13:34 |
| cdent | it's so old now it is probably out of date | 13:34 |
| smcginnis | Well wouldya look at that. | 13:35 |
| smcginnis | I do think we should probably have the deps set separately than install_command though. | 13:35 |
| smcginnis | Since that's the variable between these targets. | 13:35 |
| smcginnis | But two ways to do the same thing I suppose. | 13:36 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Update contributor guide for Train https://review.openstack.org/645581 | 13:39 |
| *** marst has quit IRC | 13:40 | |
| openstackgerrit | Merged openstack/nova master: Trivial: remove unused var from policies.base.py https://review.openstack.org/645380 | 13:47 |
| *** BjoernT has joined #openstack-nova | 13:47 | |
| *** BjoernT has quit IRC | 13:49 | |
| *** mriedem has joined #openstack-nova | 13:51 | |
| *** ivve has quit IRC | 13:51 | |
| *** BjoernT has joined #openstack-nova | 13:54 | |
| *** gbarros has joined #openstack-nova | 13:55 | |
| *** mlavalle has joined #openstack-nova | 13:56 | |
| *** awaugama has joined #openstack-nova | 13:56 | |
| *** efried is now known as fried_rice | 13:58 | |
| fried_rice | o/ | 13:58 |
| *** bnemec is now known as beekneemech | 13:59 | |
| *** jaosorior has quit IRC | 14:01 | |
| kaisers | Looking for a second +2 for another driver bugfix at https://review.openstack.org/#/c/522245/ , anyone interested? :) | 14:04 |
| *** marst has joined #openstack-nova | 14:05 | |
| mdbooth | Ooh, that's fun. | 14:06 |
| *** eharney has joined #openstack-nova | 14:06 | |
| mdbooth | In instance.save() we do: | 14:06 |
| mdbooth | if not updates: | 14:06 |
| mdbooth | ... | 14:06 |
| mdbooth | return | 14:06 |
| *** BjoernT_ has joined #openstack-nova | 14:06 | |
| mdbooth | Which, I think, means that if 2 different threads do: | 14:07 |
| mdbooth | instance.task_state='foo' | 14:07 |
| mdbooth | instance.save(expected_task_state=[None]) | 14:07 |
| mdbooth | They will *both* succeed, because the current state will never be evaluated for the second one | 14:07 |
| mdbooth | And that would break a ton of stuff in nova api | 14:08 |
| cdent | I feel like this is a known issue, but I can't remember why I think that | 14:08 |
| *** erlon has joined #openstack-nova | 14:08 | |
| mdbooth | I'm on my second interesting race condition of the day. I'm in race condition heaven. | 14:09 |
| *** BjoernT has quit IRC | 14:09 | |
| mdbooth | mriedem: Speaking of which you brought up gate failure https://bugs.launchpad.net/nova/+bug/1820337 yesterday, which I think is resolved by https://review.openstack.org/#/c/645546/ | 14:13 |
| openstack | Launchpad bug 1820337 in OpenStack Compute (nova) "test_bfv_quota_race_local_delete intermittently fails with testtools.matchers._impl.MismatchError: ['bfv'] != []" [High,In progress] - Assigned to Matthew Booth (mbooth-9) | 14:13 |
| *** jmlowe has joined #openstack-nova | 14:15 | |
| mriedem | mdbooth: comment inline | 14:18 |
| mriedem | i also noted that that bug doesn't show up on master for some reason | 14:18 |
| mriedem | but yeah this seems reasonable | 14:18 |
| mgoddard | hi nova | 14:20 |
| mgoddard | has anyone seen this after sending SIGHUP to nova-compute: | 14:20 |
| fried_rice | o/ mgoddard | 14:20 |
| mgoddard | In shutdown, no new events can be scheduled | 14:20 |
| mdbooth | mriedem: Yeah, the rocky/master thing is weird. My bet is it'll be some random quirk of eventlet scheduling I'll bet, caused by an additional context-switching operation added somewhere random and unimportant. | 14:20 |
| mgoddard | http://logs.openstack.org/40/616640/29/check/kolla-ansible-centos-source-upgrade/34ac7d4/primary/logs/system_logs/kolla/nova/nova-compute.txt.gz#_2019-03-22_13_48_58_621 | 14:21 |
| fried_rice | mgoddard: Does it go away after a little while? | 14:21 |
| mgoddard | fried_rice: I'm not sure, but I don't think so | 14:21 |
| mgoddard | fried_rice: how little a while? | 14:21 |
| fried_rice | It wouldn't surprise me that SIGHUP would bork anything that's in flight. Was that vif creation in flight at the time? | 14:22 |
| mgoddard | fried_rice: no, it's a post upgrade instance boot test | 14:23 |
| fried_rice | looks like the SIGHUP happens here: http://logs.openstack.org/40/616640/29/check/kolla-ansible-centos-source-upgrade/34ac7d4/primary/logs/system_logs/kolla/nova/nova-compute.txt.gz#_2019-03-22_13_43_57_192 | 14:24 |
| fried_rice | not sure if the errors after that are "normal" or not. | 14:24 |
| fried_rice | your spawn is five minutes later, which should be plenty of time, yah. | 14:26 |
| fried_rice | This sounds like a job for... | 14:26 |
| fried_rice | gibi: ^ any ideas? | 14:26 |
| gibi | looking | 14:27 |
| mriedem | mdbooth: maybe, i guess that eventlet thing you're trying to fix was master-only | 14:27 |
| mdbooth | mriedem: Yes, that's master only. Nova-api didn't need eventlet in Rocky, and wasn't monkey patched. | 14:28 |
| mdbooth | The gate thing is in functional, which is monkey patched separately, same on both branches. The chance causing the difference in scheduling could be literally anywhere, though. Doesn't even have to be even vaguely related. | 14:29 |
| openstackgerrit | Surya Seetharaman proposed openstack/nova-specs master: Support adding the reason behind a server lock https://review.openstack.org/638629 | 14:30 |
| mgoddard | fried_rice: gibi: do the grenade jobs use a SIGHUP? | 14:30 |
| * fried_rice has no idea | 14:31 | |
| cdent | mgoddard: unlikely | 14:31 |
| gibi | I'm not sure what should nova is expected to do at SIGHUP | 14:31 |
| gibi | I'm not sure what nova is expected to do at SIGHUP | 14:31 |
| mgoddard | rolling upgrade docs say to SIGHUP to refresh upgrade levels: https://docs.openstack.org/nova/stein/user/upgrade.html#rolling-upgrade-process | 14:31 |
| gibi | mgoddard: I understand the first log http://logs.openstack.org/40/616640/29/check/kolla-ansible-centos-source-upgrade/34ac7d4/primary/logs/system_logs/kolla/nova/nova-compute.txt.gz#_2019-03-22_13_48_58_621 as nova got an error from neutron so it would worth to check what is in the neutron log | 14:32 |
| dansmith | gibi: there's a bug about this.. oslo.service is killing us during a sighup instead of letting us do graceful things | 14:34 |
| gibi | mgoddard: ^^ | 14:34 |
| dansmith | gibi: mgoddard: https://review.openstack.org/#/c/641907/ | 14:35 |
| dansmith | I was looking at that with him early-on but I've lost all the context on it | 14:35 |
| gibi | dansmith: thanks | 14:35 |
| mgoddard | gibi: I think it's a red herring. I can't see any errors in neutron. I think that the VIF plug event pushes nova through a path that exposes this half-shutdown state | 14:35 |
| mgoddard | dansmith: thanks | 14:35 |
| dansmith | mgoddard: it's because nova has been restarted and has dropped a bunch of state on the ground | 14:36 |
| dansmith | I have to run off for a sec, biab | 14:36 |
| artom | http://logs.openstack.org/81/644881/2/check/tempest-full-py3/3b506d2/controller/logs/screen-n-cpu.txt.gz?level=WARNING#_Mar_20_20_39_18_723559 | 14:36 |
| artom | Well crap, looks like there's still something missing from that whole revert resize external events mess | 14:36 |
| *** artom is now known as temka | 14:37 | |
| mgoddard | It seems like the workaround here is going to be to restart nova-compute rather than HUP, I'll go with that | 14:40 |
| mgoddard | thanks fried_rice gibi dansmith | 14:41 |
| sean-k-mooney | cdent: regarding your lower constratins patch that is similar to mine but it also is technically wrong | 14:49 |
| *** shilpasd has quit IRC | 14:49 | |
| sean-k-mooney | cdent: placement and os-vif only use lower-constratis in our lower-constratis job | 14:49 |
| cdent | sean-k-mooney: and? | 14:50 |
| sean-k-mooney | but if we have a transitive depency that is capped in upper-constratits but not in our lower-constratis then it will be uncapped | 14:50 |
| sean-k-mooney | given os-vif and placement have relitivly few depencies we are proably fine | 14:51 |
| sean-k-mooney | but for nova or larger project we actully should use both but lower constraints needs to be passed first | 14:51 |
| mgoddard | dansmith: qq for when you return - would you recommend a full restart of all nova services rather than HUP, including those where we haven't seen issues with HUP (i.e. all except compute)? | 14:51 |
| cdent | my changes to nova's lower-constraints job make it so the upper-constraints file is not included in the test (which it had been before), and that caused conflicts | 14:52 |
| sean-k-mooney | yes and that is more correct but the actually fix should be to include lower-constratis first followed by upper | 14:52 |
| dansmith | mgoddard: yes to restart of compute instead of hup, although it'll still cause in-flight boots to fail.. don't think it's really a concern for most of the other services, so probably okay to hup them | 14:53 |
| sean-k-mooney | the pip code uses the first constratit it finds for a dep | 14:53 |
| *** Sundar has joined #openstack-nova | 14:53 | |
| sean-k-mooney | cdent: the main issue i have found is that mocking does not work correctly with the lower constrating deps as i think things moved | 14:53 |
| sean-k-mooney | cdent: ill pull down your version and rebase and see if i see the same issue | 14:54 |
| mgoddard | dansmith: at this point it's just been upgraded so will have restarted anyway - another restart won't hurt too much. I'll go with that, thanks | 14:54 |
| dansmith | ack | 14:54 |
| sean-k-mooney | i can combine both patches and see if i can get something that works fully | 14:54 |
| *** Luzi has quit IRC | 14:56 | |
| sean-k-mooney | smcginnis: and yes the cinder code https://opendev.org/openstack/cinder/src/branch/master/tox.ini#L15-L18 is what we do in os-vif and it almost right they should also have upper-constriatns listed here https://opendev.org/openstack/cinder/src/branch/master/tox.ini#L198 after lower. | 14:57 |
| smcginnis | sean-k-mooney: I don't think we want upper constraints listed in the lower constraints job on line 198. The point of that is to make sure it only uses the lower constraints. | 14:59 |
| sean-k-mooney | smcginnis: the way os-vif placement and cinder execute lower-constraitns is actully probably fine 99% of the time as the are using lower-constraints for all there direct dependcies | 14:59 |
| *** Luzi has joined #openstack-nova | 14:59 | |
| sean-k-mooney | smcginnis: if lower constratits does not list every dep we install then not having it in line 198 will leave deps uncapped | 14:59 |
| sean-k-mooney | assuming those deps are capped in upper | 15:00 |
| sean-k-mooney | that is the only case where it would be needed | 15:00 |
| smcginnis | But lower should list every dependency. | 15:00 |
| sean-k-mooney | it should but i dont think we enforce that | 15:00 |
| smcginnis | Anything in requirements.txt should be in lower-constraints.txt | 15:00 |
| *** Luzi has quit IRC | 15:00 | |
| smcginnis | The point of the job is to capture and validate all the requirement's minimum allowable versions. | 15:01 |
| sean-k-mooney | smcginnis: yes i know but we dont validate that every file listed in pip feeze is in lower-constratis | 15:01 |
| sean-k-mooney | the original file was generated that way but there is notheing to enforce it so it could be out of sync | 15:02 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix return param docstring in check_can_live_migrate* methods https://review.openstack.org/645610 | 15:02 |
| sean-k-mooney | lower-constratins also needs to have our depencies dependcies listed as we do not use our deps lower-constraitns recursivly | 15:02 |
| sean-k-mooney | anyway ill email the list once i have a working fix for nova that work with master | 15:03 |
| sean-k-mooney | i have a partial fix but as i said our unit tests mocking does not work with lower constratints as we end up using the oslo messaging amqp driver instead of the fake driver in some tests and the mocking does not work | 15:04 |
| openstackgerrit | Surya Seetharaman proposed openstack/nova master: [WIP] Add 'power-update' external event to listen to ironic https://review.openstack.org/645611 | 15:07 |
| *** cfriesen has joined #openstack-nova | 15:07 | |
| *** dpawlik has quit IRC | 15:14 | |
| *** Sundar is now known as DarthNau | 15:14 | |
| mriedem | dansmith: do you see any reason to not add a locked_reason to the instances table vs system_metadata or something? https://review.openstack.org/#/c/638629/1/specs/train/approved/add-locked-reason.rst@42 | 15:19 |
| mriedem | i get a bit of the heeby jeebys shoving more non-filterable/indexable columns in the instances table for just system metadata type purposes, but if it doesn't cause any issues for indexing and the other locked/locked_by columns are already in there, then i suppose it's fine | 15:20 |
| mriedem | i'd ask jay but he's gone | 15:20 |
| dansmith | and requires rewriting the instances table, for what is just metadata | 15:20 |
| *** marst has quit IRC | 15:21 | |
| mriedem | right - it's a schema migration on a large table | 15:21 |
| *** altlogbot_3 has quit IRC | 15:21 | |
| dansmith | i.e. non-critical information that won't be there for some instances anyway | 15:21 |
| dansmith | right | 15:21 |
| mriedem | i'm not advocating a big ass data migration to move the existing locked/locked_by to another table as a blocker on this, but seems we shouldn't pile on | 15:21 |
| dansmith | yeah | 15:21 |
| mriedem | despite it will be confusing to have a new locked_* field in a different place now, but the Instance object abstracts that | 15:22 |
| dansmith | moving it just means we have a collapse we have to do later (or leave it dangling) so moving is less good than just putting new things in the right-er place | 15:22 |
| dansmith | and really, you only need that info in the api, right? you put it on the instance and you're loading it all the time... | 15:22 |
| mriedem | correct | 15:22 |
| dansmith | how long of a reason are we proposing? | 15:23 |
| mriedem | purely non-queryable meta | 15:23 |
| mriedem | 255 | 15:23 |
| dansmith | because we get a free 255 char limit with sysmeta | 15:23 |
| dansmith | ack | 15:23 |
| mriedem | which is exactly what system_metadata value is | 15:23 |
| dansmith | yeah | 15:23 |
| dansmith | seems all around better to put it in sysmeta to me, aside from the purely-developer reason of wanting them to look like they're next to each other in one data model at the bottom of the stack | 15:25 |
| dansmith | ...which I get of course | 15:25 |
| dansmith | but. | 15:25 |
| *** altlogbot_3 has joined #openstack-nova | 15:26 | |
| dansmith | the argument of putting it instances because the other locked-by is there, is like saying we should have stored the pre-resize vm_state in instances because that's where vm_state is, which would be insane reasoning | 15:26 |
| dansmith | and since most instances are unlocked for most of their life.... | 15:26 |
| *** irclogbot_2 has quit IRC | 15:30 | |
| mriedem | ok are you going to -1? if not i'll just leave a comment with a link to this conversation | 15:31 |
| tssurya | mriedem, dansmith: I'll update the spec to add it to the metadata table | 15:31 |
| *** irclogbot_3 has joined #openstack-nova | 15:32 | |
| dansmith | tssurya: I think that's the way to go, especially since it's just changing the spec to remove the db migration, schema migration and just say "stuff it into sysmeta" :) | 15:32 |
| dansmith | it'll be much easier to merge that way too | 15:33 |
| tssurya | dansmith: haha sure, for some reason I wanted to have all the lock related info in the same place, but yea sure we could split it to the metadata | 15:33 |
| tssurya | hopefully I won't have to move the locked_by right ? | 15:33 |
| tssurya | let that stay there ? | 15:34 |
| dansmith | tssurya: no don't move the existing stuff | 15:34 |
| tssurya | cool! :) | 15:34 |
| mriedem | tssurya: i'm leaving comments on PS3 | 15:35 |
| mriedem | so hold up | 15:35 |
| tssurya | mriedem: okay | 15:36 |
| *** irclogbot_3 has quit IRC | 15:36 | |
| *** maciejjozefczyk has quit IRC | 15:37 | |
| *** irclogbot_2 has joined #openstack-nova | 15:38 | |
| *** igordc has joined #openstack-nova | 15:42 | |
| *** gaoyan has joined #openstack-nova | 15:42 | |
| mriedem | tssurya: done | 15:44 |
| tssurya | thanks | 15:44 |
| mriedem | elbragstad: so we've got this silly locked_by field on the instance object in our db which is an enum of 'admin' or 'owner', | 15:45 |
| mriedem | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4016 | 15:45 |
| mriedem | i guess in this case admin is a non-project admin, so like a system scope admin | 15:46 |
| mriedem | anyway, there is a proposal to expose that out of the rest api https://review.openstack.org/#/c/638629/3/specs/train/approved/add-locked-reason.rst@117 | 15:46 |
| mriedem | do you see any issues with future policy scope creep weirdness with that? | 15:46 |
| dansmith | sure seems like that should be a userid and not an enum | 15:46 |
| dansmith | (before we expose it I mean) | 15:47 |
| dansmith | or | 15:47 |
| mriedem | it's hella old | 15:47 |
| dansmith | I know | 15:47 |
| gmann | mriedem: dansmith as in future it can be locked by system.admin and project.admin as well we should make it user-id. | 15:47 |
| dansmith | so when we expose it, maybe we should make it possible for it to be owner|admin|other, so that later if we store a user-id, we can say "other" there | 15:47 |
| gmann | otherwise we will not know it is locked by project admin or system admin | 15:48 |
| mriedem | gmann: today if you lock as project admin it's 'owner' | 15:48 |
| mriedem | it's only 'admin' if the request comes from an admin in a different project | 15:48 |
| dansmith | tssurya: just added a comment about that | 15:49 |
| gmann | mriedem: no, it is 'admin' if project admin lock any non-admin user server of same project | 15:50 |
| tssurya | dansmith: checking | 15:50 |
| mriedem | looks like it was added in https://review.openstack.org/#/c/38196/ | 15:50 |
| dansmith | god the world was so much simpler in 2013 | 15:50 |
| mriedem | gmann: ? is_owner = instance.project_id == context.project_id | 15:50 |
| mriedem | as long as the project id matches, it's owner | 15:50 |
| mriedem | despite the role | 15:50 |
| gmann | ohh yeah we only match project id. soory | 15:51 |
| gmann | sorry | 15:51 |
| dansmith | "soory" correct canadian for "sorry" | 15:51 |
| mriedem | he's fitting right in | 15:52 |
| gmann | :) | 15:52 |
| gmann | so we assume 'admin' is system-admin which again might be confusing | 15:52 |
| mriedem | hmm, i wonder if locked_by is still used on unlock https://review.openstack.org/#/c/38196/13/nova/compute/api.py@2490 | 15:52 |
| dansmith | it's what gates whether or not you can unlock right? | 15:52 |
| gmann | I am adding lock as [system, project] scope_type in my PoC of scope_type - https://review.openstack.org/#/c/645452/2/nova/policies/lock_server.py | 15:52 |
| mriedem | ah it is | 15:53 |
| gmann | but we have overridden unlock also i think that is for admin to unlock owner server | 15:53 |
| *** gaoyan has quit IRC | 15:53 | |
| gmann | owner locked server | 15:53 |
| mriedem | os_compute_api:os-lock-server:unlock:unlock_override | 15:54 |
| mriedem | yuck | 15:54 |
| mriedem | baking policy into the db | 15:54 |
| dansmith | ready for friday to be over already? | 15:54 |
| gmann | yeah, we should remove that also. not sure any reason to keep that | 15:54 |
| gmann | are we going to expose 'locked_by' also ? not checked the spec yet | 15:56 |
| *** igordc has quit IRC | 15:57 | |
| tssurya | dansmith: umm you want to add "other" where ? as in locked_by enum schema needs to change ? or just in the response api terminology for future needs ? | 15:57 |
| dansmith | tssurya: just the api schema for the future | 15:57 |
| tssurya | okay | 15:57 |
| dansmith | we should get some agreement on that (and the actual enum value) first though | 15:58 |
| tssurya | yea | 15:58 |
| mriedem | dansmith: i replied to that comment, | 15:58 |
| dansmith | and mriedem is too busy barfing | 15:58 |
| mriedem | Instance.locked_by is just a string | 15:58 |
| mriedem | which will get spewed back out of the api response | 15:58 |
| mriedem | so if we allow storing a user_id in there in the future, it's a db schema change but shouldn't really mean any change to the instance object or api | 15:58 |
| dansmith | oh sorry, I assumed it was an enum in the json schema too | 15:58 |
| mriedem | it might be in tempest...checking | 15:58 |
| mriedem | oh nvm, we don't expose it today | 15:59 |
| gmann | we only expose 'lock' as string in API response | 15:59 |
| dansmith | it says admin or owner | 15:59 |
| *** igordc has joined #openstack-nova | 15:59 | |
| mriedem | right | 15:59 |
| gmann | we do not expose 'locked_by' | 15:59 |
| mriedem | tempest is the only thing that would have a response schema check | 15:59 |
| dansmith | mriedem: right the thing she's adding says "admin or owner" | 15:59 |
| mriedem | i know, but that's just the possible values in the db schema today | 15:59 |
| dansmith | so I assumed that meant a schema enum | 15:59 |
| mriedem | tempest could just allow 'string' | 15:59 |
| mriedem | nova doesn't have any response schema | 15:59 |
| dansmith | well let's be clear about it then because it looks like those are the only two values | 16:00 |
| dansmith | I mean in the spec | 16:00 |
| mriedem | sure | 16:00 |
| *** jangutter has quit IRC | 16:01 | |
| mriedem | 11am and i forgot what i was actually going to try and get done today... | 16:01 |
| mriedem | need a stable core to hit this https://review.openstack.org/#/c/633493/ | 16:04 |
| mriedem | https://review.openstack.org/#/c/640116/ is also trivial on queens | 16:05 |
| mriedem | for fast approval | 16:05 |
| mriedem | and https://review.openstack.org/#/c/637391/ | 16:05 |
| mriedem | once we get those we could probably release queens | 16:05 |
| mriedem | thanks dansmith | 16:07 |
| * mriedem goes to lunch | 16:07 | |
| *** mriedem is now known as mriedem_afk | 16:07 | |
| dansmith | uar | 16:08 |
| *** dustinc has joined #openstack-nova | 16:08 | |
| *** gaoyan has joined #openstack-nova | 16:09 | |
| *** helenaAM has quit IRC | 16:15 | |
| *** imacdonn has quit IRC | 16:19 | |
| *** imacdonn has joined #openstack-nova | 16:20 | |
| *** gaoyan has quit IRC | 16:21 | |
| *** janki has quit IRC | 16:22 | |
| mdbooth | https://bugs.launchpad.net/nova/+bug/1821373 | 16:24 |
| openstack | Launchpad bug 1821373 in OpenStack Compute (nova) "Most instance actions can be called concurrently" [Undecided,New] | 16:24 |
| * mdbooth will try to work on that next week, btw | 16:24 | |
| melwitt | dansmith: want to hit these release bot changes? https://review.openstack.org/645461 and https://review.openstack.org/645463 | 16:25 |
| sean-k-mooney | mdbooth: i assume you are going to remove the logic that shortcircuts if the value is set the same value and have it hit the db | 16:25 |
| mdbooth | sean-k-mooney: Probably, but I'll have to meditate carefully on the safety of whatever. | 16:26 |
| mdbooth | sean-k-mooney: And right now I'm a bit hungry and a bit checked out for the weekend :) | 16:26 |
| sean-k-mooney | mdbooth: fixing db curuptions is definetly not a firday evning activity | 16:26 |
| mdbooth | Don't try to fix db races on an empty stomach :) | 16:27 |
| dansmith | melwitt: I think you're safe to fast-approve things like that :) | 16:27 |
| melwitt | can't be too careful | 16:27 |
| melwitt | (joke) | 16:28 |
| *** fried_rice is now known as fried_rolls | 16:30 | |
| dansmith | mdbooth: hmm, check_instance_state has a task state to try to prevent some of what you describe, not sure why that's not being used (although it won't close it entirely) | 16:31 |
| dansmith | but I agree, I think either doing a dedicated task_state check in the db if we're going to punt due to no-updates will retain as much of the optimization as possible and close that window | 16:31 |
| dansmith | letting it all go through even if no updates is an option, but it's heavy | 16:32 |
| dansmith | (i.e. what we were doing before) | 16:32 |
| *** mrch_ has joined #openstack-nova | 16:38 | |
| *** rpittau is now known as rpittau|afk | 16:39 | |
| mdbooth | dansmith: Yeah. I started thinking about whether we can safely bypass the trip to the db, but that's not a Friday afternoon thought-train. | 16:40 |
| dansmith | I don't think we can entirely based on your reasoning, but we can make it efficient | 16:41 |
| dansmith | but yes, a tuesday conversation | 16:41 |
| *** dpawlik has joined #openstack-nova | 16:42 | |
| dansmith | fried_rolls: are you not approving this because of your comment or for sequencing reasons? https://review.openstack.org/#/c/644625/5 | 16:44 |
| dansmith | fried_rolls: if the former, I'll just add it to unblock progress, but it seems entirely duplicative to me | 16:44 |
| *** cfriesen has quit IRC | 16:45 | |
| *** dpawlik has quit IRC | 16:47 | |
| *** BjoernT_ has quit IRC | 16:50 | |
| openstackgerrit | melanie witt proposed openstack/nova-specs master: Move Stein implemented specs https://review.openstack.org/645749 | 16:52 |
| *** udesale has quit IRC | 17:00 | |
| *** tbachman has joined #openstack-nova | 17:09 | |
| *** dtantsur is now known as dtantsur|afk | 17:11 | |
| *** cdent has left #openstack-nova | 17:18 | |
| *** cdent has joined #openstack-nova | 17:24 | |
| *** cdent has left #openstack-nova | 17:24 | |
| *** eharney has quit IRC | 17:26 | |
| *** derekh has quit IRC | 17:26 | |
| gibi | mriedem_afk: could you hit the bandwidth spec update when you are back https://review.openstack.org/#/c/644810/ ? | 17:26 |
| *** tbachman has quit IRC | 17:29 | |
| *** tssurya has quit IRC | 17:30 | |
| openstackgerrit | Kashyap Chamarthy proposed openstack/nova-specs master: Add "CPU selection with hypervisor consideration" spec https://review.openstack.org/645814 | 17:33 |
| gmann | dansmith: mriedem_afk i think we should expose project_id in locked_by field which can be easily fetched for 'owner' case (based on DB enum value and context) but not sure how to fetch the admin project-id when server is locked by admin and GET by owner | 17:34 |
| dansmith | gmann: yeah that's the obviously difficult one | 17:35 |
| gmann | we need to change the locked_by DB from enum to UUID ? | 17:35 |
| gmann | should we change DB schema also in this spec so that we always store project-id in locked_by while lock API | 17:35 |
| dansmith | we could just start throwing a locked_by_uuid in sysmeta too, and maybe just add "id" to the enum, | 17:35 |
| mnaser | kashyap: regarding your arm change to support 'virt' machine type, does that ship in as specific release of qemu? | 17:36 |
| dansmith | but since it's an enum in the schema, it's not easy to just convert it to a freeform string | 17:36 |
| elbragstad | mriedem_afk just catching up | 17:36 |
| kashyap | mnaser: Hi | 17:36 |
| mnaser | under centos 7 with qemu-kvm, i can't find a 'virt' machine type (this is the rhel shipped one) | 17:36 |
| gmann | dansmith: ok. | 17:36 |
| kashyap | mnaser: Which change do you refer to? | 17:36 |
| mnaser | kashyap: https://github.com/openstack/nova/commit/e155baefb0bbaa0aa78e54fe9e0f10c12336c01a | 17:36 |
| kashyap | (/me is dazed due to death-by-meetings; and is almost ready to check out) | 17:37 |
| elbragstad | migrating from enum in the schema is pretty disruptive, isn't it? | 17:37 |
| * kashyap clicks | 17:37 | |
| dansmith | elbragstad: yes | 17:37 |
| mnaser | i'm assuming this controls the resulting `-M` parameter in the qemu-kvm command which is the machine type | 17:37 |
| elbragstad | we hit that with something in keystone's database once, it was a total pain | 17:37 |
| openstackgerrit | melanie witt proposed openstack/nova-specs master: Move Stein implemented specs https://review.openstack.org/645749 | 17:37 |
| mnaser | to which I don't see the `virt` option available under centos 7.6 right now | 17:37 |
| kashyap | mnaser: Top off my head, I need to check which version introduced the 'virt' board for ARM/AArch64 | 17:37 |
| kashyap | mnaser: Are you checking with the right QEMU binary? | 17:38 |
| mnaser | okay, i was curious at seeing how `virt` works for x86 machines too but just noticed it's missing. | 17:38 |
| mnaser | kashyap: on a centos 7 machine => /usr/libexec/qemu-kvm -M help | 17:38 |
| mnaser | gets me this: http://paste.openstack.org/show/748268/ | 17:38 |
| kashyap | mnaser: That is referring to x86. | 17:38 |
| mnaser | kashyap: let me check an arm box quickly | 17:39 |
| kashyap | mnaser: You need to pull in the AArch64 binary; `dnf search` for it | 17:39 |
| mnaser | i thought virt was a generic qemu-kvm machine type available for all architectures | 17:39 |
| kashyap | mnaser: No. Only for AArch64/ARM | 17:39 |
| kashyap | mnaser: But ... | 17:39 |
| mnaser | kashyap: ok, you're right, i see it under aaarch64 on an arm box | 17:39 |
| elbragstad | mriedem_afk re: your first question - i would probably write that policy check string to be something like "(role:admin and system_scope:all) or rule:owner)" | 17:40 |
| kashyap | mnaser: ... there has been some talk at last year's KVM Forum about adding a similar 'virt' board for x86 | 17:40 |
| gmann | dansmith: elbragstad so we can just extend the current enum to store 'id' also (do not modify column from enum->string)and that we expose in API, right ? | 17:40 |
| openstackgerrit | melanie witt proposed openstack/nova-specs master: Move Stein implemented specs https://review.openstack.org/645749 | 17:40 |
| mnaser | kashyap: ok, cool, sorry for the firedrill, misinformed on my side then :) | 17:40 |
| kashyap | (Also "Kata Containers" folks, who forked QEMU, also want it) | 17:40 |
| kashyap | mnaser: No worries :-) | 17:40 |
| elbragstad | gmann enum definitions are kind of immutable i think | 17:40 |
| dansmith | gmann: we could, but I'm not sure how important this really is, and it's not fair to saddle the other effort with too much I think | 17:40 |
| mnaser | kashyap: yeah, i saw NEMU which seems like it is better solved by adding a virt machine type. | 17:40 |
| dansmith | elbragstad: I think adding one thing to an enum isn't bad | 17:40 |
| dansmith | elbragstad: they're stored as an int, IIRC, so adding one doesn't cause too much disruption as long as you're not removing one or adding one that causes you to need the next size up of int | 17:41 |
| dansmith | but some database person should config | 17:41 |
| dansmith | *confirm | 17:41 |
| elbragstad | aha | 17:41 |
| gmann | dansmith: but current string 'admin' will be confusing for case of scope_type as system and proejcts | 17:41 |
| mnaser | kashyap: aha, from the NEMU readme "NEMU also introduces a new QEMU x86-64 machine type: virt. It is a purely virtual platform, that does not try to emulate any existing x86 chipset or legacy bus (ISA, SMBUS, etc) and offloads as many features to KVM as possible. This is a similar approach as the already existin AArch64 virt machine type and NEMU will only support the two virt machine types." | 17:41 |
| mnaser | that's where i saw it from. | 17:41 |
| kashyap | mnaser: There are several issues with "NEMU", and "better solved" is also debatable :-). Upstream QEMU folks are working with them to abandon the idea of a fork | 17:42 |
| kashyap | And to work with upstream. | 17:42 |
| gmann | i mean from API response side | 17:42 |
| dansmith | gmann: you mean for the proposed spec to expose it? | 17:42 |
| mnaser | kashyap: absolutely, what i meant was instead of creating nemu, we upstream all that stuff into a custom machine type :) | 17:42 |
| gmann | dansmith: in current spec- https://review.openstack.org/#/c/638629/3/specs/train/approved/add-locked-reason.rst@117 | 17:42 |
| kashyap | mnaser: Yep, that is a good idea; and x86 will "soon" get it. There have been some mind-showering sessions on QEMU usptream list | 17:42 |
| dansmith | gmann: if you're really concerned about that and are willing to do the work underneath to make it not have to, then okay | 17:42 |
| gmann | yeah | 17:42 |
| kashyap | mnaser: So expect to see a "virt" board for x86 (that is expressely designed for guests, including of course Nova instances). | 17:43 |
| mnaser | kashyap: cool, that would be super neat | 17:43 |
| * kashyap --> hungry; be back later | 17:43 | |
| gmann | because this is the only API we say 'admin' or 'owner' in response. and 'admin' will be extended in scope_type system-admin or project-admin | 17:44 |
| *** luksky has quit IRC | 17:44 | |
| dansmith | gmann: ack, well, raise that on the spec I guess | 17:44 |
| *** ganso has joined #openstack-nova | 17:45 | |
| mnaser | is "urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host='198.72.124.41', port=443): Read timed out. (read timeout=60)" under tempest.scenario.test_shelve_instance.TestShelveInstance a common failure? | 17:47 |
| elbragstad | i think i stumbled across an article like this a while ago when we were trying to add an attribute to an enum http://komlenic.com/244/8-reasons-why-mysqls-enum-data-type-is-evil/ | 17:47 |
| *** psachin has quit IRC | 17:48 | |
| *** tesseract has quit IRC | 17:48 | |
| *** BjoernT has joined #openstack-nova | 17:51 | |
| *** gmann is now known as gmann_afk | 17:52 | |
| *** mriedem_afk is now known as mriedem | 17:55 | |
| elbragstad | mriedem i thinking keeping the `admin` or `owner` details out of the database would be a good think moving forward | 17:56 |
| elbragstad | a lot of the work we're doing with scope types introduces different types of "admins", so we can't necessarily assume "admin" is always the cloud administrator/operator | 17:57 |
| dansmith | elbragstad: right, that's why we're talking about this: | 18:01 |
| dansmith | it's already in the db, but not exposed to the user | 18:01 |
| dansmith | the spec is proposing exposing that admin|user notion to the api user | 18:02 |
| dansmith | so we either need to make the api sufficiently generic or change a bunch of stuff before we can do the other part, which is locked reason | 18:02 |
| elbragstad | so a user would be able to see who locked an instance? | 18:02 |
| dansmith | gmann_afk: elbragstad mriedem: What if instead of exposing the person that locked it for now, we just expose a boolean which is "can I unlock it?" | 18:02 |
| *** IvensZambrano has quit IRC | 18:02 | |
| dansmith | elbragstad: right | 18:02 |
| dansmith | but we could paper over a lot of details by just saying "you can unlock this" or "you can not unlock this: | 18:03 |
| elbragstad | dansmith the "can I unlock it" bit seems like a good middle ground? | 18:03 |
| dansmith | right | 18:03 |
| mriedem | dansmith: that depends on policy though, which could change between the time they have that response and when they try to unlock | 18:03 |
| elbragstad | as a user - i probably care less about who locked my stuff and i just want to know if i can lock it | 18:03 |
| dansmith | at least lets us push the refactoring of the data store away from the current spec | 18:03 |
| mriedem | granted, i wouldn't expect policy to change often | 18:03 |
| dansmith | mriedem: sure, but same for exposing admin|user .. you have to now what the policy is to know whether it matters | 18:04 |
| mriedem | the 'can i unlock' is a policy check | 18:04 |
| elbragstad | well - to calculate that boolean you'd just need to run the context through that policy check - then populate that accordingly in the data model (and response?) | 18:04 |
| dansmith | right | 18:04 |
| mriedem | not the data model, but the response yes | 18:04 |
| dansmith | but if you don't use the boolean, and expose "locked_by: admin" | 18:04 |
| dansmith | the user has to know whether policy considers them an admin or not right? | 18:05 |
| dansmith | whereas "no you can't unlock this" is pretty clear | 18:05 |
| openstackgerrit | Merged openstack/nova stable/queens: libvirt: Add workaround to cleanup instance dir when using rbd https://review.openstack.org/628726 | 18:05 |
| elbragstad | ++ | 18:05 |
| openstackgerrit | Merged openstack/nova stable/queens: Avoid BadRequest error log on volume attachment https://review.openstack.org/640116 | 18:05 |
| dansmith | the fact that it could change between check and attempt is pretty much the case for everything :) | 18:05 |
| elbragstad | "admin" is kinda confusing, what is locked by a project admin, a domain admin or a system admin? | 18:05 |
| elbragstad | s/what/was/ | 18:06 |
| mriedem | in this case project admin == 'owner' | 18:06 |
| mriedem | same as if the owner of the instance locked it themselves | 18:06 |
| mriedem | it's the system admin case that is wonky | 18:06 |
| elbragstad | i'm missing the part why the system-admin case is wonky | 18:07 |
| mriedem | i mean the case where the locked_by is 'admin' today is if someone outside the project that owns the instance locks the instance | 18:07 |
| mriedem | project admin locking the instance treats locked_by as 'owner' | 18:07 |
| elbragstad | ... interesting | 18:08 |
| mriedem | idk how i feel about returning an at-the-time-of-request policy check boolean saying if you can or cannot unlock it with a subsequent unlock call, i don't think we have anything else like that in the api | 18:08 |
| elbragstad | someone outside the project? | 18:08 |
| dansmith | I totally don't understand the concern over the policy thing | 18:08 |
| dansmith | we return data (or not) based on policy | 18:09 |
| mriedem | for faults and event traceback yeah | 18:09 |
| mriedem | true | 18:09 |
| mriedem | and the numa blob that is being proposed | 18:09 |
| dansmith | and we used to for things like flavor extra specs, iirc | 18:09 |
| dansmith | and hostnames if you're admin | 18:09 |
| dansmith | or fake host ids if you're not | 18:10 |
| mriedem | it's slightly different though, those are just exposing things or not | 18:10 |
| dansmith | if it really matters, then unlockable:true or omit it entirely, but I don't think that's better :) | 18:10 |
| mriedem | i'd be much more comfortable with this locked_reason spec if we didn't have to deal with gd locked_by in the first place | 18:10 |
| *** gmann_afk is now known as gmann | 18:11 | |
| mriedem | locked_by is just internal meta and i'm not sure why we need to expose it, and it's only related to a policy check on unlock | 18:11 |
| mriedem | if you can't unlock you find out with a 403 as normal | 18:12 |
| dansmith | well, we can do that too | 18:12 |
| dansmith | but I don't understand the concern | 18:12 |
| mriedem | i wouldn't block on it | 18:12 |
| mriedem | just seems like we're maybe trying to solve a problem we're making for ourselves | 18:12 |
| mriedem | which we maybe don't need to care about | 18:13 |
| dansmith | I tell you who's making problems.. mriedem :) | 18:13 |
| mriedem | i've said a million times that guy is an insufferable asshole | 18:13 |
| dansmith | heh | 18:13 |
| elbragstad | dumb question: does exposing the attribute actually have an impact on someone being able to unlock or lock an instance? | 18:14 |
| mriedem | it's an indication if they can unlock it | 18:14 |
| gmann | i agree that 'can i unlock' is policy control things. exposing locked_by I consider the benefit of as user i know my instances is locked by someone else like admin (system-admin or project-admin) and i can ask them to unlock | 18:15 |
| dansmith | I imagine the impetus for this whole thing is, someone tries to do something, can't because locked, so they don't understand why and have to call support to figure out why.. if there was a "locked by an admin, reason: because you didn't pay" then .. they know | 18:15 |
| elbragstad | but if that attribute wasn't exposed to them, could they still build the same request and send it to nova and have it behave the same way? | 18:15 |
| dansmith | elbragstad: yes | 18:15 |
| dansmith | elbragstad: but they may not understand why it fails if they don't see it was locked by someone else | 18:15 |
| mriedem | right i see the value in the reason field for sure | 18:15 |
| gmann | then we can leave locked_by to expose | 18:16 |
| openstackgerrit | Merged openstack/nova stable/queens: Allow utime call to fail on qcow2 image base file https://review.openstack.org/633493 | 18:16 |
| openstackgerrit | Merged openstack/nova master: Update master for stable/stein https://review.openstack.org/645463 | 18:16 |
| mriedem | elbragstad: this is the check code on unlock https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/lock_server.py#L47 | 18:16 |
| openstackgerrit | Merged openstack/nova stable/stein: Update .gitreview for stable/stein https://review.openstack.org/645461 | 18:16 |
| mriedem | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4025 | 18:17 |
| *** xek_ has quit IRC | 18:17 | |
| dansmith | gmann: you're saying you think exposing "can I unlock" is bad right? | 18:17 |
| dansmith | what if we just expose locked_by_me:True|False or locked_by=user|other | 18:17 |
| gmann | yeah. that is all control by policy | 18:17 |
| dansmith | okay I don't understand that at all, but whatever :) | 18:17 |
| gmann | if locked by other then policy should say 403 | 18:17 |
| dansmith | gmann: right, but you're going to make them try to determine that instead of just doing the check ourselves? we can ask policy about it, just like we do for the conditional exposure of fault | 18:18 |
| dansmith | try the put I mean | 18:18 |
| elbragstad | if the policy is written like "(role:admin and system_scope:all) or rule:owner" and locked_by_me is True|False, is locked_by useful? | 18:18 |
| mriedem | if i'm following this unlock logic, if an admin in my project locks my server, i can unlock it. but if an admin outside my project (global god like admin) locks my server, then i can only unlock it if i pass the override policy check right? | 18:18 |
| elbragstad | oh - maybe i was missing ^ that bit | 18:19 |
| gmann | mriedem: right | 18:19 |
| mriedem | and the override policy defaults to rule:is_admin i.e. system level god admin | 18:19 |
| mriedem | in legacy terms | 18:19 |
| gmann | elbragstad: we compare only project_id (not user id) for deciding it is owner or not. | 18:20 |
| mriedem | so with dan's proposal, on response, we checking if you pass is_expected_locked_by and if not, do you pass the override policy, exactly like the unlock code, | 18:21 |
| mriedem | are we going to want to perform that policy check when listing servers with details? ^ for each server? | 18:21 |
| elbragstad | mmm - so owner == anyone with a token scoped to the project that contains the instance | 18:21 |
| gmann | humm, no we should not perform that check in show. | 18:21 |
| mriedem | elbragstad: yes | 18:22 |
| elbragstad | and admin == anyone with the `admin` role on any project | 18:22 |
| gmann | yeah | 18:22 |
| openstackgerrit | Merged openstack/nova-specs master: Update Network Bandwidth resource provider spec https://review.openstack.org/644810 | 18:22 |
| mriedem | gmann: "no we should not perform that check in show." is what we would have to do to return an "can i unlock this server" boolean | 18:22 |
| dansmith | and it's what we do when we expose (or not) fault in instance | 18:22 |
| mriedem | i don't agree on that exactly being the same | 18:23 |
| dansmith | is there some api guideline that says "only reveal if someone can do something when they try" ? | 18:23 |
| mriedem | with fault traceback it's just a show/hide | 18:23 |
| gmann | yeah, seems like. | 18:23 |
| mriedem | maybe we do a policy check on every instance when listing, for faults i mean, would have to look | 18:23 |
| dansmith | you guys keep saying "we shouldn't do that" but I haven't seen any *reason* :) | 18:23 |
| mriedem | anywho | 18:23 |
| *** jdillaman has quit IRC | 18:25 | |
| mriedem | well i guess we don't do a context.can check when listing servers to see if we can show the fault traceback https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/views/servers.py#L550 but it's close, just based on the hard-coded is_admin flag in the context | 18:25 |
| dansmith | also, locked_by_me=True|False is something we can do today, something we can do in the future if we store the locking user, and something that doesn't require a policy check on show.. why is that not okay? | 18:25 |
| mriedem | which elbragstad wants to remove | 18:25 |
| *** _alastor_ has quit IRC | 18:25 | |
| *** erlon has quit IRC | 18:25 | |
| mriedem | dansmith: you mean locked_by_owner yeah? | 18:26 |
| dansmith | sure | 18:26 |
| mriedem | where the logic would just be locked_by_owner = instance.locked_by == 'owner' | 18:26 |
| dansmith | or locked_by=Owner|NotOwner | 18:26 |
| dansmith | right | 18:26 |
| dansmith | don't say admin, just say "not you" | 18:26 |
| elbragstad | ++ | 18:27 |
| * dansmith is still waiting on the reasoning for not doing a policy check | 18:27 | |
| mriedem | i'd be more ok with doing locked_by=owner|other | 18:27 |
| mriedem | i'm trying to express this... | 18:27 |
| * dansmith expects a simpson reference youtube any second | 18:28 | |
| dansmith | *simpsons | 18:28 |
| mriedem | but doing a policy check when building a response based on policy for something that would be checked during a subsequent unlock request, seems very weird and capability-ish to me, which we don't do elsewhere. it's true we check policy when building a response to hide things for non-admins, like system details (hosts, fault traceback, etc), but those aren't capability things | 18:28 |
| gmann | I am also ok for locked_by=owner|other if we cannot expose the UUID. | 18:28 |
| mriedem | gmann: we don't store the uuid anywhere for who locked the instance | 18:28 |
| elbragstad | if instances stored user information instead of project ids, we wouldn't need two lock properties, would we? | 18:28 |
| gmann | mriedem: yeah, i mean if we do not want to change that as it will be complex | 18:29 |
| dansmith | mriedem: so the reason is we don't do exactly this elsewhere and it "feels wrong" ? | 18:29 |
| mriedem | correct | 18:29 |
| dansmith | I mean, I understand that things feel wrong.. I don't understand why this one does, but that's okay | 18:29 |
| dansmith | alright | 18:29 |
| mriedem | i could argue it'd be nice to have a "can_be_resized" flag in the server response | 18:29 |
| mriedem | that doesn't mean we're going to add that | 18:29 |
| dansmith | people have asked for that :) | 18:29 |
| mriedem | i'm sure they have | 18:29 |
| dansmith | based on all the reasons you *can't* do that | 18:30 |
| mriedem | and we've talked about that capabilities API for several years | 18:30 |
| dansmith | how's that coming? :) | 18:30 |
| mriedem | it's mostly rotted flesh in my storage room | 18:30 |
| dansmith | because to me, | 18:30 |
| mriedem | don't tell the cops please | 18:30 |
| dansmith | the capaibiltiies api is the same thing, but all in one place, which IMHO lends further support to it being a good idea, but just not doable until we have enough other ones for critical mass :) | 18:31 |
| dansmith | s/doable/palatable/ | 18:31 |
| dansmith | and that's unfortunate | 18:31 |
| dansmith | but anyway, exposing locked_by=owner|other seems like a reasonable compromise | 18:31 |
| mriedem | i'd be fine with a GET /servers/{server_id}/capabilities API | 18:32 |
| mriedem | and we start rolling shit into that like this | 18:32 |
| dansmith | to be clear, | 18:32 |
| dansmith | that means someone has to get the server, see that it's locked and a reason, then go fetch the capabilities of the server to see if they can unlock it | 18:32 |
| dansmith | which is fine, but... | 18:32 |
| mriedem | they could just try to unlock it | 18:32 |
| mriedem | get the 403 | 18:33 |
| mriedem | like always | 18:33 |
| dansmith | sure | 18:33 |
| mriedem | if locked_by=other | 18:33 |
| elbragstad | ^ that's what people do now with other APIs (not necessarily in nova) | 18:33 |
| mriedem | then there is a reasonable guess they might not be able to unlock it | 18:33 |
| mriedem | which the reason should tell them why it's locked | 18:33 |
| dansmith | I dunno, I'm just having trouble following all the logic spaghetti, but I don't really need to care | 18:33 |
| elbragstad | does the reason include something like "this was locked by an admin, pay your bill pls" | 18:34 |
| gmann | yeah lock_reason can tell, "this is locked by admin/system and ask them to unlock after xyz condition/time" | 18:34 |
| gmann | elbragstad: yeah, i think it can be | 18:34 |
| dansmith | elbragstad: I know they do, and I'm sure it doesn't set off any alarms to hit 403s, but it feels like trying sudo vi /etc/passwd on a system to see if you can, and if not, it's reported :) | 18:34 |
| gmann | IMO with lock_reason info in response can explain the locked_by things. till now we did not have lock_reason so may be locked_by was important to know | 18:35 |
| elbragstad | yeah - true... the same argument has been made to have a capabilities-like api in keystone, but we're obviously not there yet, either | 18:35 |
| dansmith | lock_reason is for humans, locked_by is for computers, IMHO | 18:35 |
| dansmith | so not really a substitute | 18:35 |
| mriedem | i think i'm going to go lock myself in the garage with the car running | 18:36 |
| dansmith | elbragstad: last time I went jiggling door handles in my neighborhood to see who has theirs locked, I was asked to not do that | 18:36 |
| elbragstad | make sure you set locked_by=owner | 18:36 |
| dansmith | mriedem: unfortunately if you have a catalytic converter that's going to take a really long time | 18:36 |
| gmann | dansmith: ok so computer purpose still can be fulfilled by locked_by=other|owner. | 18:37 |
| dansmith | gmann: yes | 18:37 |
| elbragstad | dansmith yeah - not the best u-x, i agree | 18:37 |
| *** tssurya has joined #openstack-nova | 18:37 | |
| gmann | i am thinking exposing exact UUID of locked person (admin etc) can be security leak ? | 18:38 |
| gmann | so just say you can unlock it or not via locked_by=owner|other seems appropriate for me now | 18:38 |
| gmann | for example exposing system admin id to end user etc | 18:39 |
| *** sridharg has quit IRC | 18:40 | |
| *** eharney has joined #openstack-nova | 18:40 | |
| *** tbachman has joined #openstack-nova | 18:40 | |
| openstackgerrit | melanie witt proposed openstack/nova-specs master: Move Stein implemented specs https://review.openstack.org/645749 | 18:45 |
| *** pcaruana has quit IRC | 18:45 | |
| melwitt | hm, wanted to get this other spec update done before moving specs but it's in merge conflict https://review.openstack.org/#/c/639033/1 | 18:45 |
| mriedem | someone want to send this queens change through https://review.openstack.org/#/c/643219/ | 18:47 |
| melwitt | er... I guess it's not. just somehow it makes the move specs patch in merge conflict | 18:47 |
| mriedem | i'll start the release request | 18:47 |
| mriedem | melwitt: i'll get that spec update | 18:47 |
| melwitt | mriedem: thanks. I'll get the queens patch | 18:47 |
| *** rchurch has quit IRC | 18:48 | |
| *** eharney has quit IRC | 18:52 | |
| *** eharney has joined #openstack-nova | 18:54 | |
| openstackgerrit | Merged openstack/nova-specs master: Update alloc-candidates-in-tree https://review.openstack.org/639033 | 19:01 |
| *** awaugama has quit IRC | 19:01 | |
| openstackgerrit | Merged openstack/nova stable/pike: Fix bug case by none token context https://review.openstack.org/603044 | 19:05 |
| *** tbachman_ has joined #openstack-nova | 19:05 | |
| *** tbachman has quit IRC | 19:05 | |
| *** tbachman_ is now known as tbachman | 19:06 | |
| *** fried_rolls is now known as fried_rice | 19:07 | |
| temka | Uh, do we timestamp our logs in local time? | 19:09 |
| fried_rice | dansmith: I thought to give more people time to look at it and comment. | 19:10 |
| *** tjgresha_nope has joined #openstack-nova | 19:10 | |
| *** tjgresha has joined #openstack-nova | 19:10 | |
| dansmith | fried_rice: roger | 19:11 |
| fried_rice | dansmith: Also, the thing mriedem is working on with the multiattach capability has raised an interesting question that we're still working through. Namely, where do you put the required trait (which request group)? | 19:11 |
| fried_rice | Not that that necessarily should hold up your spec, but it's giving me pause. | 19:11 |
| mriedem | fried_rice: did you see my change on that patch? | 19:11 |
| dansmith | yeah, would rather save that for implementation | 19:11 |
| mriedem | i'm not using a request group anymore | 19:11 |
| mriedem | i'm hitching a ride on the RequestSpec.flavor | 19:12 |
| mriedem | cash, grass or ..., no traits ride for free | 19:12 |
| fried_rice | mriedem: been afk for the past couple hours, if that's when you changed it. But eventually you're adding it to a request group, albeit maybe indirectly. | 19:12 |
| dansmith | mriedem: I was trying to decide if I like or hate that | 19:12 |
| mriedem | fried_rice: right indirectly | 19:12 |
| openstackgerrit | melanie witt proposed openstack/nova-specs master: Move Stein implemented specs https://review.openstack.org/645749 | 19:12 |
| dansmith | on the one hand it saves the "how do we request it" for the deeper layers, which might be good, | 19:12 |
| fried_rice | on the other hand, it isn't part of the flavor | 19:13 |
| dansmith | but also modifying the flavor, while it's the reason we added it and store it separately, strikes me as "this doesn't FEEL right" | 19:13 |
| fried_rice | that's my gut too, but I haven't really thought about it | 19:13 |
| mriedem | i did it for expediency b/c i couldn't figure out the semantics of the internal request group stuf | 19:14 |
| mriedem | *stuff | 19:14 |
| dansmith | yeah and the fact that you have to also worries me | 19:14 |
| mriedem | it kept trying to add an empty numbered resources dict, and there are no resources in my request group | 19:14 |
| mriedem | it's likely a bug in the resource request / group stuff internally | 19:14 |
| fried_rice | where should it add it? | 19:15 |
| mriedem | it was also just one of those, "oh hey i have like 2 hours left today to do something, how about i hack this together and see how it goes" thing | 19:15 |
| dansmith | doesn't the base unnumbered group correspond to the whole request? | 19:16 |
| mriedem | fried_rice: this is what i was getting | 19:16 |
| mriedem | GET /placement/allocation_candidates?limit=1000&required1=COMPUTE_VOLUME_MULTI_ATTACH&resources=MEMORY_MB%3A512%2CVCPU%3A1&resources1= | 19:16 |
| dansmith | something that is a trait on te host that is fundamental seems like it should be easy to add | 19:16 |
| mriedem | and i'd get a 400 on that empty resources1 | 19:16 |
| mriedem | the unnumbered resources there are from the flavor | 19:16 |
| fried_rice | mhm, because the API refuses to allow you to specify requiredN without a corresponding resourcesN | 19:16 |
| mriedem | the required1/resources1 were from the group i added | 19:16 |
| mriedem | ok, | 19:17 |
| fried_rice | so when whoever wrote that, they didn't see a need to not send the resourcesN through every time. | 19:17 |
| mriedem | so the problem is the RequestGroup for the flavor gets built long after my code | 19:17 |
| mriedem | i didn't want the requiredN, but i couldn't figure out how to not make it add the number | 19:17 |
| *** weshay|rover is now known as weshay | 19:17 | |
| dansmith | mriedem: right but they should be in the unnumbered one regardless of where they came from in this case | 19:17 |
| fried_rice | right, but the problem is, where *do* you want to put the trait? | 19:17 |
| mriedem | fried_rice: alongside the flavor | 19:18 |
| mriedem | flavor group | 19:18 |
| fried_rice | Okay, but if we say that, then we're *requiring* the VCPU/MEMORY_MB to come from the unnumbered group | 19:18 |
| fried_rice | which is not (yet) a requirement otherwise. | 19:18 |
| mriedem | *head explodes* | 19:18 |
| dansmith | mriedem: if I cut the cat off your car can I sit in your garage with you? | 19:18 |
| fried_rice | and it's going to explode (like mriedem's head) when we do numa. | 19:18 |
| mriedem | dansmith: there is plenty of room, laura is at work | 19:19 |
| mriedem | i'll get the lawn chairs down | 19:19 |
| * dansmith heads over with his hacksaw | 19:19 | |
| mriedem | no need, i've got one | 19:19 |
| dansmith | even better, won't have to check a bag | 19:19 |
| * fried_rice took a few lines to realize we were talking about a catalytic converter | 19:19 | |
| * fried_rice imagined fur getting caught in the hacksaw teeth and stuff | 19:19 | |
| dansmith | indeed, but catalytic is so hard to type when you only have hours left on this planet | 19:19 |
| fried_rice | So go with me on this: Can't I theoretically today put resources1:VCPU=1, resources1:MEMORY_MB:2048 into my flavor extra specs? And it'll ignore the base flavor values for vcpus and ram? | 19:22 |
| *** phasespace has joined #openstack-nova | 19:23 | |
| fried_rice | down the road, I'll get a request with no unnumbered group | 19:23 |
| fried_rice | so I'll have required=MULTIATTACH but no resources=* | 19:24 |
| fried_rice | which I think will result in the same failure | 19:24 |
| * fried_rice goes to look at the latest thing cdent did... | 19:24 | |
| mriedem | clearly we have outsmarted ourselves | 19:26 |
| mriedem | mission accomplished | 19:26 |
| fried_rice | yeah, because this | 19:27 |
| fried_rice | https://github.com/cdent/placecat/blob/d3241a6fb0d8701072986c169615787f7484a3b7/gabbits/trait-pairs.yaml#L109 | 19:27 |
| fried_rice | doesn't work, we've got an issue to work through. | 19:27 |
| fried_rice | I have to go do phone things now :( | 19:27 |
| mriedem | whole bunch of stable/pike changes we could move through https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:stable/pike+label:Code-Review=1 | 19:32 |
| openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Refix disk size during live migration with disk over-commit https://review.openstack.org/631372 | 19:33 |
| *** igordc has quit IRC | 19:34 | |
| elbragstad | i assume by previous the previous discussion, capabilities in nova stalled out? | 19:36 |
| * elbragstad sucks at typing | 19:36 | |
| *** jmlowe has quit IRC | 19:40 | |
| *** mriedem has quit IRC | 19:46 | |
| *** DarthNau has quit IRC | 19:46 | |
| *** BjoernT has quit IRC | 19:47 | |
| EmilienM | hi | 19:47 |
| EmilienM | we need https://review.openstack.org/#/c/626952/ in Stein, please | 19:47 |
| *** mriedem has joined #openstack-nova | 19:47 | |
| mriedem | elbragstad: yup | 19:48 |
| EmilienM | if https://review.openstack.org/#/c/626952/ doesn't land, we'll have to carry that patch downstream | 19:48 |
| EmilienM | we can't deploy Nova with SSL atm | 19:48 |
| EmilienM | not sure why it's being postponed to Train, really | 19:48 |
| gmann | melwitt: ack your comment on policy improvement spec. I am doing QA release things today. I will ping you on monday to discuss that and probably next plan. | 19:48 |
| EmilienM | melwitt ^ | 19:48 |
| elbragstad | mriedem is the main reason just because there aren't enough people to work on it or was it something else? i tried looking for a spec in review to find the answer but didn't see any | 19:49 |
| *** luksky has joined #openstack-nova | 19:50 | |
| melwitt | EmilienM: I don't think it's being postponed, should go into rc2. were you ok with that mriedem? | 19:50 |
| mriedem | melwitt: idk about rc2. mdbooth said as long as it's merged on master (Train) he's happy to backport internally rather than upstream. | 19:50 |
| EmilienM | +1 for rc2, thanks | 19:50 |
| dansmith | last I asked, fried_rice said it was waiting until after rc something | 19:50 |
| dansmith | right, that ^ | 19:50 |
| melwitt | oh | 19:50 |
| dansmith | but I really don't know | 19:51 |
| EmilienM | I would rather avoid downstream only backports | 19:51 |
| EmilienM | but I don't really care as I don't maintain this rpm downstream | 19:51 |
| mriedem | and i'd rather avoid putting some crazy eventlet thing in rc2 that we don't see in the gate | 19:51 |
| EmilienM | it's just ugly that the rest of users will have the same issue as we have | 19:51 |
| melwitt | from what mdbooth said it is a regression though, for those running an unlucky combo of package versions right | 19:51 |
| mriedem | we gate on bionic which is py36 and don't see this upstream in CI, so i can't explain the difference | 19:51 |
| mriedem | yeah maybe | 19:52 |
| mriedem | i've not followed the ML thread | 19:52 |
| dansmith | mdbooth also said it was fine for train | 19:52 |
| fried_rice | dansmith: At this point I'm waiting for you to re+2 | 19:53 |
| fried_rice | since you had done it before | 19:53 |
| melwitt | ok... I was just thinking if it's a regression it would be nice to fix it for stein ppl but if most think it should be train-only, then I'll go with the consensus | 19:53 |
| dansmith | fried_rice: I haven't reviewed it in its current form and you said you'd ping me if you needed something | 19:53 |
| fried_rice | okay. ping. | 19:53 |
| dansmith | fried_rice: I'm not super comfortable with it at this point (I reviewed it initially a while ago) | 19:53 |
| dansmith | so I say let melwitt do it if she thinks it's a good idea | 19:53 |
| melwitt | cool gmann, thanks | 19:54 |
| mriedem | i have to spend my requisite 2 hours per month to look at fixing https://launchpad.net/bugs/1790204 before writing up my monthly internal report... | 19:54 |
| openstack | Launchpad bug 1790204 in OpenStack Compute (nova) "Allocations are "doubled up" on same host resize even though there is only 1 server on the host" [High,Triaged] | 19:54 |
| dansmith | ten revisons ago and a week prior | 19:54 |
| mriedem | honestly my +2 on that is mostly, (1) it's not in rc1, (2) it's eventlet and i'm not ever comfortable with eventlet stuff, (3) lots of people have been working on it and say it fixes the problem in a reproduction system so (4) i'm going to trust mdbooth on it | 19:55 |
| fried_rice | yes, and I'm willing to do the same, but I wanted to give dansmith the opportunity to look at it again because he seemed to understand it (10 patch sets and a week ago). | 19:56 |
| melwitt | yeah, I was going to say, that's the best my +2 could be too, for the same reasons | 19:56 |
| melwitt | eventlet monkey patch ordering is not really my forte | 19:56 |
| dansmith | fried_rice: don't wait on me | 19:56 |
| fried_rice | okay. | 19:56 |
| fried_rice | EmilienM: +A | 19:57 |
| EmilienM | fried_rice: thank you | 19:58 |
| mriedem | i'm still not really comfortable with pushing this into an rc2 | 20:00 |
| mriedem | i'd be much cooler with landing it in train, and then if non-OSP people come along after stein GA saying they have the same issue (maybe zigo - he tends to tickle weird stuff like this), then we say ok let's backport and do a quick 19.0.1 on stable | 20:01 |
| mriedem | or thode or coreycb | 20:02 |
| mriedem | you know, other distro gremlins :) | 20:02 |
| mriedem | i say that with the utmost affection | 20:02 |
| *** tbachman has quit IRC | 20:04 | |
| *** EmilienM has left #openstack-nova | 20:07 | |
| coreycb | mriedem: we'll be ramping up our testing including SSL in the next couple of weeks. so we haven't hit that yet. | 20:14 |
| mriedem | coreycb: ack cool | 20:17 |
| coreycb | mriedem: will keep you posted. we're py3 only at this point so i imagine that we'll hit it as well. grrr eventlet. | 20:18 |
| *** igordc has joined #openstack-nova | 20:19 | |
| *** jmlowe has joined #openstack-nova | 20:20 | |
| mriedem | py3 what? | 20:21 |
| mriedem | 3.5? | 20:21 |
| mriedem | or bionic 3.6? | 20:21 |
| mriedem | oh you mean just py3 support | 20:21 |
| mriedem | yeah apparently osp didn't hit issues with py3.5 | 20:21 |
| coreycb | py3.6 for bionic-stein. and py3.7 too but that is for disco-stein so not a common target. | 20:22 |
| openstackgerrit | Merged openstack/nova master: Fix return param docstring in check_can_live_migrate* methods https://review.openstack.org/645610 | 20:41 |
| *** ralonsoh has quit IRC | 21:06 | |
| *** mdbooth_ has joined #openstack-nova | 21:06 | |
| *** mdbooth has quit IRC | 21:09 | |
| *** igordc has quit IRC | 21:16 | |
| *** igordc has joined #openstack-nova | 21:16 | |
| *** mchlumsky has quit IRC | 21:30 | |
| mriedem | so i guess if your cloud supports strict affinity groups + resize, you *have* to enable allow_resize_to_same_host yeah? otherwise you could just never resize any servers in an affinity group | 21:35 |
| mriedem | which is fun | 21:35 |
| mriedem | seems allow_resize_to_same_host should really just be a weigher rather than a true/false config | 21:35 |
| *** mgoddard has quit IRC | 21:47 | |
| *** mgoddard has joined #openstack-nova | 21:47 | |
| *** zhubx007 has quit IRC | 21:57 | |
| *** zhubx007 has joined #openstack-nova | 21:57 | |
| openstackgerrit | Merged openstack/nova stable/queens: Fix resource tracker updates during instance evacuation https://review.openstack.org/643219 | 22:09 |
| *** cfriesen has joined #openstack-nova | 22:13 | |
| mriedem | dansmith: i don't suppose you remember off the top of your head why we used force_hosts/nodes for the rebuild to same host run through the scheduler where we still want to use the filters, iow, why didn't we use requested_destination? | 22:14 |
| mriedem | the former skips filters, the later restricts to a host but runs filters | 22:14 |
| mriedem | i think we could have avoided this https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L584 | 22:15 |
| mriedem | i know that was all a crazy mess at the time and we were just trying to get something working | 22:15 |
| dansmith | mriedem: because we have to run the numa filter right? | 22:28 |
| dansmith | I mean, I barely remember this, but I remember we needed to run the numa filter even though we knew where it was going to validate that the new image still works with the requested numa topology on the host or whatever | 22:28 |
| mriedem | yeah that's not my point, i'm just wondering why we used force_hosts over requested_destination | 22:29 |
| mriedem | requested_destination will still run the filters | 22:29 |
| mriedem | and avoid needing to look for that secret hint here https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L584 | 22:29 |
| dansmith | oh I thought you said one doesn't run the filters | 22:29 |
| mriedem | force_hosts doesn't normally | 22:29 |
| mriedem | which is why that condition had to be added | 22:29 |
| dansmith | [15:14:55] <mriedem>the former skips filters, the later restricts to a host but runs filters | 22:29 |
| mriedem | we could have just avoided that by using requested_destination | 22:29 |
| dansmith | oh I see | 22:30 |
| dansmith | then I dunno | 22:30 |
| dansmith | :D | 22:30 |
| mriedem | just rushing i'm guessing | 22:30 |
| dansmith | could be | 22:30 |
| mriedem | i'm basically needing to do some similar hackery for resize to same host and got digging into that code is all | 22:30 |
| dansmith | we did that in dublin under threat of snowpocalypse (i.e. 1" of snow) | 22:30 |
| openstackgerrit | Merged openstack/nova master: Eventlet monkey patching should be as early as possible https://review.openstack.org/626952 | 22:31 |
| openstackgerrit | melanie witt proposed openstack/nova-specs master: Move Stein implemented specs https://review.openstack.org/645749 | 22:37 |
| *** whoami-rajat has quit IRC | 22:41 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Alternative fix to bug 1790204 (does not work yet) https://review.openstack.org/645954 | 22:48 |
| openstack | bug 1790204 in OpenStack Compute (nova) "Allocations are "doubled up" on same host resize even though there is only 1 server on the host" [High,Triaged] https://launchpad.net/bugs/1790204 | 22:48 |
| mriedem | fried_rice: i tried my alternative and it just blasted me right in my stupid face ^ | 22:48 |
| mriedem | dansmith: oh yeah teehee https://review.openstack.org/#/c/645954/1/nova/compute/api.py | 22:49 |
| mriedem | i think it's time for me to start working on a project where i'm just doing application development on top of something else where the shit can roll downhill rather than breaking my head working on essentially an operating system | 22:54 |
| *** ivve has joined #openstack-nova | 22:58 | |
| *** tosky has quit IRC | 23:01 | |
| *** mriedem has quit IRC | 23:04 | |
| *** luksky has quit IRC | 23:20 | |
| *** tbachman has joined #openstack-nova | 23:24 | |
| *** tbachman has quit IRC | 23:24 | |
| *** tbachman has joined #openstack-nova | 23:25 | |
| *** igordc has quit IRC | 23:27 | |
| *** igordc has joined #openstack-nova | 23:27 | |
| *** tssurya has quit IRC | 23:29 | |
| *** igordc has quit IRC | 23:33 | |
| *** N3l1x has quit IRC | 23:34 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!