338 lines
15 KiB
Plaintext
338 lines
15 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "0ec2f9d2_d13799e9",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 30742
|
|
},
|
|
"writtenOn": "2022-02-08T09:15:01Z",
|
|
"side": 1,
|
|
"message": "recheck",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "52a7286a_3a85cf70",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 30742
|
|
},
|
|
"writtenOn": "2022-02-09T10:54:17Z",
|
|
"side": 1,
|
|
"message": "recheck",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "882b1acc_6dbe1900",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 10459
|
|
},
|
|
"writtenOn": "2022-02-09T14:27:46Z",
|
|
"side": 1,
|
|
"message": "I suspect the failure in python-tempestconf-tempest-devstack-admin-train may be caused by the fact we are still using run-tempest instead of run-tempest-26 for that release, basically missing the changes introduced in tempest.git by If49ab0c31aca5b7837636727096a9bc83f891b1b (https://review.opendev.org/c/openstack/tempest/+/787455).\n\nSo that should be probably fixed (of course in a separate patch).",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "51133b43_f1dc6992",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 30742
|
|
},
|
|
"writtenOn": "2022-02-10T11:52:33Z",
|
|
"side": 1,
|
|
"message": "Thanks Luigi for pointing out the change. I will put a separate patch for the tempest change happened.",
|
|
"parentUuid": "882b1acc_6dbe1900",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "8e0cb044_b8111b1c",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 22873
|
|
},
|
|
"writtenOn": "2022-02-10T11:55:53Z",
|
|
"side": 1,
|
|
"message": "a good catch! thanks!\nhttps://review.opendev.org/c/openinfra/python-tempestconf/+/828680",
|
|
"parentUuid": "882b1acc_6dbe1900",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "68ffc367_18047376",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 10459
|
|
},
|
|
"writtenOn": "2022-02-10T22:59:35Z",
|
|
"side": 1,
|
|
"message": "recheck",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "142e7e75_b244ae85",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 22873
|
|
},
|
|
"writtenOn": "2022-02-11T13:08:48Z",
|
|
"side": 1,
|
|
"message": "lgtm, thanks",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "3910af9b_ab5b88b0",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 12393
|
|
},
|
|
"writtenOn": "2022-02-16T12:02:44Z",
|
|
"side": 1,
|
|
"message": "Based on Abhishek comment, looks good to me, Waiting for tosky final ack on this.",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "964fb824_f68c23d6",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 10459
|
|
},
|
|
"writtenOn": "2022-02-16T15:24:57Z",
|
|
"side": 1,
|
|
"message": "No, sorry, I still have a question.",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "06b781dc_332896f3",
|
|
"filename": "config_tempest/services/image.py",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 61,
|
|
"author": {
|
|
"id": 10459
|
|
},
|
|
"writtenOn": "2022-02-14T14:43:25Z",
|
|
"side": 1,
|
|
"message": "I\u0027m sorry, but I think I overlooked this.\nIf num_stores is 1, image-feature-enabled.import_image will be set to True.\nBut shouldn\u0027t this be enabled when the stores found are more than 1?",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "77e4ad26_71bdd9e8",
|
|
"filename": "config_tempest/services/image.py",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 61,
|
|
"author": {
|
|
"id": 9303
|
|
},
|
|
"writtenOn": "2022-02-14T14:59:57Z",
|
|
"side": 1,
|
|
"message": "Luigi, the plan is to remove single store configuration support from glance and with multistore config I can define single store like;\n\nenabled_backends \u003d \u0027fast\u0027: \u0027rbd\u0027\n\nSo to avoid future regression, I have suggested that we should use \u003e 0 here.\n\n(You can see detail explanation in previous comments)",
|
|
"parentUuid": "06b781dc_332896f3",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "e222bdb9_265c77fd",
|
|
"filename": "config_tempest/services/image.py",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 61,
|
|
"author": {
|
|
"id": 10459
|
|
},
|
|
"writtenOn": "2022-02-16T15:24:57Z",
|
|
"side": 1,
|
|
"message": "Sorry, it\u0027s not clear to me.\n\nDoes it mean that, when you have multistore enabled but you define a single store (as it will happen in the future when you remove the single store configuration support), the import_image feature still works? Does it mean that feature is really linked to the availability of \"multistore config\" and not the number of store?",
|
|
"parentUuid": "77e4ad26_71bdd9e8",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "09a137f8_14b36ed9",
|
|
"filename": "config_tempest/services/image.py",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 61,
|
|
"author": {
|
|
"id": 9303
|
|
},
|
|
"writtenOn": "2022-02-16T15:38:12Z",
|
|
"side": 1,
|
|
"message": "Right, I can define single store define using multistore config as shown in above comment.\n\nImport image is independent of store configuration, it will work whether you have single store or multiple stores. Only thing is if you have single store configured then you will not be able to use copy-image import plugin which requires more than one store configured.\n\nLet me know if you have any further queries around it.",
|
|
"parentUuid": "e222bdb9_265c77fd",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "5e3f7e1d_6551b7a1",
|
|
"filename": "config_tempest/services/image.py",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 61,
|
|
"author": {
|
|
"id": 10459
|
|
},
|
|
"writtenOn": "2022-02-16T15:56:15Z",
|
|
"side": 1,
|
|
"message": "I think my confusion stems also from the fact that \"single store\" may either mean \"current way of configuring a store\" and \"a single store configured with multistore configuration\".\n\nCan you please clarify your previous sentence by distinguishing the two?\n\nI can read your sentence in two different ways:\n\n1) import image works a) if glance is configured through multistore configuration, regardless of the amount of storages (one or more)\n\n2) import image works if glance is configured to have multiple stores, which requires the new multistore way of configuring.\n\n\nIf this is 1), then the current condition based on the stores is not exactly clear, the function should be _is_multistore_configured rather than _get_number_of_stores\n\nif this is 2), the the condition should be \u003e1.\n\nOf course it\u0027s possible there is a condition 3) which I haven\u0027t considered.",
|
|
"parentUuid": "09a137f8_14b36ed9",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "331d5110_78eb83c2",
|
|
"filename": "config_tempest/services/image.py",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 61,
|
|
"author": {
|
|
"id": 9303
|
|
},
|
|
"writtenOn": "2022-02-16T17:22:28Z",
|
|
"side": 1,
|
|
"message": "1) import image works a) if glance is configured through multistore configuration, regardless of the amount of storages (one or more) \n\nThis is correct\n\n2) import image works if glance is configured to have multiple stores, which requires the new multistore way of configuring.\n\nThis is totally wrong\n\n\nAs of now you can configure stores two ways;\n\n1. Old way\n[glance_store]\nstores \u003d file,swift,http\ndefault_store \u003d file\nfilesystem_store_datadir \u003d /path/to/file/system\n\n\n2. Multistore way\n2.1 more than one store\n[Default]\nenabled_backends \u003d fast:rbd,slow:file\n\n[glance_store]\ndefault_backend \u003d fast\n\n[fast]\n\u003crbd_specific_options\u003e\n\n[slow]\n\u003cfile_store_specific_options\u003e\n\n2.2 only one store\n[Default]\nenabled_backends \u003d fast:rbd\n\n[glance_store]\ndefault_backend \u003d fast\n\n[fast]\n\u003crbd_specific_options\u003e\n\nAFAIK, this option is required to run tests to import image in multiple stores and copy existing image in multiple stores. The first case will be possible to run even if single store is configured, but the later one i.e. copy image will not work if there is only single store configured.",
|
|
"parentUuid": "5e3f7e1d_6551b7a1",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "e764b1ae_808b7bb0",
|
|
"filename": "config_tempest/services/image.py",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 61,
|
|
"author": {
|
|
"id": 10459
|
|
},
|
|
"writtenOn": "2022-02-17T10:21:45Z",
|
|
"side": 1,
|
|
"message": "Thanks, so \"1) import image works a) if glance is configured through multistore configuration, regardless of the amount of storages (one or more) \nThis is correct\"\n\nThen at least the commit message should be changed.\"\nThis is an attempt to populate tempestconf with\nmultistore feature enabled if and only if multiple\nstores are available.\"\n\nBut then, shouldn\u0027t the function be check whether the feature is enabled, and be called differently (_is_multistore_enabled) rather then checking the amount of stores?",
|
|
"parentUuid": "331d5110_78eb83c2",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "68be3dd3_224cacfd",
|
|
"filename": "config_tempest/services/image.py",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 61,
|
|
"author": {
|
|
"id": 9303
|
|
},
|
|
"writtenOn": "2022-02-17T10:58:22Z",
|
|
"side": 1,
|
|
"message": "\u003e Thanks, so \"1) import image works a) if glance is configured through multistore configuration, regardless of the amount of storages (one or more) \n\u003e This is correct\"\n\u003e \n\u003e Then at least the commit message should be changed.\"\n\u003e This is an attempt to populate tempestconf with\n\u003e multistore feature enabled if and only if multiple\n\u003e stores are available.\"\n\u003e \n\u003e But then, shouldn\u0027t the function be check whether the feature is enabled, and be called differently (_is_multistore_enabled) rather then checking the amount of stores?\n\nAgree, As per the commit message I thought the patch is to check if multistore is configured but internally it seems to run import workflow like import images in more than one store or copying existing images in more than one store. I think we should add a note here to explain why we need more than 1 store and set it here if num_stores \u003e 1.",
|
|
"parentUuid": "e764b1ae_808b7bb0",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "dca8a138_e3765105",
|
|
"filename": "config_tempest/services/image.py",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 61,
|
|
"author": {
|
|
"id": 9303
|
|
},
|
|
"writtenOn": "2022-02-17T11:01:23Z",
|
|
"side": 1,
|
|
"message": "\u003e \u003e Thanks, so \"1) import image works a) if glance is configured through multistore configuration, regardless of the amount of storages (one or more) \n\u003e \u003e This is correct\"\n\u003e \u003e \n\u003e \u003e Then at least the commit message should be changed.\"\n\u003e \u003e This is an attempt to populate tempestconf with\n\u003e \u003e multistore feature enabled if and only if multiple\n\u003e \u003e stores are available.\"\n\u003e \u003e \n\u003e \u003e But then, shouldn\u0027t the function be check whether the feature is enabled, and be called differently (_is_multistore_enabled) rather then checking the amount of stores?\n\u003e \n\u003e Agree, As per the commit message I thought the patch is to check if multistore is configured but internally it seems to run import workflow like import images in more than one store or copying existing images in more than one store. I think we should add a note here to explain why we need more than 1 store and set it here if num_stores \u003e 1.\n\n\nScratch my latest comment, I think we should rename this method to \u0027_is_multistore_enabled\u0027 and set the option in tempest.conf based on that.",
|
|
"parentUuid": "68be3dd3_224cacfd",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "43f8f942_1adf937e",
|
|
"filename": "config_tempest/services/image.py",
|
|
"patchSetId": 13
|
|
},
|
|
"lineNbr": 61,
|
|
"author": {
|
|
"id": 30742
|
|
},
|
|
"writtenOn": "2022-03-08T15:56:07Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "dca8a138_e3765105",
|
|
"revId": "c72239914116fde2e4a90bedd1b14b2f263aae47",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
}
|
|
]
|
|
} |