| *** bhagyashris is now known as bhagyashris|off | 05:35 | |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Xena RC-1 release notes https://review.opendev.org/c/openstack/glance/+/809129 | 07:39 |
|---|---|---|
| opendevreview | jinyuanliu proposed openstack/python-glanceclient master: Clean up extra spaces https://review.opendev.org/c/openstack/python-glanceclient/+/809155 | 08:33 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Add protection testing for metadef namespaces https://review.opendev.org/c/openstack/glance-tempest-plugin/+/800902 | 11:11 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Add protection testing for metadef namespaces https://review.opendev.org/c/openstack/glance-tempest-plugin/+/800902 | 11:58 |
| abhishekk | RC1 reno patch up for review, https://review.opendev.org/c/openstack/glance/+/809129 | 13:42 |
| * dansmith is reviewing already | 13:42 | |
| abhishekk | cool, thank you | 13:42 |
| dansmith | left you a question | 13:43 |
| abhishekk | ack, ++ | 13:44 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Xena RC-1 release notes https://review.opendev.org/c/openstack/glance/+/809129 | 13:47 |
| abhishekk | I think we are good for Xena now | 13:54 |
| abhishekk | except for metadef tempest-plugin patches, I will give ack to release of tempest-plugin patch and then we can backport it to stable branch | 13:55 |
| dansmith | oh we have to release the tempest plugin? | 13:56 |
| dansmith | I had assumed not | 13:57 |
| abhishekk | hmm, we need to | 13:57 |
| dansmith | okay | 13:58 |
| abhishekk | wait, we don't do releases for tempest-plugin, you are right | 13:59 |
| abhishekk | gmann, I think we don't create stable branch but do the release, right ? | 14:04 |
| abhishekk | croelandt, last review request for this cycle, https://review.opendev.org/c/openstack/glance/+/809129 | 14:05 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Implement API protection testing for metadef objects https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802793 | 14:10 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Implement API protection testing for metadef resource types https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802792 | 14:10 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Implement API protection testing for metadef properties https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802794 | 14:10 |
| opendevreview | Pranali Deore proposed openstack/glance-tempest-plugin master: Implement API protection testing for metadef tags https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802795 | 14:10 |
| gmann | abhishekk: we do not create stable branch but plugins needs to release a tag for cycle support/end-of-support etc | 14:10 |
| abhishekk | gmann, ack, so last date for release is 17th ? | 14:11 |
| gmann | abhishekk: https://docs.openstack.org/tempest/latest/tempest_and_plugins_compatible_version_policy.html | 14:11 |
| gmann | abhishekk: as they are with cycle-with-intermediary release model I think 13th was deadline? but you can check in release channel I think other plugins are also not tagged yet (tempest tag is also not merged in release side) | 14:12 |
| croelandt | abhishekk: is it just me, or does this one seem easier than the previous ones? :) | 14:13 |
| abhishekk | croelandt, :D | 14:13 |
| abhishekk | gmann, https://review.opendev.org/c/openstack/releases/+/808782 | 14:14 |
| abhishekk | this commit mentions 17 as a dead line | 14:14 |
| gmann | abhishekk: perfect | 14:14 |
| abhishekk | ok, that means we have couple of days to work on pending changes and get those merged | 14:15 |
| gmann | abhishekk: yeah, 17th is in case PTL does not respond on release patches. | 14:15 |
| abhishekk | so is there a backdoor to push it further > | 14:15 |
| gmann | abhishekk: if anything very urgent then we can wait but as things are tested with plugins master branch so there is no missing testing for xena cycle things. | 14:15 |
| abhishekk | ack | 14:16 |
| abhishekk | sounds good then | 14:16 |
| croelandt | Is there any reason why os_hash_algo and os_hash_value may not be set on an image? Is there an option to disable the multihash feature or something? | 14:17 |
| gmann | abhishekk: main idea behind release wise tag for tempest & plugins was to have a known compatible tags between tempest and plugins so that any production cloud can install them easily. but as you know tempest and plugins master support many stable branch so other version also works fine as we test in upstream too | 14:17 |
| gmann | abhishekk: so if you release now or after few patches that is no so much difference as such | 14:17 |
| abhishekk | yep, got it | 14:18 |
| abhishekk | will wait for today or else give the ack on the patch | 14:18 |
| abhishekk | thank you for explaining in detail | 14:18 |
| abhishekk | croelandt, I think those are set automatically | 14:18 |
| croelandt | I got a user downstream who has no os_hash_algo nor os_hash_value on his images | 14:19 |
| croelandt | I cannot reproduce it, a Horizon dev tried to reproduce it, no luck | 14:20 |
| croelandt | This seems to happen only when they create a snapshot of an instance | 14:20 |
| croelandt | it's driving me crazy | 14:20 |
| abhishekk | ack, brb | 14:20 |
| dansmith | maybe direct snapshot? | 14:21 |
| dansmith | i.e. nova creates the snapshot in ceph directly and just tells glance about it? | 14:21 |
| croelandt | oh that is a thing? | 14:22 |
| gmann | pdeore: abhishekk dansmith lbragstad on rbac test. one suggestion. I think making create_namespace() taking client object or project_id for which namespace needs to be created will be easy code to read/maintain and test can pass their required client, project_id they want to test on - https://review.opendev.org/c/openstack/glance-tempest-plugin/+/800902/27/glance_tempest_plugin/tests/rbac/v2/base.py#91 | 14:22 |
| dansmith | croelandt: kindof an important one :) | 14:22 |
| croelandt | dansmith: so Nova creates the snapshot, and then creates a "pointer" in Glance to the rbd location? | 14:22 |
| gmann | I think that is what lbragstad points on his comment ? | 14:23 |
| dansmith | croelandt: location yeah | 14:23 |
| dansmith | gmann: I dunno, it only ever needs to use the namespace client to do that | 14:23 |
| gmann | dansmith: client i mean admin, system, reader etc | 14:24 |
| dansmith | gmann: I think the thing that would help clean all that up is not keep re-writing self.client to be "the one client I'm mostly testing here" and just create them all like self.images_client, self.namespace_client, self.objects_client, etc | 14:24 |
| croelandt | dansmith: so in that case we don't hash the data? | 14:24 |
| dansmith | croelandt: wouldn't surprise me.. we have t read it all in order to calculate the hash right? and in those cases, we never do (I assume) | 14:24 |
| gmann | dansmith: yeah that is true, self.client are bad things and many in tempest too. if we want to test what all persona can create/access namespace then passing that persona in create_namespace() is easy. | 14:25 |
| dansmith | gmann: I also think the getattr(cls, f'os_{cls.credentials[0]}') pattern is pretty dangerous | 14:25 |
| dansmith | gmann: because I changed the ordering of cls.credentials at one point and that broke things and I couldn't figure out why | 14:25 |
| gmann | yeah that is why I had hard time to understand the things in those tests | 14:26 |
| lbragstad | yeah - that's obtuse | 14:26 |
| dansmith | I dunno if lbragstad copied that from someone, but I know he *loves* f-strings | 14:26 |
| dansmith | but in that case, it seems pretty not great to me | 14:26 |
| lbragstad | yeah - i used the keystone tests as a template | 14:27 |
| lbragstad | and i kept it consistent for the time being | 14:27 |
| dansmith | alright, you can only keep using that excuse so many times :D | 14:27 |
| gmann | :) | 14:27 |
| dansmith | but yeah, especially in these metadef cases, | 14:27 |
| lbragstad | f-strings aside, the ordering part is probably the more fragile part, yeah? | 14:27 |
| dansmith | where we have a bunch of clients all kinda stacked up, and lots of creds, I just think namespacing them and making them explicit would be better | 14:28 |
| dansmith | lbragstad: yeah it's just the creds[0] thing | 14:28 |
| lbragstad | right - ok | 14:28 |
| dansmith | lbragstad: f-strings make that less likely for a reader to realize there's code in that string, which is why I hate them, but yeah | 14:28 |
| gmann | +1. explicit is much correct/easy. | 14:28 |
| dansmith | normally I would kinda skim over strings when looking for conditional code, and ... f-strings break that | 14:29 |
| * dansmith stops ranting about f-strings | 14:29 | |
| lbragstad | yeah - some sort of method that iterates the credentials and exposes them without an implied order enforced is a good idea | 14:29 |
| lbragstad | i feel like that should be in tempest proper | 14:29 |
| dansmith | yeah, maybe self.${cred}.${thing}_client pattern would be nice | 14:30 |
| gmann | lbragstad: you mean self.os_system_admin etc ? | 14:30 |
| lbragstad | well - guess we could just do that | 14:31 |
| lbragstad | and not alias anything, right? | 14:31 |
| gmann | that is we have in tempest. self.system_admin|reader.<x_client> | 14:31 |
| lbragstad | right now we do things like, | 14:31 |
| lbragstad | self.admin = self.os_system_admin.namespaces_client | 14:32 |
| gmann | yeah alias are mistake in tempest and as they were so many we never got rid of those completely especially self.client | 14:32 |
| lbragstad | hmm | 14:32 |
| lbragstad | ok | 14:32 |
| dansmith | gmann: ++ | 14:32 |
| lbragstad | well - if we don't think aliasing and shortening the names is important, then we could just define the credentials list and we don't have to rely on an order in setup_clients() | 14:32 |
| lbragstad | and it removes the "wait, what is self.client() again?" | 14:33 |
| lbragstad | when you're reading the tests | 14:33 |
| dansmith | right | 14:33 |
| gmann | yes | 14:33 |
| dansmith | I do like short names, no doubt, | 14:33 |
| dansmith | but it would also be easy to, at the top of a test, do "client = self.admin.namespaces_client" | 14:33 |
| gmann | reading test is more imp i think as we donot have great test docuemntation | 14:33 |
| gmann | at least within that test file | 14:33 |
| lbragstad | same, but i'm fine reading a bit more if it means the tests are more robust | 14:34 |
| gmann | and if we have in test file then it is easy to know test requested those creds and using them in their tests | 14:35 |
| gmann | *same test file | 14:35 |
| croelandt | dansmith: so the whole point of direct snapshots is to be quicker, which explains why it would not make sense to compute the hash? | 14:48 |
| dansmith | well, the whole point of direct snapshots is to avoid sending the bits from ceph->nova->glance->ceph when ceph can just CoW the disk and return a new reference (or whatever) | 14:49 |
| dansmith | but the side-effect is that we don't actually handle the bits, we just handle the reference | 14:49 |
| dansmith | and as such, I'm not sure how we *could* know the hash, unless ceph was able to tell us (and I expect it doesn't/cant') | 14:50 |
| croelandt | ok | 14:52 |
| croelandt | so, fun fact | 14:52 |
| croelandt | If you want to edit an image that does not have os_hash_* set, you can using the CLI | 14:52 |
| croelandt | but Horizon is like "NOPE" | 14:53 |
| croelandt | Then people ask me why I hate GUI programs, and why I don't like the Web | 14:53 |
| dansmith | "edit" meaning what? just update some other properties? | 14:54 |
| dansmith | I'm guessing that's either a bug, or an ill-placed safeguard on horizon's part | 14:54 |
| croelandt | yeah I think they're trying to edit some properties | 14:55 |
| croelandt | and the "edit" button is greyed out | 14:55 |
| croelandt | I'll check with them that it is possible to do itthrough the CLI | 14:55 |
| croelandt | but yeah it feels like it's a check from Horizon, something like "if any of the properties is NULL, then something must be wrong" | 14:56 |
| dansmith | yeah | 14:57 |
| * abhishekk scrolling back | 14:59 | |
| abhishekk | so it looks like glance-tempest-plugin needs some more refactoring | 15:01 |
| opendevreview | Merged openstack/glance master: Xena RC-1 release notes https://review.opendev.org/c/openstack/glance/+/809129 | 17:23 |
| abhishekk | RC1 release patch - https://review.opendev.org/c/openstack/releases/+/808625 | 17:43 |
| * abhishekk signing out for the day | 18:15 | |
| *** timburke_ is now known as timburke | 20:59 | |
| *** prometheanfire is now known as Guest1 | 21:20 | |
| *** promethe- is now known as prometheanfire | 21:53 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!