Wednesday, 2020-08-26

*** slaweq_ has joined #openstack-nova00:10
*** slaweq has quit IRC00:12
*** gyee has quit IRC00:26
*** tbachman has quit IRC00:38
*** tbachman_ has joined #openstack-nova00:38
*** tbachman has joined #openstack-nova00:42
*** tbachman_ has quit IRC00:43
*** takamatsu has quit IRC00:45
*** spatel has quit IRC00:51
*** takamatsu has joined #openstack-nova00:51
openstackgerritGhanshyam Mann proposed openstack/nova master: Migrate default policy file from JSON to YAML  https://review.opendev.org/74805900:54
*** JamesBenson has joined #openstack-nova01:38
*** Liang__ has joined #openstack-nova01:41
*** JamesBenson has quit IRC01:42
*** Yumeng has joined #openstack-nova02:15
*** boxiang_ has quit IRC02:30
*** boxiang_ has joined #openstack-nova02:30
*** Liang__ has quit IRC02:36
*** Liang__ has joined #openstack-nova02:37
*** dklyle has quit IRC02:40
*** david-lyle has joined #openstack-nova02:40
openstackgerritYingji Sun proposed openstack/nova master: Set different VirtualDevice.key  https://review.opendev.org/71356502:51
*** chenhaw has quit IRC02:51
*** chenhaw has joined #openstack-nova02:52
openstackgerritMerged openstack/nova stable/stein: Should not skip volume_size check for bdm.image_id == image_ref case  https://review.opendev.org/74455302:55
*** sapd1_x has joined #openstack-nova03:17
*** rcernin_ has joined #openstack-nova03:25
*** rcernin has quit IRC03:25
openstackgerritMerged openstack/nova stable/ussuri: compute: Do not allow rescue attempts using volume snapshot images  https://review.opendev.org/74270603:38
*** JamesBenson has joined #openstack-nova03:39
*** JamesBenson has quit IRC03:44
*** links has joined #openstack-nova03:54
*** Liang__ has quit IRC03:57
*** psachin has joined #openstack-nova03:57
*** chenhaw has quit IRC04:00
*** chenhaw has joined #openstack-nova04:00
*** mkrai has joined #openstack-nova04:05
*** dave-mccowan has quit IRC04:07
openstackgerritMerged openstack/nova stable/rocky: compute: Allow snapshots to be created from PAUSED volume backed instances  https://review.opendev.org/72917704:14
*** jmlowe has quit IRC04:18
openstackgerritElancheran S proposed openstack/nova stable/ussuri: Removes the delta file once image is extracted  https://review.opendev.org/74803404:25
*** jmlowe has joined #openstack-nova04:29
*** evrardjp has quit IRC04:33
*** evrardjp has joined #openstack-nova04:33
openstackgerritTony Su proposed openstack/nova master: Provider Config File: Functions to merge provider configs to provider tree  https://review.opendev.org/67652204:51
*** ratailor has joined #openstack-nova05:15
*** sridharg has joined #openstack-nova05:21
*** sridharg has quit IRC05:22
*** sridharg has joined #openstack-nova05:23
*** JamesBenson has joined #openstack-nova05:45
*** belmoreira has joined #openstack-nova05:46
*** belmoreira has quit IRC05:47
*** JamesBenson has quit IRC05:50
*** jsuchome has joined #openstack-nova05:52
*** redrobot has quit IRC05:53
*** mvorwerk_ has joined #openstack-nova06:11
*** mvorwerk has quit IRC06:13
*** slaweq_ has quit IRC06:14
*** david-lyle has quit IRC06:21
*** slaweq_ has joined #openstack-nova06:35
*** ralonsoh has joined #openstack-nova06:42
*** mkrai_ has joined #openstack-nova06:45
*** slaweq_ is now known as slaweq06:48
*** mkrai has quit IRC06:48
*** vishalmanchanda has joined #openstack-nova06:49
*** tesseract has joined #openstack-nova06:57
*** brinzhang_ has quit IRC06:59
*** brinzhang has joined #openstack-nova07:03
*** belmoreira has joined #openstack-nova07:12
openstackgerritKeigo Noha proposed openstack/nova master: Change default num_retries for glance to 3  https://review.opendev.org/74038907:18
*** yonglihe has joined #openstack-nova07:21
gibigmann: cool. I marked the policy by as implemented07:35
brinzhanggibi: hi, good morning07:39
*** tosky has joined #openstack-nova07:40
gibibrinzhang: hi07:40
brinzhanggibi: https://review.opendev.org/#/c/715326/19/nova/compute/manager.py@3654 this you mean, we should add the kwargs['accel_uuids'] to the rebuild() and _rebuild_default_impl() interfact, right?07:40
brinzhangthat we should change the virt/driver.py and virt/ironic/driver.py rebuild() functions?07:41
gibibrinzhang: yes, I mean that this parameter should be part of the https://github.com/openstack/nova/blob/master/nova/virt/driver.py#L285 interface07:42
brinzhanggibi: does it need to change the irnic driver? https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L166907:43
brinzhangs/irnic/ironic/07:43
gibiyes07:43
gibiit will be a parameter that the ironic driver gets but ignores at the moment07:44
brinzhangyeah, we will add this parameter, and send a mail to the ML later ^07:46
brinzhanggibi: thanks07:46
*** Luzi has joined #openstack-nova07:55
gibibrinzhang: thanks07:57
openstackgerritAlexandre Arents proposed openstack/nova master: Add a lock to prevent race during detach/attach of interface  https://review.opendev.org/74795708:01
*** mgoddard has quit IRC08:10
*** mgoddard has joined #openstack-nova08:11
bauzasgibi: fwiw, I'm starting to sheperd the rest of the provider config series08:17
*** dosaboy has quit IRC08:17
bauzasshepherd* even08:17
gibibauzas: thanks.08:17
*** dosaboy has joined #openstack-nova08:17
bauzasit needed me to look back at the spec, but thanks for having started to review the series08:18
gibiunfortunately the implementation changed ownwer couple of times so there are thing that needs (needed) detective work to understand the context08:21
*** rmart04 has joined #openstack-nova08:23
rmart04Hey All, Apologies if this isn't the right place to ask, but are there any obvious memory limitations gotchas when using dedicated cpu policy? I'm seeing OOM killing off my instances when the host has plenty of memory left :/ (Stein / CentOS7 Packages / KVM / Kolla-Ansible)08:26
openstackgerritElod Illes proposed openstack/nova stable/queens: compute: Allow snapshots to be created from PAUSED volume backed instances  https://review.opendev.org/72917808:27
openstackgerritBrin Zhang proposed openstack/nova master: Cyborg evacuate support  https://review.opendev.org/71532608:30
*** tosky has quit IRC08:32
*** tosky has joined #openstack-nova08:33
*** yonglihe has quit IRC08:33
*** arne_wiebalck has quit IRC08:33
*** luyao has quit IRC08:33
*** luyao has joined #openstack-nova08:34
*** arne_wiebalck has joined #openstack-nova08:35
*** Yumeng has quit IRC08:36
*** derekh has joined #openstack-nova08:37
*** yonglihe has joined #openstack-nova08:40
stephenfinlyarwood, gibi: So I've got three series on the go rn. What would I need to pay you to get at least one of them moving? :)08:42
gibistephenfin: the vpmem is on my menu for today08:43
gibiis that one of the three08:43
gibi?08:43
stephenfinunless you meant vTPM, no :(08:43
gibivtpm08:43
stephenfinthen yes \o/08:43
gibicool08:43
stephenfinhurrah08:43
gibithen you can expect review on that in the coming hours08:43
stephenfinawesome. Thanks, gibi :)08:44
gibino problemo, it is in the runway slot so it in focus :)08:44
openstackgerritStephen Finucane proposed openstack/nova master: Add type hints to 'nova.compute.manager'  https://review.opendev.org/74286308:46
openstackgerritStephen Finucane proposed openstack/nova master: Don't unset Instance.old_flavor, new_flavor until necessary  https://review.opendev.org/74199508:46
openstackgerritStephen Finucane proposed openstack/nova master: privsep: Add support for recursive chown, move_tree operations  https://review.opendev.org/74286408:46
openstackgerritStephen Finucane proposed openstack/nova master: Add type hints to 'nova.virt.libvirt.utils'  https://review.opendev.org/74286508:46
openstackgerritStephen Finucane proposed openstack/nova master: Add support for resize and cold migration of emulated TPM files  https://review.opendev.org/63993408:46
*** adriant has quit IRC08:48
*** adriant has joined #openstack-nova08:48
stephenfinalex_xu: Can you look at https://review.opendev.org/#/c/744021/ and its predecessor today? You probably have appropriate context08:54
stephenfinalex_xu: I suspect it impacts the mixed CPU work too08:54
openstackgerritBrin Zhang proposed openstack/nova master: Cyborg evacuate support  https://review.opendev.org/71532608:58
stephenfinrmart04: Using the dedicated CPU policy implies a single-node NUMA topology by default, and using a NUMA topology implies consuming memory from a single host NUMA node for each guest NUMA node09:01
stephenfinrmart04: That's a roundabout way of saying that I suspect you haven't reserved memory on per-node basis and may have allocated something like hugepages unevenly across NUMA nodes09:02
stephenfinrmart04: nova won't let you overcommit against a host NUMA node using nova instances, but it can't account for host processes or huge page allocations consuming memory on that node. You've to do that manually09:05
*** sapd1_x has quit IRC09:05
stephenfinrmart04: There are two config options of value here: 'reserved_host_memory_mb', which is a total amount of memory to reserve across all NUMA nodes, and 'reserved_huge_pages' which is a per-node reservation that applies when you're using explicit pagesizes09:07
bauzasstephenfin: gibi: fwiw, after the provider series, I was heading to review your vTPM series09:07
stephenfinrmart04: I suggest whacking up the first ('reserved_host_memory_mb'). It would be good to inspect idle memory consumption without instances on the host and use that value with some buffer.09:07
stephenfinrmart04: The second is rarely used. Unless you're using explicit page sizes for all instances, I wouldn't worry about it for now09:08
stephenfinrmart04: Hope that helps09:08
stephenfinbauzas: awesome. Thanks :)09:08
*** dtantsur|afk is now known as dtantsur09:09
lyarwoodstephenfin: just catching up with some downstream stuff but I'll get into your reviews after that09:13
bauzasalex_xu: gibi: I have a logging concern with https://review.opendev.org/#/c/693460/3609:16
bauzasalex_xu: gibi: or we will get a lot of logs if operators don't use this new feature :)09:17
gibibauzas: you mean https://review.opendev.org/#/c/693460/36/nova/compute/resource_tracker.py@120 ?09:18
gibibauzas: I see one extra log per compute node at compute service startup09:18
gibialso that config has a default value so it is set even if not specified in the config file09:19
bauzasgibi: correct, this won't be very talkative, but we then try to merge inventories even if the operator doesn't use it09:19
bauzasgibi: https://review.opendev.org/#/c/693460/36/nova/compute/resource_tracker.py@116509:19
bauzasand https://review.opendev.org/#/c/676029/51/nova/compute/provider_config.py@39109:20
bauzasall of this seems not needed09:20
bauzasgibi: I would have preferred a way to avoid calling all of this if no YAML files were there09:22
gibiCan you check from oslo config if the config value is set explicitly by the user or just the default value is applied by oslo?09:23
gibibecause current calling down to get_provider_configs is the only way to detect if there are config files09:24
*** spatel has joined #openstack-nova09:26
*** k_mouza has joined #openstack-nova09:26
gibiregarding the actual logging: I can see a reasoning to log what provider config files are loaded if any instead of logging if nothing is loaded09:28
*** spatel has quit IRC09:30
stephenfinreal trivial patch for the VMWare driver here https://review.opendev.org/#/c/713565/09:39
*** rcernin_ has quit IRC09:39
*** k_mouza has quit IRC09:40
openstackgerritStephen Finucane proposed openstack/nova master: Move revert resize under semaphore  https://review.opendev.org/74774609:41
openstackgerritStephen Finucane proposed openstack/nova master: Cleanup 'drop_move_claim' and '_drop_move_claim'  https://review.opendev.org/74774709:41
openstackgerritStephen Finucane proposed openstack/nova master: manager: Address TODO  https://review.opendev.org/74774809:41
openstackgerritStephen Finucane proposed openstack/nova master: manager: Move context manager up one level  https://review.opendev.org/74567409:41
openstackgerritStephen Finucane proposed openstack/nova master: compute: Add type hints for resize functions  https://review.opendev.org/74534109:41
openstackgerritStephen Finucane proposed openstack/nova master: WIP: compute: Add more type hints for resize functions  https://review.opendev.org/74567509:41
stephenfinlyarwood: are you respinning that nova-image-download-via-rbd series again today to add the missing '__init__.py' file etc.?09:42
openstackgerritMerged openstack/nova stable/ussuri: libvirt: Provide VIR_MIGRATE_PARAM_PERSIST_XML during live migration  https://review.opendev.org/74797209:45
gibistephenfin: I left a question in https://review.opendev.org/#/c/741995/10/nova/compute/manager.py@a493209:46
lyarwoodstephenfin: yeah09:52
*** sridharg has quit IRC09:54
stephenfingibi: replied09:54
gibistephenfin: thanks it is make sense now.09:55
*** k_mouza has joined #openstack-nova09:57
bauzasgibi: sorry, missed your ping09:57
stephenfingibi: fwiw, that patch is identical to this bug fix for a ResourceTracker race. I just didn't want to make the series depend on each other https://review.opendev.org/#/c/744958/09:57
stephenfingibi: Have been bugging melwitt and dansmith for reviews on that though so you're off the hook for reviews there :P09:57
*** gryf has quit IRC09:58
gibistephenfin: if it is identical then I can add my +2 there :)09:58
*** mkrai_ has quit IRC09:58
*** mkrai__ has joined #openstack-nova09:58
stephenfinyup, iirc cherry-picked and change-id modified to mark them as independent09:59
*** mkrai has joined #openstack-nova10:02
*** mkrai__ has quit IRC10:02
bauzasstephenfin: just saw https://review.opendev.org/#/c/747744/10:03
bauzaswill try to look at it too10:03
stephenfinnice one10:04
bauzasstephenfin: starting to look at the vTPM series but... wow, I need a beer :p10:06
gibiisn't it a bit too early for a beer? ;)10:07
lyarwoodblasphemy10:08
alex_xustephenfin: checking now10:08
alex_xubauzas: yea, as gibi said, it is just one log at startup, should be ok10:08
bauzasalex_xu: I changed my thoughts10:08
bauzasI don't wanna hold for a nit10:09
bauzasbut... this change needs a rebase either way :)10:09
bauzasgibi: technically, it's beer o'time10:10
bauzasten past nonn10:10
bauzasnoon10:10
bauzasif that's too early, then I don't know when10:11
alex_xubauzas: yea, that is due to tony_su rebase the previous patch, actually it needn't, just just gate failed, he should recheck instead of rebase.10:12
bauzasalex_xu: the branch I see from gerrit pulls the old revision10:13
bauzasalex_xu: what he can do tho is wait for the dependent change to be merged and then indeed a recheck should be fine10:14
*** cgoncalves has quit IRC10:14
gibibauzas: I rest my case, it is really afternoon now10:16
openstackgerritStephen Finucane proposed openstack/nova master: Provider Config File: Enable loading and merging of provider configs  https://review.opendev.org/69346010:18
alex_xubauzas: yea, that is my thought also10:18
stephenfinalex_xu, bauzas: There you go10:18
elodlyarwood: the persist_xml patch has merged in ussuri. what do you think about triggering a release in ussuri? do you mind if i create a release patch?10:18
alex_xustephenfin: hah, thanks :)10:19
bauzasstephenfin: thanks10:19
* bauzas goes away for a barbecue grilling, and then I'm back in 1 hour and half10:20
elodlyarwood: or should we wait until it gets merged all the way till stein and release ussuri / train / stein together?10:20
openstackgerritStephen Finucane proposed openstack/nova master: Deprecate filters that have been replaced by placement filters  https://review.opendev.org/74560510:22
gibibauzas: enjoy10:23
lyarwoodelod: if you have time feel free to create the release change for ussuri now10:23
lyarwoodelod: I was going to wait but there's nothing stopping us from releasing as it lands in each branch tbh10:24
elodlyarwood: ok, thanks, doing it now10:25
lyarwoodawesome thanks :)10:26
*** sridharg has joined #openstack-nova10:26
*** cgoncalves has joined #openstack-nova10:31
elodlyarwood , gibi : https://review.opendev.org/#/c/748156/10:35
elodseems minor version bump required anyway, but if you see other patch that should listed also that requires minor bump, too, just let me know and I'll update the commit message10:37
gibielod: +110:39
*** Luzi has quit IRC10:42
openstackgerritStephen Finucane proposed openstack/nova master: Deprecate filters that have been replaced by placement filters  https://review.opendev.org/74560510:47
*** sridharg has quit IRC10:53
*** mkrai has quit IRC10:55
rmart04Hi Stephenfin, thanks for this info. I'm just digesting what you've written. I am currently not using huge pages, but I think what you are saying is that one of the numa nodes ran out of memory, not the entire host. I'm not sure how reserved_host_memory_mb will help when the host has lots of memory available (250GB), i'm guessing we just ran out of memory on one particuar numa node? I should mention, we have10:58
rmart04 one large VM on this host with numa_nodes:2.10:58
stephenfinrmart04: Yes, exactly. We lock memory to a single NUMA node so it the instance tries to allocate memory and there's none on _that node_, you're flat out of luck10:59
*** jangutter has joined #openstack-nova11:00
stephenfinDoes the per-node memory allocation of that very large VM bump up against the per-host node total? Are your DIMMs evenly split between host NUMA nodes?11:00
stephenfini.e. you've 250GB available in total, but perhaps only a small bit on one particular NUMA node11:01
sean-k-mooneythe OOM reaper in the kernel runs per numa node so if a numa node runs out of memory it will start killing process if the kerenl needs to allcoate even if the other node is entirely empty11:01
rmart04So Host has 768GB split evenly across numa nodes. There is one instance, 750GB in size, and has seen more than half its memory used successfully in the past. I'm not sure how to see how much memory a particular numa node has available? Is that easy to see?11:02
sean-k-mooneyif you are using numa but not pinning or hugepages11:02
alex_xustephenfin: are we ensure the operator finish the pin_cpu_set to dedicated_cpu_set/sahred_cpu_set change before update to V release?11:02
sean-k-mooneythen we will place all the vms on numa node 0 and never use the second numa node11:02
sean-k-mooneyim working on fixing that this morning11:02
rmart04cpu_policy is dedicated, so  I believed its pinned11:03
alex_xustephenfin: or the operator can do that change when upgrade U to V?11:03
*** jangutter_ has quit IRC11:03
sean-k-mooneyah in that case we will stack all vms on the first numa node untill we run our of pinned cpus then use the next one11:03
stephenfinalex_xu: A user can continue to use the legacy vcpu_pin_set in U. I'm hoping to drop support for it in V, but I have too many series in flight at the moment so it might slip to W11:04
stephenfinalex_xu: If that's what you mean?11:04
sean-k-mooneyalex_xu: i think the plan is to remove vcpu_pin_set in v so they have to upgrade in U11:04
sean-k-mooneyideally they would do it in train11:04
sean-k-mooneybut they can do it in U11:04
sean-k-mooneyrmart04: you can see it in /sys11:05
sean-k-mooneyill get the path on sec11:05
alex_xustephenfin: sean-k-mooney ,I'm thinking of 744021 may stop the upgrade from U to V if the operate transft from legacy to new config in that time11:06
sean-k-mooneyrmart04: cat /sys/bus/node/devices/node*/meminfo11:06
alex_xuif we force that legacy config change finished in U, then it should be fine11:07
sean-k-mooneyalex_xu: its there because we had customer upgraind into an invlaid config11:08
stephenfinalex_xu: I don't understand the issue11:08
sean-k-mooneythey had vms with the old configuration where they were using isolate but the new config options11:08
stephenfinA user should never get into a situation where they have new-style config but a pinned instance is consuming VCPU instead of PCPU11:08
sean-k-mooneyso they were using vcpus instead of pcpus11:08
sean-k-mooneyeven thogh the host reported pcpus11:09
stephenfinThis patch should be backported to Ussuri and Train11:09
sean-k-mooneyyes11:09
stephenfinIs there something upgrade specific I'm missing, perhaps?11:10
sean-k-mooneyalex_xu: it was an edgecase we forgot to block, we taught it would be blocked by the numa toplogy filter but the check was missing11:10
alex_xuif the host with legacy config is running isolated thread instance, but the 744021 stop the isolated thread instance? (I'm still try to ramp up my mind)11:10
sean-k-mooneyalex_xu: if its using legacy configs it will allow it to boot11:10
sean-k-mooneyalex_xu: it only changes the behavior for new configs11:11
alex_xuoh, right11:11
sean-k-mooneyalex_xu: we dont allow the shared and dedicated set to overlap so the only time they contian the same values is if we are using the legacy config options11:12
sean-k-mooneyso  and host_cell.pcpuset != host_cell.cpuset11:12
sean-k-mooneyis an indirect check for is the host using new config options11:13
lyarwoodelod: sorry missed that https://review.opendev.org/#/c/747357/ wasn't included11:13
sean-k-mooneythat will also work in the schduler11:13
lyarwoodelod: can we wait for that and then release?11:13
sean-k-mooneysince we cant actully check the config directly there11:13
jsuchomelyarwood: gibi stephenfin https://review.opendev.org/#/c/574301/ finally green again, any further changes necessary?11:14
*** jangutter has quit IRC11:14
*** gryf has joined #openstack-nova11:15
*** jangutter has joined #openstack-nova11:15
elodlyarwood: oh, of course, sorry, I forgot to check what else on the queue :S11:15
openstackgerritMerged openstack/nova master: Provider Config File: Functions to merge provider configs to provider tree  https://review.opendev.org/67652211:16
lyarwoodjsuchome: https://review.opendev.org/#/c/746904/ - I just need to respin this after lunch and I think we should be good to go11:17
alex_xusean-k-mooney: but with new config, why we meet the case fallback to query VCPU?11:17
alex_xufallback query should find the host with legacy config11:18
* bauzas is back11:19
sean-k-mooneyalex_xu: not if all host are using the new configs and you forget to trun off the fallback11:19
sean-k-mooneythe fallback can select the vcpu on host with the new config options11:20
sean-k-mooneythere is noting in the fallback query to prevent that11:20
sean-k-mooneywe were relying on the numa toplogy filter to handel that case and this check was missing11:20
sean-k-mooneyalex_xu: for what its worth i was 99% certin the numa toplogy filter would block this untill i repoduced the issue locally11:21
alex_xusean-k-mooney: if forget turn the fallback off, the scheduler will chocie the host with shared_cpu_set?11:23
alex_xuoh, it is not, since host.pcpu is empty11:24
sean-k-mooneyso with the fall back enabled on a cloudl with all new config options11:28
sean-k-mooneyand smt enabled11:28
sean-k-mooneywhen we do the initall query the hyperthreading forbiden trait will cause the PCPU queury to retrun no hosts since they all have smt enabled11:29
alex_xusean-k-mooney: it is new deployed host, with dedicated and shared cpu set, that allows the mixed instance. But the fallback still enabled, then in the case of there is no more pCPU, then scheduler fallback to VCPU. Then host with shared cpu set allow that scheduling?11:29
sean-k-mooneythen the fallback with vcpus will return allocation agains tthe vcpu inventories11:29
sean-k-mooneyalex_xu: yes basicly11:30
sean-k-mooneyso what happens is we end up claiming vCPUs in placement and then pin the vm to the cpu_dedicated_set cpus11:31
alex_xuif that is the case, a shared thread instance also can be scheduled to that host, right?11:31
sean-k-mooneyyes11:31
alex_xuemm...that is still wrong11:32
sean-k-mooneywell shared instance and pinned can mix on the same host as of train11:32
sean-k-mooneythat is ok11:32
sean-k-mooneybut the allocation are wrong11:32
alex_xuso we should ask the user turn off the fallback before upgrading to V?11:32
sean-k-mooneywe shoudl remove it when we removed vcpu_pin_set11:33
*** JamesBenson has joined #openstack-nova11:33
sean-k-mooneywith https://review.opendev.org/#/c/744021/3 its safe to leave it on11:33
sean-k-mooneyas placement will still do the wrong thing but the numa toplogy filter or the hardware module on teh compute host will block it11:33
alex_xubut it doesn't fix the case for shared thread instance, right?11:33
sean-k-mooneythat is not broken11:34
sean-k-mooneyif you have cpu_policy=shared then it will request vcpus11:34
noonedeadpunkhey everyone! I was wondering if it's possible to prohibit users to use ephemeral storage? I was trying to look through the policies but the only thing I found and was related is to allow use zero disk flavors. While it allows to use cinder volumes, it doesn't prohibits ephemeral ones...11:34
sean-k-mooneythe fallback only applies to cpu_policy=dedicated11:34
sean-k-mooneyand cpu_thread_policy only applies to cpu_polcy=dedicated11:35
alex_xuyes11:35
noonedeadpunkEventually while I was looking I saw other ppl was also wondering about that opportuninty11:35
sean-k-mooneynoonedeadpunk: yes you can11:35
sean-k-mooneyvia config11:35
alex_xuI mean the case cpu_thread_policy is shared and cpu_policy is dedicated11:35
sean-k-mooneyoh in that case we dont claim vcpus at all11:35
sean-k-mooneywell we will11:36
sean-k-mooneybecause of the bug11:36
sean-k-mooneyalex_xu: the vm is still being pinned to the cpu_dedicated_set11:36
sean-k-mooneyand its emulator treads will stll be pinned to the cpu_shared_set11:37
sean-k-mooneywe just wont claim PCPUs in plament11:37
sean-k-mooneynoonedeadpunk: https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.max_local_block_devices11:37
sean-k-mooneynoonedeadpunk: you can set that to 011:37
alex_xuwait, the filter will stop that, since there is no enough pcpu in hostnuma.pcpu11:37
sean-k-mooneyalex_xu: no it wont11:38
noonedeadpunksean-k-mooney: oh, thanks! I was thinking about https://docs.openstack.org/nova/ussuri/configuration/config.html#libvirt.images_type setting to noop or smth but saw that's not an option11:38
alex_xuoh...I see now, due to hyper thread11:38
sean-k-mooneyalex_xu: it will only stop it if thos pcpus are claimed11:38
sean-k-mooneyso if the host is more or less empty it will spawn if its getting full then yes it can reject it11:39
sean-k-mooneybut its not always going to11:39
sean-k-mooneynoonedeadpunk: are you trybing to force all vms to be boot from volume11:40
noonedeadpunkyep11:40
*** JamesBenson has quit IRC11:40
sean-k-mooneyah i see ya so that config option is the way to go11:40
*** JamesBenson has joined #openstack-nova11:40
noonedeadpunktbh ideal scenario would be to force all except some selected group11:40
sean-k-mooneybut just be aware we consider swap to be a local disk too11:40
sean-k-mooneynoonedeadpunk: this is a per host option by the way11:41
sean-k-mooneyso you can set it on a subset of hosts11:41
sean-k-mooneyand ideally use an aggreate to group them for scheduling11:41
noonedeadpunkOh, so it's effective on compute, not on api/scheduler hosts?11:41
sean-k-mooneynoonedeadpunk: correct11:41
noonedeadpunkOk, good to know!11:41
noonedeadpunkthanks for the help and sorry for disturbing:)11:41
noonedeadpunkreally wasn't able to find this specific option with code words I was thinking about)11:42
sean-k-mooneynoonedeadpunk: what release are you on11:42
alex_xusean-k-mooney: ah, i see now. The host has enough pcpu for that instance, but the instance forbidden the hyperthread, that is why the placement reject, then fallback to VCPU. And then filter begin to filter based pcpu again, then the scheduling success11:42
sean-k-mooneyalex_xu: exactly11:42
alex_xufinally i see that11:42
alex_xusean-k-mooney: thanks11:42
sean-k-mooneyits an edgecase to and edgecase but customers hit it imeditely11:42
noonedeadpunksean-k-mooney: I think train mostly11:43
noonedeadpunk(maybe some U)11:43
sean-k-mooneyok so i think trian has isolated aggreates support one sec whil i get the link11:43
sean-k-mooneyhttps://docs.openstack.org/nova/latest/reference/isolate-aggregates.html11:43
sean-k-mooneywhat you can do is add a custom CUSTOM_BFV_ONLY trait to a subset of host and add it as a required trait in your BFV flavors11:44
*** raildo has joined #openstack-nova11:44
sean-k-mooneyif you configure the aggreate to requrie it then it will block non BFV flavor from landing on those hosts11:45
noonedeadpunkis scheduler.enable_isolated_aggregate_filtering option also compute specific?11:45
sean-k-mooneythen on those host you can also set the config option if you want too11:45
noonedeadpunk(I think not)11:45
sean-k-mooneynoonedeadpunk: that is cloud wide11:45
sean-k-mooneybut this allows you to use traits and host aggrate to dynamically force certen behavior11:46
noonedeadpunkyeah, that's pretty handy, thanks!11:46
sean-k-mooneyin this case it will prevent vms without the trait form landing on host in a given aggreate11:46
sean-k-mooneynoonedeadpunk: this would allow you to get teh same effect without needing to modify config options11:46
noonedeadpunk(I would probably still prefer this to be configurable by policy though tbh, but whatever)11:46
sean-k-mooneyin what way11:47
sean-k-mooneyhave a specific bfv_only flag of some kind? per host or cloud wide?11:47
sean-k-mooneynoonedeadpunk: im trying to understand if there is a gap that we might need to address beyond having a topic doc on how to use the exisitng feature to do this11:48
noonedeadpunkSo yeah, we have `os_compute_api:servers:create:zero_disk_flavor` so maybe option like  `os_compute_api:servers:create:local_drive` which by default allowed for everyone11:48
noonedeadpunkbut this can be overriden and local_drive can be created only to specific groups of users11:49
sean-k-mooneyah to block flavor creation with root, ephmeral or swap11:49
openstackgerritMerged openstack/nova stable/ussuri: libvirt: Do not reference VIR_ERR_DEVICE_MISSING when libvirt is < v4.1.0  https://review.opendev.org/74735711:49
sean-k-mooneythe only real issue i see with that is root ephmerla and swap might not be local11:50
openstackgerritMerged openstack/nova master: Set different VirtualDevice.key  https://review.opendev.org/71356511:50
sean-k-mooneye.g. if you are using ceph for storage11:50
noonedeadpunkAnd eventually currently I am....11:50
sean-k-mooneybut we proably could add a policy flag for each filed gmann is that a reasonable thing to do?11:50
sean-k-mooneygmann: question i have is it resonable to add a policy flag to restirct creating flavor with non 0 root, swap or ephemeral storage11:51
noonedeadpunkso, if I have ceph backend configured, than max_local_block_devices is not really applicable?11:51
sean-k-mooneynoonedeadpunk: more or less yes11:52
sean-k-mooneyi think it techniall is still enforced but its not really local11:52
noonedeadpunkah, ok, then this should work11:52
sean-k-mooneyim not sure if the point where we check that option know what storage is configured11:53
sean-k-mooneywe might have a skip for the rbd backend11:53
sean-k-mooneynoonedeadpunk: storage is one area i understand but avoid so the details are not something i rember and have to lookup each time11:54
* bauzas is backhttps://review.opendev.org/#/c/741500/10 ?11:55
bauzaswhoops11:55
bauzasgibi: any reason why you based https://review.opendev.org/#/c/742407/4 on top of https://review.opendev.org/#/c/741500/10 ?11:55
sean-k-mooneybauzas: a little over eger to be back :)11:55
* bauzas can't translate :)11:56
gibibauzas: I have no ide how I made that11:56
noonedeadpunkOk, so I have the following situation: some of the users, despite we don't have non-zero flavors in terms of disks, some of them still press "do not create volume" button in horizon, which creates ephemeral volume in ceph with size equall to the image size. And then they return crying that all data has been lost in case of some reboot/rebuilt/etc11:56
openstackgerritBalazs Gibizer proposed openstack/nova master: Undeprecate the vmwareapi driver  https://review.opendev.org/74240711:56
bauzasgibi: cool11:56
sean-k-mooneyits merged now so it does not really matter11:56
sean-k-mooneythe vtpm patch11:56
bauzasyup11:56
bauzasbut this was confusing me11:57
bauzasas I was looking at the hairy vtpm series11:57
gibibauzas: thanks for noticing11:57
sean-k-mooneygibi: you proably were reviewing stephens patch before11:57
bauzas(and gosh, this spec is... terrible https://specs.openstack.org/openstack/nova-specs/specs/victoria/approved/add-emulated-virtual-tpm.html )11:57
gibisean-k-mooney: could be11:57
sean-k-mooneyhehe its a good way to get extra review :P11:58
sean-k-mooneyput random patches into a seriese in the runway11:58
gibilol11:58
gibiI make a mental note to do it more often11:58
gibi:D11:58
sean-k-mooneywith the amount of patches in stephens seriese im not even sure he would notice one more when he is rebaseing them all11:59
gibi:D11:59
gibibtw, stephenfin: I've finished reading and commenting the vtpm series11:59
sean-k-mooneyit looks like they are all approved12:00
sean-k-mooneyat least for spwan12:00
sean-k-mooneythe move operation are still pending12:00
*** bbowen has quit IRC12:01
gibiyeah, spawn seems pretty solid to me12:01
sean-k-mooneythis will make efried happy to see12:01
bauzasI'm just reviewing the rest of the series12:03
bauzasbut honestly, mho is that we tried to be gentlemen with vtpm usage, by overcomplicating what we were providing as a feature :)12:03
bauzastrying to have vtpm instances be pets is nice12:03
*** bbowen has joined #openstack-nova12:04
bauzasbut we're pulling swift and trying to do maths to get whether we can still migrate an emulated file device or not, which overcomplicates things12:04
bauzasbut this is what it is, the spec is approved12:04
sean-k-mooneywell we try to make it behave like a real server12:05
bauzasthis is cloud12:05
sean-k-mooneyyep12:06
bauzasmost of the operations are admin-managed12:06
sean-k-mooneyin a cloud that would be atypical12:06
bauzasso in this case, trying to solve all the problems is nice, but this comes with a cost12:06
sean-k-mooneywell one of the proposal was no move operations12:06
bauzasbut the ship has sailed.12:06
sean-k-mooneywhich was very non admin frendly12:07
bauzasI'm okay with creating a fresh new tpm device with move operations12:07
bauzaswhat I'm struggling with is trying to reuse the existing data12:07
bauzasthis should be app-managed12:07
sean-k-mooneythe check is pretty simple12:07
sean-k-mooneyif anything changes version or type then data is not perserved12:07
sean-k-mooneyif the version and type do not change we copy it12:08
stephenfinbauzas: well, don't worry about swift. None of that is done12:08
sean-k-mooneythe special case is shelve12:08
stephenfinIs we might never do it. For now, shelve is simply blocked12:08
sean-k-mooneywhere we ahve to store it somewhere which is swift12:08
sean-k-mooneybauzas: the alternitive was to stick it in glance as a second image12:09
sean-k-mooneyor not store it at all12:09
bauzassean-k-mooney: yup, I saw in the series12:11
bauzasthe shelve case is the most complicated12:11
bauzasI also have concerns with rebuild12:11
bauzastrying to reuse the existing backup file is nice, but as a user I'd expect a fresh vTPM device if I rebuild12:12
sean-k-mooneyrebuild preseves the data if the version/type of tpm does not change12:12
sean-k-mooneybauzas: that fails the do what hardware does guideline12:12
bauzasthis data is ephemeral, right?12:12
sean-k-mooneybauzas: if you reinstall the os on your laptop your tpm is not cleared12:12
bauzasah, you're right, if you wanna mock the baremetal case12:13
bauzasbut in this case, all moves are just data loss12:13
bauzasespecially when I read https://specs.openstack.org/openstack/nova-specs/specs/victoria/approved/add-emulated-virtual-tpm.html#migrations-and-their-ilk12:13
bauzasbrr...12:13
sean-k-mooneybauzas: one of the usecases for this is storing encyrption keys or certs for authentication12:13
sean-k-mooneyso if you have data volumes or use the tpm as a cert store and use rebuild to upgrade your application you would not want to loose that data12:14
bauzasgibi: thoughts on keeping a list of CI maintainers somewhere in https://review.opendev.org/#/c/742407/512:14
bauzasgibi: if you have remembrance of anything about this...12:14
openstackgerritMerged openstack/nova master: Fix indentation nits  https://review.opendev.org/74676512:14
openstackgerritMerged openstack/nova master: Remove deprecated scheduler filters  https://review.opendev.org/74480012:14
sean-k-mooneybauzas: all move operation in the sepc are currently data perserving12:15
bauzasI do understand the usecase12:15
sean-k-mooneybauzas: we just copy the tpm data form one host to another12:15
bauzasI'm just saying we are adding more than necessary12:15
bauzasbut again, the ship has sailed12:15
sean-k-mooneybauzas: well it was felt that without perserving the data on move ops we could not resonably support them12:16
openstackgerritMerged openstack/nova master: tests: Add helpers for rebuild, cold migrate, and shelve/unshelve  https://review.opendev.org/74779212:16
sean-k-mooneybauzas: complexity is a valid concern but i think the current semantics are what i would expect12:16
bauzassean-k-mooney: yup, and again, I won't bother12:19
bauzaswe had a consensus, I do respect it.12:20
efriedA lot of blood, sweat, and tears went into getting that spec to where it is.12:20
efriedWhich involved some very heavy tradeoffs and compromises along the way.12:21
efriedThere's some saying like, "You know it's a good compromise when nobody is happy."12:21
*** _erlon_ has joined #openstack-nova12:22
efriedSpecifically to address the notion of pets: if you want a TPM, your VM is already a pet.12:22
gibiefried: o/12:23
efriedI will say that I am happy to see things like vTPM and provider config merging now -- two releases after I stopped working on them, which was multiple releases after I (and others) started working on them.12:23
elodlyarwood: btw, should we wait this to merge too? >>> https://review.opendev.org/#/c/744550/12:23
lyarwoodelod: that's less important tbh12:24
lyarwoodelod: just a nice to have, the other change fixes an issue introduced by an earlier change12:24
sean-k-mooneyefried: yep it will be nice to see them in the victoria project update hopefully as highlights12:26
elodlyarwood: ok, then I've updated the release patch12:27
elodthe other patch unfortunately had to recheck anyway, so it would mean at least 3-4 hrs to merge...12:28
bauzasgibi: lyarwood: stephenfin: -1 for relnote on https://review.opendev.org/#/c/631363/6112:32
bauzasstephenfin: I'm currently digging the long list of https://review.opendev.org/#/q/topic:bp/add-emulated-virtual-tpm+status:merged but I can't find where we defined docs12:33
gibiack, do we need to pull it out of the gate?12:33
gibibauzas: doc is in the next patch12:33
bauzasah right12:33
gibihttps://review.opendev.org/#/c/739213/1712:33
bauzasthat's what I just saw12:34
bauzasokay, I think we can leave as it is since lyarwood already approved the above patch but this sucks :(12:34
bauzasand I personally feel we should rewrite the reno file to be more explicit by saying "sorry folks, only spawn as of now"12:34
bauzasgiven what efried said on the life of pets, once people see TPM devices around in nova, they gonna start using those without knowing they're stukc12:35
bauzasstephenfin: ^12:35
gibibauzas: at least the doc is explicit about what does not work with vtpm12:35
bauzas(provided we don't merge the move ops changes in the next 2 weeks obviously)12:35
stephenfinbauzas: I can do a follow-up12:35
bauzasstephenfin: wfm12:36
stephenfincool. gimme a sec12:36
bauzasand yeah, docs are expicit12:36
bauzasexplicit even12:36
lyarwoodyeah I was under the impression that things were going to be enabled before GA anyway so it would be a waste of time detailing everything in the releasenote12:36
efriedBTW, it'll be a good idea to take your cues from jroll and penick if you have questions about how these are going to be used irl. They're the primary consumer driving this work, at least initially.12:37
lyarwoodas opposed to updating the docs12:37
bauzasstephenfin: cool, and then say we support resize, you could then amend your renofile to include "spawn [and resize]"12:37
bauzaslyarwood: I never predict on things being merged12:37
openstackgerritLee Yarwood proposed openstack/nova master: rbd: Move rbd_utils out of libvirt driver under nova.storage  https://review.opendev.org/74690412:37
openstackgerritLee Yarwood proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD  https://review.opendev.org/57430112:37
openstackgerritLee Yarwood proposed openstack/nova master: Bring back allowed_direct_url_schemes in support of RBD image download  https://review.opendev.org/72809512:37
openstackgerritLee Yarwood proposed openstack/nova master: zuul: Add devstack-plugin-ceph-compute-qcow2 to experimental queue  https://review.opendev.org/74322012:37
bauzaslyarwood: and I'm on the likes to consider master as something usable as of now12:38
lyarwoodbauzas: right but my point is that it's easier to update the docs than a reno IMHO12:38
bauzaslyarwood: but I know this is an utopia12:38
bauzaslyarwood: well, it's just a file change, but either way12:38
bauzasdocs are good for getting all the details12:38
bauzasrelease notes are good for knowing what's in the release12:38
openstackgerritStephen Finucane proposed openstack/nova master: releasenotes: Detail support for server ops with vTPM  https://review.opendev.org/74821512:40
stephenfinbauzas, gibi, lyarwood: nice and generic ^12:40
bauzas++12:40
sean-k-mooneylyarwood: updating a reno is not that hard12:41
sean-k-mooneywe can even backport reno updtes it does actully work12:41
stephenfinsean-k-mooney: it is tedious though12:41
bauzastedious in what ?12:41
stephenfinespecially when you've already got detailed info in docs12:41
bauzasstephenfin: well, docs were giving me a 40412:42
*** mkrai has joined #openstack-nova12:42
stephenfinbauzas: to update incrementally as you tweak features12:42
bauzashence the -112:42
sean-k-mooneyyes as long as the reno is not wrong and directs you to the docs12:42
sean-k-mooneythen keeping the docs up to date makes sense12:42
lyarwoodbauzas: docs are the next change in the series12:42
stephenfinoh, I thought that was okay since it's fixed in the next change?12:42
bauzasI'm still on the side to have a digested list of features12:42
bauzasstephenfin: I'd say I passed over ti12:42
bauzasit12:42
bauzasbut again, and I know I'm now maybe in the minority, I don't like merging broken code that will be fixed in the next change of the series12:43
sean-k-mooneystephenfin: i have done the update in the past its not that bad same as updating the docs12:43
bauzaseven if the change in question is next in the queue12:43
bauzasthat gives us bad habits12:43
stephenfintrue. It's not broken code though, in fairness :)12:44
lyarwoodare we classing releasenotes as code now?12:44
bauzasit's broken docs, this is worst, indeed :p12:44
lyarwoodops stephenfin beat me to it12:44
stephenfinlyarwood: coke, please12:44
lyarwoodyou're only getting warm tap water from me mate12:44
bauzasin our example, we could have left the change as it is, with a reno file just giving the exact things you provided in your FUP, without providing a broken link12:45
bauzasand in the next change, we could have provided the docs with an amended reno file that'd add the link12:45
bauzasthat's what I just call good habits12:45
stephenfinyup, that's fair12:46
stephenfinIn this instance though, are you okay to leave it in the gate queue or should I pull it out and rework it?12:46
bauzasstephenfin: on a fun side, should I *really* review https://review.opendev.org/#/c/742863/9 and all the type hints changes, given all the discussions we had in the past ? :D12:46
bauzasstephenfin: nah, I said I can assume this being fixed automatically12:47
stephenfinokay, cool12:47
bauzasstephenfin: so, tbh, I guess you're doing https://review.opendev.org/#/c/742863/9/nova/compute/manager.py unrelated to the vTPM series, right?12:48
stephenfinbauzas: Up to you :) It is really trivial, if that helps (not much hints being added, just getting mypy passing)12:48
bauzasahah, I see12:48
stephenfinyeah, see my reply to lyarwood12:48
stephenfinI'm adding new type functions in the next change and I want to annotate them and validate those annotations. To enable validation, I need to fix a couple of issues with the file. That's what I'm doing here.12:48
bauzasyou added type hints for the vtpm changes but this didn't work, gotcha12:48
stephenfinexactly12:48
stephenfinit felt wrong to add type hints but not validate them, and I couldn't enable validation without some additional work12:49
stephenfinso I did that work separately since it's not totally related12:49
stephenfinif that makes sense12:49
bauzasstephenfin: yeah12:49
bauzasthat's not I wanna argue against type hints12:50
bauzasif you wanna use them, this works for me12:50
bauzasbut let's not make it mandatory for all of us12:50
stephenfinyeah, to be clear, I'm not asking it of anyone who doesn't want to add them12:51
*** smcginnis has joined #openstack-nova12:51
sean-k-mooneyonce a file has first been enabeld for type hints you can add them inline12:51
sean-k-mooneyinstead of as a seperate patch12:51
stephenfinbut I'd like to add them to my code. I find them helpful12:51
*** ratailor has quit IRC12:51
sean-k-mooneybut when your ading a new file you often have to update unrelated code to get it to pass12:51
sean-k-mooneywell semingly unrelated12:51
bauzasthere are two ways of seeing type hints12:52
stephenfinsean-k-mooney: yeah, that's what I'm doing for new code at least12:53
stephenfinfor expected backports, melwitt and others have asked I do them separately to keep changes to the backport to a minimum, which I can deal with :)12:53
bauzaseither as a terrible waste of time for reviewers, or as an opportunity for code readability and static checks12:53
sean-k-mooneybauzas: yep i am stongly in teh latter camp12:53
stephenfins/type hints/tests/12:53
bauzasI'm not opiniated on any of both, I just try to be pragmatic12:53
*** ratailor has joined #openstack-nova12:53
sean-k-mooneylack of type hints is why i hated python for the first 2-3 years of working on openstack12:54
stephenfinsame argument can be made, though the value of tests is definitely more significant12:54
bauzasmy biggest concern being this a py3 feature, but given we now have a good consensus on things we can vs we can't do, I'm okay12:54
sean-k-mooneywehn we started adding the parms comment it helped a bit but not as much as type hints12:54
stephenfinbut it's also dead code from the perspective of production12:54
bauzasadding only type hints on new code seems reasonable... but...12:54
stephenfinthe current situation works for me (add them as you're going, and only if you want to)12:55
stephenfinbauzas: to be clear, new code or modified code12:55
bauzasthere is a trade-off, the more we're using them, the higher chances we can have merge conflicts with backporting12:55
sean-k-mooneyyes which i think is ok12:55
sean-k-mooneywe will have merge conflitcs anyway12:55
sean-k-mooneydue to python3 vs python 212:56
lyarwoodha if you think this is going to cause merge conflicts wait until you see stephenfin's func test series :D12:56
sean-k-mooneywhich one there is one every cycle :P12:56
lyarwoodit's all worthwhile however :)12:56
sean-k-mooneyyep stephenfin has been making the func tests more and more useful12:57
bauzaswell, I don't know for you folks but I'm working in a company that promises 5-year support for OpenStack12:57
stephenfinas do I. I think we add a little _too_ much weight to preserving backportability, at the expense of making master worse12:57
stephenfini.e. don't move these lines around because it'll make the backport harder, even though doing so would make things significantly easier to grok on master12:57
stephenfingibi: good spot on the chown thing12:57
lyarwoodbauzas: you missed a downstream discussion around stable/train while you were out FWIW12:57
bauzasdamn, it's a cabal !12:57
stephenfingibi: Functional tests should have caught that. I don't know why it didn't for sure but I suspect the dumb mocking of that cleanup routine is the culprit though12:58
*** ftarasenko has quit IRC12:58
sean-k-mooneylyarwood: did i also miss that i dont rememebr it12:58
* sean-k-mooney may have blocked out all knowladge of it as a defence mechanium not sure12:58
gibistephenfin: if you can catch those things in func test then I can ease on my tempest requirements12:58
stephenfinlyarwood: you mean the question of backporting functional test rework changes12:58
bauzaslyarwood: I feel like I was going to be caught in Varennes12:58
bauzasthat's a coup !12:58
lyarwoodsean-k-mooney: I think you were there12:59
lyarwoodstephenfin: yeah12:59
sean-k-mooneyoh that12:59
stephenfinwhat does a Frenchman know of coups12:59
sean-k-mooneydont we always backport them12:59
sean-k-mooneyto keep as close to master as possible12:59
sean-k-mooneyto make future backports simpler12:59
lyarwoodnot always12:59
*** bnemec has joined #openstack-nova12:59
stephenfinsometimes yes, sometimes no12:59
sean-k-mooneyok i guess it depend on how invasive it is12:59
stephenfinlyarwood's suggesting we always do, even if there are quite a few, since it will ultimately make our lives easier13:00
sean-k-mooneyjust being pargmatic about it13:00
stephenfinand it's test only so impact on stability should be non-existent13:00
lyarwoodyup13:00
bauzasI see13:00
*** ftarasenko has joined #openstack-nova13:00
bauzaswell, for functional test only changes, this could work13:00
sean-k-mooneybauzas: i think it came up in the context of one of melwitt patches where she would have had to rewrite the tests if she did not backport a refactor patch13:01
sean-k-mooneyand i suggested she shoudl backport the refactor13:01
sean-k-mooneyto avoid that and keep the backport clean13:01
stephenfingibi: Also, it's always been broken, i.e. blame efried, not me :P https://review.opendev.org/#/c/639934/30/nova/virt/libvirt/utils.py@69113:01
bauzassean-k-mooney: which makes sense from a pragmatic pov13:02
*** mkrai has quit IRC13:02
* stephenfin stops shifting blame and starts fixing stuff13:02
efriedI blame reviewers13:02
*** mkrai has joined #openstack-nova13:02
bauzasbut again, we all sign-off for maintaining code for half a decade, refrain your style changes13:02
sean-k-mooneybauzas: or backport them13:02
bauzaswe all = those who wear some head coverage with a color (that is less red and more blue)13:03
sean-k-mooneythe more consitent we are the eaiser it is to grock the code13:03
efriedI don't actually recognize that code; I may get to blame mdbooth13:03
bauzassean-k-mooney: I'd say that providing style changes in an implementation patch is terrible and we should avoid this13:03
efriedor cfriesen13:04
gibistephenfin, efried: I can take the blame found the issue :)13:04
gibifinding even13:04
sean-k-mooneyright but type hints are not a style change13:04
sean-k-mooneythey are a type of testing13:04
lyarwoodhave to say I disagree bauzas, we'd never be able to move the codebase forward otherwise13:04
sean-k-mooneyits technically a liniting enhancement13:04
stephenfinas someone who's paid to work on the libvirt module, so do I13:05
bauzasgibi: the story of my life https://i.pinimg.com/564x/c5/de/f4/c5def4bd565b44f4d27e1b7478dfa528.jpg13:05
stephenfinso. much. cruft.13:05
sean-k-mooneybauzas: hehe13:05
lyarwoodawww ./me hugs nova.virt.libvirt13:05
stephenfinheh13:05
bauzaslyarwood: stephenfin: I don't say we shouldn't modify code13:05
sean-k-mooneybauzas: worst part of git blame. who wrote this crap.. damb it was was me...13:06
*** bhagyashris is now known as bhagyashris|away13:06
stephenfinbauzas: well, take things like this https://review.opendev.org/#/c/631363/61/nova/tests/unit/virt/libvirt/fakelibvirt.py@13513:06
efriedHah! Yes! I get to blame Chris! https://review.opendev.org/#/c/639934/7..8/nova/virt/libvirt/utils.py@63013:06
stephenfinpersonally, I think that's a good thing13:06
gibiefried: good, he is not around to pass it forward :)13:07
efriedPerfect13:07
sean-k-mooneyoh recursive=true13:07
stephenfinI mean, I'm touching that code and the additional change makes the whole thing better13:08
bauzasstephenfin: again, no disagreement, just saying that the more we can decouple style changes from implementation changes, the better it would be as we could pull those style changes down to the stable branches without also pulling fancy new features13:08
sean-k-mooneystephenfin: did you see if this is need for livemiration by the way. i think we dont have too copy for live migration but we never validated that13:08
sean-k-mooneystephenfin: so currently we do just to be safe13:08
stephenfinsean-k-mooney: Currently I have live migration blocked because I haven't validated it13:09
sean-k-mooneyok well when you do can you try commenting out the tpm data copy13:09
stephenfinI do have a DevStack env on your cluster that I'll use to validate it once the rest has merged though13:09
sean-k-mooneyand see if it works13:09
bauzasstephenfin: fortunately, git is smart enough to handle conflicts on blank lines13:09
stephenfinsean-k-mooney: If we're talking about the same thing, then that's for cold migration13:09
bauzas(or it should)13:09
sean-k-mooneyhttps://review.opendev.org/#/c/639934/30/nova/virt/libvirt/utils.py@69113:09
sean-k-mooneythe save and restore13:09
stephenfinYes, that is necessary. I checked the libvirt docs and included the relevant snippet in the commit message13:10
sean-k-mooneyits needed for cold migration13:10
stephenfin Libvirt13:10
stephenfinwon't do this automatically for us since cold migrations, or offline13:10
stephenfinmigrations in libvirt lingo, do not currently support "copying13:10
stephenfinnon-shared storage or other file based storages", which includes the13:10
stephenfinvTPM device [1].13:10
sean-k-mooneystephenfin: but the qemu example seamed to imply it is not for live13:10
sean-k-mooneybut there was not clear statement of that13:10
stephenfinah, yes13:11
stephenfinwell13:11
stephenfin<stephenfin> sean-k-mooney: Currently I have live migration blocked because I haven't validated it13:11
stephenfinso I can try when I do the validation, sure :)13:11
sean-k-mooneystephenfin: cool13:11
sean-k-mooneystephenfin: it was an open question in the spec to be figured out when we do the implemnation so jsut reminding you13:11
stephenfinbauzas: Fair. I guess it's just a matter of where to draw the line13:11
stephenfinI'd be less conservative about this stuff that you are, I suspect13:11
bauzasstephenfin: if any of us can just think every time we write on maintainability and upgrades, this would just be perfect.13:12
bauzasstephenfin: totally, and I recognize me being a lame coworker13:12
bauzas:p13:12
stephenfinyeah, stop being so lame!13:13
stephenfin;)13:13
sean-k-mooneystephenfin: https://github.com/qemu/qemu/blob/master/docs/specs/tpm.rst#migration-with-the-tpm-emulator13:14
sean-k-mooneysince there is no copy in those steps i was sepcualtign it was in the testvm.bin file13:15
sean-k-mooneybut its also posible this was on the same host so -tpmstate dir=/tmp/mytpm1 could be acceable13:16
*** dave-mccowan has joined #openstack-nova13:17
openstackgerritLucas Alvares Gomes proposed openstack/nova master: DO NOT REVIEW: Test OVN devstack module  https://review.opendev.org/74822613:18
* bauzas bailing out for 1 hour13:19
*** sapd1_x has joined #openstack-nova13:22
*** nweinber has joined #openstack-nova13:23
*** brinzhang has quit IRC13:41
*** brinzhang has joined #openstack-nova13:41
*** ratailor has quit IRC13:46
*** brinzhang has quit IRC13:48
*** brinzhang has joined #openstack-nova13:48
*** Liang__ has joined #openstack-nova13:49
openstackgerritLee Yarwood proposed openstack/nova master: Ensure source compute is up when confirming a resize  https://review.opendev.org/69929113:50
lyarwoodgibi: ^ would you be able to take a look at this when you have time?13:50
*** Liang__ is now known as LiangFang13:50
gibilyarwood: sue13:50
lyarwoodgibi: thanks13:50
gibisure13:50
lyarwoodstephenfin: the rebase to remove the merge conflict just removed your +2 if you can readd13:51
*** mkrai has quit IRC13:52
stephenfinwill do13:54
lyarwoodthanks13:54
*** jangutter_ has joined #openstack-nova13:56
*** jangutter has quit IRC13:59
openstackgerritMerged openstack/nova master: libvirt: Add emulated TPM support to Nova  https://review.opendev.org/63136314:10
openstackgerritMerged openstack/nova master: docs: Add docs for vTPM support  https://review.opendev.org/73921314:10
lyarwoodbauzas / melwitt: https://review.opendev.org/#/c/747973/ - should be ready if either of you have time today.14:10
openstackgerritMerged openstack/nova master: tests: Add reproducer for bug #1889633  https://review.opendev.org/74402014:10
openstackbug 1889633 in OpenStack Compute (nova) "Pinned instance with thread policy can consume VCPU" [High,In progress] https://launchpad.net/bugs/1889633 - Assigned to Stephen Finucane (stephenfinucane)14:10
openstackgerritMerged openstack/nova master: hardware: Reject requests for no hyperthreads on hosts with HT  https://review.opendev.org/74402114:11
*** mkrai has joined #openstack-nova14:11
bauzaslyarwood: ack, will look14:29
openstackgerritAlexandre Arents proposed openstack/nova master: Add a lock to prevent race during detach/attach of interface  https://review.opendev.org/74795714:31
*** derekh has quit IRC14:32
*** derekh has joined #openstack-nova14:32
lyarwoodbauzas: thanks :)14:36
*** dustinc has joined #openstack-nova14:37
openstackgerritStephen Finucane proposed openstack/nova master: Add type hints to 'nova.virt.libvirt.utils'  https://review.opendev.org/74286514:39
openstackgerritStephen Finucane proposed openstack/nova master: Add support for resize and cold migration of emulated TPM files  https://review.opendev.org/63993414:39
stephenfingibi: Turns out a more granular mocking of migrate_disk_and_power_off is quite the bit of work. Have pushed up all the other fixes. Will keep working on that to get full coverage14:44
*** lpetrut has joined #openstack-nova14:46
gmannsean-k-mooney: noonedeadpunk: RE on policy for flavor ephemeral storage control.  we can add policy rule for that and allow everyone by default so that we do not break existing usage but my concern is, if restricted by policy then it will control all request not specific to host or storage, is that fine?14:50
bauzasstephenfin: https://review.opendev.org/#/c/744958/7 and https://review.opendev.org/#/c/741995/10 look the same, I guess it's PEBKAC ?14:50
sean-k-mooneygmann: flavor creation is admin only by default14:50
sean-k-mooneygmann: so the new policy would also have to be the same14:50
gmannsean-k-mooney: ah yeah with current default.14:50
openstackgerritLucian Petrut proposed openstack/nova master: Avoid invalid file name, preventing git clone on win32  https://review.opendev.org/74825014:51
bauzasstephenfin: wow https://review.opendev.org/#/q/owner:stephenfin%2540redhat.com+status:open14:51
bauzasdo you sleep overnight?14:51
sean-k-mooneygmann: can we make default dynamic14:51
gmannsean-k-mooney: but i was reading chat, is noonedeadpunk use case for specific host to control such VM creation?14:51
sean-k-mooneygmann: e.g. default it to whatever teh current flavor create policy is14:51
gmannsean-k-mooney: dynamic?14:51
gmannsean-k-mooney: yeah, we can do that14:51
sean-k-mooneygmann: no i dont think so14:52
sean-k-mooneygmann: i think noonedeadpunk wanted to restrct who could create flavors with local storage14:52
sean-k-mooneythere are ways to prevent host from booting gues with local sotrage alredy14:52
lpetruthi, looks like we have an unusual issue on Windows: https://review.opendev.org/#/c/748250/ a release note filename containing pipes prevents Nova from being cloned on Windows :)14:52
gmannsean-k-mooney: i see14:52
sean-k-mooneygmann: either via config or using a trait and isolated aggreates feature14:53
noonedeadpunkto be exact, I wanted to restrict users who can create VMs with ephemeral storages14:53
gmannok14:53
sean-k-mooneynoonedeadpunk: that is slightly different then14:53
noonedeadpunkFor example I generally want to prohibit usage of ephemeral/local storages,m but what 1 groups of users to be still able to do that14:53
sean-k-mooneyone way to do that is to make the ephemeral flavors private14:53
openstackgerritAlex Xu proposed openstack/nova master: Provider Config File: Enable loading and merging of provider configs  https://review.opendev.org/69346014:54
sean-k-mooneyso make all you public flaovr bfv only14:54
stephenfinbauzas: They are identical, but I didn't want to make the two series depend on each other14:54
noonedeadpunkbut even with lavor with 0 disk it's possible to create server without cinder volume14:54
openstackgerritStephen Finucane proposed openstack/nova stable/ussuri: tests: Add reproducer for bug #1889633  https://review.opendev.org/74825114:54
openstackbug 1889633 in OpenStack Compute (nova) ussuri "Pinned instance with thread policy can consume VCPU" [High,Triaged] https://launchpad.net/bugs/188963314:54
stephenfinbauzas: I'll rebase once one of them lands14:54
openstackgerritStephen Finucane proposed openstack/nova stable/ussuri: hardware: Reject requests for no hyperthreads on hosts with HT  https://review.opendev.org/74825214:54
noonedeadpunks/lavor/flavor/14:54
sean-k-mooneythen use flavor access to allow access to the other falvors to a specific tenant or set of tenants14:54
gmannsean-k-mooney: noonedeadpunk we can default it to 'os_compute_api:os-flavor-manage:create' rule itself so whatever create policy has in any deployment (default or overridden ) stays the same with new policy check.14:54
alex_xustephenfin: gibi ^ rebased 693460, the conflict at admin doc14:54
bauzasstephenfin: I'd have appreciated a -2 somewhere :)14:54
bauzasor a DNM14:55
sean-k-mooneygmann: that would not actully address noonedeadpunk usecase14:55
sean-k-mooneyit would do what i asked you14:55
stephenfinbauzas: well that would have defeated the purpose, wouldn't it? :P14:55
sean-k-mooneywe would need a policy on spwan that blocked ephemeral flavors14:55
lyarwoodelod: https://review.opendev.org/#/c/747358/ should also be ready now btw14:55
bauzasstephenfin: one is a bugfix, the other one is a feature that is proritized in a runwayu14:55
lyarwoodelod: unless you want me to squash14:55
sean-k-mooneyunless you were in a specific group14:55
bauzasstephenfin: so, honestly... :D14:56
sean-k-mooneybut i think falvor access and tenants achive a similar result14:56
noonedeadpunksean-k-mooney: I mean there's no way to create a flavor that prohibit usage of ephemeral/local volumes14:56
gmannsean-k-mooney: yeah14:56
*** dklyle has joined #openstack-nova14:56
sean-k-mooneynoonedeadpunk: well the flavor specifcy the disk usage14:56
sean-k-mooneyyou cant consme disk based on image metadata14:57
stephenfinYeah, I wasn't sure how else to do it without ending up with a 15+ patch series :-\14:57
sean-k-mooneybut you can restict who can use that flavor14:57
noonedeadpunkyes, but even with 0 disk and 0 swap - ephemeral will still be created if volume is not provided?14:57
*** tkajinam has quit IRC14:57
stephenfinespecially given I couldn't place it at the bottom of either series14:57
sean-k-mooneynoonedeadpunk: if you dont use boot form volume14:57
sean-k-mooneyalthough i argued that its a bug that that is allows14:57
noonedeadpunkYeah, so what I would be expecting is that nave printed that it's not allowed for instance14:58
sean-k-mooneyi wanted 0 disk to mean bfv only14:58
sean-k-mooneyother felt that would break some pepole14:58
*** lpetrut has quit IRC14:58
sean-k-mooneynoonedeadpunk: we may or may not beable to contole that by policy dependingo n what info is currenlty passed to server create14:59
noonedeadpunkThat why I was talking about policy, which can be configured if just reject users request when he tries to boot server without volume with 0 disk in flavor14:59
gmannflavor creation control is not needed really but booting VM control with new policy14:59
sean-k-mooneygmann: yes it would have to be a check on server create14:59
sean-k-mooneyeither an api check or policy14:59
stephenfinbauzas: if you can hit https://review.opendev.org/#/c/747744/ then I can drop one of those patches and rebase both series onto one15:00
sean-k-mooneyoh donwstream call got to go15:00
noonedeadpunklike the point here is that ppl on public clouds usually not sure what they want, and just go the easiest path, which results in lost data for them...15:00
bauzasstephenfin: yeah, that's possible15:00
*** hemna has quit IRC15:01
gmannsean-k-mooney:  noonedeadpunk: in that case we can default the new policy with this https://github.com/openstack/nova/blob/a7735d5e379c20c59cfb639f9f4d339bcffca2f9/nova/policies/servers.py#L16715:01
*** hemna has joined #openstack-nova15:02
*** LiangFang has quit IRC15:03
openstackgerritStephen Finucane proposed openstack/nova stable/train: tests: Add reproducer for bug #1889633  https://review.opendev.org/74825415:04
openstackbug 1889633 in OpenStack Compute (nova) ussuri "Pinned instance with thread policy can consume VCPU" [High,In progress] https://launchpad.net/bugs/1889633 - Assigned to Stephen Finucane (stephenfinucane)15:04
openstackgerritStephen Finucane proposed openstack/nova stable/train: hardware: Reject requests for no hyperthreads on hosts with HT  https://review.opendev.org/74825515:04
noonedeadpunkI mean like `os_compute_api:servers:create:zero_disk_flavor` is already checking the flavor. Maybe somewhere near it's possible to check if flavor has zero size disk, but has no volume attached?15:05
noonedeadpunkI'm not really familiar with nova codebase as it's so huge...15:05
sean-k-mooneyyou proably need the block device mappings15:06
* noonedeadpunk already got lost in suggestions.......15:07
sean-k-mooneythat said i think we have a bfv flag somewhere15:07
noonedeadpunkAt least I hope I told userstory I'm trying to implement in an understandable way....15:08
sean-k-mooney you did15:08
sean-k-mooneyyou want to restict who can boot a vm without bfv15:08
sean-k-mooneyusing policy and roles15:09
sean-k-mooneyright15:09
noonedeadpunkideally yes. How to restrict it on whole cloud I guess you've already answered and it's with max_local_block_devices15:10
noonedeadpunkeventually that will probably work for me as well15:10
sean-k-mooneyyes the config option will also block it much latere on the compute host  rather then api15:11
noonedeadpunk(in the worst case scenario would use isolate aggregates which will be also tricky because of the need to separate resources)15:12
noonedeadpunkbut yeah, checking that on api level might be more efficient15:12
noonedeadpunkand produce more understandable output15:12
sean-k-mooneyan api check woudl allow us to retrun a specific api respoce saying why15:14
sean-k-mooneycurrently it would jsut get a no valid host15:14
noonedeadpunkyeah15:14
sean-k-mooneywhich is not a great user experice15:14
gmann zero_disk_flavor policy already control the zero disk boot  if it is not bfv15:14
noonedeadpunkzero_disk_flavor eventually can only prohibit spawning a VM when it's created with flavor with 0 disk15:15
sean-k-mooneyoh so the edgecase it did not fix was allowing bfz when disk >015:15
sean-k-mooneythere is an edgecase that it breaks15:15
noonedeadpunkeven if its bfv15:15
gmannhttps://github.com/openstack/nova/blob/eef4b5435e7cdfe53ee9d9265d96c7dd278d9e93/nova/compute/api.py#L73015:17
sean-k-mooney ya htat is blocking using 0 disk flavor without bfv15:17
noonedeadpunkI think it's pretty different15:17
noonedeadpunkyeah15:18
sean-k-mooneyit does not prevent createing vms with non 0 disk flavors15:18
noonedeadpunkeventually... it blocks  0 disk flavor with bfv as well. At least blocked on train for me...15:18
sean-k-mooneyit should not15:18
gmannyeah, it should not block that15:19
sean-k-mooneythat check should only block non bfv vms with 0 disk flavors15:19
noonedeadpunkoh, yeah, maybe you're right15:19
noonedeadpunksorry15:19
*** dklyle has quit IRC15:19
noonedeadpunkactually, even when instance has 0 disk flavor it can be still created without bfv15:20
sean-k-mooneynot with that policy15:20
noonedeadpunkwhich also can't be covered with that policy15:20
sean-k-mooneythat is what it block15:20
sean-k-mooneybut on train it might now be enabeld by default15:20
noonedeadpunkoh, indeed, yeah, you're right15:20
sean-k-mooneyits relitivly new15:21
noonedeadpunkso yeah, edgecase when flavor >015:21
noonedeadpunkI just got it overriden after upgrade to train, so mixing a bit, sorry15:21
gmannit is there  before train and with admin only. before stein it was default with admin-or-owner15:21
noonedeadpunkso like another option is to drop all non-0 flavors and use that policy15:22
gmannok so the case left here is to allow flavor.disk>0 in bfv case ?15:22
sean-k-mooneynoonedeadpunk: yes15:23
sean-k-mooneynoonedeadpunk: and create private flavor with disk if you need that and only give access to sepcific tenants15:23
gmannor instead of drop, control the flavor access15:23
sean-k-mooneythe flavor access i tenatnt based rather then role based is the only wrinkel with that15:24
sean-k-mooneybut it would work15:24
noonedeadpunkok, thank you very much for your time. You gave me 2 options which is more than enough at this time for me:)15:25
*** sapd1_x has quit IRC15:25
*** gyee has joined #openstack-nova15:26
*** dklyle has joined #openstack-nova15:26
noonedeadpunkI just really thought that I had issues with creating even with volumes and 0 disk flavors when had os_compute_api:servers:create:zero_disk_flavor locked for admin15:27
sean-k-mooneyif you can repoduce it simply then file a bug if you hit it15:27
sean-k-mooneyas that should work15:27
noonedeadpunkyeah, sure! thanks again!15:30
*** belmoreira has quit IRC15:36
rmart04thanks @sean-k-mooney, graphing this in grafana now, still unsure what the best approach is here though to stop OOMs. Was StephenFin saying that huge pages may help me over come this?15:53
openstackgerritVlad Gusev proposed openstack/nova stable/ussuri: replace the "hide_hypervisor_id" to "hw:hide_hypervisor_id"  https://review.opendev.org/74718915:58
*** xek has quit IRC16:01
bauzasstephenfin: gave you a terrible -1 https://review.opendev.org/#/c/747744/216:05
* stephenfin shakes fist and clicks16:05
* bauzas hides16:05
stephenfinrmart04: I was suggesting explicit page size allocations would help16:05
bauzasstephenfin: I know I'm a mean person16:06
stephenfinbecause then you can do per-node reserved memory amounts16:06
stephenfine.g. 'hw:mem_page_size=small'16:06
*** dklyle has quit IRC16:06
*** dklyle has joined #openstack-nova16:07
sean-k-mooneythat will force a singel numa node unless you also use hw:numa_nodes=X16:07
sean-k-mooneyrmart04: do you allow ram oversubscription16:08
sean-k-mooneyrmart04: and are you using train or newer?16:08
rmart04We're using Stein and no RAM oversubscription16:09
sean-k-mooneyyou are using pinning yes16:09
rmart04Yes16:09
sean-k-mooneyok so in that case you should use hugepages16:09
sean-k-mooneyusing explcit small pages could help but there hugepages will improve performance16:09
openstackgerritBalazs Gibizer proposed openstack/nova master: Prevent starting services with older than N-1 computes  https://review.opendev.org/73848216:09
sean-k-mooneyyou are already using pinning so you cannot live migrete16:10
sean-k-mooneyadding hugepages wont make that worse in this case16:10
rmart04ok JohnGarbut also suggested this to me recently, i'll look into it and see how i get on. Does the host need any explicit configuration or is it just flavor metadata specs?16:10
*** artom has quit IRC16:10
sean-k-mooneyrmart04: the host needs to have memroy preaccloated as hugepages16:10
sean-k-mooneyyou can do tha that on the kernel commandline16:11
sean-k-mooneyyou have 768 GB and are running a 750GB guest right16:11
sean-k-mooneyso you would allocate 750GB of 1GB hugepages ideally16:12
sean-k-mooneythen set hw:mem_page_size=large in the flavor16:12
sean-k-mooneydefault_hugepagesz=1G hugepagesz=1G hugepages=75016:12
sean-k-mooneythat is what you need to add to the kernel command line in grub16:13
rmart04ah OK amazing, thank you16:13
sean-k-mooneyyou have only 1 numa node on the host currently right?16:13
rmart04No two16:13
sean-k-mooneyand the vm has how many numa nodes? 216:14
rmart04yes16:14
sean-k-mooneyok then that makes sense16:14
sean-k-mooneyotherwise i was wondering how the vm booted :)16:14
rmart04I managed to get over that hurdle :P16:15
sean-k-mooneyrmart04: you should see >20% incresse form hugepages in most workloads16:15
*** mvorwerk_ has quit IRC16:15
sean-k-mooneymore if its memory or io bound16:15
sean-k-mooneyless if it compute bound16:15
sean-k-mooneyor disk bound16:15
rmart04ok great, thanks for all your help16:17
*** tesseract has quit IRC16:18
*** mvorwerk has joined #openstack-nova16:18
sean-k-mooneyby the way the reason i ask about oversubsrcitip is hugepage cant be swapped out ot disk but they also will never be reap as a result of OOM16:18
sean-k-mooneyso it shoudl solve your issue although there is a very small change that the qemu process can still be selected16:18
*** dosaboy has quit IRC16:19
*** links has quit IRC16:19
*** dosaboy has joined #openstack-nova16:19
sean-k-mooneyqemu will be using relitvly little non hugepage memeory in this config so it will have a low memery pressur score vs other process and memeory will be balance evenly across both host numa nodes for the vm16:20
* bauzas goes off for the day16:20
yoctozeptomorning nova folks - today's question is when nova compute host name can get different from nova hypervisor host name and how16:20
* yoctozepto too busy to read the code...16:21
*** psachin has quit IRC16:21
sean-k-mooneyyoctozepto: it will be different for ironic16:21
yoctozeptosean-k-mooney: true, let's focus on nova+kvm :-)16:22
sean-k-mooneyout side of that it shoudl be the same unless you set teh HOST config element or you add a fqdn to you /etc/hostname16:22
yoctozeptoactually evaluating noonedeadpunk's https://review.opendev.org/728629 (for masakari)16:22
* noonedeadpunk patching16:22
stephenfinyoctozepto: You can manually override '[DEFAULT] host'16:23
sean-k-mooneyyoctozepto: this is how its set https://github.com/openstack/nova/blob/master/nova/conf/netconf.py#L50-L7016:23
stephenfinit defaults to socket.gethostname()16:23
stephenfinwhat sean-k-mooney just linked to16:23
sean-k-mooneyyep16:23
sean-k-mooneyso they should be the same unless you change that config option16:23
sean-k-mooneyif you use socket.fqdn() in your code it may not agreee16:24
sean-k-mooneyi fixed this in cyborg16:24
yoctozeptothen I would be changing the thing for nova-compute but not for the hypervisor, right?16:24
sean-k-mooneyyes16:24
sean-k-mooneyif you set [default]/host to something16:24
yoctozeptook, so where does the name for the hypervisor spawn and how's it kept linked? :D16:25
sean-k-mooneyhyperviour_hostname and host will not be the same for libvirt16:25
sean-k-mooneyor other virt dirvers16:25
sean-k-mooneyhost is the compute service host and is set by that config and hypervisor_hostname is set by the virt driver16:25
openstackgerritStephen Finucane proposed openstack/nova master: functional: Don't inherit from 'ProviderUsageBaseTestCase'  https://review.opendev.org/74827116:26
sean-k-mooneythe libvirt driver get it form libvirt16:26
sean-k-mooneyand it calls socket.gethostname internally in c16:26
stephenfinbauzas: https://review.opendev.org/74827116:26
sean-k-mooneysocket.gethostname is a fucntion provided by libc and pythons socket module jsut call the libc version16:26
sean-k-mooneysocket.gethostname() reads /etc/hostname if it exits on linux16:27
yoctozeptosean-k-mooney: ah, so it's consulted each time with libvirt? I guess that's why I had a hard time figuring this one out16:27
sean-k-mooneyyes16:27
yoctozeptoyeah, the rest makes sense16:27
stephenfinsean-k-mooney: https://review.opendev.org/#/c/748250/16:27
yoctozeptoI do wonder how noonedeadpunk (and some others) ended up desynchronizing them16:27
bauzasstephenfin: I don't get your comment, you're saying you can't just use the InstanceMixin ?16:28
noonedeadpunkI have situnation where socket.gethostname and socket.getfqdn are different things16:28
bauzasstephenfin: directly I mean ?16:28
yoctozeptonoonedeadpunk: hmm, ah16:28
yoctozeptobut then sean-k-mooney was only about gethostname16:28
yoctozeptoyou mean libvirt is actually doing getfqdn?16:28
bauzasstephenfin: of course, you'd have to specify the latest microversion but is that terrible to do ?16:28
stephenfinbauzas: Not really. I'd have to copy-paste all of this https://github.com/openstack/nova/blob/master/nova/tests/functional/integrated_helpers.py#L1107-L113216:29
sean-k-mooneyyoctozepto: no its getting the hostname16:29
stephenfinbauzas: Which seems worse than having functions available that I'm not using16:29
noonedeadpunksean-k-mooney: but I think that libvirt checks for the socket.getfqdn...16:29
sean-k-mooneystephenfin: lol ok16:29
stephenfin'_IntegratedTestBase' is basically what you're asking for but with all that boilerplate inline16:29
yoctozeptothen there is a conflict between your testimonies, noonedeadpunk and sean-k-mooney :-)16:29
sean-k-mooneynoonedeadpunk: it does not16:30
yoctozeptoinvestigation ongoing!16:30
yoctozepto:D16:30
sean-k-mooneynoonedeadpunk: unless they changed it wich would be a breaking change16:30
stephenfinthere are some differences around policy but I'm resolving those in separate patches16:30
noonedeadpunksean-k-mooney: http://paste.openstack.org/show/797181/16:30
yoctozeptohttps://github.com/libvirt/libvirt/blob/7ba4838a18afb609d900190c57b0cbfb842ccbdf/src/util/virutil.c#L47016:31
noonedeadpunkI can judge only by this...16:31
stephenfinmelwitt: want to unbork cloning on Windows? https://review.opendev.org/#/c/748250/16:31
yoctozeptoit seems they are training to canonicalize on master16:31
yoctozeptothat's how they ended up with an fqdn16:31
bauzasstephenfin: technically, you'd need to do things like https://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/test_bug_1849409.py16:31
sean-k-mooneyyoctozepto: noonedeadpunk we cant change this behavior16:32
yoctozeptoso yeah, they b0rked16:32
sean-k-mooneyits used in plamcnet16:32
bauzasstephenfin: but that should be it16:32
yoctozeptosean-k-mooney: what's plamcnet? ;d16:32
noonedeadpunksean-k-mooney: yeah, that's fine) just sharing feedback about fqdn vs gethostname - fighting this 3 years I guess...16:32
sean-k-mooneythe placement service16:32
yoctozeptonoonedeadpunk: but now we know16:32
yoctozeptosean-k-mooney: ack16:32
elodlyarwood: no need to squash, it's good as it is. will review now16:33
sean-k-mooneynoonedeadpunk: it is technially allowed to return an fqdn if you put an fqdn in /etc/hostname which you should not do but some people do16:33
stephenfinbauzas: Yes, but those are nearly identical. The only difference is the use of the CinderFixture and fake image service (essentially GlanceFixture)16:33
yoctozeptook, this was a very worthwhile chat, thanks sean-k-mooney, stephenfin and noonedeadpunk16:33
yoctozeptosean-k-mooney: you can also get trapped by dns16:33
stephenfinbauzas: oh, and the cast as call16:33
sean-k-mooneythe masikari patch is not correct by the way16:34
melwittstephenfin: sure, but curious what does your comment mean about sphinx not finding the reno? does that mean the release note will disappear from the release notes? I had thought it would still be included even if the name is changed?16:34
yoctozeptosean-k-mooney: any reviews appreciated!16:34
yoctozeptoany and all* I mean16:34
stephenfinmelwitt: it'll find the reno with the new name but complain about a file matching the old name being missing16:34
bauzasstephenfin: yup, we would probably just need another base testclass which wouldn't pull all the placementesque methods16:34
sean-k-mooneyyoctozepto: the seach is using hyperviour_hostname in the host filed16:34
noonedeadpunksean-k-mooney: it's more complicated than that.. In /etc/hostname I have uacloud-nova03. But the thing is that python gets it not from /etc/hostname, but from /etc/hosts16:34
stephenfini.e. "this existed in the past but I can't find it now; skipping"16:34
sean-k-mooneythat only works for libvirt16:35
melwittstephenfin: ah ok. thanks16:35
bauzasstephenfin: but in the meantime, instancemixin seems better to use IMHO + those extra lines16:35
yoctozeptosean-k-mooney: nah, that's bad naming16:35
stephenfinbauzas: We have it. It's call '_IntegratedTestBase'16:35
yoctozeptoactually masakari uses this-called-string for corosync and nova-compute16:35
yoctozeptoand asssumes that they are the same16:35
stephenfinbauzas: look at the superclasses for that https://github.com/openstack/nova/blob/master/nova/tests/functional/integrated_helpers.py#L100816:35
yoctozeptosomehow they thought it always equals hypervisors...16:35
yoctozeptowell, I would have as well probably16:35
stephenfin*It's called16:36
bauzasstephenfin: I do remember the pain writing functional tests that were requiring duplicate code16:36
sean-k-mooneyyoctozepto: maybe its not clear if https://docs.openstack.org/api-ref/compute/?expanded=list-compute-services-detail#list-compute-services is the hyperviour_hostname or the host value form teh cofnig16:37
sean-k-mooneyi suppect its the host value form the config16:37
yoctozeptosean-k-mooney: it is, it's just that noonedeadpunk kept the old name, I commented on that too16:38
sean-k-mooneyok so its the host value16:38
sean-k-mooneynot the hypervisor_hostname16:38
noonedeadpunkAt fisrt I was trying to keep changes minimal)16:38
*** mkrai has quit IRC16:38
yoctozeptonoonedeadpunk: all in all, this patch makes masakari a bit better but still kinda delays the issue that one could actually want to monitor the hypervisors, and have their names agree with those in corosync cluster16:38
bauzaseither way, it's becoming late for me, \o16:39
sean-k-mooneynoonedeadpunk: thats an good goal generaly but but names existingon the compute node recored in teh db16:39
sean-k-mooneyso we should try to use hypervisor_hostname and host name differently depending on which we mean16:39
noonedeadpunkYeah, I see it now. Just wanted to made it as bug fix to get backported at first :p16:40
noonedeadpunkbut you're totally right16:40
*** k_mouza has quit IRC16:41
yoctozeptonoonedeadpunk: if we straighten the logic right, then you know who's the core who will accept those backports :-)16:41
sean-k-mooneynoonedeadpunk: these are teh rules libvirt uses by the way16:42
sean-k-mooneyhttps://github.com/libvirt/libvirt/blob/7ba4838a18afb609d900190c57b0cbfb842ccbdf/src/util/virutil.c#L470-L48916:42
*** k_mouza has joined #openstack-nova16:44
sean-k-mooneythe did modify this 8 months ago https://github.com/libvirt/libvirt/commit/26d9748ff114a060ee751959d108d062f737f5d916:45
noonedeadpunksean-k-mooney: I think that C way of gethostname may differ from python implementation....16:46
noonedeadpunkAnd I'm not C guy unfortunatelly...16:46
sean-k-mooneythey did not in the past but now they are calling a new fucntion16:46
sean-k-mooneythe python way called the c function16:47
yoctozeptosean-k-mooney: I linked to that libvirt code just above :-)16:47
sean-k-mooneylibvirt is now using g_get_host_name();16:47
sean-k-mooneyinstead of gethostname16:47
*** k_mouza has quit IRC16:48
*** lpetrut has joined #openstack-nova16:49
*** lpetrut has quit IRC16:49
noonedeadpunkYeah, so things may broke for me with some libvirt update... Anyway that would mean time to work on naming convention:) Like I have only 1 such region - the first one I've built...16:49
noonedeadpunkbut it created corner case....16:49
*** dklyle has quit IRC16:50
*** ralonsoh has quit IRC16:51
sean-k-mooney6.0 is the default in ubuntu and centos 816:52
sean-k-mooneyi know ooo hardcodes the fqdn in /etc/hostname16:52
yoctozeptonoonedeadpunk: masakari needs fixing nonetheless ;p16:52
sean-k-mooneyand i think it also manually sets the [default]/host value16:52
sean-k-mooneynova depends on teh actull hostname not changing16:53
*** mkrai has joined #openstack-nova16:53
*** mkrai has quit IRC16:55
*** artom has joined #openstack-nova16:56
*** dtantsur is now known as dtantsur|afk16:57
*** mvorwerk has quit IRC16:58
sean-k-mooneyyoctozepto: noonedeadpunk this is where we call libvirt if you are interested https://github.com/openstack/nova/blob/master/nova/virt/libvirt/host.py#L97616:59
sean-k-mooneyand that is used here https://github.com/openstack/nova/blob/a7735d5e379c20c59cfb639f9f4d339bcffca2f9/nova/virt/libvirt/driver.py#L841817:00
*** mvorwerk has joined #openstack-nova17:00
yoctozeptosean-k-mooney: thanks, I just did not realize it's libvirt telling us its name, but thanks to you it's clear for me now17:00
sean-k-mooneyya genericly its the hypervior e.g vmware or hyperv or libvirt that provides this17:02
*** mkrai has joined #openstack-nova17:02
openstackgerritMerged openstack/nova stable/train: libvirt: Provide VIR_MIGRATE_PARAM_PERSIST_XML during live migration  https://review.opendev.org/74797317:02
openstackgerritMerged openstack/nova master: releasenotes: Detail support for server ops with vTPM  https://review.opendev.org/74821517:03
noonedeadpunksean-k-mooney: just one more stupid question... trying to find host-evacuate api call in https://docs.openstack.org/api-ref/compute/ but don't see for some reason (only server evacuate)17:04
*** sapd1_x has joined #openstack-nova17:04
yoctozeptosean-k-mooney: https://docs.openstack.org/api-ref/compute/?expanded=evacuate-server-evacuate-action-detail#evacuate-server-evacuate-action ?17:05
yoctozeptonoonedeadpunk: ^17:05
yoctozeptoit's server (instance/vm) that's getting evacuated17:05
noonedeadpunkand how does https://docs.openstack.org/nova/rocky/admin/evacuate.html#evacuate-all-instances work? It gets list of instances and evacuate one by one ?17:06
*** mkrai has quit IRC17:07
*** derekh has quit IRC17:08
yoctozeptonoonedeadpunk: that's what I would assume, could use checking the code17:09
sean-k-mooneynoonedeadpunk: host-evacuate is not an api action17:09
sean-k-mooneynoonedeadpunk: its a client command17:09
noonedeadpunkok, got it17:09
noonedeadpunkyeah17:10
sean-k-mooneynoonedeadpunk: http://www.danplanet.com/blog/2016/03/03/evacuate-in-nova-one-command-to-confuse-us-all/17:10
sean-k-mooneydansmith's blog that is basically mandatory reading on host-evacuate and how it related to every thing else17:11
noonedeadpunkoh, ok, now I know under what conditions our client loose all of their data from ephemeral drives...17:11
sean-k-mooneyyoctozepto: host-evacuate does not call teh evacuate api17:11
noonedeadpunkwhen masakari calls instance evacuate17:11
sean-k-mooneyyoctozepto: it does cold migrate17:11
sean-k-mooneyactully no it does evacuate17:12
sean-k-mooneyi should read the blog more often17:12
noonedeadpunk"The core of the evacuate process in nova is actually rebuild, which in many cases is a destructive operation"17:13
noonedeadpunkso it's not evacuate and you;re right17:13
sean-k-mooneyfrom the blog17:13
sean-k-mooney"The nova host-evacuate command does not translate directly to a server-side operation, but is more of a client-side macro or “meta operation.” When you call this command, you provide a hypervisor hostname, which the client uses to list and trigger evacuate operations on each instance running on that hypervisor. You would use this command post-failure (just like the single-instance evacuate17:13
sean-k-mooneycommand) to trigger evacuations of all the instances on a failed compute host."17:13
sean-k-mooneynoonedeadpunk: no it is evacuate17:14
sean-k-mooneynoonedeadpunk: but evacuate does not mean what you think it does17:14
noonedeadpunkyeah17:14
noonedeadpunkso when masakari calls instance evacuate when this instance is not volume based - it get's wiped out17:15
sean-k-mooneyevacuate only preseved data if you use bfv or are on shared storage17:15
sean-k-mooneynoonedeadpunk: unless nova is using ceph via the rbd imageages type or /var/lib/nova is on nfs17:15
noonedeadpunkand I guess even with rbd drive rebuild is destructove for non-bfv?17:15
noonedeadpunkhm...17:16
sean-k-mooneynoonedeadpunk: no its not with ceph we detech its on shared storage17:16
noonedeadpunkhm.....17:16
* dansmith is enjoying the heck out of this17:16
sean-k-mooneydansmith: its proably in your blog post17:17
noonedeadpunkthen I still don't get why I got situations when ephemeral got lost or wiped out during node crush....17:17
noonedeadpunkbut whatever)17:17
noonedeadpunkI know how to disable this :p17:17
sean-k-mooneydansmith: i keep it bookmarked as a reference document17:18
*** bnemec has quit IRC17:18
dansmithyeah, I should charge admission17:18
sean-k-mooneynoonedeadpunk: ootnote: In the case of volume-backed instances, the root disk of the instance is usually in a common location such as on a SAN device. In this case, the root disk is not destroyed, but any other instance state is recreated (which includes memory, ephemeral disk, swap disk, etc).17:19
noonedeadpunkyeah, so it's re-created from the image?17:19
noonedeadpunkas rebuild do17:19
noonedeadpunkwhich is destructive and all changes made on vm are gone which is eventually as intended?17:20
sean-k-mooneythe root disk shoudl not be but if you have extra ephermeral disk they proably are i dont know how addtional ephemeral disks work with ceph. i now swap is store as a ceph volume17:20
yoctozeptosean-k-mooney, noonedeadpunk: I guess masakari should include a warning in big, red font about the necessity to have HA storage first before running masakari17:20
noonedeadpunkI guess so...17:21
yoctozeptoit's obvious to us (well, me at least) but years of practice has proven it's not entirely that obvious to all of users17:21
sean-k-mooneyyoctozepto: if masikari automate evacuate yes17:21
*** ganso has quit IRC17:21
sean-k-mooneyor keep all your data on a cinder data volume17:21
yoctozeptoyeah, that's what I meant17:21
noonedeadpunkI mean I have ceph for everything.... But still non bfv instances get's wiped out during evacuations...17:21
noonedeadpunkok, whatever)17:21
sean-k-mooneynoonedeadpunk: ill check the ceph code quickly but i tought we dedect ceph and preserved it.17:22
*** k_mouza has joined #openstack-nova17:22
noonedeadpunkmaybe it's some recent change? Ie T or U?17:22
sean-k-mooneywe pass an on_shared_storage flag internllay which should be set to true for ceph and nfs17:23
sean-k-mooneythats set here https://github.com/openstack/nova/blob/20459e3e88cb8382d450c7fdb042e2016d5560c5/nova/api/openstack/compute/evacuate.py#L9717:24
sean-k-mooneyoh you have to pass it as an option17:24
noonedeadpunksean-k-mooney: yoctozepto seems like another patch to masakari?:)17:25
sean-k-mooneynoonedeadpunk: https://docs.openstack.org/nova/latest/reference/api-microversion-history.html#id1217:25
sean-k-mooneyso before 2.14 you had to pas it17:25
sean-k-mooneyafter that its automatic17:25
noonedeadpunk2.12 was soooooo long ago....17:26
*** k_mouza has quit IRC17:26
*** ganso has joined #openstack-nova17:27
sean-k-mooneyis that what massikari is using17:27
sean-k-mooneyhttps://github.com/openstack/nova/blob/20459e3e88cb8382d450c7fdb042e2016d5560c5/nova/compute/manager.py#L3504 we now ask the dirver if the insance is on shared storage17:27
noonedeadpunkhm. I think I'm reading it wrong.. But it returns None for 2.14+ ? https://github.com/openstack/nova/blob/20459e3e88cb8382d450c7fdb042e2016d5560c5/nova/api/openstack/compute/evacuate.py#L4517:28
sean-k-mooneyform 2.14 on17:28
sean-k-mooneynoonedeadpunk: yes 2.14 on it returns none and we ask the driver17:28
noonedeadpunkah17:28
sean-k-mooneywhich does self.image_backend.backend().17:29
sean-k-mooney                                is_shared_block_storage()17:29
sean-k-mooneyso unless you have differen image_backend on different hosts that should be correct17:29
sean-k-mooneyrbd returns true https://github.com/openstack/nova/blob/20459e3e88cb8382d450c7fdb042e2016d5560c5/nova/virt/libvirt/imagebackend.py#L95817:30
sean-k-mooneyso you should see this log message when you evacuate https://github.com/openstack/nova/blob/20459e3e88cb8382d450c7fdb042e2016d5560c5/nova/compute/manager.py#L3514-L351617:30
sean-k-mooneyand we should keep the disk content17:30
noonedeadpunklooks like this, yes17:31
* noonedeadpunk goes to find node to drop17:31
sean-k-mooneywhat api microversion is masikari using17:31
noonedeadpunk2.53 for master...17:33
sean-k-mooneyit looks like it does not set one https://github.com/openstack/masakari/blob/cd95f6660c14f506603f0864ca24dd25c278a6a8/masakari/engine/drivers/taskflow/host_failure.py#L262-L26317:33
sean-k-mooneyso nova client will default to the latest17:34
noonedeadpunkok, on rocky it was 2.1417:34
* noonedeadpunk upgraded from rocky month ago17:34
*** rmart04 has quit IRC17:34
yoctozeptowe care about stein+ atm17:35
noonedeadpunk++17:35
sean-k-mooney2.14 was mitaka17:35
sean-k-mooneyso you should be good17:35
sean-k-mooneywe have been preseviing shared storage vms for a very long time17:36
*** mvorwerk has quit IRC17:36
*** mvorwerk has joined #openstack-nova17:38
*** mvorwerk has quit IRC17:45
*** artom has quit IRC17:45
*** mvorwerk has joined #openstack-nova17:46
*** sapd1_x has quit IRC17:47
*** k_mouza has joined #openstack-nova17:56
*** belmoreira has joined #openstack-nova17:57
noonedeadpunksean-k-mooney: just tested and during evacuate data is really preserved18:00
sean-k-mooneygood that would have been a nasty bug if that was regressed18:00
openstackgerritStephen Finucane proposed openstack/nova master: Change default num_retries for glance to 3  https://review.opendev.org/74038918:00
*** k_mouza has quit IRC18:01
*** mvorwerk has quit IRC18:02
noonedeadpunkTIL18:02
openstackgerritMerged openstack/nova stable/ussuri: compute: Validate a BDMs disk_bus when provided  https://review.opendev.org/74455018:02
sean-k-mooneynoonedeadpunk: the api does not guarentee that data is preserved unless you are using bfv18:03
sean-k-mooneynoonedeadpunk: so unless you deployed the cloud and know the vm is on shared storage when you dont use bfv dont rely on that behavior18:03
*** mvorwerk has joined #openstack-nova18:04
noonedeadpunkI see, yeah, I know that bfv is really preferable. But it's hard to explain in terms of public clouds, when even with no non-0 disk flavors ppl still do use ephemeral. But good part of them, is that they can handle ISO's while cinder wasn't able to boot ISO image properly last time I've checked...18:06
openstackgerritMerged openstack/nova master: rbd: Move rbd_utils out of libvirt driver under nova.storage  https://review.opendev.org/74690418:06
sean-k-mooneynoonedeadpunk: well i have no problem iwht non bfv instance18:06
sean-k-mooneyi like the ceph backend for example18:07
noonedeadpunkyes, sure ceph backend is everywhere for me:)18:07
sean-k-mooneybut its jsut driver devied if evauate preserves data or not18:07
sean-k-mooneyfor the libvirt dirver it does preseve data it the sorate is shared18:07
sean-k-mooneyfor other drivers that may not be the case18:07
noonedeadpunkdunno, I need better investigation on my side, it was just good lead, but no.. And I wasn't able to reproduce by far so yeah...18:08
sean-k-mooneyironic for example i would expect to always recreate the server altouhg if its boot form volue it might persve it18:08
*** mvorwerk has quit IRC18:12
*** mvorwerk has joined #openstack-nova18:14
openstackgerritStephen Finucane proposed openstack/nova master: Use absolute path during qemu img rebase  https://review.opendev.org/73924618:21
*** brinzhang_ has joined #openstack-nova18:21
*** brinzhang has quit IRC18:25
*** belmoreira has quit IRC18:30
*** belmoreira has joined #openstack-nova18:32
sean-k-mooneystephenfin: by the way for my libvirt/os-vif patch i have another pathc im addedin beneath it18:36
sean-k-mooneybut ill adress your commend when i push both18:36
*** mvorwerk has quit IRC18:36
stephenfinsean-k-mooney: Okay. You've got reviews on https://review.opendev.org/#/c/745605/ too so if you get a chance to address that, I can re +2 tomorrow18:37
* stephenfin had a Python Ireland meeting and is now going for food o/18:37
sean-k-mooneyya ill take a look at it18:38
*** mvorwerk has joined #openstack-nova18:38
*** mvorwerk has quit IRC18:58
*** mvorwerk has joined #openstack-nova18:58
*** hamalq has joined #openstack-nova19:02
openstackgerritJiri Suchomel proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD  https://review.opendev.org/57430119:04
*** zzzeek has quit IRC19:07
*** mvorwerk has quit IRC19:08
*** zzzeek has joined #openstack-nova19:09
*** mvorwerk has joined #openstack-nova19:10
*** belmoreira has quit IRC19:12
openstackgerritMerged openstack/nova master: Add type hints to 'nova.compute.manager'  https://review.opendev.org/74286319:22
openstackgerritMerged openstack/nova master: Avoid invalid file name, preventing git clone on win32  https://review.opendev.org/74825019:22
*** mvorwerk has quit IRC19:24
*** zzzeek has quit IRC19:25
*** mvorwerk has joined #openstack-nova19:26
*** zzzeek has joined #openstack-nova19:28
*** zzzeek has quit IRC19:35
*** zzzeek has joined #openstack-nova19:36
*** tosky has quit IRC19:38
*** jsuchome has quit IRC19:46
*** mvorwerk_ has joined #openstack-nova19:47
*** mvorwerk has quit IRC19:48
*** artom has joined #openstack-nova19:55
*** k_mouza has joined #openstack-nova19:57
openstackgerritsean mooney proposed openstack/nova master: libvirt: delegate ovs plug to os-vif  https://review.opendev.org/60243220:00
openstackgerritsean mooney proposed openstack/nova master: pass vifs_already_plugged when reverting a migration  https://review.opendev.org/74829620:00
mnaserdoes anyone know why regex based filtering is no longer working in the api with ussuri?20:00
mnaseri cant track any commits that might have caused it20:01
*** nweinber has quit IRC20:01
mnaseropenstack --debug server list --all-projects --name 'foo' returns two servers which are 'foo1.bar' and 'foo2.bar'20:01
mnaserbut `openstack --debug server list --all-projects --name 'foo\d'` returns nothing20:01
*** k_mouza has quit IRC20:01
sean-k-mooneydid you check the client20:02
sean-k-mooneyim not sure if this is api side20:02
sean-k-mooneyit might be20:02
sean-k-mooneywell i think it is but maybe there were some clinet changes that broke it20:02
mnasersean-k-mooney: its not, i looked into the api-ref20:03
mnaserit says you can send actual regex20:03
sean-k-mooneystring20:03
sean-k-mooney20:03
sean-k-mooneyFilters the response by a server name, as a string. You can use regular expressions in the query. For example, the ?name=bob regular expression returns both bob and bobb. If you must match on only bob, you can use a regular expression that matches the syntax of the underlying database server that is implemented for Compute, such as MySQL or PostgreSQL.20:03
mnaser_regex_instance_filter() in https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py hasn't been touched for 5-8 years20:03
mnaseraccording to blame20:04
sean-k-mooneyso if this behavior changed its likel a db change20:04
mnaseri mean, it went from myslq to mysql20:04
mnaserlet me check if maybe its an option ;\20:04
sean-k-mooneythis was the last change to that code https://github.com/openstack/nova/commit/117fad897d5310d66cc2e690f3cd32e72614d8fd20:06
mnaserso it looks like it should be sending a REGEXP query20:07
mnaserlet me try it out against the actual db server20:08
mnaserindeed, the regexp doesnt work20:09
mnaser`SELECT * FROM instances WHERE display_name REGEXP 'foo\d';` returns an empty set20:09
sean-k-mooneytry 'foo\d.*'20:11
sean-k-mooneyfoo\d would not match foo1.bar20:11
sean-k-mooneyhttps://regex101.com/r/uDuQBs/1/20:14
mnasersean-k-mooney: ok so it turns out that mysql seemingly uses a differnt regex lib than mariadb20:14
mnaserhttps://dev.mysql.com/doc/refman/5.7/en/regexp.html#regexp-operators20:14
sean-k-mooneyyour current regex is not correct however20:15
sean-k-mooneyfoo\d will not match foo1.bar20:15
sean-k-mooneyit will match foo120:15
sean-k-mooneyfoo\d is the same as foo[0-9]20:16
mnasersean-k-mooney: right, i agree with you on that, but the regex impl using in mysql does not20:16
mnaserSELECT display_name FROM instances WHERE display_name REGEXP 'foo[[:digit:]].'; works20:16
sean-k-mooneyit looks like they want you to double escape20:17
sean-k-mooneyso you could try foo\\d20:17
mnasersean-k-mooney: nope20:18
mnasersee doc above, [[:digit:]] is what they use instead of \d according to that20:19
sean-k-mooneyok will this is not part of the nova api defintion20:19
mnaseri guess mariadb and mysql are different when it comes to this20:19
sean-k-mooneymaybe maybe not20:19
sean-k-mooney\d works in pyton20:19
sean-k-mooneybut it might not work in sql20:19
mnaseryeah well the fact we're relying on the db to do the filtering makes it very unpredictable20:19
mnaserbecause the regex can be different dependign on the backend20:19
sean-k-mooneyya well this is not a portable part of teh api20:21
sean-k-mooneywe dont actully give any guarenttes this will work20:21
sean-k-mooneyi know we looked at removing it at one point20:21
sean-k-mooneyit is called out in the api ref at least20:22
sean-k-mooneyIf you must match on only bob, you can use a regular expression that matches the syntax of the underlying database server that is implemented for Compute, such as MySQL or PostgreSQL.20:22
*** bbowen has quit IRC20:22
*** bbowen has joined #openstack-nova20:22
sean-k-mooneymnaser: for what its worth that regex support is from the nova v1 api20:27
sean-k-mooneyit predates v2 or microverions20:27
sean-k-mooneyi belive it even predates novas use of specs20:28
sean-k-mooneyso the fact it works at al is somewhat surprising20:28
melwittagreed, but apparently other people have known this works, someone fixed a thing related to it 3 years ago https://review.opendev.org/50676020:31
sean-k-mooneyoh im sure people still use it20:33
sean-k-mooneyjust its not portable and it proably not well tested20:34
*** lbragstad has quit IRC20:35
*** lbragstad has joined #openstack-nova20:35
*** vishalmanchanda has quit IRC20:37
melwittyeah20:37
sean-k-mooneyapparently sqlalcamey support regex filters20:37
sean-k-mooneyhttp://xion.io/post/code/sqlalchemy-regex-filters.html20:37
sean-k-mooneynick_regexp = '^' + re.escape(nick) + r'\d+$'20:37
sean-k-mooney    return session.query(Person).filter(Person.nick.regexp(nick_regexp)).all()20:37
sean-k-mooneymelwitt: mnaser so if we wanted to make it portable in the futrue we could use those to do so20:38
sean-k-mooneythat would standardise on python regex syntax20:38
mnaseroooh20:38
mnaseryes i like that approach20:38
melwittproject idea for mnaser xD20:38
sean-k-mooneywith a microversion since those are a thing now20:38
sean-k-mooneyso  you can got old and busted or new an shiny20:39
mnasermelwitt, sean-k-mooney: doesn't sound too harsh, probably once we're done upgrading everything to ussuri and this k8s operator for openstack project20:40
*** bnemec has joined #openstack-nova20:40
sean-k-mooneyare you collaberating with redhat on the k8s operator20:40
mnaseri dont think redhat is building a k8s operator for openstack, we're building https://opendev.org/vexxhost/openstack-operator20:41
mnaserall work is in gerrit / testing using tempest so nothing exotic in that sense20:41
sean-k-mooneyhttps://github.com/openstack-k8s-operators20:41
sean-k-mooneymnaser: redhat is20:41
mnaserlol, welp, TIL20:41
mnaserthis is very new20:42
sean-k-mooneymnaser: and we will be supporting it in the next major verions20:42
mnasergosh, this is exactly what we're doing20:42
mnaserlol20:42
sean-k-mooneyaltough proably as tech preview20:42
mnasersean-k-mooney: who should i reach out to lol20:42
sean-k-mooneythat is a good question i can follow up internally tommorow and ask20:43
mnasersean-k-mooney: let me know because it pretty much already just works here... so we're really doing the same thing essentially :)20:44
sean-k-mooneymdbooth and mschuppert are involed with the nova part20:44
mnaseri.e. the keystone todo is already done in our case, heh20:44
sean-k-mooneydo you have a fully contiarerised nova contol plane20:45
sean-k-mooneyoh you are actully doing it in opendev20:46
mnasersean-k-mooney: yes :)20:46
sean-k-mooneynice20:47
mnaserand comptues too, the only thing that runs on the physical nodes is openvswitch and libvirt20:47
mnaserthe plan is to move openvswitch to containers too because we rely on the kernel data path so that should be ok20:47
mnaserand libvirt, planning to move towards the split daemon model and run only the kvm virt driver on the host (and i think even then that has a way of being containerized but not being killed)20:47
sean-k-mooneykolla uses ovs and libvirt in continers20:47
mnaserright but they have the advantage of managing a docker container and access to host20:48
mnaseri only can do things via k8s api20:48
sean-k-mooneyright but you can run it with pid=host and as a deamonset20:48
*** damien_r has quit IRC20:48
sean-k-mooneythat way the qemu instance are not tied to the lifetime of the libvirt pod20:48
mnaserwell you could but there's like some tricky things to do with that in making sure the process doesnt disappear/etc/etc -- but yeah, not impossible!20:49
sean-k-mooneythey are doint this in the nova operator i think20:49
sean-k-mooneythey started with runnign the contoles in vms and only contianerisn the comptue nodes20:49
sean-k-mooneythey are currently working on the contol plane20:50
mnaseryeah we just haven't gotten around that bit yet, but yeah, totally possible, we run nova-compute in containers too20:50
sean-k-mooneynova-compute is trivial20:50
sean-k-mooneylibvirt is harder20:50
mnaseryep20:51
mnaseranyways, yeah, we should totally talk because we're probably doing the same thing..20:51
sean-k-mooneyyep20:51
sean-k-mooneyand as i said this will be supported as a deployment model going forward20:51
sean-k-mooneyin our downstream product20:52
sean-k-mooneyit was anouched at the redhat summit a few months ago20:52
mnaserit's honestly the way to go :>20:52
*** mvorwerk_ has quit IRC20:52
openstackgerritMerged openstack/nova master: Ensure source compute is up when confirming a resize  https://review.opendev.org/69929121:03
*** slaweq has quit IRC21:18
*** k_mouza has joined #openstack-nova21:33
*** ganso has quit IRC21:40
*** bnemec has quit IRC21:42
*** ganso has joined #openstack-nova21:43
*** zzzeek has quit IRC21:45
*** zzzeek has joined #openstack-nova21:48
*** ganso has quit IRC21:52
*** bnemec has joined #openstack-nova21:52
*** ganso has joined #openstack-nova21:53
*** brinzhang0 has joined #openstack-nova22:00
*** raildo_ has joined #openstack-nova22:01
*** raildo has quit IRC22:04
*** brinzhang_ has quit IRC22:04
*** k_mouza has quit IRC22:06
*** bnemec has quit IRC22:09
*** ftarasenko has quit IRC22:17
*** bnemec has joined #openstack-nova22:21
*** rcernin has joined #openstack-nova22:40
*** bnemec has quit IRC22:44
*** bnemec has joined #openstack-nova22:48
*** dklyle has joined #openstack-nova22:50
*** JamesBenson has quit IRC22:50
*** bnemec has quit IRC22:54
*** tkajinam has joined #openstack-nova23:01
*** zzzeek has quit IRC23:10
*** zzzeek has joined #openstack-nova23:11
*** bnemec has joined #openstack-nova23:21
*** brinzhang0 has quit IRC23:22
*** brinzhang0 has joined #openstack-nova23:23
*** bnemec has quit IRC23:28
*** hamalq has quit IRC23:35
*** jamesdenton has quit IRC23:36
*** bnemec has joined #openstack-nova23:50

Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!