)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d4e713af1fa6161eaec718f0c161fc618e427829","unresolved":true,"context_lines":[{"line_number":13,"context_line":"minimal reservations, maximal reservations, allocation_ratio and step_size."},{"line_number":14,"context_line":"This blueprints proposes to use provider.yaml and the placement API to set"},{"line_number":15,"context_line":"these values."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I122fd0b9b9630a59059062e2186ac5fdd238e5bc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"c251f7fd_11ffff10","line":16,"updated":"2022-09-22 07:54:11.000000000","message":"If you add the this line then the automation will add a link to this patch on the bp\u0027s whiteboard. \n\nblueprint: compute-customizable-inventory","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":28542,"name":"Eigil Obrestad","email":"eigil@obrestad.org","username":"obrestad"},"change_message_id":"301207934196dd3ea474110b34181fe1a0d775ce","unresolved":false,"context_lines":[{"line_number":13,"context_line":"minimal reservations, maximal reservations, allocation_ratio and step_size."},{"line_number":14,"context_line":"This blueprints proposes to use provider.yaml and the placement API to set"},{"line_number":15,"context_line":"these values."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I122fd0b9b9630a59059062e2186ac5fdd238e5bc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"8bc1291f_55a80f42","line":16,"in_reply_to":"c251f7fd_11ffff10","updated":"2022-11-07 11:15:41.000000000","message":"Done","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d4e713af1fa6161eaec718f0c161fc618e427829","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"bdc80577_d0deee47","updated":"2022-09-22 07:54:11.000000000","message":"Thank you Eigil. This is a well written spec. I\u0027m OK with the use case and the high level proposed change. I have request for details inline and I have a possible alternative to think about. ","commit_id":"1275a41bbea3988e5f80bddfe7c1f9ab21195841"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0d3f690b4410b941d30814969e3f732cd46dd4ba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6fb4740d_58b535a5","updated":"2022-09-22 08:27:58.000000000","message":"in general i do like this.\n\ni aggree wthat we should add more deatil on the provider.yaml changes\n\nwe should note that there will be a schma change to enable it and add an example of what that will look like\n\nso please add an exmaple of what it will look like to change the min/max ram cpu or disk. ","commit_id":"1275a41bbea3988e5f80bddfe7c1f9ab21195841"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3210b0e93f5644df70993bfd8eb1a82c2ff9adc8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"20eb17e9_c0143f78","updated":"2022-11-15 13:38:54.000000000","message":"Lets define it explicitly which fields we are supporting setting.","commit_id":"44adb7a1414dd58930ab94ec3d0824ff7618fdd0"}],"specs/2023.1/approved/compute-customizable-inventory.rst":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d4e713af1fa6161eaec718f0c161fc618e427829","unresolved":true,"context_lines":[{"line_number":19,"context_line":"Problem description"},{"line_number":20,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Currently using the placement API to change the values of the default resource"},{"line_number":23,"context_line":"classes in a compute-nodes inventory would give the expected result in terms of"},{"line_number":24,"context_line":"scheduling, but changes of these values are not persistent as nova-compute is"},{"line_number":25,"context_line":"regularly updating placement with its default values."}],"source_content_type":"text/x-rst","patch_set":1,"id":"ed293a2e_06122ce8","line":22,"range":{"start_line":22,"start_character":44,"end_line":22,"end_character":54},"updated":"2022-09-22 07:54:11.000000000","message":"the min_unit, max_unit values\n\nor do we need to allow setting more values like reserved, step_size?","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":28542,"name":"Eigil Obrestad","email":"eigil@obrestad.org","username":"obrestad"},"change_message_id":"301207934196dd3ea474110b34181fe1a0d775ce","unresolved":false,"context_lines":[{"line_number":19,"context_line":"Problem description"},{"line_number":20,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Currently using the placement API to change the values of the default resource"},{"line_number":23,"context_line":"classes in a compute-nodes inventory would give the expected result in terms of"},{"line_number":24,"context_line":"scheduling, but changes of these values are not persistent as nova-compute is"},{"line_number":25,"context_line":"regularly updating placement with its default values."}],"source_content_type":"text/x-rst","patch_set":1,"id":"95d28dbb_df87c5fb","line":22,"range":{"start_line":22,"start_character":44,"end_line":22,"end_character":54},"in_reply_to":"ed293a2e_06122ce8","updated":"2022-11-07 11:15:41.000000000","message":"Currently you can change \"allocation_ratio\" through the api and nova will not overwrite it with defaults; and I think that also make sense for the other values, including step_size and reserved.","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d4e713af1fa6161eaec718f0c161fc618e427829","unresolved":true,"context_lines":[{"line_number":32,"context_line":"having large VM\u0027s from being scheduled on small compute nodes. The rationale is"},{"line_number":33,"context_line":"that if the operator allow a single VM to get access to all physical CPUs on a"},{"line_number":34,"context_line":"hypervisor a single user can utilize all available resources, and thus affect"},{"line_number":35,"context_line":"other instances on the same node. "},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Such large VMs should be scheduled on larger hypervisors where not all CPU\u0027s"},{"line_number":38,"context_line":"will be allocated to the same instance.Similarly an operator might want to"}],"source_content_type":"text/x-rst","patch_set":1,"id":"ef0baddb_fa705773","line":35,"range":{"start_line":35,"start_character":33,"end_line":35,"end_character":34},"updated":"2022-09-22 07:54:11.000000000","message":"remote the trailing whitespace","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":28542,"name":"Eigil Obrestad","email":"eigil@obrestad.org","username":"obrestad"},"change_message_id":"301207934196dd3ea474110b34181fe1a0d775ce","unresolved":false,"context_lines":[{"line_number":32,"context_line":"having large VM\u0027s from being scheduled on small compute nodes. The rationale is"},{"line_number":33,"context_line":"that if the operator allow a single VM to get access to all physical CPUs on a"},{"line_number":34,"context_line":"hypervisor a single user can utilize all available resources, and thus affect"},{"line_number":35,"context_line":"other instances on the same node. "},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Such large VMs should be scheduled on larger hypervisors where not all CPU\u0027s"},{"line_number":38,"context_line":"will be allocated to the same instance.Similarly an operator might want to"}],"source_content_type":"text/x-rst","patch_set":1,"id":"c3e87bfc_96299ebc","line":35,"range":{"start_line":35,"start_character":33,"end_line":35,"end_character":34},"in_reply_to":"ef0baddb_fa705773","updated":"2022-11-07 11:15:41.000000000","message":"Done","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d4e713af1fa6161eaec718f0c161fc618e427829","unresolved":true,"context_lines":[{"line_number":55,"context_line":"Parsing of the provider.yaml should allow setting the inventory for the"},{"line_number":56,"context_line":"resource-classes VCPU and MEMORY_MB in addition to the currently allowed"},{"line_number":57,"context_line":"CUSTOM_* classes to allow the operators define these values through"},{"line_number":58,"context_line":"configuration-files if that is preferred over using the placement API."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Alternatives"},{"line_number":61,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"39648617_201230b9","line":58,"updated":"2022-09-22 07:54:11.000000000","message":"Thanks for mentioning both aspects. I think in the implementation we can make sure that simply reading these value from the config file will result in nova applying what was in the config file to placement, without the need to read the value first back from placement. But this is implementation details so what you wrote there is good as is.","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0d3f690b4410b941d30814969e3f732cd46dd4ba","unresolved":true,"context_lines":[{"line_number":55,"context_line":"Parsing of the provider.yaml should allow setting the inventory for the"},{"line_number":56,"context_line":"resource-classes VCPU and MEMORY_MB in addition to the currently allowed"},{"line_number":57,"context_line":"CUSTOM_* classes to allow the operators define these values through"},{"line_number":58,"context_line":"configuration-files if that is preferred over using the placement API."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Alternatives"},{"line_number":61,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"4f867194_6cd836e4","line":58,"in_reply_to":"39648617_201230b9","updated":"2022-09-22 08:27:58.000000000","message":"the read from placement is to enable you to change teh values form the api or via provider.yaml\n\nit is just making nova treat its hardcoded value as an initial value which is only set if creating a inventory for the first time.","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":28542,"name":"Eigil Obrestad","email":"eigil@obrestad.org","username":"obrestad"},"change_message_id":"301207934196dd3ea474110b34181fe1a0d775ce","unresolved":false,"context_lines":[{"line_number":55,"context_line":"Parsing of the provider.yaml should allow setting the inventory for the"},{"line_number":56,"context_line":"resource-classes VCPU and MEMORY_MB in addition to the currently allowed"},{"line_number":57,"context_line":"CUSTOM_* classes to allow the operators define these values through"},{"line_number":58,"context_line":"configuration-files if that is preferred over using the placement API."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Alternatives"},{"line_number":61,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"02441bc0_4bd1992f","line":58,"in_reply_to":"4f867194_6cd836e4","updated":"2022-11-07 11:15:41.000000000","message":"Yes; my intention lines up with what Sean wrote here.","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d4e713af1fa6161eaec718f0c161fc618e427829","unresolved":true,"context_lines":[{"line_number":56,"context_line":"resource-classes VCPU and MEMORY_MB in addition to the currently allowed"},{"line_number":57,"context_line":"CUSTOM_* classes to allow the operators define these values through"},{"line_number":58,"context_line":"configuration-files if that is preferred over using the placement API."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Alternatives"},{"line_number":61,"context_line":"------------"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"fd2363d8_472ca219","line":59,"updated":"2022-09-22 07:54:11.000000000","message":"I feel we need to have a bit more detail here about how the provider.yaml file will look like. So either an example or a change in the schema[1] needs to be described here.\n\n[1] https://github.com/openstack/nova/blob/1025c9879341d44db33c4cc501435364dd185a9e/nova/compute/provider_config.py#L35","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":28542,"name":"Eigil Obrestad","email":"eigil@obrestad.org","username":"obrestad"},"change_message_id":"301207934196dd3ea474110b34181fe1a0d775ce","unresolved":false,"context_lines":[{"line_number":56,"context_line":"resource-classes VCPU and MEMORY_MB in addition to the currently allowed"},{"line_number":57,"context_line":"CUSTOM_* classes to allow the operators define these values through"},{"line_number":58,"context_line":"configuration-files if that is preferred over using the placement API."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Alternatives"},{"line_number":61,"context_line":"------------"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"18c10e9c_2f61803c","line":59,"in_reply_to":"0505ec3b_c49b868f","updated":"2022-11-07 11:15:41.000000000","message":"Agreed. Patchset 3 will come soon with the proposed schema changes.","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0d3f690b4410b941d30814969e3f732cd46dd4ba","unresolved":true,"context_lines":[{"line_number":56,"context_line":"resource-classes VCPU and MEMORY_MB in addition to the currently allowed"},{"line_number":57,"context_line":"CUSTOM_* classes to allow the operators define these values through"},{"line_number":58,"context_line":"configuration-files if that is preferred over using the placement API."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Alternatives"},{"line_number":61,"context_line":"------------"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"0505ec3b_c49b868f","line":59,"in_reply_to":"fd2363d8_472ca219","updated":"2022-09-22 08:27:58.000000000","message":"yes so currently provider.yaml only allows you to create addtional invetores \n\nwe need to add a new section “existing” to be able to reference the already creaty invenorties and we need to remove the restriction on requireing CUSTOM_ resosuce clases","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d4e713af1fa6161eaec718f0c161fc618e427829","unresolved":true,"context_lines":[{"line_number":64,"context_line":"reserved_host_memory_mb and similar. Even though that code-wise is a simpler"},{"line_number":65,"context_line":"alternative it is desirable to use the provider.yaml file instad as that will"},{"line_number":66,"context_line":"be a common interfaces for such changes."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Data model impact"},{"line_number":69,"context_line":"-----------------"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"902736a8_b428eb9a","line":67,"updated":"2022-09-22 07:54:11.000000000","message":"I\u0027m wondering about another alternative...\n\nToday nova blindly sets max_unit and min_unit. But those are not mandatory fields in the Placement API, so nova can choose not to set it ever. And let Placement default it. I think this way the admin can go and update the min_unit, max_unit value directly via Placement API and nova won\u0027t override it during the periodic updates.\n\nNova sets these values here periodically to some defaults[1]. If we don\u0027t set max_unit there, then placement will default it to big enough number [2] when no value is provided. But if it is an update then I believe placement will not overwrite the value in the DB with the default.\n\nSo for me the question which interface we want to use to define max_unit, min_unit\na) nova\u0027s provider.yaml that nova will translate to placement API call\nb) change nova not to set default in the placement API calls and let the admin set these values via the Placement API directly separately.\n\n[1] https://github.com/openstack/nova/blob/1025c9879341d44db33c4cc501435364dd185a9e/nova/virt/libvirt/driver.py#L8800-L8801\n[2] https://github.com/openstack/placement/blob/b9c1167facd5050c5f8f53644039e28b464ec010/placement/handlers/inventory.py#L48","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0d3f690b4410b941d30814969e3f732cd46dd4ba","unresolved":true,"context_lines":[{"line_number":64,"context_line":"reserved_host_memory_mb and similar. Even though that code-wise is a simpler"},{"line_number":65,"context_line":"alternative it is desirable to use the provider.yaml file instad as that will"},{"line_number":66,"context_line":"be a common interfaces for such changes."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Data model impact"},{"line_number":69,"context_line":"-----------------"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"f2377159_264a004d","line":67,"in_reply_to":"902736a8_b428eb9a","updated":"2022-09-22 08:27:58.000000000","message":"i really dislike when max is larger then total.\n\nbut yes if we just wanted to enable chanign it form the api we could either have nova set the vaule only for generation 0 or never set it\n\nthe other alternitive is to use the aggreate flavor affinity filter\nwhich was designed to adress the usecuase of limiting what flavors can be scheduled to a aset of hosts.\n\nfor legacy reasons its called the aggreate type affinity filter\n https://docs.openstack.org/nova/latest/admin/scheduling.html#aggregatetypeaffinityfilter","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":28542,"name":"Eigil Obrestad","email":"eigil@obrestad.org","username":"obrestad"},"change_message_id":"301207934196dd3ea474110b34181fe1a0d775ce","unresolved":false,"context_lines":[{"line_number":64,"context_line":"reserved_host_memory_mb and similar. Even though that code-wise is a simpler"},{"line_number":65,"context_line":"alternative it is desirable to use the provider.yaml file instad as that will"},{"line_number":66,"context_line":"be a common interfaces for such changes."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Data model impact"},{"line_number":69,"context_line":"-----------------"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"b73053fa_3b9cc202","line":67,"in_reply_to":"f2377159_264a004d","updated":"2022-11-07 11:15:41.000000000","message":"A simpler alternative than changing the logic with provider.yaml is to simply have nova set the vaule only for generation 0 and if the opreator changes it the changes persist. So; basically the first of the two changes in this blueprint.","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d4e713af1fa6161eaec718f0c161fc618e427829","unresolved":true,"context_lines":[{"line_number":110,"context_line":"This spec updates the libvirt-driver, but it might make sense to have the same"},{"line_number":111,"context_line":"behaviour in other drivers. For drivers not having this functionality we simply"},{"line_number":112,"context_line":"will have the current behaviour where the driver regularly resets the values to"},{"line_number":113,"context_line":"their defaults."},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"Upgrade impact"},{"line_number":116,"context_line":"--------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"980069fa_ddd7794d","line":113,"updated":"2022-09-22 07:54:11.000000000","message":"If we go with the provider.yamls solution then we will have a driver agnostic way to overwrite what the virt driver provides. The provider.yaml is applied at the resource tracker level[1] and so this runs for all virt driver that using placement. \n\n[1] https://github.com/openstack/nova/blob/1025c9879341d44db33c4cc501435364dd185a9e/nova/compute/resource_tracker.py#L1296","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0d3f690b4410b941d30814969e3f732cd46dd4ba","unresolved":true,"context_lines":[{"line_number":110,"context_line":"This spec updates the libvirt-driver, but it might make sense to have the same"},{"line_number":111,"context_line":"behaviour in other drivers. For drivers not having this functionality we simply"},{"line_number":112,"context_line":"will have the current behaviour where the driver regularly resets the values to"},{"line_number":113,"context_line":"their defaults."},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"Upgrade impact"},{"line_number":116,"context_line":"--------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"c9894c2a_8cc7bc1a","line":113,"in_reply_to":"980069fa_ddd7794d","updated":"2022-09-22 08:27:58.000000000","message":"this would be implemented in the compute manager/ resouce tracker not the driver level not the virt driver level in general yes.","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":28542,"name":"Eigil Obrestad","email":"eigil@obrestad.org","username":"obrestad"},"change_message_id":"301207934196dd3ea474110b34181fe1a0d775ce","unresolved":false,"context_lines":[{"line_number":110,"context_line":"This spec updates the libvirt-driver, but it might make sense to have the same"},{"line_number":111,"context_line":"behaviour in other drivers. For drivers not having this functionality we simply"},{"line_number":112,"context_line":"will have the current behaviour where the driver regularly resets the values to"},{"line_number":113,"context_line":"their defaults."},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"Upgrade impact"},{"line_number":116,"context_line":"--------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"f54cc02d_90f25350","line":113,"in_reply_to":"c9894c2a_8cc7bc1a","updated":"2022-11-07 11:15:41.000000000","message":"Do I understand the code correctly that vaules set in the driver [1] will be overwritten by the ones from providers.yaml as the driver-code is called[2] before the yaml-file is parsed[3]?\n\n[1] https://github.com/openstack/nova/blob/1025c9879341d44db33c4cc501435364dd185a9e/nova/virt/libvirt/driver.py#L8785-L8805\n[2] https://github.com/openstack/nova/blob/1025c9879341d44db33c4cc501435364dd185a9e/nova/compute/resource_tracker.py#L1240-L1241\n[3] https://github.com/openstack/nova/blob/1025c9879341d44db33c4cc501435364dd185a9e/nova/compute/resource_tracker.py#L1296","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3210b0e93f5644df70993bfd8eb1a82c2ff9adc8","unresolved":false,"context_lines":[{"line_number":110,"context_line":"This spec updates the libvirt-driver, but it might make sense to have the same"},{"line_number":111,"context_line":"behaviour in other drivers. For drivers not having this functionality we simply"},{"line_number":112,"context_line":"will have the current behaviour where the driver regularly resets the values to"},{"line_number":113,"context_line":"their defaults."},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"Upgrade impact"},{"line_number":116,"context_line":"--------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"2eb54192_86c2fc94","line":113,"in_reply_to":"f54cc02d_90f25350","updated":"2022-11-15 13:38:54.000000000","message":"Yes I think you are correct.","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d4e713af1fa6161eaec718f0c161fc618e427829","unresolved":true,"context_lines":[{"line_number":138,"context_line":"* Update nova/virt/libvirt/driver.py to only insert default-values into"},{"line_number":139,"context_line":"  placement if they dont already exist."},{"line_number":140,"context_line":"* Update the provider.yaml validator to allow setting VCPU and MEMORY_MB in"},{"line_number":141,"context_line":"  addition to the CUSCOM_* classes."},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"Dependencies"},{"line_number":144,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"c2e7df18_9f29d27b","line":141,"range":{"start_line":141,"start_character":18,"end_line":141,"end_character":25},"updated":"2022-09-22 07:54:11.000000000","message":"nit: CUSTOM","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":28542,"name":"Eigil Obrestad","email":"eigil@obrestad.org","username":"obrestad"},"change_message_id":"301207934196dd3ea474110b34181fe1a0d775ce","unresolved":false,"context_lines":[{"line_number":138,"context_line":"* Update nova/virt/libvirt/driver.py to only insert default-values into"},{"line_number":139,"context_line":"  placement if they dont already exist."},{"line_number":140,"context_line":"* Update the provider.yaml validator to allow setting VCPU and MEMORY_MB in"},{"line_number":141,"context_line":"  addition to the CUSCOM_* classes."},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"Dependencies"},{"line_number":144,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"9430ce8f_d847fbe6","line":141,"range":{"start_line":141,"start_character":18,"end_line":141,"end_character":25},"in_reply_to":"c2e7df18_9f29d27b","updated":"2022-11-07 11:15:41.000000000","message":"Done","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d4e713af1fa6161eaec718f0c161fc618e427829","unresolved":true,"context_lines":[{"line_number":151,"context_line":"This change is only utilizing existing functionality, so regular unit testing"},{"line_number":152,"context_line":"should suffice. If it is desirable to test that a VM being scheduled actually"},{"line_number":153,"context_line":"are scheduled to a less favorable node if the more favorable nodes are"},{"line_number":154,"context_line":"restricted to not allow a certain flavor, a more extensive test is needed."},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Documentation Impact"},{"line_number":157,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"221c3169_e950f075","line":154,"updated":"2022-09-22 07:54:11.000000000","message":"We have a set of functional tests for the provider.yaml code path here[1]. I would extend that too. \n\nRegarding validating the scheduling constraints. I don\u0027t think we need to test that here as that is an existing placement functionality tested in placement.\n\n[1] https://github.com/openstack/nova/blob/master/nova/tests/functional/compute/test_resource_tracker.py#L480","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0d3f690b4410b941d30814969e3f732cd46dd4ba","unresolved":true,"context_lines":[{"line_number":151,"context_line":"This change is only utilizing existing functionality, so regular unit testing"},{"line_number":152,"context_line":"should suffice. If it is desirable to test that a VM being scheduled actually"},{"line_number":153,"context_line":"are scheduled to a less favorable node if the more favorable nodes are"},{"line_number":154,"context_line":"restricted to not allow a certain flavor, a more extensive test is needed."},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Documentation Impact"},{"line_number":157,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"e07f3eb5_40d38657","line":154,"in_reply_to":"221c3169_e950f075","updated":"2022-09-22 08:27:58.000000000","message":"ya  we can add some functional test to assert the values are set right in placcment but we dont need to validate that placment then does the right thing.\n\nthat is test by placement","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":28542,"name":"Eigil Obrestad","email":"eigil@obrestad.org","username":"obrestad"},"change_message_id":"301207934196dd3ea474110b34181fe1a0d775ce","unresolved":false,"context_lines":[{"line_number":151,"context_line":"This change is only utilizing existing functionality, so regular unit testing"},{"line_number":152,"context_line":"should suffice. If it is desirable to test that a VM being scheduled actually"},{"line_number":153,"context_line":"are scheduled to a less favorable node if the more favorable nodes are"},{"line_number":154,"context_line":"restricted to not allow a certain flavor, a more extensive test is needed."},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Documentation Impact"},{"line_number":157,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7044b061_7a04746c","line":154,"in_reply_to":"e07f3eb5_40d38657","updated":"2022-11-07 11:15:41.000000000","message":"Ack","commit_id":"4381cc9fe3e9bfc82038b9577f53c2d1a415381a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3210b0e93f5644df70993bfd8eb1a82c2ff9adc8","unresolved":true,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"The provider.yaml schema should be updated to allow setting additional"},{"line_number":56,"context_line":"inventories than the currently allowed \"additional\" inventories with names"},{"line_number":57,"context_line":"matching \"CUSTOM_*\". Inventories for VCPU and MEMORY_MB should be allowed to be"},{"line_number":58,"context_line":"set in a new section called \u0027existing\u0027."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"An example of the use of an \u0027existing\u0027 section in provider.yamle would look"}],"source_content_type":"text/x-rst","patch_set":5,"id":"6a6c0b39_d9c8d4cd","line":57,"range":{"start_line":57,"start_character":21,"end_line":57,"end_character":32},"updated":"2022-11-15 13:38:54.000000000","message":"We don\u0027t really want to allow setting all the fields in the inventories. I.e. `total` of VCPUs should not be settable via provider.yaml. But we can allow setting, `min_value`, `max_value`, and `step_size`. We should not allow setting `reserved` as that is set via `reserved_host_cpus` config option. Or we need to deprecate that config option. We should not allow setting `allocation_ratio` as that is also settable via existing config option or we should start deprecating that config option too.","commit_id":"44adb7a1414dd58930ab94ec3d0824ff7618fdd0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"380592edc47a5a5e34cd967954a68e182176ffce","unresolved":true,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"The provider.yaml schema should be updated to allow setting additional"},{"line_number":56,"context_line":"inventories than the currently allowed \"additional\" inventories with names"},{"line_number":57,"context_line":"matching \"CUSTOM_*\". Inventories for VCPU and MEMORY_MB should be allowed to be"},{"line_number":58,"context_line":"set in a new section called \u0027existing\u0027."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"An example of the use of an \u0027existing\u0027 section in provider.yamle would look"}],"source_content_type":"text/x-rst","patch_set":5,"id":"4f4a3752_041a7da8","line":57,"range":{"start_line":57,"start_character":21,"end_line":57,"end_character":32},"in_reply_to":"2c37927b_8c477ecd","updated":"2022-11-15 18:06:29.000000000","message":"well i do want use to be able to modify some atributes of the VCPU and PCPUS inventories. specifical the min_unit, max_uint and step_size and the step size\nthey are the main usecase for this.\n\ni am not against allowing reserved or allocation ratio if we also look at deprecating the current way to do that or declare it an error to use both\nand check for that.\n\nit might be nicer long term to only allow changing this via provider.yaml\ni agree on total not bing setable.\n\n\nim oke to scope this to `min_value`, `max_value`, and `step_size for now\nbut i think that those three shoudl be allowed on all standard resouce classes.\n\nso we can set a min and/or max cpu,ram,disk amount for a given host.","commit_id":"44adb7a1414dd58930ab94ec3d0824ff7618fdd0"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"dd16b5b60805e04af4452fbc937e06878b3758a7","unresolved":true,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"The provider.yaml schema should be updated to allow setting additional"},{"line_number":56,"context_line":"inventories than the currently allowed \"additional\" inventories with names"},{"line_number":57,"context_line":"matching \"CUSTOM_*\". Inventories for VCPU and MEMORY_MB should be allowed to be"},{"line_number":58,"context_line":"set in a new section called \u0027existing\u0027."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"An example of the use of an \u0027existing\u0027 section in provider.yamle would look"}],"source_content_type":"text/x-rst","patch_set":5,"id":"2c37927b_8c477ecd","line":57,"range":{"start_line":57,"start_character":21,"end_line":57,"end_character":32},"in_reply_to":"6a6c0b39_d9c8d4cd","updated":"2022-11-15 17:34:46.000000000","message":"Agreed with gibi. I don\u0027t want us to be able to modify some inventory resouce classes like VCPU or PCPU.\n\nMaybe we should discuss which RCs we could be updating.","commit_id":"44adb7a1414dd58930ab94ec3d0824ff7618fdd0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"380592edc47a5a5e34cd967954a68e182176ffce","unresolved":true,"context_lines":[{"line_number":63,"context_line":".. code-block:: yaml"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"  meta:"},{"line_number":66,"context_line":"    schema_version: \u00271.0\u0027"},{"line_number":67,"context_line":"  providers:"},{"line_number":68,"context_line":"    - identification:"},{"line_number":69,"context_line":"        name: \u0027computeN.fqdn\u0027"}],"source_content_type":"text/x-rst","patch_set":5,"id":"e0eb2917_ef93cddc","line":66,"range":{"start_line":66,"start_character":21,"end_line":66,"end_character":24},"updated":"2022-11-15 18:06:29.000000000","message":"so we will need to bump the schema version to 1.1 for this as its an additive change","commit_id":"44adb7a1414dd58930ab94ec3d0824ff7618fdd0"}]}