Monday, 2016-12-05

*** bswartz has joined #openstack-meeting-cp02:27
*** tovin07 has joined #openstack-meeting-cp02:50
*** coolsvap has joined #openstack-meeting-cp04:30
*** prateek has joined #openstack-meeting-cp05:28
*** mgagne has quit IRC05:53
*** mgagne has joined #openstack-meeting-cp05:55
*** mgagne is now known as Guest5173705:55
*** tovin07_ has joined #openstack-meeting-cp06:25
*** dhellmann_ has joined #openstack-meeting-cp06:53
*** dhellmann has quit IRC06:53
*** dhellmann_ is now known as dhellmann06:56
*** garloff has quit IRC07:48
*** brault has joined #openstack-meeting-cp08:09
*** rarcea has joined #openstack-meeting-cp08:16
*** MarkBaker has joined #openstack-meeting-cp10:24
*** beekhof has quit IRC10:27
*** tovin07 has quit IRC10:44
*** tovin07_ has quit IRC10:48
*** sdague has joined #openstack-meeting-cp10:52
*** Daviey has quit IRC11:43
*** Daviey has joined #openstack-meeting-cp11:43
*** prateek has quit IRC12:59
*** xyang1 has joined #openstack-meeting-cp13:55
*** lamt has joined #openstack-meeting-cp14:01
*** coolsvap has quit IRC14:27
*** prateek has joined #openstack-meeting-cp14:44
*** lamt has quit IRC14:46
*** edtubill has joined #openstack-meeting-cp15:02
*** gouthamr has joined #openstack-meeting-cp15:39
*** prateek has quit IRC15:53
*** garloff has joined #openstack-meeting-cp16:01
*** lamt has joined #openstack-meeting-cp16:38
*** garloff has quit IRC16:42
ildikov#startmeeting cinder-nova-api-changes17:00
openstackMeeting started Mon Dec  5 17:00:16 2016 UTC and is due to finish in 60 minutes.  The chair is ildikov. Information about MeetBot at http://wiki.debian.org/MeetBot.17:00
openstackUseful Commands: #action #agreed #help #info #idea #link #topic #startvote.17:00
*** openstack changes topic to " (Meeting topic: cinder-nova-api-changes)"17:00
openstackThe meeting name has been set to 'cinder_nova_api_changes'17:00
*** mriedem has joined #openstack-meeting-cp17:00
mriedemo/17:00
ildikovscottda ildikov DuncanT ameade cFouts johnthetubaguy jaypipes takashin alaski e0ne jgriffith tbarron andrearosa hemna erlon mriedem gouthamr ebalduf patrickeast smcginnis diablo_rojo gsilvis  xyang1 raj_singh lyarwood17:00
scottdahi17:00
ildikovmriedem: scottda: hi :)17:01
ildikovlet's wait a bit longer to see who's around for today's meeting17:01
DuncanTHi17:02
DuncanTI'm here, though I've had no real input on this for ages17:02
ildikovDuncanT: no worries, let me give a very short summary17:03
ildikovlast week we decided to go with CRUD operations for the new Cinder API in the sense of having attachment_create, attachment_update, attachment_get and attachment_remove17:04
ildikovattachment_create reserves the volume and creates a volume attachment17:04
ildikovif the connector is not specified in the call it stops there17:04
ildikovotherwise finishes the attachment process17:04
DuncanTSo that does the state change (attaching or attached) too17:05
ildikovNova can call it from the API for instance and call attachment_update when has the connector17:05
DuncanT?17:05
ildikovDuncanT: it should handle state changes17:05
mriedemi've finally reviewed johnthetubaguy's spec again https://review.openstack.org/#/c/373203/17:06
mriedem47 comments :)17:06
mriedemi have some concerns in there17:06
ildikovDuncanT: we haven't agreed on the exact state machine, I hope to have that when jgriffith updates the code based on what we agreed at last week17:06
DuncanTildikov: So for the instance migration case, we create two attachments for the same volume?17:06
mriedemDuncanT: yes17:06
ildikovDuncanT: if you mean live migrate, then yes17:06
mriedemsame instance, same volume, different hosts per attachment17:06
DuncanTThat makes sense. Thanks.17:06
ildikovmriedem: does any of your concerns affect the high levels we agreed last week?17:07
mriedemoh that's right, johnthetubaguy is out this week, wedding + honeymoon17:07
* smcginnis sneaks in17:07
mriedemildikov: honestly i didn't catch the meeting last week17:07
mriedemildikov: some of my main concerns in john's spec are (1) nova setting the vol attachment status to 'error'17:08
ildikovmriedem: then with what I summarized at the beginning of this one?17:08
mriedemCRUD ops in cinder api for attachments is fine17:08
ildikovmriedem: ok, we haven't touched on those "details"17:08
mriedemmy concerns in the nova spec are really around interactions17:08
mriedemand some smaller things like i don't think nova needs a config option to tell it to do the new flow17:08
mriedembut my main issue is nova setting the vol attachment status to error17:09
ildikovI think that comes from the direction of creating new attachment in almost ever use case Nova has17:09
ildikovif we can update the attachment, then changing the state to error does not make much sense17:10
mriedemwell, the spec is also a bit contradictory in places about either deleting the attachment on failure, or setting it to error status17:10
smcginnismriedem: Yeah, I think on failure probably better to just delete and retry. Or not.17:10
mriedemimo nova shouldn't touch the state on an external resource,17:11
mriedemif nova wants to set the instance to error state, so be it17:11
ildikovI don't think either that Nova should change the attachment sate to error17:11
mriedemthe shelve stuff in there isn't fully fleshed out, but it's late in the spec and i'm assuming john ran out of steam17:12
ildikovsounds like too many things to clean up after a while, but it's unclear that when and by whom17:12
mriedemi'm trying to walk a fine line before john (both johns) rage quitting17:12
mriedems/before/between/17:12
mriedemanyway, john garbutt is out this week17:12
mriedemmy comments are in the spec17:12
mriedemi don't really have anything else to share today17:12
smcginnisSo the nova side issues have an impact on the cinder side ones?17:13
ildikovmriedem: we can move forward with the Cinder parts this week as if my understanding is correct your comments are more about how Nova is using the Cinder API17:13
ildikovmriedem: is this assumption correct?17:13
smcginnisI think we mostly have agreement at a high level. Just wondering if we can proceed on the cinder side and work out the nova bits as we go.17:13
smcginnisildikov: Thinking the same. ;)17:14
mriedemi haven't looked at the cinder spec,17:14
mriedemthe nova spec calls out some assumptions about how cinder handles state transitions for the attachment17:14
mriedembut we could change that over time with microversions if needed17:14
mriedemas nova starts to use it17:14
ildikovmriedem: it's short and high level mainly captures CRUD17:15
mriedemthere are some things in the nova spec about upgrading old attachments to the new state model...that might be worth looking into17:15
mriedemlike if cinder will have issues with nova creating a new attachment record for an already attached volume17:15
ildikovmriedem: I think if we go on and implement that under a next API microversion in Cinder then it should be a pretty good first step, but I'll look into the state changes part17:15
ildikovalthough I think it's mainly Cinder's business what the volume state is...17:16
mriedemi.e. nova has the bdm.connection_info, i'm assuming nova would pass that in on attachment create/update, but what would cinder do with that?17:16
smcginnisildikov: +117:16
ildikovmriedem: you mean Nova gets back the connection_info fro Cinder and stores it, right?17:17
mriedemno17:17
mriedemi'm talking about migrating existing attachments to the new model17:17
mriedeme.g. we have a server with a volume that was attached in kilo17:17
mriedemit doesn't have any attachment record in cinder17:17
mriedemnova has the bdm with the connection_info17:17
ildikovah, ok17:18
mriedemgarbutt's spec says that to upgrade nova will be creating new style attachments for existing bdms17:18
mriedemso the volume is already attached, we just need to get it into the new state of things so that a later detach, using the new flow, will work17:18
ildikovI think we were talking about an upgrade script kind of thing earlier17:18
mriedemi *think* that just means nova creates the attachment record in cinder and passes the connection_info, connector, host and server id17:19
mriedembut with the volume already being in-use, and probably multiattach=False, will cinder puke on that?17:19
ildikovthat's the same instance so it will not17:19
mriedemin other words, cinder will need some kind of 'oh this is for an existing attachment thing, let's not change state and just store it for our records' kind of logic17:19
hemnawhy would nova pass the cinder connection_info during detach?17:20
mriedemhemna: this isn't detach17:20
* hemna is confused17:20
mriedemhemna: did you show up late?17:20
hemnayah I did sorry17:20
ildikovalthough it's also the same host, I'm not sure what we agreed on that, most probably nothing in details17:20
*** diablo_rojo has joined #openstack-meeting-cp17:20
hemnatrying to read the scrollback17:20
mriedemi don't think this is a major hurdle,17:21
hemnaoh migrating existing attachments17:21
mriedemjust saying, there are cases like this that are probably not fully thought through yet on the cinder side, which we might have to microversion as nova starts using the new apis17:21
mriedemhemna: yes17:21
ildikovhemna: we are talking about the situation, when we are upgrading the system so we have volume attachments that were created with the old Cinder, so they don't have all the data, etc.17:21
mriedemyeah, nova has the data17:21
hemnaso is cinder supposed to save the connection_info in the attachments table for each attach?17:21
mriedemand wants to give it to cinder17:22
mriedemyes17:22
ildikovhemna: also in Nova we need to add the attachment_id and additional stuff to the BDM that we don't store today to get to a consistent environment17:22
hemnaah yah17:22
mriedemthe old connection_info won't have the 'shared_host_connection' flag either...17:22
hemnado we have a way to get that attachment_id in the API during a migration?17:22
mriedemso when we go to detach the old style connection we'd be lacking those details..unless we refresh that at some point17:23
hemnathe shared_host_connection comes from the cinder driver though17:23
mriedemhemna: when upgrading, nova creates the attachment in cinder, gets the attachment id back and stores it in the nova bdm17:23
mriedemhemna: i know,17:23
hemnaunless Cinder calls the driver to ask for it, we can't get it17:23
mriedembut it's not in the kilo era attachment17:23
mriedemthat's why i'm thinking cinder might need to refresh it or something17:23
mriedemi guess if os-initialize_connection is idempotent today, this should probably be ok17:24
hemnaso I presume that the nova migration process should trigger the fetching of that17:24
hemnayup17:25
mriedemso,17:25
hemnathe volume manager would have to call initialize_connection to ask for it17:25
hemnaand would also get the rest of the connection_info at that point too17:25
hemnaeven though Nova has a copy of it in the bdm17:25
mriedemjohn's spec says that when nova calls update_attachment, it passes the os-brick connector, and gets the connection_info back - i guess for our upgrade scenario, nova can just be creating the attachment and updating it with the connector, and then brick can refresh the connection at that point17:26
mriedemthen cinder doesn't know if nova is actually attaching a new volume or just record keeping an old legacy attachment17:26
*** Guest51737 is now known as mgagne17:26
hemnathat *should* be ok17:26
hemnaFC is easy17:27
*** mgagne has quit IRC17:27
*** mgagne has joined #openstack-meeting-cp17:27
hemnaiSCSI does issues some rescans, etc17:27
hemnabut tht should be ok too, for an existing attachment17:27
ildikovwith the new Cinder API it sounds like an attachment_create with specifying all the info to it and delete the old one if the new attachment is created successfully17:28
mriedemdelete what old one?17:28
mriedemthere is no old one if the volume attachment is from kilo17:28
ildikovthen what was the "creating" part of your description above?17:29
mriedemnot sure i follow17:30
mriedemi'm talking about a nova periodic task or something that's migrating old vol attachment data to the new world17:30
mriedemthose old vol attachments won't have a literal 'attachment' record yet17:30
mriedemb/c when those were created, the attachment API didn't exist17:30
hemnathere will be an existing attachment record in cinder already for those17:31
mriedemso nova's periodic task or some migration script will need to create those attachments for existing volume bdms17:31
ildikovbut the attachments are saved in the DB today too17:31
ildikovwith an attachment_id17:31
mriedemhmmm17:31
ildikovI think it might even be a separate table, I would need to check that17:31
hemnavolume_attachment17:31
mriedemit won't have the connector17:31
hemnaI created that17:31
ildikovhemna: yeap, that's the one17:31
mriedemor the host?17:31
hemnacorrect17:31
hemnait won't have the connector17:31
mriedemi'm not sure what needs to be consolidated there17:31
hemnaor the host17:31
ildikovmriedem: yeap, with less info as you say, but it exists17:31
mriedembut if it already exists, then i guess nova just needs to get the attachments for a given volume/server/host and then update them with the connector17:32
mriedemand then store that attachment id in the bdm17:32
hemnayah17:32
hemnathe attachment will have the instance_uuid in there17:32
mriedemhemna: will it?17:34
mriedemi thought that was something new getting stored in like mitaka17:34
hemnayup17:34
mriedemeven going back to juno attachments?17:34
mriedemor essex17:34
mriedem:)17:34
hemnaI think that was stored in the volume table17:34
mriedemmaybe cinder didn't exist until folsom :L)17:34
hemnaprior to my adding the volume_attachment table17:34
mriedemyeah, so point being...17:34
mriedemdata migration ftw17:34
hemnaI think we still stored the instance_uuid17:34
mriedemand people with pets they haven't touched in 4 years17:35
ildikovwhat about creating a new attachment record and do cleanup for the old data?17:35
hemnahttps://github.com/openstack/cinder/blob/grizzly-eol/cinder/db/sqlalchemy/migrate_repo/versions/001_cinder_init.py#L17917:36
mriedemnot sure17:36
hemna:)17:36
mriedemhemna: yeah, but was that copied into the volume_attachments table when it was added?17:37
mriedemi.e. was that data migrated?17:37
hemnayes it was migrated17:37
hemnaI wrote that migration17:37
mriedemok17:37
hemnaexisting attachments got copied into the volume_attachment table with the instance_uuid17:37
mriedemalright, so maybe we just have nova call update_attachment and pass in the connector,17:37
mriedemand that's it17:37
mriedemcinder returns the attachment_id and nova stores it in the bdm17:37
mriedemand we consider that done17:37
hemnathat should work17:37
mriedemi mean, nova's migration is really just, get me all BDMs for instances on this host with a volume_id set but no attachment_id value17:38
mriedemonce we update those with the attachment_id, we don't migrate them again17:38
hemnayup17:38
hemnathat should be good17:38
mriedemthen we drop that after ocata-eol or whatever17:38
mriedempike-eol at this point probably..17:38
mriedembut anyway17:38
hemna+117:39
ildikovsounds good to me17:39
ildikovI hope to get the Cinder API landed soon and have the code up for Nova by the end of Ocata the latest17:39
mriedemi can go back and update garbutt's spec and my comments/questions about upgrades with this info17:40
mriedemi.e. nova shouldn't need to create a new attachment17:41
mriedemdo we know that griffith plans on using the existing volume_attachments table?17:41
ildikovmriedem: that would be great to have that updated17:41
hemnaso...17:41
hemnafor whatever reason he created some key/value pair table for attachments17:41
hemnaI never understood the need for it17:41
ildikovI think he planned to use a new one, but then we touched on that briefly that we might have that as a next step rather than a starting point17:42
hemnasince the volume_attachment table exists and just needs the new columns to support the shared flag, connector and connection_info17:42
ildikovI would guess part of the issue is the info we store is different per driver, etc.17:42
hemnaeverything would be there in that normalized table17:42
hemnabut whatevs17:42
mriedemok, so you guys are going to have to sort that out17:42
ildikovmriedem: on my TODO list, it's added as a TODO in the Cinder spec too to ensure we have an eye on that17:43
mriedem"I would guess part of the issue is the info we store is different per driver, etc." so store an 'extras' column with a json blob of driver cruft17:43
hemnait shouldn't affect the Nova <---> Cinder API interactions17:43
hemnait just makes the implementation in Cinder more difficult for no reason.17:43
mriedemhemna: right, as long as it doesn't leak out of the API i think we're fine17:43
ildikovhemna: +117:43
ildikovmriedem: if it does then we did something wrong IMHO17:44
hemnayah17:44
hemnahttps://review.openstack.org/#/c/387712/6/cinder/db/sqlalchemy/migrate_repo/versions/087_add_attachment_specs.py17:45
hemnafwiw17:45
smcginnisildikov: Is the Cinder spec up to date with the latest?17:46
ildikovsmcginnis: yes17:46
smcginnisildikov: Great, thanks for doing that.17:47
ildikovsmcginnis: I'm still waiting for some input from jgriffith, but the outcome of the last meeting is captured in the spec17:47
smcginnisSounds like we need some of his time. That would be good to have him check off on it and add any other details from in his head.17:48
ildikovsmcginnis: I added a ref to the Nova spec as it's detailed regarding the interaction, so it contains examples regarding usage17:48
smcginnisildikov: +117:48
ildikovsmcginnis: he's traveling today AFAIK, but I'll ping him later this week17:48
smcginnisildikov: Yeah, I think he's on a plane right now or in route. Hopefully he has some time later in the week.17:49
ildikovsmcginnis: do you have a rough guess on until when we should have the code ready to have it merged in Ocata?17:49
*** harlowja has joined #openstack-meeting-cp17:50
smcginnisildikov: I'm fine up until O-3 as long as we have some confidence in the code. At least in that it won't break anything existing and we can start iterating with microversions if tweaks are needed once Nova starts trying to use it.17:50
hemnaare multinode tests working?  re: live migration17:51
ildikovsmcginnis: I believe it's simple enough to serve the purpose and as it's completely new we should have less issues with it17:51
smcginnisildikov: I'm hoping so.17:51
mriedemhemna: which multinode tests? nova or cinder?17:52
smcginnishemna: I know we have multinode running.17:52
mriedemor both17:52
smcginnishemna: For cinder.17:52
ildikovscottda: are you available to help out with microversions?17:52
mriedemwe had to disable the volume-backed live migration tests for lvm in the gate17:52
hemnaildikov, well the API is new and not used yet, but his patch does change the volume manager quite a bit17:52
smcginnishemna: But can't remember what exactly it's covering at this point.17:52
mriedemb/c the failure rate was too high17:52
*** Rockyg has joined #openstack-meeting-cp17:52
hemna:(17:52
scottdaildikov: Yes, always17:52
ildikovscottda: coolio, tnx for confirming!17:52
mriedemvolume-backed ceph live migration might be more stable17:52
scottdaildikov: I've already given jgriffith Some POC code to help him moving forward17:53
hemnaattaches will work or they won't17:53
ildikovscottda: sounds great, tnx17:53
hemnait's live migration that has always been a PITA IMHO17:53
hemnaanyway, I think we need to get his patch out of merge conflict17:54
hemnaand get to testin git17:54
hemnatesting it17:54
ildikovhemna: +117:56
smcginnis3 minutes. Anything else today?17:56
hemnanot I17:56
ildikovmriedem: BTW, do you think you can check this one too: https://review.openstack.org/#/c/335358/ ? :)17:56
mriedemthis was the volume-backed live migration bug anyway https://bugs.launchpad.net/nova/+bug/152489817:56
openstackLaunchpad bug 1524898 in OpenStack Compute (nova) "Volume based live migration aborted unexpectedly" [High,Confirmed]17:56
mriedemildikov: i don't know when17:57
ildikovmriedem: I think it would pretty much make our lives easier with moving to the new Cinder API if we got this cleanup in17:57
ildikovmriedem: if you have an opinion about removing the do_check_attach flag that could give me some work17:58
ildikovmriedem: I mean what hemna asked on the last patch set17:58
mriedemi have to swap all of that context back in, which is going to take time, and have lots of things going on right now17:59
mriedemso i'll get to it when i can17:59
ildikovmriedem: ok17:59
ildikovmriedem: please try to do it before Christmas17:59
* ildikov likes Christmas gifts :)18:00
smcginnismriedem: A christmas present for ildikov18:00
ildikovsmcginnis: +1 :)18:00
mriedemhumbug18:00
mriedemi don't get any gifts18:00
ildikovmriedem: I will stop annoying you with the reminders18:00
ildikovmriedem: and will not remind you to anything else this year18:01
ildikovmriedem: I think we can almost say that it would be a present from me to you ;)18:01
smcginnisWe're over time..18:02
mriedemon that note, can we wrap up so i can get some lunch? :)18:02
ildikovyeap, we can18:02
ildikovtnx everyone :)18:02
smcginnisThanks18:02
ildikov#endmeeting18:02
*** openstack changes topic to "OpenStack Meetings || https://wiki.openstack.org/wiki/Meetings"18:02
openstackMeeting ended Mon Dec  5 18:02:50 2016 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)18:02
openstackMinutes:        http://eavesdrop.openstack.org/meetings/cinder_nova_api_changes/2016/cinder_nova_api_changes.2016-12-05-17.00.html18:02
openstackMinutes (text): http://eavesdrop.openstack.org/meetings/cinder_nova_api_changes/2016/cinder_nova_api_changes.2016-12-05-17.00.txt18:02
*** mriedem has quit IRC18:02
openstackLog:            http://eavesdrop.openstack.org/meetings/cinder_nova_api_changes/2016/cinder_nova_api_changes.2016-12-05-17.00.log.html18:02
*** diablo_rojo has quit IRC18:08
*** piet has joined #openstack-meeting-cp18:14
*** rarcea has quit IRC18:33
*** ameade_ has joined #openstack-meeting-cp19:23
*** SergeyLukjanov2 has joined #openstack-meeting-cp19:26
*** noama has quit IRC19:28
*** ameade has quit IRC19:28
*** SergeyLukjanov has quit IRC19:28
*** sheeprine has quit IRC19:28
*** mrhillsman has quit IRC19:28
*** jroll has quit IRC19:28
*** SergeyLukjanov2 is now known as SergeyLukjanov19:28
*** sheeprine has joined #openstack-meeting-cp19:28
*** ameade_ is now known as ameade19:31
*** noama has joined #openstack-meeting-cp19:35
*** jroll has joined #openstack-meeting-cp19:36
*** codebauss has joined #openstack-meeting-cp19:38
*** codebauss is now known as mrhillsman19:38
*** gouthamr has quit IRC19:38
*** MarkBaker has quit IRC20:30
*** diablo_rojo has joined #openstack-meeting-cp20:33
*** edtubill has quit IRC20:34
*** kencjohnston has left #openstack-meeting-cp21:00
*** kencjohnston has joined #openstack-meeting-cp21:00
*** garloff has joined #openstack-meeting-cp21:00
*** lamt has quit IRC21:03
*** edtubill has joined #openstack-meeting-cp21:28
*** edtubill has quit IRC21:39
*** lamt has joined #openstack-meeting-cp21:50
*** diablo_rojo has quit IRC22:03
*** r1chardj0n3s has left #openstack-meeting-cp22:07
*** beekhof has joined #openstack-meeting-cp22:13
*** piet has quit IRC23:00
*** xyang1 has quit IRC23:16
*** benj_ has quit IRC23:43
*** benj_ has joined #openstack-meeting-cp23:44
*** lamt has quit IRC23:44
*** sdague has quit IRC23:55

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!