)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34076,"name":"Jakub Darmach","email":"jakub@stackhpc.com","username":"darmach"},"change_message_id":"553cbee9744f47c983054eb5541c6723ead5c469","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1e698adb_62640780","updated":"2025-04-08 13:09:43.000000000","message":"Looks nice, should streamline the inner workings. @jake.yip@ardc.edu.au would you take a look?","commit_id":"04468dd8a0e6019197499b6805ca8ae5f2e39af9"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"40cbfcdd25130cbba095cff40d161c6a60207889","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"687ae032_ce45702c","updated":"2025-04-09 04:53:51.000000000","message":"Minor logic issue to note.\n\nI\u0027ll continue reviewing this week, as time allows.","commit_id":"04468dd8a0e6019197499b6805ca8ae5f2e39af9"},{"author":{"_account_id":36412,"name":"Scott Davidson","email":"sdavidson327@gmail.com","username":"sd109"},"change_message_id":"624a1d95b12b8d37226b5be2a08d538cc462e786","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"42e7b893_efb0e766","updated":"2025-04-17 11:12:56.000000000","message":"I\u0027m seeing similar issues with cluster deletion in my testing to those Dale is reporting. When a `coe cluster delete` command is issued the `openstack-cluster` Helm release does get uninstalled so that all VMs and other OpenStack resources are deleted but then, before the Magnum cluster object deletion is complete, the Helm release is re-installed.\n\nFor some reason this re-install doesn\u0027t seem to actually recreate the CAPI objects on the management cluster (and therefore no new OpenStack resources are created) which is not a Helm behaviour I\u0027ve ever seen before - possibly some kind of race condition between uninstalling and re-installing the Helm release.\n\nThis leaves the cluster in a state where all OpenStack resources are correctly deleted and so are most of the CAPI k8s resources on the management cluster but the Helm release for the user cluster is still present on the management cluster and so is the cluster-autoscaler pod for the (now non-existent) workload cluster.\n\nFWIW these cluster deletion issues may have been masked in DevStack testing due to [1] which is now fixed upstream but is not yet in a tagged release. To get the CAPI janitor fixes into a DevStack environment you can run:\n\n```\nhelm repo add capi-janitor https://azimuth-cloud.github.io/cluster-api-janitor-openstack\nhelm repo update\nhelm upgrade -n capi-janitor-system cluster-api-janitor-openstack capi-janitor/cluster-api-janitor-openstack --reuse-values --version 0.8.2-dev.0.main.4\n```\n\n[1] https://github.com/azimuth-cloud/cluster-api-janitor-openstack/pull/167","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"af2f1cb1ebbfa6408e6f46942ec14547d9faecd2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"94c40a55_db3cd6cb","updated":"2025-04-14 00:35:08.000000000","message":"This is not behaving quite right for me in my testing - possibly my configuration but I think the patchset could be improved.\n\nLet me know your thoughts and if you see the same issues.","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"f66397362d01cf0876f29279c8acb9c54c7731e7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"99bc3473_8b59d881","updated":"2025-04-14 04:02:59.000000000","message":"This test case is leaving behind clusters in `DELETE_IN_PROGRESS` state (which may transition to DELETE_COMPLETE) but the CAPI cluster and Helm deployment remains.\n\n$ openstack coe cluster create --cluster-template \u003ctemplate\u003e test10; sleep 0.5; openstack coe cluster delete test10\nRequest to create cluster 1166eb5d-f18a-4829-91e7-113d99a1d06d accepted\nRequest to delete cluster test10 has been accepted.\n\nThis environment has a single magnum-conductor running, so the race isn\u0027t between two magnum-conductors.","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":36695,"name":"Niklaus Hofer","display_name":"Niklaus \u0027vimja\u0027 Hofer","email":"niklaus.hofer@stepping-stone.ch","username":"vimja"},"change_message_id":"88854c3723983f707a4e3e82041bd84bacc78943","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"51cedc35_f2abd5c9","updated":"2025-04-11 08:00:32.000000000","message":"We tested the change in our test OpenStack cloud. From what we have seen it appears to be working just fine - everything that\u0027s worked before is still working plus we are no longer able to trigger the race conditions we were seeing before.\n\nThis get the thumbs up from me!","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"d153114e66151a121df5578442509d307b901cf6","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"207c4c1f_be8cc6bb","in_reply_to":"99bc3473_8b59d881","updated":"2025-04-14 04:10:53.000000000","message":"Note: This occurs after placing the `self._update_helm_release(context, cluster)` inside the `CREATE_IN_PROGRESS`/`UPDATE_IN_PROGRESS` if statement as I suggested in a previous review comment.\n\nI expect it occurs before this code change also, but have not tested.","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3165c2a5_9f0a1d72","updated":"2025-11-05 10:02:24.000000000","message":"I think we should have a tempest test that really tests this","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"}],"magnum_capi_helm/driver.py":[{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"af2f1cb1ebbfa6408e6f46942ec14547d9faecd2","unresolved":true,"context_lines":[{"line_number":389,"context_line":"            )"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        # Apply changes to the Helm release to sync with updated cluster state"},{"line_number":392,"context_line":"        self._update_helm_release(context, cluster)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        if cluster.status in {"},{"line_number":395,"context_line":"            fields.ClusterStatus.CREATE_IN_PROGRESS,"}],"source_content_type":"text/x-python","patch_set":3,"id":"1c1bbb7e_5f3f791a","line":392,"updated":"2025-04-14 00:35:08.000000000","message":"Is this in the right place? Perhaps it should be within the `if` statement that checks for UPDATE or CREATE in progress?\n\nIn my testing, when the cluster state is DELETE_IN_PROGRESS, the uninstall has been done and then this triggers a new installation of the helm chart.\n\nDo you see the same behaviour?","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":34213,"name":"Grzegorz Bialas","display_name":"Grzegorz Bialas","email":"grzegorz@stackhpc.com","username":"gbialas"},"change_message_id":"13ef82896fad78a4a0d227dd12206184334b5d95","unresolved":true,"context_lines":[{"line_number":389,"context_line":"            )"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        # Apply changes to the Helm release to sync with updated cluster state"},{"line_number":392,"context_line":"        self._update_helm_release(context, cluster)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        if cluster.status in {"},{"line_number":395,"context_line":"            fields.ClusterStatus.CREATE_IN_PROGRESS,"}],"source_content_type":"text/x-python","patch_set":3,"id":"8ef8e79a_6232dd92","line":392,"in_reply_to":"1c1bbb7e_5f3f791a","updated":"2025-11-03 10:36:01.000000000","message":"@dale@catalystcloud.nz I have added check for status of node group, this seems to improve this behaviour. I couldn’t fully replicate full re-installation of chart In what test scenario did it happened? How many clusters/node groups did you spawned and encountered that ?\nHowever I could replicate behaviour that Scott is mentioning here https://review.opendev.org/c/openstack/magnum-capi-helm/+/946055/comments/42e7b893_efb0e766 and this seems to fix it.","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"af2f1cb1ebbfa6408e6f46942ec14547d9faecd2","unresolved":true,"context_lines":[{"line_number":1041,"context_line":"        nodegroups \u003d cluster.nodegroups"},{"line_number":1042,"context_line":"        for ng in nodegroups:"},{"line_number":1043,"context_line":"            self._validate_allowed_flavor(context, ng.flavor_id)"},{"line_number":1044,"context_line":"            if self._get_autoscale_enabled(cluster):"},{"line_number":1045,"context_line":"                self._validate_allowed_node_counts(cluster, ng)"},{"line_number":1046,"context_line":""},{"line_number":1047,"context_line":"        # we generate this name (on the initial create call only)"}],"source_content_type":"text/x-python","patch_set":3,"id":"d42e445a_13c06a0e","line":1044,"updated":"2025-04-14 00:35:08.000000000","message":"This change appears to be unrelated to the rest of the patchset.\n\nCould you please submit this separately in case we need to revert the other parts of this.","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":10910,"name":"Stig Telfer","email":"stig.openstack@telfer.org","username":"stigtelfer"},"change_message_id":"90a23ffbd5372cae7fef928fab83cfcdbb0cc2cc","unresolved":true,"context_lines":[{"line_number":1041,"context_line":"        nodegroups \u003d cluster.nodegroups"},{"line_number":1042,"context_line":"        for ng in nodegroups:"},{"line_number":1043,"context_line":"            self._validate_allowed_flavor(context, ng.flavor_id)"},{"line_number":1044,"context_line":"            if self._get_autoscale_enabled(cluster):"},{"line_number":1045,"context_line":"                self._validate_allowed_node_counts(cluster, ng)"},{"line_number":1046,"context_line":""},{"line_number":1047,"context_line":"        # we generate this name (on the initial create call only)"}],"source_content_type":"text/x-python","patch_set":3,"id":"e4f69b0c_92746bdd","line":1044,"in_reply_to":"d42e445a_13c06a0e","updated":"2025-11-05 08:41:30.000000000","message":"Thanks for checking Dale, but this change fixes an issue found by the unit tests relating to the decoupling of helm release update from cluster creation. IIRC validation of autoscaling parameters was not happening in the context of the cluster creation request.","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":1041,"context_line":"        nodegroups \u003d cluster.nodegroups"},{"line_number":1042,"context_line":"        for ng in nodegroups:"},{"line_number":1043,"context_line":"            self._validate_allowed_flavor(context, ng.flavor_id)"},{"line_number":1044,"context_line":"            if self._get_autoscale_enabled(cluster):"},{"line_number":1045,"context_line":"                self._validate_allowed_node_counts(cluster, ng)"},{"line_number":1046,"context_line":""},{"line_number":1047,"context_line":"        # we generate this name (on the initial create call only)"}],"source_content_type":"text/x-python","patch_set":3,"id":"e665cd9f_6f7fea3a","line":1044,"in_reply_to":"e4f69b0c_92746bdd","updated":"2025-11-05 10:02:24.000000000","message":"That still looks like something we\u0027d like to be broken out to a separate patch","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":388,"context_line":"                cluster"},{"line_number":389,"context_line":"            )"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        # Apply changes to the Helm release to sync with updated cluster state"},{"line_number":392,"context_line":"        if cluster.status in {"},{"line_number":393,"context_line":"            fields.ClusterStatus.DELETE_IN_PROGRESS,"},{"line_number":394,"context_line":"        }:"}],"source_content_type":"text/x-python","patch_set":5,"id":"aac2ad9f_ce9fb1bd","line":391,"updated":"2025-11-05 10:02:24.000000000","message":"So, first of all the format is # NOTE: or # NOTE(nick):\nI don\u0027t think in is needed, cluster.status \u003d\u003d $state is enough","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":392,"context_line":"        if cluster.status in {"},{"line_number":393,"context_line":"            fields.ClusterStatus.DELETE_IN_PROGRESS,"},{"line_number":394,"context_line":"        }:"},{"line_number":395,"context_line":"            LOG.debug(\"Cluster %s is being deleted\", cluster.uuid)"},{"line_number":396,"context_line":"        else:"},{"line_number":397,"context_line":"            self._update_helm_release(context, cluster)"},{"line_number":398,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"0d977742_1f57cd24","line":395,"updated":"2025-11-05 10:02:24.000000000","message":"This is very ambiguous message - it\u0027s being deleted and what will not happen? What is the process that is reporting this? If we\u0027re logging this - it should be useful","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":820,"context_line":"            additionalStorageClasses\u003dadditional_storage_classes,"},{"line_number":821,"context_line":"        )"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"    def _process_node_groups(self, cluster):"},{"line_number":824,"context_line":"        nodegroups \u003d cluster.nodegroups"},{"line_number":825,"context_line":"        nodegroup_set \u003d []"},{"line_number":826,"context_line":"        for ng in nodegroups:"}],"source_content_type":"text/x-python","patch_set":5,"id":"c971a21f_670405bc","line":823,"updated":"2025-11-05 10:02:24.000000000","message":"I\u0027d say commit message needs an explanation why are we not passing nodegroups now to that function","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":825,"context_line":"        nodegroup_set \u003d []"},{"line_number":826,"context_line":"        for ng in nodegroups:"},{"line_number":827,"context_line":"            # Don\u0027t include nodegroups flagged for deletion"},{"line_number":828,"context_line":"            if ng.status \u003d\u003d fields.ClusterStatus.DELETE_IN_PROGRESS:"},{"line_number":829,"context_line":"                continue"},{"line_number":830,"context_line":"            if ng.role !\u003d NODE_GROUP_ROLE_CONTROLLER:"},{"line_number":831,"context_line":"                nodegroup_item \u003d dict("}],"source_content_type":"text/x-python","patch_set":5,"id":"4db1d646_769d9c1f","line":828,"updated":"2025-11-05 10:02:24.000000000","message":"Just add that to next if statement instead of creating one that does continue?","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":839,"context_line":"                nodegroup_set.append(nodegroup_item)"},{"line_number":840,"context_line":"        return nodegroup_set"},{"line_number":841,"context_line":""},{"line_number":842,"context_line":"    # Note: only call this from the context of the reconciliation loop"},{"line_number":843,"context_line":"    def _update_helm_release(self, context, cluster):"},{"line_number":844,"context_line":""},{"line_number":845,"context_line":"        image_id, kube_version, os_distro \u003d self._get_image_details("}],"source_content_type":"text/x-python","patch_set":5,"id":"d0ad75c9_ab8d2dc2","line":842,"updated":"2025-11-05 10:02:24.000000000","message":"uppercase NOTE","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":888,"context_line":"                    \"enabled\": self._get_autoheal_enabled(cluster),"},{"line_number":889,"context_line":"                },"},{"line_number":890,"context_line":"            },"},{"line_number":891,"context_line":"            \"nodeGroups\": self._process_node_groups(cluster),"},{"line_number":892,"context_line":"            \"addons\": {"},{"line_number":893,"context_line":"                \"openstack\": {"},{"line_number":894,"context_line":"                    \"csiCinder\": self._storageclass_definitions("}],"source_content_type":"text/x-python","patch_set":5,"id":"e12567da_b76bde58","line":891,"updated":"2025-11-05 10:02:24.000000000","message":"ditto","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":1062,"context_line":"        self._create_appcred_secret(context, cluster)"},{"line_number":1063,"context_line":"        self._ensure_certificate_secrets(context, cluster)"},{"line_number":1064,"context_line":""},{"line_number":1065,"context_line":"        # Helm release is updated on next run of the reconciliation loop"},{"line_number":1066,"context_line":""},{"line_number":1067,"context_line":"    def update_cluster("},{"line_number":1068,"context_line":"        self, context, cluster, scale_manager\u003dNone, rollback\u003dFalse"}],"source_content_type":"text/x-python","patch_set":5,"id":"a0366945_b6f6ec21","line":1065,"updated":"2025-11-05 10:02:24.000000000","message":"ditto","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":1111,"context_line":"        if nodes_to_remove:"},{"line_number":1112,"context_line":"            LOG.warning(\"Removing specific nodes is not currently supported\")"},{"line_number":1113,"context_line":""},{"line_number":1114,"context_line":"        # Helm release is updated on next run of the reconciliation loop"},{"line_number":1115,"context_line":""},{"line_number":1116,"context_line":"    def upgrade_cluster("},{"line_number":1117,"context_line":"        self,"}],"source_content_type":"text/x-python","patch_set":5,"id":"1be8299e_b4235acd","line":1114,"updated":"2025-11-05 10:02:24.000000000","message":"ditto","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":1139,"context_line":"        cluster.save()"},{"line_number":1140,"context_line":"        cluster.refresh()"},{"line_number":1141,"context_line":""},{"line_number":1142,"context_line":"        # Helm release is updated on next run of the reconciliation loop"},{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"    def create_nodegroup(self, context, cluster, nodegroup):"},{"line_number":1145,"context_line":"        nodegroup.status \u003d fields.ClusterStatus.CREATE_IN_PROGRESS"}],"source_content_type":"text/x-python","patch_set":5,"id":"de48302d_40a489fe","line":1142,"updated":"2025-11-05 10:02:24.000000000","message":"ditto","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":1148,"context_line":"            self._validate_allowed_node_counts(cluster, nodegroup)"},{"line_number":1149,"context_line":"        nodegroup.save()"},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"        # Helm release is updated on next run of the reconciliation loop"},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"    def update_nodegroup(self, context, cluster, nodegroup):"},{"line_number":1154,"context_line":"        nodegroup.status \u003d fields.ClusterStatus.UPDATE_IN_PROGRESS"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f7c8462_b77430cd","line":1151,"updated":"2025-11-05 10:02:24.000000000","message":"ditto","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":1157,"context_line":"            self._validate_allowed_node_counts(cluster, nodegroup)"},{"line_number":1158,"context_line":"        nodegroup.save()"},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":"        # Helm release is updated on next run of the reconciliation loop"},{"line_number":1161,"context_line":""},{"line_number":1162,"context_line":"    def delete_nodegroup(self, context, cluster, nodegroup):"},{"line_number":1163,"context_line":"        nodegroup.status \u003d fields.ClusterStatus.DELETE_IN_PROGRESS"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a5e48ec_8d41fcc6","line":1160,"updated":"2025-11-05 10:02:24.000000000","message":"ditto","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":1164,"context_line":"        nodegroup.save()"},{"line_number":1165,"context_line":""},{"line_number":1166,"context_line":"        # Remove the nodegroup being deleted from the nodegroups"},{"line_number":1167,"context_line":"        # on next run of the reconciliation loop"},{"line_number":1168,"context_line":""},{"line_number":1169,"context_line":"    def create_federation(self, context, federation):"},{"line_number":1170,"context_line":"        raise NotImplementedError(\"Will not implement \u0027create_federation\u0027\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"de200146_c7b2bd66","line":1167,"updated":"2025-11-05 10:02:24.000000000","message":"ditto","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"}],"magnum_capi_helm/helm.py":[{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"40cbfcdd25130cbba095cff40d161c6a60207889","unresolved":true,"context_lines":[{"line_number":117,"context_line":"        except processutils.ProcessExecutionError as exc:"},{"line_number":118,"context_line":"            # If other updates are being applied concurrently, we can get:"},{"line_number":119,"context_line":"            # another operation (install/upgrade/rollback) is in progress"},{"line_number":120,"context_line":"            if ("},{"line_number":121,"context_line":"                exc.stderr"},{"line_number":122,"context_line":"                and ("},{"line_number":123,"context_line":"                    \"another operation (install/upgrade/rollback) is in \""}],"source_content_type":"text/x-python","patch_set":2,"id":"416fa638_9a3c8c3e","line":120,"updated":"2025-04-09 04:53:51.000000000","message":"This `if` statement needs brackets added.\n\nThis is currently in the format of `if (a and b or c)`, which should be changed to `if (a and (b or c))`, otherwise `c` is evaluated even when `a` evaluates to False.","commit_id":"04468dd8a0e6019197499b6805ca8ae5f2e39af9"},{"author":{"_account_id":10910,"name":"Stig Telfer","email":"stig.openstack@telfer.org","username":"stigtelfer"},"change_message_id":"1283b6d9f28c6ced8948e692f0b430a04ca67ef1","unresolved":false,"context_lines":[{"line_number":117,"context_line":"        except processutils.ProcessExecutionError as exc:"},{"line_number":118,"context_line":"            # If other updates are being applied concurrently, we can get:"},{"line_number":119,"context_line":"            # another operation (install/upgrade/rollback) is in progress"},{"line_number":120,"context_line":"            if ("},{"line_number":121,"context_line":"                exc.stderr"},{"line_number":122,"context_line":"                and ("},{"line_number":123,"context_line":"                    \"another operation (install/upgrade/rollback) is in \""}],"source_content_type":"text/x-python","patch_set":2,"id":"b72ff63b_e429a174","line":120,"in_reply_to":"416fa638_9a3c8c3e","updated":"2025-04-10 12:33:22.000000000","message":"Thanks Dale, good catch","commit_id":"04468dd8a0e6019197499b6805ca8ae5f2e39af9"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"af2f1cb1ebbfa6408e6f46942ec14547d9faecd2","unresolved":true,"context_lines":[{"line_number":206,"context_line":"            )"},{"line_number":207,"context_line":"            return True"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        if chart_meta[\"chart\"] !\u003d chart_name:"},{"line_number":210,"context_line":"            LOG.debug("},{"line_number":211,"context_line":"                f\"{release_name}: chart name update \""},{"line_number":212,"context_line":"                f\"{chart_meta[\u0027chart\u0027]}-\u003e{chart_name}: \""}],"source_content_type":"text/x-python","patch_set":3,"id":"cc1dec0e_74178f5a","line":209,"updated":"2025-04-14 00:35:08.000000000","message":"When I run this, the value for chart_name is `oci://example.com/capi-helm-charts/openstack-cluster`, and the chart_meta[\"chart\"] value is `openstack-cluster`.\n\nThis causes repeated `helm upgrade` every update loop, while the cluster is creating which doesn\u0027t seem ideal.\n\nDo I have an \u0027odd\u0027 config compared to what you use, or could this check be improved to a more accurate one?","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":36412,"name":"Scott Davidson","email":"sdavidson327@gmail.com","username":"sd109"},"change_message_id":"4a83c3aee0c35e678ecbfa3ac3a3b659ab7ea721","unresolved":true,"context_lines":[{"line_number":206,"context_line":"            )"},{"line_number":207,"context_line":"            return True"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        if chart_meta[\"chart\"] !\u003d chart_name:"},{"line_number":210,"context_line":"            LOG.debug("},{"line_number":211,"context_line":"                f\"{release_name}: chart name update \""},{"line_number":212,"context_line":"                f\"{chart_meta[\u0027chart\u0027]}-\u003e{chart_name}: \""}],"source_content_type":"text/x-python","patch_set":3,"id":"e0d80a7b_92b3200f","line":209,"in_reply_to":"0fa36769_3c31471d","updated":"2025-11-06 10:27:29.000000000","message":"This looks like a difference between using a standard Helm chart repository [1] for publishing the charts, which is what we do in azimuth-cloud/capi-helm-charts, and using an OCI registry to publish the Helm charts [2], which I presume is what Dale is using to publish his fork of the charts. Since Helm supports both methods, the driver should probably support both too.\n\n[1] https://helm.sh/docs/topics/chart_repository\n[2] https://helm.sh/docs/topics/registries/","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":34213,"name":"Grzegorz Bialas","display_name":"Grzegorz Bialas","email":"grzegorz@stackhpc.com","username":"gbialas"},"change_message_id":"221fda738e7a92add855dd0bada795c0169710e6","unresolved":true,"context_lines":[{"line_number":206,"context_line":"            )"},{"line_number":207,"context_line":"            return True"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        if chart_meta[\"chart\"] !\u003d chart_name:"},{"line_number":210,"context_line":"            LOG.debug("},{"line_number":211,"context_line":"                f\"{release_name}: chart name update \""},{"line_number":212,"context_line":"                f\"{chart_meta[\u0027chart\u0027]}-\u003e{chart_name}: \""}],"source_content_type":"text/x-python","patch_set":3,"id":"0fa36769_3c31471d","line":209,"in_reply_to":"cc1dec0e_74178f5a","updated":"2025-11-04 12:46:02.000000000","message":"Hi! How is your config different from standard one? How did you triggered this?  I can’t reproduce this. I have addd some additional logging and it always give me this: \n2025-11-04 12:37:44.483 7 DEBUG magnum_capi_helm.helm [-] whitey1-wu2bvasx5p2m: whitey_debug name: openstack-cluster-\u003eopenstack-cluster status deployedversion: 0.10.1-\u003e0.10.1","commit_id":"e818aa503546fb43b9149c26f3e05e4ad646d486"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":195,"context_line":"                raise"},{"line_number":196,"context_line":"            return True"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        if ("},{"line_number":199,"context_line":"            \"revision\" not in chart_meta"},{"line_number":200,"context_line":"            or \"chart\" not in chart_meta"},{"line_number":201,"context_line":"            or \"status\" not in chart_meta"}],"source_content_type":"text/x-python","patch_set":5,"id":"3d310d27_65ed132f","line":198,"updated":"2025-11-05 10:02:24.000000000","message":"define a list of strings to check and convert that to a loop?","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"ea2dce0a7decf9bb23a99c8898319dba9fe8c207","unresolved":true,"context_lines":[{"line_number":202,"context_line":"            or \"version\" not in chart_meta"},{"line_number":203,"context_line":"        ):"},{"line_number":204,"context_line":"            LOG.debug("},{"line_number":205,"context_line":"                f\"{release_name}: incomplete chart metadata: update needed\""},{"line_number":206,"context_line":"            )"},{"line_number":207,"context_line":"            return True"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"811f382a_7f7c6d08","line":205,"updated":"2025-11-05 10:02:24.000000000","message":"All these debug logs are nice, but the operator or developer doesn\u0027t really know what do they relate to - should we add a cluster uuid, anything?","commit_id":"1cd4c31773f69d10dcb1aaaccb16cfcc7bf5e3b3"}]}