)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"74027bf592a5a323e53bc593fb205490891ba597","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8f156e54_c6dc0491","updated":"2023-07-24 07:54:39.000000000","message":"Clearly, we should add some unit tests here, but I think this is a good idea, and seems like the correct approach.","commit_id":"10ea0c88f0181cffc60a7fe58a5337c86ea1a054"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"23e2652ca5e75baf7c6eccad2e72773495d58cc5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8ecadabd_6f251be6","updated":"2023-07-20 19:34:08.000000000","message":"Testing this against Ironic tempest here: https://review.opendev.org/c/openstack/ironic/+/889122","commit_id":"10ea0c88f0181cffc60a7fe58a5337c86ea1a054"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"5ca5d289ba7ce6eefdae5bbfc94b1dac3152607b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"ecdeee62_ad58bbb0","updated":"2023-08-31 16:47:07.000000000","message":"Going to document this in a specless blueprint for next cycle, but John and I worked out a potential new flow which is nicer:\n- Nova always sends the relevant project_id in instance_info\n- The existing Ironic feature to set automatic_lessee will be enhanced to use this metadata if set to true.\n\nThis has a few benefits:\n- No new configuration in Nova to enable this\n- Exactly one config in openstack to enable this behavior (Ironic\u0027s [conductor]automatic_lessee \u003d true)\n- Ironic could potentially use that metadata later for other reasons, if desired.","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"3522bd5091ca057d22d9d306870235b9b352323a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"ef938fac_cf449c56","updated":"2023-08-25 21:22:15.000000000","message":"I\u0027m not the most familiar with Nova\u0027s model, but are flavor specs the right way to manage things which are generally turned on/off per Ironic deployment? There\u0027s not a reasonable use case for having lessee set on some nodes booted but not all. It seems like that might be a duplicate of what the policy config essentially does on the Ironic side; if an administrator did not want a given project to have access to the Ironic node API it\u0027s leasing, they would configure that via role assignments and policy in Ironic.\n\nIt just seems like it\u0027d be much less ergonomic to have this in flavor-specs, especially when we already have guarantees that there will never be more than one Ironic cluster behind a given nova-compute service.","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"93e72697be4f23dde7c95c2f5b56ccad4c9b92c9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"907e3469_130d74f9","updated":"2023-08-28 16:24:14.000000000","message":"Putting a procedural -2 lock for ensuring that the feature is properly tracked by an approved blueprint. You can reach me over IRC (bauzas on #openstack-nova) if you require further details of the nova process.","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"bd7a06d477f72562d0a29a9328c6b6924e71f90d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"d3d567aa_aa757e19","updated":"2023-08-25 20:49:06.000000000","message":"This has been extensively tested in devstack and is working. It is ready for review.","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"24cbc86d344728a8e2ea819c661ffe2fb6de5365","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"2b4034a0_af557bc3","updated":"2023-08-25 21:02:54.000000000","message":"im not sure this should be done vai a config option\nit proably should be a flavor extra spec or similar\n\nthe other reason for the -1 is this proably should be a specless blueprint which mean this shoudl be defered to the c cycle","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b3f952b667e687e03914b54a3f506ef30719ae68","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"af289ab6_0f033745","in_reply_to":"13fdbd30_fe504f81","updated":"2023-08-28 10:05:16.000000000","message":"that sounds like a bug in the ironic drvier to me. it should be useign the users token for all comunicaiton that does not requrie admin in the ironic api.\n\nstephen finucan started on convergitn the ironic driver to using the sdk i belive\ngieven we are rewritign large amount of the code for that anyway i think we should at least look at fixign the incorrect use of the admin client.\nhttps://review.opendev.org/q/topic:bp%252Fopenstacksdk-in-nova\n\nim wokdering what the best way to move this forward is. its easy to say just write a spec with the problemsteamet/usecase and we can go form there but perhaps we can conitue to discuss here or on irc or a meet call.","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"652200a10bf7e968660f1a0b33441f3e8fe078a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"d5b1629c_4bf978b4","in_reply_to":"8655a8a8_204f4e66","updated":"2023-08-28 14:16:59.000000000","message":"that really sounds like there should be metadata on the ironic node that we read to know what to do.","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"2df4efde4ff345a92684d7f9cdfff629aa8869b7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8655a8a8_204f4e66","in_reply_to":"af289ab6_0f033745","updated":"2023-08-28 14:06:31.000000000","message":"\u003e that sounds like a bug in the ironic drvier to me. it should be useign the users token for all comunicaiton that does not requrie admin in the ironic api.\n\nSo, not exactly. There\u0027s a few ways you can identify a project as owning a node in Ironic:\n- Default/legacy state: nodes are not associated with a project, and are administrated by system scoped admins\n- Node ownership: nodes are associated with a node.owner set to a project. Project-scoped permissions can be used then in Ironic\u0027s API\n- Node leasing: nodes are associated with a node.lessee set to a project; lessee is generally only set when a node is provisioned. Project-scoped permissions can be set on these in Ironic\u0027s API (and would generally include API calls that make sense to use when you have a node provisioned to you; such as read only information about the node or the ability to perform maintenance items on them).\n- Both: A node owner is set, and a node lessee is set on deploy. This allows a graduated permissions approach: the owner has certain access, and the lessee has certain (usually less) access. There are deployments using this approach in Ironic, where the Nova compute is using a project-scoped token that can only see nodes owned by a given project.\n\nThis idea of a potential owner and/or lessee makes it so that the lines between what is admin in Nova\u0027s API and what is not admin may vary based on deployment -- in the default/legacy case, all actions taken are admin. This is the detail that John and I went over when he said he thought it was unlikely we could properly pass around token information and suggested we take this, more direct approach.","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"a52e5b5df44ad2076e12ea8554459c20cb149cb3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"13fdbd30_fe504f81","in_reply_to":"c564d423_e4006770","updated":"2023-08-27 23:06:50.000000000","message":"Two things:\n1) I designed this with a post node-sharding world in mind (with peer_list deprecated). We already have config keys though, like partition_key, which if configured inconsistently across computes in peer_list cause bad behavior.\n\nSecondly, about flavor specs... I can see the appeal of the API-discoverability of the flavor specs. This is just basically a switch that opts Ironic into the same kind of multi-tenancy that has been supported with other projects already (via service tokens with a user piggybacked).\n\nThe original design of this (and how it works for standalone users), is for Ironic to extract the user information from the token and set lessee if the administrator configures it ([conductor]/automatic_lessee\u003dtrue). This doesn\u0027t work with Nova because it unconditionally uses the service/admin identity to communicate with Ironic. When I talked to John Garbutt in person about this problem in July, he indicated to me that solving that problem would be extremely difficult or impossible so suggested this as a workaround.\n\nPart of why I like this being a config option is, frankly, it makes it easier to set it up as the default way to run Ironic.\n\nI\u0027m happy to rewrite the change if you\u0027re certain flavor specs are the right route; I want the functionality exposed for Ironic user through whatever means you dictate, I\u0027m just curious what the best way is for us to lay it out such that one day, it could be the default... (and/or if solving the original \u0027problem\u0027 of preserving original-calling-user information on calls to Ironic so we can do it is the right solution; but if so that\u0027s a much larger change with a larger change of causing breakage).","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c5584e9f228c9d0fbcd3c080c3011237830bcf5c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"36e3ed05_933ea6a5","in_reply_to":"ecdeee62_ad58bbb0","updated":"2023-08-31 17:22:44.000000000","message":"ya that sounds like a reasonable approch to me","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"644f4e1697bbebf5ca8f4bc53abea13780b87a25","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c564d423_e4006770","in_reply_to":"ef938fac_cf449c56","updated":"2023-08-26 17:14:16.000000000","message":"so normally flavor/image propertiese are prefer to set per instance options that dont change for the lifetime of the instance.\n\n\nthe reason i am hesitating to take the conf approch is we try very very hard to not have any config driven api behavior.\n\ni.e. if there is a user visable api functionalty diffent between creating a instance on cloud a and cloud b that shoudl genreally not be enabel vai a config option.\n\n\nso on one hand i get why you want to enable this via a gloabl config option for the ironic driver but on the other hand that is a iterop issue because on one cloud a simple server create will enable the ablity to interact with a limit portion of the ironic api and on other cloud it wont and you will not be able to tell which is the case form the nova api.\n\nso that kind of where the problem is ideally we woudl have no config driven api behavior and remove any we currently have so im hesitent to intentionally add new ironic speicific config driven api behavior.\n\ni would like to see what other think.\n\ni wonder if this is instead somthing that could be driven form teh ironic side.\ni.e. could we have admins tag the ironic nodes and when nova select a node which happens to have \"auto_assign_lease\u003dtrue\" set then we would use the lease api to asign the lease?\n\nironic when enroling the nodes could provide a simple way to make that a default if it wanted and that would have a similr effect.\n\nthere are also complexities due to the peer_list withusing config options. i.e. if you dont set the same value on all ironic computes then depenign on what host the instace is maged by it woudl get diffetn behaivor. if we take the approch of using a flaovr extra spec of a tag/metadata on the ironic node we dont have that problem.","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"}],"nova/conf/ironic.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"93e72697be4f23dde7c95c2f5b56ccad4c9b92c9","unresolved":true,"context_lines":[{"line_number":99,"context_line":"             \u0027when ironic [conductor]/automatic_lessee is enabled or when \u0027"},{"line_number":100,"context_line":"             \u0027using a project-scoped token that relies on the lessee field \u0027"},{"line_number":101,"context_line":"             \u0027for access.\u0027"},{"line_number":102,"context_line":"    )"},{"line_number":103,"context_line":"]"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"04724da7_5277020e","line":102,"updated":"2023-08-28 16:24:14.000000000","message":"I hate to block things for bureaucratic reasons, but this new config option actually enables a feature that users can broadly use for multi-tenancy (in other words, this config knob isn\u0027t just for performance optimization or other kinds of non-feature development).\n\nAlas, in Nova we do have a process for accepting feature requests so we can reasonably align ourselves on reviewing the implementation patches. Here, this patch appears very late on my radar in our cycle and no blueprint was created against it for tracking.\nCould you please accordingly file a blueprint so we could properly track it during the Caracal timeframe ?\nThanks.\n\nDetails of our Nova process can be found here https://docs.openstack.org/nova/latest/contributor/process.html#spec-and-blueprint-approval-freeze","commit_id":"52e118d3489935743c4e34d0a7bbca037f9c8271"}],"nova/virt/ironic/patcher.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"74027bf592a5a323e53bc593fb205490891ba597","unresolved":true,"context_lines":[{"line_number":120,"context_line":"            patch.append({\u0027path\u0027: \u0027/instance_info/traits\u0027,"},{"line_number":121,"context_line":"                          \u0027op\u0027: \u0027add\u0027, \u0027value\u0027: traits})"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"        if CONF.ironic.set_lessee_on_deploy:"},{"line_number":124,"context_line":"            patch.append({\u0027path\u0027: \u0027/lessee\u0027, \u0027op\u0027: \u0027add\u0027,"},{"line_number":125,"context_line":"                          \u0027value\u0027: instance.project_id})"},{"line_number":126,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"eaafd8b1_c36f6ece","line":123,"updated":"2023-07-24 07:54:39.000000000","message":"Part of me was thinking we should set this earlier, when we set instance-uuid, but I think this is much better, we add and remove it with the instance_info. That seems more correct.","commit_id":"10ea0c88f0181cffc60a7fe58a5337c86ea1a054"}]}