)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"aace3c3841441282efb7b42dc29f9956c163b37a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e7a553ea_e740026b","updated":"2025-02-25 14:35:12.000000000","message":"I am bit worried about backwards compatibility concerns here. Although I don\u0027t have a solution to offer really, I am afraid.","commit_id":"c806a7b88d5dfa86c475a0b2cb5b683b82ddb5b3"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"5d3f12d93e59a342d3dfcb3d4d92acdb1f88b306","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6d4e4712_e93f9ccc","in_reply_to":"5c117539_6be79151","updated":"2025-03-04 02:03:25.000000000","message":"Relevant capi-helm-charts code: https://github.com/azimuth-cloud/capi-helm-charts/blob/main/charts/openstack-cluster/templates/node-group/openstack-machine-template.yaml#L71\n\nIf this was changed to the v1beta1 format instead of the magnum driver, then no change would be required to values being passed in. Upgrading the chart is a requirement to support v1beta1, and it would include this change also.\n\nIt\u0027s another option; I\u0027m not sure which is preferred. It might be harder to define the chart values, and the driver remains with older-style YAML...","commit_id":"c806a7b88d5dfa86c475a0b2cb5b683b82ddb5b3"},{"author":{"_account_id":34212,"name":"Matt Anson","email":"matta@stackhpc.com","username":"m-anson","status":"StackHPC"},"change_message_id":"d008056c0e7eef676be0a16cb795c0bd4fbb4528","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"32c0ff64_4308f5b2","in_reply_to":"6d4e4712_e93f9ccc","updated":"2025-03-10 19:21:33.000000000","message":"I\u0027ve got a proposed fix pretty much as described in this comment chain: https://github.com/azimuth-cloud/capi-helm-charts/pull/501.\n\nIMHO, I think *not* changing the driver code and handling this down in the Helm charts is probably the preferred route to fix this for now.","commit_id":"c806a7b88d5dfa86c475a0b2cb5b683b82ddb5b3"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"e1a2e7f76c4c9065efb475f5a18ceb9eb8280209","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5c117539_6be79151","in_reply_to":"e7a553ea_e740026b","updated":"2025-03-04 01:55:12.000000000","message":"Hmm yeah, changing the **driver** makes it depend explicitly on CAPO v1beta1. That\u0027s okay by me (as my company has upgraded to CAPO 0.10), but magnum-capi-helm needs to make sure it\u0027s declared a breaking change and probably put a compatibility matrix somewhere to say which CAPO versions are supported by which driver versions.\n\nOne way to provide a wider support with the same driver would be to make this change in the CAPI Helm Chart repo. Right now the values for `nodeGroup.machineNetworking.ports` are free-form and applied as YAML.\n\nChanging the Helm chart means updating the **helm chart** version will require CAPO v1beta1 (and thus, there\u0027s a compatibility matrix in there), but the driver remains agnostic and there\u0027s no forced upgrade for older Clusters not using a new Helm Chart (eg. older `capi_helm_chart_version` label).\n\nThere\u0027s an upgrade required here somewhere, but how we communicate it and how widely we support older CAPO is the choice we have.\n\nI think it\u0027s reasonable to require this(with communication/docs!) - I\u0027m not sure I support looking up and supporting older v1alpha7 manifests as that will add to complexity of this codebase for little gain.","commit_id":"c806a7b88d5dfa86c475a0b2cb5b683b82ddb5b3"},{"author":{"_account_id":34212,"name":"Matt Anson","email":"matta@stackhpc.com","username":"m-anson","status":"StackHPC"},"change_message_id":"d008056c0e7eef676be0a16cb795c0bd4fbb4528","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"498c7d20_7c5845d9","updated":"2025-03-10 19:21:33.000000000","message":"Outside of fixing this particular issue, we could push all of the values-templating down into capi-helm-charts by allowing the values passed to the charts to be overridden by a JSON-formatted cluster(template) label.\n\nThe driver code could/should continue to pass a sensible set of default values to get a cluster working, but which could be overridden by the contents of the JSON-formatted label.\n\nThis would be a nice addition to the driver that StackHPC could contribute over the coming months.","commit_id":"caa738f0df822341ce688100c8ccab56a4c612da"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"8f602810c5dcb5476e6615aebfff4e8f13add91d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5783acda_2b659720","updated":"2025-05-07 10:06:06.000000000","message":"Update: This issue was resolved in helm charts[1] instead of magnum helm driver.\n\n[1] https://github.com/azimuth-cloud/capi-helm-charts/pull/501","commit_id":"caa738f0df822341ce688100c8ccab56a4c612da"},{"author":{"_account_id":35263,"name":"Matt Crees","email":"mattc@stackhpc.com","username":"mattcrees"},"change_message_id":"e384c5ac5166f4ecc86f03cdee25388589f47767","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6680396f_29ebb1fc","in_reply_to":"5783acda_2b659720","updated":"2025-05-07 11:00:40.000000000","message":"Indeed, I\u0027ll abandon this patch","commit_id":"caa738f0df822341ce688100c8ccab56a4c612da"}],"magnum_capi_helm/driver.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"aace3c3841441282efb7b42dc29f9956c163b37a","unresolved":true,"context_lines":[{"line_number":957,"context_line":"                            {},"},{"line_number":958,"context_line":"                            {"},{"line_number":959,"context_line":"                                \"network\": {"},{"line_number":960,"context_line":"                                    \"filter\":"},{"line_number":961,"context_line":"                                        \"name\": extra_network_name,"},{"line_number":962,"context_line":"                                },"},{"line_number":963,"context_line":"                                \"securityGroups\": [],"}],"source_content_type":"text/x-python","patch_set":1,"id":"e843d60e_8c765363","line":960,"updated":"2025-02-25 14:35:12.000000000","message":"Are you OK to make sure this code is covered in a unit test please?\n\n(I know that wouldn\u0027t make any difference to the bug we are finding here).\n\nThe problem we have here is that adding this will break older clouds, so we might have to bump the minimum version of the capi helm charts that we support here (and equally, state we break when v1beta1 crds are not being used)\n\n... I am not sure I like the breaking change here. Although I am not really sure how we avoid that.\n\nI guess we could use the k8s API to detect which version of the CRD is being used, any only do the new format when we see v1beta1 registered? I am not really sure that works though. Yuck.","commit_id":"c806a7b88d5dfa86c475a0b2cb5b683b82ddb5b3"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9390e8991b45ef51499eedc7d98a20b29247f822","unresolved":true,"context_lines":[{"line_number":957,"context_line":"                            {},"},{"line_number":958,"context_line":"                            {"},{"line_number":959,"context_line":"                                \"network\": {"},{"line_number":960,"context_line":"                                    \"filter\":"},{"line_number":961,"context_line":"                                        \"name\": extra_network_name,"},{"line_number":962,"context_line":"                                },"},{"line_number":963,"context_line":"                                \"securityGroups\": [],"}],"source_content_type":"text/x-python","patch_set":1,"id":"d5e2908a_fa89f4ee","line":960,"in_reply_to":"e843d60e_8c765363","updated":"2025-02-25 14:37:36.000000000","message":"PS, I think we need some extra curly braces on here, to create the extra dictionary.","commit_id":"c806a7b88d5dfa86c475a0b2cb5b683b82ddb5b3"}],"releasenotes/notes/fix-external-network-name-e597e5458ad1c717.yaml":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"aace3c3841441282efb7b42dc29f9956c163b37a","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Fixes an issue where, from CAPO version 0.10.0 onwards,"},{"line_number":5,"context_line":"    \"extra_network_name\" would not work."},{"line_number":6,"context_line":"    `LP#2099997 \u003chttps://bugs.launchpad.net/magnum/+bug/2099997\u003e`__"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"97433cca_9e22c4d8","line":6,"updated":"2025-02-25 14:35:12.000000000","message":"I think we need to tell people that to use this feature, you must now use at least v1beta1... and older versions will break horribly... although this seems pretty bad.","commit_id":"c806a7b88d5dfa86c475a0b2cb5b683b82ddb5b3"}]}