| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-01 | |||
| 18:42:19 | andymccr | dansmith: if you run api_db sync when cell_v2 create hasnt run it fails saying "cell doesnt exist" on an upgrade | |
| 18:42:29 | openstackgerrit | Feodor Tersin proposed openstack/nova-specs master: Add ScaleIO ephemeral storage backend https://review.openstack.org/304715 | |
| 18:42:35 | andymccr | but on a greenfield deploy if you run "cell_create" without api_db sync having happened it says "cant write to nova_api database" | |
| 18:42:55 | dansmith | ah, okay, I see | |
| 18:43:22 | dansmith | that's a very atypical example, so maybe we work around that one specifically instead of building in a lot of "check this before doing a thing" for other stuff? | |
| 18:43:52 | dansmith | andymccr: how do you know if flavors need to be created? | |
| 18:44:00 | dansmith | (i.e. upgrade or new install) | |
| 18:44:48 | andymccr | we don't create flavors as part of the deploy - that's up to the deployer afterwards if they want (or in testing the tempest role sets up the bits) | |
| 18:45:00 | dansmith | oh okay I thought you did | |
| 18:45:00 | andymccr | but the create bits are idempotent so if they exist it doesnt bomb or anything | |
| 18:45:21 | dansmith | okay, so, the migration uses flavors to determine if it should fail | |
| 18:45:34 | dansmith | depending on where this runs you could probably do the same | |
| 18:45:39 | mriedem | sdague: sfinucan: i've figured out that https://review.openstack.org/#/c/434339/ introduces a bug | |
| 18:45:48 | mriedem | in the 204 case | |
| 18:46:01 | dansmith | andymccr: also you could sync to before the blocker, do the create, then finish the sync and that would work for both cases | |
| 18:46:36 | andymccr | dansmith: how can i specify to sync to before the blocker? | |
| 18:46:54 | mriedem | fixing | |
| 18:46:59 | andymccr | dansmith: if i understand what you mean you do the first part of the sync, then create the cell if it doesnt exist, then finish the sync | |
| 18:47:13 | dansmith | andymccr: sync --version 123 | |
| 18:47:19 | dansmith | andymccr: yup | |
| 18:47:58 | mriedem | andymccr: there is also a nova-manage cell_v2 list_cells command | |
| 18:48:04 | dansmith | andymccr: that blocker was attempting to prevent people from screwing themselves by not reading the docs, but I see how it puts you in a tight spot | |
| 18:48:11 | mriedem | you could parse that for the nova_cell0 entry | |
| 18:48:12 | dansmith | mriedem: that too | |
| 18:48:32 | dansmith | andymccr: with nova-status we may be able to get away from doing that as much in the future | |
| 18:48:51 | andymccr | mriedem: that would tell if cells exist, so in the case of greenfield i still need an api_db sync first - in upgrades i wouldn't | |
| 18:48:54 | mriedem | yeah, we did this blocker migration before we had nova-status | |
| 18:49:04 | andymccr | mriedem: the key one would be a "has an api_db sync ever run successfully" command | |
| 18:49:16 | dansmith | andymccr: db version | |
| 18:49:33 | mriedem | yeah | |
| 18:49:38 | mriedem | nova-manage api_db version | |
| 18:49:39 | dansmith | andymccr: 'nova-manage api_db version' to see if you're at 0, before the blocker, or after, then do the right thing | |
| 18:49:45 | mriedem | that will tell you if you've synced | |
| 18:50:30 | openstackgerrit | Sujitha proposed openstack/nova master: Remove mox from nova.tests.unit.virt.xenapi.test_vmops[2] https://review.openstack.org/439770 | |
| 18:50:58 | andymccr | mriedem: dansmith thanks a bunch i'll play around with those and try see if i can get something working. that sounds like it may work | |
| 18:51:06 | dansmith | andymccr: cool | |
| 18:51:13 | andymccr | owalsh-afk: i'll let you know what i come up with | |
| 18:51:20 | andymccr | it might be helpful for tripleo too | |
| 18:51:42 | melwitt | dansmith: how can you tell what version the blocker is? | |
| 18:51:53 | dansmith | melwitt: look in the tree? | |
| 18:52:01 | dansmith | melwitt: this would be a special case for this blocker | |
| 18:52:28 | melwitt | so 030 I guess | |
| 18:52:56 | dansmith | melwitt: yep | |
| 18:53:37 | sdague | mriedem: really, because there is a unit test for that | |
| 18:53:59 | mriedem | sdague: it doesn't model the requests.Response.__bool__ override | |
| 18:54:05 | mriedem | sdague: i've got the fix, just opening a separate bug for it | |
| 18:54:08 | sdague | https://review.openstack.org/#/c/434339/3/nova/tests/unit/test_metadata.py@907 | |
| 18:54:12 | mriedem | yeah, it's borked | |
| 18:54:20 | sdague | mriedem: ok, what's wrong with the unit test? | |
| 18:54:48 | sdague | oh, did it skip a step? | |
| 18:55:09 | mriedem | yes | |
| 18:55:25 | sdague | also... why do we believe people are going to return a 204 from this who's purpose is to fetch data? | |
| 18:55:36 | mriedem | i thought about that too | |
| 18:55:43 | sdague | like, I don't even understand why we would believe that's a thing | |
| 18:55:45 | mriedem | and i don't know, but mikal built it into the tests | |
| 18:55:54 | sdague | right, but I think he was cantriping something else | |
| 18:56:13 | sdague | those tests and the use of requests status codes look very much like copy paste from some other pattern | |
| 18:56:29 | sdague | anyway, it's probably fine to just make sure that the json decode exception is handled | |
| 18:56:52 | mriedem | it is, it handles a TypeError and ValueError | |
| 18:56:59 | sdague | ok, then we are fine, right? | |
| 18:57:00 | mriedem | but that results in a warning for a successful response | |
| 18:57:04 | mriedem | no | |
| 18:57:20 | sdague | I'm not sure I buy that 204 is here intentionally | |
| 18:57:31 | mriedem | well, | |
| 18:57:38 | mriedem | it doesn't work for me because it started making my tests in https://review.openstack.org/#/c/435563/ fail | |
| 18:57:39 | sdague | the fact that the 204 unit test returned content, makes me again think this is randomly just added | |
| 18:57:44 | mriedem | because i was expecting one warning, not two | |
| 18:57:57 | mriedem | the 204 unit test doesn't return content | |
| 18:57:58 | sdague | mriedem: ... that I don't accept :) | |
| 18:58:04 | sdague | mriedem: it used to | |
| 18:58:28 | sdague | https://review.openstack.org/#/c/434339/3/nova/tests/unit/test_metadata.py@849 that's why that had to be changed | |
| 18:58:32 | mriedem | request_mock.return_value.status_code = response_code | |
| 18:58:45 | mriedem | the actual runtime was changed to not look at status_code | |
| 18:58:58 | mriedem | and so it just looks at request_mock.return_value which is truthy i guess | |
| 18:59:04 | sdague | mriedem: but the unit test simulating the 204 was returning content previously | |
| 18:59:26 | mriedem | yeah true | |
| 18:59:27 | sdague | which makes me really really believe none of this was part of the design, it was just a cantrib | |
| 18:59:33 | sdague | cantrip | |
| 18:59:40 | sdague | so stop reading so much meaning into it :) | |
| 18:59:43 | mriedem | i don't know wtf a cantrip is | |
| 18:59:48 | sdague | magic spell | |
| 19:00:05 | mriedem | duped by those aussies again! | |
| 19:00:12 | sdague | spin around 3 times, I don't get a cold, ok, just do that every day | |
| 19:00:21 | sdague | the talisman will keep the tigers away | |
| 19:00:32 | mriedem | headache is a demon in my head? | |
| 19:00:40 | sdague | yep, pretty much | |
| 19:02:53 | mriedem | so i'm still going to handle the if res and res.text case, because we don't know how people are going to use this, there could be like a first time checkin thing that returns data, and later calls don't, idk | |
| 19:03:02 | mriedem | i suspect that's maybe what mikal was thinking | |
| 19:03:19 | mriedem | that would seem pretty crappy though | |
| 19:03:29 | openstackgerrit | John Garbutt proposed openstack/nova-specs master: Add spec to use Cinder's new attachment APIs https://review.openstack.org/373203 | |
| 19:14:31 | sdague | mriedem: well, we're not handling any of the 30x stuff in real ways either, like processing the redirects | |
| 19:15:15 | sdague | mriedem: honestly, I think this is being overthought, this is a custom service that the op is writing, if they write it to do bad things, .... there is only so much protection here | |
| 19:16:13 | sdague | especially as this is in the metadata code which is ... you know, funky :) | |
| 19:19:13 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Check for 204 case in DynamicVendorData https://review.openstack.org/439785 | |
| 19:22:21 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Only create vendordata_dynamic ksa session if needed https://review.openstack.org/435563 | |
| 19:27:21 | mriedem | Vek: andreykurilin: tdurakov: we should have had a release note with this https://review.openstack.org/#/c/439141/ | |
| 19:27:42 | andreykurilin | oh.. | |
| 19:27:43 | andreykurilin | sure | |
| 19:27:49 | mriedem | we can just roll one on top | |
| 19:28:02 | Gorian | huh. I got pinged here, but it's been unusally active, so I don't have history that far back... | |