17:00:35 <gibi> #startmeeting nova notification
17:00:36 <openstack> Meeting started Tue May  8 17:00:35 2018 UTC and is due to finish in 60 minutes.  The chair is gibi. Information about MeetBot at http://wiki.debian.org/MeetBot.
17:00:37 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
17:00:39 <openstack> The meeting name has been set to 'nova_notification'
17:00:54 <gibi> let's have a quick one
17:00:55 <mriedem> o/
17:01:19 <gibi> I have two things on my list
17:01:38 <gibi> #link https://review.openstack.org/#/c/563269 Add notification support for trusted_certs
17:02:05 <gibi> it seemed problematic last week as the trusted_certs field is lazy loaded
17:02:35 <gibi> but I think we agreed with the author to add this only to the instance.create and instance.rebuild notification where as the cert cannot change elsewhere
17:02:54 <gibi> but I wanted to mention here so you can disagree :)
17:03:07 <mriedem> hmm,
17:03:15 <mriedem> well, alternatively we add a config option like we have for bdms
17:03:37 <mriedem> because otherwise the REST API returns something that isn't in the instance payloads
17:03:45 <mriedem> for everything but instance create and rebuild
17:03:45 <gibi> yeah I listed the alternatives here https://review.openstack.org/#/c/563269/6/nova/notifications/objects/instance.py@122
17:04:34 <gibi> we have precedent for each alternative
17:04:54 <gibi> bdms uses config option, keypair only added to instance.create
17:05:08 <mriedem> i just left a comment,
17:05:17 <mriedem> microversion 2.54 allows changing keypair on instance rebuild
17:05:29 <mriedem> so we're missing something for that, i didn't know we only sent keypairs during instance.create notifications
17:05:52 <mriedem> for trusted_certs, i'd follow the bdm config option route
17:06:10 <mriedem> since anyone using that is going to have to configure the system differently anyway
17:06:14 <gibi> I can dig up how we reasoning if the keypair solution from past reviews
17:07:48 <gibi> that was the keypair patch #link https://review.openstack.org/#/c/463001/
17:08:03 <gibi> based on the commit message tags are also handled like keypair
17:09:08 <gibi> anyhow if you feel that the config way is more appropriate in this case then I'm not against that
17:09:41 <mriedem> i don't care too much either way
17:09:52 <mriedem> but i think the rebuid notification is missing the new keypair now, which is a bug
17:09:57 <mriedem> for microversion 2.54
17:10:15 <gibi> mriedem: I agree on the keypair rebuild bug
17:10:23 <gibi> I can file the bug report and push a quick fix tomorrow
17:10:52 <gibi> ahh it wont be a quick one
17:11:11 <gibi> the rebuild notification needs a payload type to incorporate the keypair
17:11:23 <gibi> similarly as proposed in the trusted cert patch
17:11:53 <mriedem> looking at "instance.rebuild.end" it has a key_name field
17:12:00 <mriedem> https://docs.openstack.org/nova/latest/reference/notifications.html#versioned-notification-samples
17:13:07 <gibi> I see. Then the difference is that every instance action has a key_name in the payload but the instance.create has the full keypair object in it
17:13:46 <mriedem> right
17:14:18 <gibi> so keypair is different from trusted_cert
17:14:20 <mriedem> yup i see the KeypairPayload in the instance.create.start notification
17:15:00 <gibi> this means that we dont have a bug in 2.54
17:15:06 <mriedem> well,
17:15:09 <gibi> I assume the key_name field will be correct
17:15:13 <mriedem> it is,
17:15:34 <mriedem> but if instance.create.* needed a KeypairPayload field, presumably that same full KeypairPayload should be in instance.rebuild if we rebuild with a new keypair
17:16:12 <gibi> OK, I rephrase my statement. We have a smaller bug in 2.54
17:16:24 <mriedem> i'm not sure why we have a full keypairs field for instance.create in the first place
17:16:33 <mriedem> yes agree it's low priority
17:17:29 <mriedem> anyway, what was your other thing to discuss?
17:17:31 <gibi> the keypair addition was part of https://blueprints.launchpad.net/nova/+spec/additional-notification-fields-for-searchlight so I guess it was requested by searchlight
17:17:38 <mriedem> oh
17:17:55 <mriedem> "The list of KeyPair objects containing at least the
17:17:55 <mriedem> name field of the original KeyPair objects based on instance.keypairs"
17:18:04 <mriedem> so key_name would have already satisfied that
17:18:22 <mriedem> so let's just forget it for rebuild unless someone ever asks
17:19:13 * gibi throws away the post it containing the task for that bug report
17:19:51 <gibi> going back to trusted_cert, is it OK to have that in instance.create and rebuild only?
17:20:05 <gibi> or we would like to transition towards a config option?
17:20:56 <mriedem> doesn't really matter that much to me,
17:21:07 <gibi> OK, then let's keep it as is
17:21:13 <mriedem> i don't like the inconsistency, but i guess it's being consistent with one of the inconsistencies
17:21:43 <gibi> yeah, we created a bit less inconsistent notification interface than the legacy was but it is still not perfect
17:21:50 <gibi> anyhow the second item on my list
17:22:07 <gibi> #link https://review.openstack.org/#/c/554212 Add PENDING vm state
17:22:26 <gibi> it proposes a new notification but I think the goal can be achived with the existing versioned instance.update
17:22:45 <gibi> so I'm -1 on the current proposal https://review.openstack.org/#/c/554212/4/specs/rocky/approved/introduce-pending-vm-state.rst@87
17:23:48 <gibi> the PoC code proposed a lot of new fields like cell mapping but I feel they only need the information that the server ended up in PENDING state
17:23:52 <mriedem> "nova_object.data":{                  "old_state":"building",                "state":"pending",                "old_task_state":"scheduling",                "new_task_state":null             },
17:23:57 <mriedem> sure, lgtm
17:24:07 <gibi> OK, thanks
17:24:28 <gibi> that was all from me, do you have something to discuss?
17:24:54 <mriedem> nope
17:25:05 <gibi> then let's close this. Thanks for the meeting
17:25:09 <gibi> #endmeeting