c2a831c519
Patch Set 9: Code-Review-1
(8 comments)
Patch-set: 9
Reviewer: Gerrit User 11655 <11655@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1, 804379f05e4a4fbc74132966c3ed6209529588b3
Attention: {"person_ident":"Gerrit User 11655 \u003c11655@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_11655\u003e replied on the change"}
158 lines
8.4 KiB
Plaintext
158 lines
8.4 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "fcc9d2f8_6a5d83fb",
|
|
"filename": "ironic_tempest_plugin/tests/api/admin/test_allocations.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 234,
|
|
"author": {
|
|
"id": 11655
|
|
},
|
|
"writtenOn": "2024-04-03T20:34:19Z",
|
|
"side": 1,
|
|
"message": "So what is happening, and causing these tests to fail, is the tests by default attempt to set a version for the request and include some extra fields.\n\nFor example, this can be found in the test output (https://b82af873f398e35b9667-d46fd59902d68f923f6bb91c5de228d0.ssl.cf2.rackcdn.com/913926/9/check/ironic-tempest-functional-python3/949d14e/testr_results.html)\n\n \u0027x-openstack-ironic-api-version\u0027: \u00271.1\u0027\n \n Which is the client saying \"I\u0027m using this version\"\n \n Except, if you look at the body we sent, you see \"resource_class\" which was added in a much later version of ironic!\n \n Body: {\"resource_class\": \"031e4d60-c510-4a46-983d-b7e92c2ad36c\", \"chassis_uuid\": \"345de938-bfa7-49eb-9810-66a8126512fb\", \"properties\": {\"cpu_arch\": \"x86_64\", \"cpus\": 8, \"local_gb\": 10, \"memory_mb\": 4096}, \"driver\": \"fake-hardware\"}\n \nAnd got back:\n\n Body: b\u0027{\"error_message\": \"{\\\\\"faultcode\\\\\": \\\\\"Client\\\\\", \\\\\"faultstring\\\\\": \\\\\"Request not acceptable.\\\\\", \\\\\"debuginfo\\\\\": null}\"}\u0027\n \nThe *huge* hint:\n\nhttps://b82af873f398e35b9667-d46fd59902d68f923f6bb91c5de228d0.ssl.cf2.rackcdn.com/913926/9/check/ironic-tempest-functional-python3/949d14e/controller/logs/screen-ir-api.txt has the following:\n\nMar 30 09:40:50.606084 np0037197416 devstack@ir-api.service[49476]: [pid: 49476|app: 0|req: 252/252] 173.231.255.75 () {66 vars in 1257 bytes} [Sat Mar 30 09:40:50 2024] POST /baremetal/v1/chassis \u003d\u003e generated 655 bytes in 152 msecs (HTTP/1.1 201) 8 headers in 380 bytes (1 switches on core 0)\nMar 30 09:40:50.641139 np0037197416 devstack@ir-api.service[49476]: DEBUG ironic.api.controllers.v1.node [None req-2344ff07-bbd8-4b2d-96fb-5da41e3aa885 None tempest-TestAllocationsWithJsonExtSupport-1289873436-system-admin] Field resource_class is not acceptable in version 1.1 {{(pid\u003d49476) reject_fields_in_newer_versions /opt/stack/ironic/ironic/api/controllers/v1/node.py:378}}\nMar 30 09:40:50.641800 np0037197416 devstack@ir-api.service[49476]: DEBUG ironic.api.method [None req-2344ff07-bbd8-4b2d-96fb-5da41e3aa885 None tempest-TestAllocationsWithJsonExtSupport-1289873436-system-admin] Client-side error: Request not acceptable. {{(pid\u003d49476) format_exception /opt/stack/ironic/ironic/api/method.py:124}}\n\nWhere things go sideways for the other tests is we\u0027re attempting calls still with 1.1 since I guess the class maximum api version is short circuiting some of the version logic.",
|
|
"revId": "05da8a8055b8dddeeca2228097558121c2cce8f5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "89efc6f7_db11b917",
|
|
"filename": "ironic_tempest_plugin/tests/api/admin/test_allocations.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 280,
|
|
"author": {
|
|
"id": 11655
|
|
},
|
|
"writtenOn": "2024-04-03T20:34:19Z",
|
|
"side": 1,
|
|
"message": "you likely want request_microversion here, as well.",
|
|
"revId": "05da8a8055b8dddeeca2228097558121c2cce8f5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "25e9577e_0f7d0ae3",
|
|
"filename": "ironic_tempest_plugin/tests/api/admin/test_nodes.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 189,
|
|
"author": {
|
|
"id": 11655
|
|
},
|
|
"writtenOn": "2024-04-03T20:34:19Z",
|
|
"side": 1,
|
|
"message": "this knob only impacts skip checking, I think you need to set request_microversion here, and on other tests.",
|
|
"range": {
|
|
"startLine": 189,
|
|
"startChar": 4,
|
|
"endLine": 189,
|
|
"endChar": 31
|
|
},
|
|
"revId": "05da8a8055b8dddeeca2228097558121c2cce8f5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "0376fd11_aa9115ba",
|
|
"filename": "ironic_tempest_plugin/tests/api/admin/test_nodes.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 233,
|
|
"author": {
|
|
"id": 11655
|
|
},
|
|
"writtenOn": "2024-04-03T20:34:19Z",
|
|
"side": 1,
|
|
"message": "What is happening here, is your getting a 404 back, so the check is actually wrong\n\n Response - Headers: {\u0027date\u0027: \u0027Sat, 30 Mar 2024 09:42:50 GMT\u0027, \u0027server\u0027: \u0027Apache/2.4.52 (Ubuntu)\u0027, \u0027x-openstack-ironic-api-minimum-version\u0027: \u00271.1\u0027, \u0027x-openstack-ironic-api-maximum-version\u0027: \u00271.91\u0027, \u0027x-openstack-ironic-api-version\u0027: \u00271.1\u0027, \u0027openstack-request-id\u0027: \u0027req-0a18df93-a413-487d-b649-b3bd6e640fa4\u0027, \u0027content-type\u0027: \u0027application/json\u0027, \u0027content-length\u0027: \u0027158\u0027, \u0027connection\u0027: \u0027close\u0027, \u0027status\u0027: \u0027404\u0027, \u0027content-location\u0027: \u0027https://173.231.255.75/baremetal/v1/nodes/3fc3def1-fc5e-4b3d-a1de-859e4b9459c5.json\u0027}\n Body: b\u0027{\"error_message\": \"{\\\\\"faultcode\\\\\": \\\\\"Client\\\\\", \\\\\"faultstring\\\\\": \\\\\"Node 3fc3def1-fc5e-4b3d-a1de-859e4b9459c5.json could not be found.\\\\\", \\\\\"debuginfo\\\\\": null}\"}\u0027\n}}}\n\n\nThe important thing is version 1.1 is being requested, which is wrong.",
|
|
"range": {
|
|
"startLine": 229,
|
|
"startChar": 0,
|
|
"endLine": 233,
|
|
"endChar": 0
|
|
},
|
|
"revId": "05da8a8055b8dddeeca2228097558121c2cce8f5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "a9b6a8d4_a5ae7923",
|
|
"filename": "ironic_tempest_plugin/tests/api/admin/test_nodes.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 263,
|
|
"author": {
|
|
"id": 11655
|
|
},
|
|
"writtenOn": "2024-04-03T20:34:19Z",
|
|
"side": 1,
|
|
"message": "Traceback (most recent call last):\n File \"/opt/stack/tempest/.tox/tempest/lib/python3.10/site-packages/ironic_tempest_plugin/tests/api/admin/test_nodes.py\", line 263, in test_update_node_with_json\n self.assertRaisese(lib_exc.NotFound, self.client.update_node,\nAttributeError: \u0027TestNodesWithoutJsonExtSupport\u0027 object has no attribute \u0027assertRaisese\u0027\n\nIn other word, self.assertRaises(...",
|
|
"revId": "05da8a8055b8dddeeca2228097558121c2cce8f5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "3a208b65_eb5fb618",
|
|
"filename": "ironic_tempest_plugin/tests/api/admin/test_portgroups.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 164,
|
|
"author": {
|
|
"id": 11655
|
|
},
|
|
"writtenOn": "2024-04-03T20:34:19Z",
|
|
"side": 1,
|
|
"message": "you\u0027ll want to set request_microversion here as well.",
|
|
"revId": "05da8a8055b8dddeeca2228097558121c2cce8f5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "202bb06c_902562c8",
|
|
"filename": "ironic_tempest_plugin/tests/api/admin/test_portgroups.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 220,
|
|
"author": {
|
|
"id": 11655
|
|
},
|
|
"writtenOn": "2024-04-03T20:34:19Z",
|
|
"side": 1,
|
|
"message": "Also here.",
|
|
"revId": "05da8a8055b8dddeeca2228097558121c2cce8f5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "ff759bab_d771f85b",
|
|
"filename": "ironic_tempest_plugin/tests/api/admin/test_portgroups.py",
|
|
"patchSetId": 9
|
|
},
|
|
"lineNbr": 226,
|
|
"author": {
|
|
"id": 11655
|
|
},
|
|
"writtenOn": "2024-04-03T20:34:19Z",
|
|
"side": 1,
|
|
"message": "so setting the version *is* good, it helps, but I think you can do it on the class level without explicitly setting the min here.",
|
|
"range": {
|
|
"startLine": 224,
|
|
"startChar": 0,
|
|
"endLine": 226,
|
|
"endChar": 39
|
|
},
|
|
"revId": "05da8a8055b8dddeeca2228097558121c2cce8f5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
}
|
|
]
|
|
} |