| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-09-13 | |||
| 12:09:21 | stephenfin | I already did, but it's crashing and burning | |
| 12:09:25 | stephenfin | https://review.opendev.org/c/openstack/nova/+/860829/3 | |
| 12:11:10 | sean-k-mooney | that looks like the change you made is actully the problem | |
| 12:11:17 | sean-k-mooney | and the previous version was likely correct | |
| 12:11:17 | stephenfin | yup | |
| 12:11:43 | stephenfin | I did test with another reproducer locally but I must have missed something | |
| 12:12:21 | sean-k-mooney | well its actully failing in tempest too | |
| 12:12:32 | sean-k-mooney | so its not just a test artifact | |
| 12:13:16 | sean-k-mooney | sqlalchemy.exc.InvalidRequestError: One or more mappers failed to initialize - can't proceed with initialization of other mappers. Triggering mapper: 'mapped class BlockDeviceMapping->block_device_mapping'. Original exception was: Instance.block_device_mapping and back-reference BlockDeviceMapping.instance are both of the same direction symbol('MANYTOONE'). Did you mean to | |
| 12:13:18 | sean-k-mooney | set remote_side on the many-to-one side ? | |
| 12:14:06 | bauzas | that's why I want to be on par with the existing | |
| 12:14:33 | bauzas | I'm very afraid of any performance hit we may introduce if we wrongly do | |
| 12:14:44 | sean-k-mooney | it was workign before he added the extra changes | |
| 12:14:51 | bauzas | the problem here is that we don't know if the other-way relationship is needed | |
| 12:15:31 | sean-k-mooney | you mean even though it passses tempest, unit test, functional test adn api test | |
| 12:15:39 | sean-k-mooney | without this change | |
| 12:15:56 | sean-k-mooney | you still think we dont actuly have a stong enough indication that its not used | |
| 12:17:01 | sean-k-mooney | the revers relateion would be lookign up instance by a security group | |
| 12:17:27 | sean-k-mooney | https://docs.openstack.org/api-ref/compute/#list-security-groups-by-server | |
| 12:17:47 | sean-k-mooney | the proxy api only allows you to look up security groups of an instnace | |
| 12:18:04 | sean-k-mooney | so we have no public api for the reverse coralation | |
| 12:18:57 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Replace use of backref https://review.opendev.org/c/openstack/nova/+/860829 | |
| 12:18:57 | opendevreview | Stephen Finucane proposed openstack/nova master: objects: Stop fetching from security_groups table https://review.opendev.org/c/openstack/nova/+/860850 | |
| 12:18:58 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove unused relationships https://review.opendev.org/c/openstack/nova/+/894635 | |
| 12:21:18 | bauzas | sean-k-mooney: yeah, while it was difficult to find whether we use instance.bdm, I just guess that instance.securit_group is way less largely used :) | |
| 12:21:55 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/main/api.py#L2992-L3228 | |
| 12:22:23 | sean-k-mooney | looking at the code there is nothign that woudl be trying to look up instance by the security group | |
| 12:22:45 | sean-k-mooney | the closest thing is security_group_in_use | |
| 12:22:56 | sean-k-mooney | and that jsut directly uses the SecurityGroupInstanceAssociation table | |
| 12:25:00 | sean-k-mooney | bauzas: anyway looks like stephen removed foreign_keys=uuid, | |
| 12:25:07 | sean-k-mooney | so that presumabnle fixes it locally? | |
| 12:25:15 | stephenfin | yup | |
| 12:25:39 | sean-k-mooney | but honestly i woudl have been much happer merging the older version before we added this. i dont want stephen to respien to drop it | |
| 12:25:44 | stephenfin | shouldn't have been there since there's no foreign key column on that side (many-to-one) | |
| 12:25:45 | sean-k-mooney | but i dont like adding dead code | |
| 12:28:07 | bauzas | the code is already dead | |
| 12:28:46 | bauzas | stephenfin explained me that backref implicetely creates a return relationship | |
| 12:28:58 | bauzas | so we already have this, silently | |
| 12:29:21 | bauzas | some "code" may use this implicit relationship for getting the values | |
| 12:29:30 | bauzas | like, instance.bdm | |
| 12:29:53 | bauzas | fortunately, we have nova objects that are the DB facade | |
| 12:39:43 | auniyal | while writing a functional test for server having tags I am getting this error: https://paste.opendev.org/show/bM4cijMNqvGY70FuEDFG/ | |
| 12:39:55 | auniyal | I have set api_version = '2.43' | |
| 12:40:05 | auniyal | because its required in CLI | |
| 12:40:22 | auniyal | is there anything else I should look for ? | |
| 12:41:22 | auniyal | bauzas, sean-k-mooney ^ | |
| 12:49:11 | sean-k-mooney | its tags not tag | |
| 12:49:31 | sean-k-mooney | well there are two ways | |
| 12:49:38 | sean-k-mooney | what eactully are you executing | |
| 12:50:00 | sean-k-mooney | you can add a tag by doing a put to /servers/{server_id}/tags/{tag} | |
| 12:50:18 | sean-k-mooney | or you do a put to /servers/{server_id}/tags with {tags": ["tag1", "tag2"]} as the boday | |
| 12:51:29 | sean-k-mooney | https://docs.openstack.org/api-ref/compute/#server-tags-servers-tags | |
| 12:52:04 | sean-k-mooney | if your doing this as part of server create its also tags not tag | |
| 13:06:06 | auniyal | I am adding tag while creating server - https://paste.opendev.org/show/bJBibgysdZSdO5iMAZEg/ | |
| 13:06:28 | sean-k-mooney | that not adding a tag to a server | |
| 13:06:33 | sean-k-mooney | thats addign a tag to a network | |
| 13:06:43 | sean-k-mooney | its not the same thing | |
| 13:06:50 | auniyal | ack | |
| 13:07:03 | auniyal | so I am trying device tagging | |
| 13:07:22 | auniyal | once device is tagged, I'll verify it in metadata.json | |
| 13:07:31 | sean-k-mooney | per the api docs | |
| 13:07:33 | sean-k-mooney | A bug has caused the tag attribute to no longer be accepted starting with version 2.37. Therefore, network interfaces could only be tagged in versions 2.32 to 2.36 inclusively. Version 2.42 has restored the tag attribute. | |
| 13:08:04 | sean-k-mooney | so in the network you call it tag | |
| 13:08:15 | sean-k-mooney | and your using 2.43 | |
| 13:08:22 | sean-k-mooney | so that shoudl be fixed | |
| 13:08:24 | auniyal | yes, 2.43 | |
| 13:09:37 | sean-k-mooney | so https://paste.opendev.org/show/bE1IV21v1cdSzRKNWKrw/ | |
| 13:09:40 | sean-k-mooney | should be valid | |
| 13:09:48 | sean-k-mooney | just s/tags/tag/ | |
| 13:11:48 | sean-k-mooney | https://github.com/openstack/nova/commit/e80e2511cf825671a479053cc8d41463aab1caaa | |
| 13:11:52 | sean-k-mooney | that is the change that fixed it | |
| 13:12:05 | auniyal | this is full what I have right now - https://paste.opendev.org/show/bymbRPlBXG3nmY1Nm4zy/ | |
| 13:13:03 | sean-k-mooney | this is the api sample test that validates it https://github.com/openstack/nova/blob/master/nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-req.json.tpl | |
| 13:14:34 | sean-k-mooney | you do not need admin_api=ture by the way | |
| 13:15:05 | auniyal | ack | |
| 13:15:39 | sean-k-mooney | so i think the problem is | |
| 13:15:42 | sean-k-mooney | self.api = self.useFixture( | |
| 13:15:44 | sean-k-mooney | nova_fixtures.OSAPIFixture(api_version='v2.1')).api | |
| 13:16:02 | sean-k-mooney | you are seting api_version but i dont think that will work as you expect | |
| 13:17:47 | bauzas | sorry auniyal can't help, today is the last day before RC1 | |
| 13:18:03 | bauzas | IMHO you need to set the microversion correctly | |
| 13:18:13 | auniyal | so yeah, I tried this nova_fixtures.OSAPIFixture(api_version='v2.42')) and got https://paste.opendev.org/show/bHOTRcnwwpPb1lP8BbH5/ | |
| 13:18:31 | bauzas | that's not how you set the microversion | |
| 13:18:34 | sean-k-mooney | ya so when we set version in teh class like that | |
| 13:18:39 | bauzas | please look at other tests | |
| 13:18:42 | sean-k-mooney | we have explcitly coded the base class to use them | |
| 13:18:47 | bauzas | you have a class value | |
| 13:19:04 | sean-k-mooney | (test.TestCase, integrated_helpers.InstanceHelperMixin) wont make api_version = '2.42' | |
| 13:19:13 | sean-k-mooney | work at the class level | |
| 13:20:06 | sean-k-mooney | this is hte base your inheriting form | |
| 13:20:08 | sean-k-mooney | https://github.com/openstack/nova/blob/53012f1c55072c42ced267a2b1adef0a669d9f45/nova/test.py#L151 | |
| 13:23:08 | sean-k-mooney | the functionaltiy you are tyring to use si part of _IntegratedTestBase | |
| 13:23:10 | sean-k-mooney | https://github.com/openstack/nova/blob/53012f1c55072c42ced267a2b1adef0a669d9f45/nova/tests/functional/integrated_helpers.py#L1239 | |
| 13:24:02 | sean-k-mooney | it does https://github.com/openstack/nova/blob/53012f1c55072c42ced267a2b1adef0a669d9f45/nova/tests/functional/integrated_helpers.py#L1307-L1311 | |
| 13:24:26 | sean-k-mooney | if this is a regression test you shoudl just set the microversion on the fixture | |
| 13:24:49 | auniyal | microversion as latest | |
| 13:24:51 | sean-k-mooney | if this is for geneal testing then you proably shoudl be extendign the exitig test | |
| 13:25:14 | sean-k-mooney | https://github.com/openstack/nova/blob/53012f1c55072c42ced267a2b1adef0a669d9f45/nova/tests/functional/test_servers.py#L61C7-L61C18 | |
| 13:25:43 | sean-k-mooney | auniyal: microveion as lates means nothing of your parten classes dont read that class veraible | |
| 13:25:47 | sean-k-mooney | thats the point im making | |