| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-12 | |||
| 08:35:37 | openstackgerrit | XiaojueGuan proposed openstack/nova master: Update links in README https://review.openstack.org/574644 | |
| 08:52:11 | openstackgerrit | Zhenyu Zheng proposed openstack/nova master: WIP Add full traceback to ExceptionPayload in versioned notifications https://review.openstack.org/564092 | |
| 09:14:58 | Kevin_Zheng | ping gibi | |
| 09:21:27 | gibi | Kevin_Zheng: pong | |
| 09:21:49 | Kevin_Zheng | Hi, thanks for the advise on exception payload patch | |
| 09:22:12 | Kevin_Zheng | the notification_sample_base.NotificationSampleTestBase.ANY is good | |
| 09:22:54 | Kevin_Zheng | but the sample file still seems hard to implement | |
| 09:23:24 | Kevin_Zheng | a typical traceback is at at least 10 lines | |
| 09:23:51 | gibi | Kevin_Zheng: do you mean it will be ugly in the sample file? | |
| 09:23:54 | Kevin_Zheng | it will be review-able to put it in the json file | |
| 09:23:57 | Kevin_Zheng | yeah | |
| 09:24:10 | Kevin_Zheng | I didn't update the sample yet | |
| 09:24:12 | Kevin_Zheng | just local | |
| 09:24:26 | Kevin_Zheng | I'm going to fix all the samples in next patch | |
| 09:24:30 | Kevin_Zheng | patchset | |
| 09:24:58 | gibi | Kevin_Zheng: does the uglyness came from the fact that a traceback is naturally a multiline string and it is now force into a single line string in json | |
| 09:25:01 | gibi | ? | |
| 09:25:33 | Kevin_Zheng | yeah, and I cannot find a good way to make it multiline | |
| 09:25:35 | Kevin_Zheng | in json | |
| 09:42:44 | gibi | Kevin_Zheng: sorry got distracted locally | |
| 09:43:38 | gibi | Kevin_Zheng: could you just add the beginning of a traceback maybe the first line including the \n at the end then '...' to signal to the reader the there is more | |
| 09:43:53 | Kevin_Zheng | yeah, that might work | |
| 09:44:07 | Kevin_Zheng | I will add them tonight | |
| 09:44:30 | gibi | Kevin_Zheng: thanks a lot! | |
| 09:46:04 | ratailor | bauzas, Do you think, this comment is valid on https://review.openstack.org/#/c/401009/11/nova/compute/api.py | |
| 09:56:35 | lyarwood | lyarwood | |
| 09:56:37 | lyarwood | argh | |
| 10:24:31 | openstackgerrit | garyk proposed openstack/nova master: VMware: populate datastore refs at init https://review.openstack.org/574688 | |
| 10:57:27 | openstackgerrit | Chris Dent proposed openstack/nova master: Move db using provider unit tests to functional https://review.openstack.org/572280 | |
| 10:57:28 | openstackgerrit | Chris Dent proposed openstack/nova master: Adjust db using allocation unit tests https://review.openstack.org/572281 | |
| 10:57:29 | openstackgerrit | Chris Dent proposed openstack/nova master: Remove nova dependencies from test_resource_provider https://review.openstack.org/572282 | |
| 11:03:41 | openstackgerrit | jiang wei proposed openstack/nova master: Add action initiator attribute to the payload https://review.openstack.org/536243 | |
| 11:21:55 | openstackgerrit | Merged openstack/nova stable/queens: Change consecutive build failure limit to a weigher https://review.openstack.org/573239 | |
| 11:36:49 | openstackgerrit | Chris Dent proposed openstack/nova master: Provide a direct interface to placement https://review.openstack.org/572576 | |
| 11:46:09 | openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Add microversion for nested allocation candidate https://review.openstack.org/565487 | |
| 11:46:10 | openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Fix nits in placement-return-all-resources series https://review.openstack.org/573604 | |
| 12:04:01 | Shilpa | stephenfin: Hi | |
| 12:25:21 | openstackgerrit | Chris Dent proposed openstack/nova master: Ensure that os-traits sync is attempted only at start of process https://review.openstack.org/553857 | |
| 13:00:00 | openstackgerrit | Brianna Poulos proposed openstack/nova master: Plumb trusted_certs through libvirt driver image paths https://review.openstack.org/561262 | |
| 13:04:06 | openstackgerrit | Bhagyashri Shewale proposed openstack/nova master: libvirt: Don't report DISK_GB if sharing https://review.openstack.org/560459 | |
| 13:36:51 | mriedem | bauzas: looks like you and i were reviewing https://review.openstack.org/#/c/479949/ at the same time | |
| 13:36:57 | bauzas | mriedem: correct | |
| 13:37:03 | mriedem | the answer to some of your questions is, it's later in the series, | |
| 13:37:10 | bauzas | I had this tab open for a while | |
| 13:37:13 | mriedem | but i don't exactly know how enable_certificate_validation is meant to be used | |
| 13:37:34 | bauzas | well, tbh, I think we need to understand how to upgrade | |
| 13:37:56 | mriedem | brianna is sometimes in irc, but don't see her now | |
| 13:37:59 | bauzas | if folks want to stop using the opt, fine with me, but then, you at least need a nova-status check and a upgrade note | |
| 13:38:22 | bauzas | AFAICU, the opt was introduced as a temporary way | |
| 13:38:33 | bauzas | hence it being deprecated from the inception | |
| 13:38:46 | bauzas | so I understand why they want to raise an exception | |
| 13:39:06 | bauzas | but we at least need to think about operators already using it | |
| 13:39:11 | mriedem | i guess i need to understand https://review.openstack.org/#/c/457678/ | |
| 13:39:27 | bauzas | right, I was on https://github.com/openstack/nova/commit/05bf7269 | |
| 13:39:53 | bauzas | (I'm glad I'm done with other things so I can be back reviewing and participating \o/ ) | |
| 13:40:31 | mriedem | what i don't understand is this part from the config option help: | |
| 13:40:32 | mriedem | "During image signature verification nova will first verify the validity of the | |
| 13:40:32 | mriedem | image's signing certificate using the set of trusted certificates associated | |
| 13:40:32 | mriedem | with the instance." | |
| 13:40:43 | mriedem | at this point in the series, the instance doesn't have trusted certs | |
| 13:40:46 | mriedem | that comes later in the api change | |
| 13:41:19 | bauzas | yeah I saw | |
| 13:41:45 | bauzas | mriedem: in https://github.com/openstack/nova/commit/05bf7269 L113, they explain what they want | |
| 13:42:07 | mriedem | right that's what i asked about here https://review.openstack.org/#/c/479949/76/nova/image/glance.py@385 | |
| 13:42:42 | mriedem | this is the API change https://review.openstack.org/#/c/486204/109/nova/compute/api.py@970 | |
| 13:42:54 | mriedem | elif (CONF.glance.verify_glance_signatures and | |
| 13:42:54 | mriedem | CONF.glance.enable_certificate_validation and | |
| 13:42:54 | mriedem | CONF.glance.default_trusted_certificate_ids): | |
| 13:42:54 | mriedem | certs_to_return = objects.TrustedCerts( | |
| 13:42:54 | mriedem | ids=CONF.glance.default_trusted_certificate_ids) | |
| 13:43:15 | mriedem | ^ is essentially this glance download() logic except it doesn't use default_trusted_certificate_ids | |
| 13:45:01 | bauzas | mriedem: mmm, I don't like the API magically guessing which certs IDs to use | |
| 13:46:09 | bauzas | mriedem: so, IIUC, we store the certs in the instance ? | |
| 13:46:37 | bauzas | mriedem: and then, later on, the virt driver passes the certs to the glance API | |
| 13:47:03 | mriedem | if the user provides trusted cert IDs, we use those, | |
| 13:47:31 | mriedem | if the user does not provide trusted cert IDs, but image sig verification is enabled, and default_trusted_certificate_ids is set, we use those | |
| 13:47:34 | bauzas | oh, and if it doesn't but nova is configured, we store the default ones | |
| 13:47:37 | mriedem | yes they are persisted with the instance | |
| 13:47:45 | mriedem | by default, there are no default ones, but yes | |
| 13:47:59 | bauzas | ok, so, follow my logic about why I don't like us persisting the defaults | |
| 13:48:19 | bauzas | say I'm an operator and I did set some default certs | |
| 13:48:36 | bauzas | some users went by, and some instances now persisted my default certs | |
| 13:48:40 | bauzas | that's awesome | |
| 13:49:02 | bauzas | but then, I need to remove those default certs because of any security concern | |
| 13:49:10 | bauzas | revoke it, if you prefer | |
| 13:49:14 | bauzas | but then I'm stuck | |
| 13:49:27 | mriedem | you can rebuild those instances | |
| 13:49:41 | bauzas | because I don't know whether the cert is in the instance was persisted because the user asked for it, or because we defaulted it | |
| 13:50:04 | mriedem | well, they are IDs | |
| 13:50:06 | bauzas | of course, yeah you can rebuild | |
| 13:50:20 | mriedem | so if you were looking at the db, then you know which cert IDs were your defaults that you're revoking | |
| 13:50:41 | bauzas | but from a guessing logic, I have no idea whether my IDs that are in the instance are those given by the user or by a very old configuration | |
| 13:51:24 | mriedem | i'm not sure how much this matters, | |
| 13:51:49 | mriedem | if the instance is storing cert IDs for defaults that you've revoked, those cert IDs aren't getting used - they were used when spawning the instance | |
| 13:52:10 | mriedem | they would fail if you tried to rebuild the instance w/o providing new trusted certs | |
| 13:52:18 | sahid | dansmith, jaypipes, you might have already think about that but... what about to add a 'version' attribute for our objects fields | |
| 13:52:27 | mriedem | or rebuilding and specifying trusted_certs=None to get the new defaults i guess | |
| 13:52:33 | sahid | something like fields.IntegerField(version=2.0, nullable=True) | |
| 13:52:55 | sahid | so we could remove that obj_make_compatible method from all of our objects | |
| 13:52:58 | mriedem | sahid: there is already a version on the object | |