Earlier  
Posted Nick Remark
#openstack-nova - 2020-06-22
13:35:17 dansmith gibi: I commented
13:36:14 dansmith gibi: making it non-public and specific to "decide if we should report zero" addresses my original concern I guess, but I don't understand what the problem currently is
13:38:36 gibi dansmith: my problem that it makes a coupling between nova.virt.libvirt.imagebackend.Image.cache and nova.virt.libvirt.imagecache.ImageCacheManager.cache_dir_is_on_same_dev_as_instances_dir as the later assumes how the former will create the directory
13:39:39 dansmith gibi: cache_dir is a property of the imagecache no?
13:40:09 gibi for me the reasoning like "the directory does not exists therefore it occupies 0 space" is easier to accept than "the directory is on the same dev as it is not created but we know that when it is created it will be a call to mkdir that creates it on the same dev"
13:40:19 dansmith are you just saying that the behavior of creating the cache_dir if it doesn't exist is something in the libvirt code?
13:40:52 dansmith gibi: until the directory exists, the same exact thing is returned right? zero?
13:41:52 tbarron sean-k-mooney: ack, cinder volumes are intended to have a life-cycle independent of compute instances or compute instance hosts
13:41:55 dansmith once the directory exists, we'll report what we see, which will almost definitely be the same dev, but if not, we'll report the value according to how the directory is at that point
13:42:11 gibi dansmith: the behavior of get_disk_usage() is the same in my PS2 and in PS5
13:42:49 gibi but I think the implementation is better strucutred in PS2
13:42:54 dansmith right, so I don't see that we're making any different assumptions
13:43:04 tbarron sean-k-mooney: so the cinder lvm backend is useful for testing iscsi but not so much for production deployments
13:43:21 dansmith gibi: well, I disagree because I think that a property should explode for a known condition
13:43:37 dansmith gibi: but make it not a property (and rename it) and you can have that structure
13:43:48 gibi dansmith: I accep that I'm ready to make that an internal helper instead of a public property
13:44:00 dansmith I think a property shouild /not/ explode I meant
13:44:07 gibi yeah, I agree ^^
13:45:09 gibi just to make sure I understand your point. Is it OK for you if change the property to an private helper method?
13:45:46 dansmith I don't like it, but it addresses the problem I had with PS2
13:46:40 gibi why don't you like it?
13:49:00 dansmith well, because as it is, the property has utility beyond what you're doing here. You're just changing it to "should I report zero for cache" which is a single conditional and might as well just be in the if statement of the get_disk_usage()
13:49:14 dansmith doesn't seem worth it being a helper to me
13:49:52 dansmith but all I really meant is that _I_ would keep it the way it is in PS5, but it matters to me less than you, so you should change it
13:50:15 dansmith what matters to me is not having that should-be-useful-but-dangerous public property
13:52:42 gibi dansmith: thanks
14:04:43 openstackgerrit Dan Smith proposed openstack/nova master: DNM: Try to make a glance multistore job https://review.opendev.org/734184
14:25:46 openstackgerrit Dan Smith proposed openstack/nova master: DNM: Try to make a glance multistore job https://review.opendev.org/734184
14:27:16 openstackgerrit Elod Illes proposed openstack/nova stable/train: Check cherry-pick hashes in pep8 tox target https://review.opendev.org/737279
15:08:06 jsuchome hey dansmith ... regular reminder about https://review.opendev.org/#/c/574301 once you have time...
15:28:00 dansmith jsuchome: I know, I haven't forgotten
15:45:46 openstackgerrit Balazs Gibizer proposed openstack/nova master: Guard against missing image cache directory https://review.opendev.org/736964
16:29:46 stephenfin melwitt: could you look at https://review.opendev.org/708617 too?
16:30:12 melwitt stephenfin: sure, will do
16:30:18 stephenfin thanks
17:22:33 openstackgerrit Stephen Finucane proposed openstack/nova master: fakelibvirt: Remove nova-network remnants https://review.opendev.org/737329
17:25:56 openstackgerrit Ghanshyam Mann proposed openstack/nova stable/stein: Make greande jobs n-v for EM and oldest stable https://review.opendev.org/737332
17:27:48 openstackgerrit Ghanshyam Mann proposed openstack/nova stable/stein: Make greande jobs n-v for EM and oldest stable https://review.opendev.org/737332
17:34:47 sean-k-mooney dansmith: you can increase the job timeout in the zull.yaml if you need to for the multistore job
17:35:05 sean-k-mooney dansmith: it looks like you glance api change made it this time https://zuul.opendev.org/t/openstack/build/18e4701c1a374bf09269778479160f25/log/controller/logs/etc/glance/glance-api_conf.txt
17:35:27 dansmith yep, and it asked for the copy
17:35:32 dansmith I think something else likely broke, looking now
17:35:37 dansmith Jun 22 15:42:22.928857 ubuntu-bionic-rax-iad-0017311577 nova-compute[23701]: INFO nova.virt.libvirt.imagebackend [None req-ca48174a-0bf4-4341-8d45-fcf69cc9a3de tempest-DeleteServersAdminTestJSON-1752858908 tempest-DeleteServersAdminTestJSON-1752858908] Asking glance to copy image e6b1a7d0-ccd8-4be3-bef7-69c68fca4313 to our rbd store robust
17:36:14 dansmith Jun 22 15:52:23.076886 ubuntu-bionic-rax-iad-0017311577 nova-compute[23701]: ERROR nova.compute.manager [instance: 2cb1f8e2-a6a3-4f42-b6e2-de6823c71e25] nova.exception.ImageUnacceptable: Image e6b1a7d0-ccd8-4be3-bef7-69c68fca4313 is unacceptable: Copy to store robust timed out
17:36:53 sean-k-mooney it might be a slow node
17:37:12 sean-k-mooney you could relax some of the times outs for image/volume creation
17:37:23 dansmith it waited ten minutes
17:37:38 dansmith that should be more than long enough to copy a cirros image on any node I think
17:38:03 sean-k-mooney ya fair point :)
17:38:33 sean-k-mooney i was more thinking it was a slow host becaue it hit the 2 hour job time out
17:38:46 sean-k-mooney althougyh i guess enough 10 minute wait would have the same effect
17:39:28 dansmith I think it's just because each time we went to spawn an instance, it waited ten minutes before failing,
17:39:35 dansmith which linearized is enough to run the timeout
17:40:11 dansmith https://zuul.opendev.org/t/openstack/build/18e4701c1a374bf09269778479160f25/log/controller/logs/screen-g-api.txt#7466
17:40:18 dansmith glance was failing to update its own property I think
17:40:35 sean-k-mooney right this si the image convertion https://zuul.opendev.org/t/openstack/build/18e4701c1a374bf09269778479160f25/log/controller/logs/screen-g-api.txt#440
17:41:03 sean-k-mooney so it looks like the inital import conversion worked
17:42:04 dansmith the devstack conversion you mean?
17:42:14 dansmith had it not, nova wouldn't have even tried to boot on it, so yeah
17:42:27 dansmith and the devstack patch I have wouldn't have gotten past waiting for the image to go active
17:43:05 sean-k-mooney dansmith: yes the intial devstack conversion seam to have worked fine so the failure after after the qcow has been converted to raw and stored in teh file backedn
17:43:26 dansmith yep
17:43:54 sean-k-mooney well if nothing else i guess glance can now use your patch to test that...
17:44:43 sean-k-mooney os_glance_importing_to_stores seams like a strange name for a property on the image
17:45:08 dansmith that's the task status property
17:45:23 sean-k-mooney https://github.com/openstack/glance/blob/92492cf50461e214b777c707148886a8e87f340d/releasenotes/notes/import-multi-stores-3e781f2878b3134d.yaml#L25 yep
17:46:18 sean-k-mooney i guess the import-form-copy is modifying that to add the rbd store
17:48:27 dansmith right, the glance tasks modify that property to tell us what is happening
17:52:06 sean-k-mooney dansmith: i wonder if this could be related to who owns the image
17:52:20 sean-k-mooney devstack uploads it as admin correct
17:52:33 sean-k-mooney but tempest is running with its own tenats
17:52:42 dansmith well, that's the obvious thing, but the task should be using an admin context for this kind of metadata updating
17:52:47 dansmith and they say it should
17:52:49 sean-k-mooney so perhaps they do not have permission to modify that porperty
17:53:22 sean-k-mooney that would be the logical thing to do yes
17:53:29 sean-k-mooney but manybet its not
17:53:58 sean-k-mooney https://github.com/openstack/glance/commit/1754c9e2b085ba0fc37a4369488c92a40268997a add the copy image support so im just skiming it quickly to see what it does
18:01:23 sean-k-mooney home ok i dont see how the propery gets updated in that but i also dont know how glance works internally so its not suprising.
18:01:32 sean-k-mooney oh time for a call...
18:19:53 dansmith melwitt: a while back I asked about getting admin credentials for glance and you pointed me to something I ignored because I decided I didn't need admin
18:20:03 dansmith melwitt: do you remember that and if so can you point me again?
18:26:35 melwitt heh, sec
18:29:19 melwitt dansmith: it might have been this commit https://github.com/openstack/nova/commit/aab4b7a0e2504c04e08389145bcb1414dea63631
18:29:58 melwitt just as an example of a place where we needed to use an admin cred to make a particular API call
18:34:08 dansmith melwitt: okay that's just a flag to the neutron client right?
18:34:47 dansmith I thought there was something more general
18:37:46 melwitt dansmith: yeah, I think when I linked you I was just saying, it is normal/expected for us to have to selectively use admin to call other APIs and that was a recent example of us doing it
18:38:33 dansmith oh, okay, that's common in a lot of places, yeah.. what I need is a way to get admin creds to talk to glance
18:38:54 dansmith I don't really know how we do that for neutron.. I think long ago we had credentials in our config, but that's gone now right?
18:41:28 melwitt I don't know off the top of my head. I thought we did have creds but I don't know about them being gone. I'm looking through the code now to see if it's obvious
18:42:34 dansmith I thought there was some service user thing we use now, but yeah I don't really know
18:53:03 melwitt based on this code block, there are supposed to be creds used from nova.conf https://github.com/openstack/nova/blob/f1ebc15dfc8ffb7f23b2cb9879f0ca9376931a90/nova/network/neutron.py#L191
19:01:42 melwitt and here's a config file from a nova-next run showing what look to be service user creds for neutron and placement https://zuul.opendev.org/t/openstack/build/785733b6379b40a5982f710a62302c21/log/controller/logs/etc/nova/nova_cell1_conf.txt#40
19:07:26 dansmith melwitt: sorry in three conversations here
19:07:38 dansmith melwitt: yeah, okay, I thought we had moved past that at some point, but it looks like not
19:07:40 melwitt np. I'm still gathering info
19:08:16 melwitt we implemented this https://specs.openstack.org/openstack/nova-specs/specs/ocata/implemented/use-service-tokens.html which says it should have docs for setting up the service user stuff in conf but I don't find any docs so far
19:08:23 dansmith glance is kinda half requiring admin to do the image copy-to-rbd thing.. if that's intentional, then we'll need admin creds for glance too, which really sucks
19:08:29 dansmith heh
19:08:56 dansmith maybe it's done and devstack is just still using the old method?

Earlier   Later