)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"7862891d402d630df632734df967fa4a46300153","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1899d3cc_3fee527e","updated":"2024-01-19 03:55:46.000000000","message":"Needs test cases for the additional control plane resize feature.\n\nAlso noteworthy that the API is referencing the driver here, perhaps the same could be achieved with RPC calls to the conductor to maintain separation. Thoughts on this appreciated.","commit_id":"2d501bc62ca1880366532dbad9afc15f0403afb4"},{"author":{"_account_id":8064,"name":"Jake Yip","email":"jake.yip@ardc.edu.au","username":"jake"},"change_message_id":"f6b777b4f5adb8bda8f3fb3239f02f5900602bf3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6c0947ba_5cb4f125","updated":"2024-01-31 08:28:37.000000000","message":"-1 for visibility, let\u0027s discuss in meeting","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"}],"magnum/api/controllers/v1/cluster.py":[{"author":{"_account_id":8064,"name":"Jake Yip","email":"jake.yip@ardc.edu.au","username":"jake"},"change_message_id":"f6b777b4f5adb8bda8f3fb3239f02f5900602bf3","unresolved":true,"context_lines":[{"line_number":482,"context_line":"    @base.Controller.api_version(\"1.10\")  # noqa"},{"line_number":483,"context_line":"    @expose.expose(ClusterID, body\u003dCluster, status_code\u003d202)"},{"line_number":484,"context_line":"    @validation.enforce_cluster_type_supported()"},{"line_number":485,"context_line":"    @validation.enforce_cluster_volume_storage_size()"},{"line_number":486,"context_line":"    def post(self, cluster):  # noqa"},{"line_number":487,"context_line":"        return self._post(cluster)"},{"line_number":488,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"434b2103_661d96c6","line":485,"updated":"2024-01-31 08:28:37.000000000","message":"you may want to try doing the validation here, as a new decorator. the validation class have access to magnum.drivers.common.driver already.","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"43fd07f8e6961d465a9b01dff08056a51b29ada8","unresolved":false,"context_lines":[{"line_number":482,"context_line":"    @base.Controller.api_version(\"1.10\")  # noqa"},{"line_number":483,"context_line":"    @expose.expose(ClusterID, body\u003dCluster, status_code\u003d202)"},{"line_number":484,"context_line":"    @validation.enforce_cluster_type_supported()"},{"line_number":485,"context_line":"    @validation.enforce_cluster_volume_storage_size()"},{"line_number":486,"context_line":"    def post(self, cluster):  # noqa"},{"line_number":487,"context_line":"        return self._post(cluster)"},{"line_number":488,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"69799101_71910dba","line":485,"in_reply_to":"2714b33a_3f846fda","updated":"2024-05-29 02:27:15.000000000","message":"Done","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"35665b8a2cc38e46afb4cb38463404234f552b1d","unresolved":true,"context_lines":[{"line_number":482,"context_line":"    @base.Controller.api_version(\"1.10\")  # noqa"},{"line_number":483,"context_line":"    @expose.expose(ClusterID, body\u003dCluster, status_code\u003d202)"},{"line_number":484,"context_line":"    @validation.enforce_cluster_type_supported()"},{"line_number":485,"context_line":"    @validation.enforce_cluster_volume_storage_size()"},{"line_number":486,"context_line":"    def post(self, cluster):  # noqa"},{"line_number":487,"context_line":"        return self._post(cluster)"},{"line_number":488,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"2714b33a_3f846fda","line":485,"in_reply_to":"434b2103_661d96c6","updated":"2024-02-29 03:07:58.000000000","message":"This works okay, and good to confirm that other code in magnum-api is doing driver lookups, so this isn\u0027t a new concept. I\u0027ll make this change so we can see what it looks like.\n\nThis decorator covers the create cluster action, the resize action (in cluster_actions.py) would be possible to do with a decorator but the existing `_resize` function already does several validation steps, so it\u0027d be going against the grain to use the decorator pattern there.","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"}],"magnum/conductor/handlers/cluster_conductor.py":[{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"2154fbe875921260497b873063e4280d5a9530b4","unresolved":true,"context_lines":[{"line_number":308,"context_line":"        allow_update_status \u003d ("},{"line_number":309,"context_line":"            fields.ClusterStatus.CREATE_COMPLETE,"},{"line_number":310,"context_line":"            fields.ClusterStatus.UPDATE_COMPLETE,"},{"line_number":311,"context_line":"            fields.ClusterStatus.UPDATE_FAILED,"},{"line_number":312,"context_line":"            fields.ClusterStatus.RESUME_COMPLETE,"},{"line_number":313,"context_line":"            fields.ClusterStatus.RESTORE_COMPLETE,"},{"line_number":314,"context_line":"            fields.ClusterStatus.ROLLBACK_COMPLETE,"}],"source_content_type":"text/x-python","patch_set":2,"id":"2b907c60_9e322852","line":311,"updated":"2024-01-24 20:19:13.000000000","message":"As discussed in meeting - I will remove this, UPDATE_FAILED covers too many failure cases to assume it\u0027s safe to upgrade. Some are safe, but likely some are not.","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"f4636717ce4afb680607d19e210fb128d7462d6d","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        allow_update_status \u003d ("},{"line_number":309,"context_line":"            fields.ClusterStatus.CREATE_COMPLETE,"},{"line_number":310,"context_line":"            fields.ClusterStatus.UPDATE_COMPLETE,"},{"line_number":311,"context_line":"            fields.ClusterStatus.UPDATE_FAILED,"},{"line_number":312,"context_line":"            fields.ClusterStatus.RESUME_COMPLETE,"},{"line_number":313,"context_line":"            fields.ClusterStatus.RESTORE_COMPLETE,"},{"line_number":314,"context_line":"            fields.ClusterStatus.ROLLBACK_COMPLETE,"}],"source_content_type":"text/x-python","patch_set":2,"id":"afde10af_b6f82ba7","line":311,"in_reply_to":"2b907c60_9e322852","updated":"2024-02-29 04:11:50.000000000","message":"Done","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"}],"magnum/drivers/common/driver.py":[{"author":{"_account_id":8064,"name":"Jake Yip","email":"jake.yip@ardc.edu.au","username":"jake"},"change_message_id":"f6b777b4f5adb8bda8f3fb3239f02f5900602bf3","unresolved":true,"context_lines":[{"line_number":193,"context_line":"        raise NotImplementedError(\"Subclasses must implement \""},{"line_number":194,"context_line":"                                  \"\u0027resize_cluster\u0027.\")"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    @property"},{"line_number":197,"context_line":"    def supported_master_sizes(self):"},{"line_number":198,"context_line":"        # Permitted control plane sizes are low uneven numbers."},{"line_number":199,"context_line":"        # This maintains best features of etcd\u0027s raft consensus."},{"line_number":200,"context_line":"        return [1, 3, 5, 7]"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def validate_master_size(self, node_count):"},{"line_number":203,"context_line":"        if node_count not in self.supported_master_sizes:"}],"source_content_type":"text/x-python","patch_set":2,"id":"6739199a_085284a2","line":200,"range":{"start_line":196,"start_character":0,"end_line":200,"end_character":27},"updated":"2024-01-31 08:28:37.000000000","message":"This is a driver level config and maybe should not be in common?","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"94b9d6bf54335551aafc5ef57b3dcb6e3518d0d2","unresolved":true,"context_lines":[{"line_number":193,"context_line":"        raise NotImplementedError(\"Subclasses must implement \""},{"line_number":194,"context_line":"                                  \"\u0027resize_cluster\u0027.\")"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    @property"},{"line_number":197,"context_line":"    def supported_master_sizes(self):"},{"line_number":198,"context_line":"        # Permitted control plane sizes are low uneven numbers."},{"line_number":199,"context_line":"        # This maintains best features of etcd\u0027s raft consensus."},{"line_number":200,"context_line":"        return [1, 3, 5, 7]"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def validate_master_size(self, node_count):"},{"line_number":203,"context_line":"        if node_count not in self.supported_master_sizes:"}],"source_content_type":"text/x-python","patch_set":2,"id":"8bf74668_49810680","line":200,"range":{"start_line":196,"start_character":0,"end_line":200,"end_character":27},"in_reply_to":"2c07ba39_8b9e0e2c","updated":"2024-07-13 15:21:24.000000000","message":"I feel like a mod operation would be better as a default, then at least on our side we wouldn\u0027t have to make any changes.","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"35665b8a2cc38e46afb4cb38463404234f552b1d","unresolved":true,"context_lines":[{"line_number":193,"context_line":"        raise NotImplementedError(\"Subclasses must implement \""},{"line_number":194,"context_line":"                                  \"\u0027resize_cluster\u0027.\")"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    @property"},{"line_number":197,"context_line":"    def supported_master_sizes(self):"},{"line_number":198,"context_line":"        # Permitted control plane sizes are low uneven numbers."},{"line_number":199,"context_line":"        # This maintains best features of etcd\u0027s raft consensus."},{"line_number":200,"context_line":"        return [1, 3, 5, 7]"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def validate_master_size(self, node_count):"},{"line_number":203,"context_line":"        if node_count not in self.supported_master_sizes:"}],"source_content_type":"text/x-python","patch_set":2,"id":"2c07ba39_8b9e0e2c","line":200,"range":{"start_line":196,"start_character":0,"end_line":200,"end_character":27},"in_reply_to":"6739199a_085284a2","updated":"2024-02-29 03:07:58.000000000","message":"I think some value does belong here, it\u0027s the default applied to all clusters and used in `validate_master_size` on cluster creation.\n\nThis function is the default allowed and is intended to be used for validation in two places:\n1) Creation of a cluster. `POST /v1/clusters` validates with `validate_master_size`\n2) Resize of a cluster\u0027s master nodegroup. `POST /v1/clusters/{cluster_ident}/actions/resize` validates with `validate_master_resize`.\n\nThe driver can then choose to override the `supported_master_sizes` property if desired, to support a different set of valid sizes - or ignore the property altogether by overriding these new functions `validate_master_size` and `validate_master_resize`.\n\nCurrently in all Magnum drivers, there are wsme restrictions for `\u003e0` in `master_count`, and validation to allow exactly 1 when `master_lb_enabled` is False, but nothing disallowing an even numbered set of nodes on cluster create.\n\nThis patchset does change that, so a release note seems appropriate, but I\u0027m okay with this low-and-odd-numbered limitation by default for all drivers. What do you think?","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"82443a8ddc83b2e82d43e3e9b981a833706a8535","unresolved":true,"context_lines":[{"line_number":193,"context_line":"        raise NotImplementedError(\"Subclasses must implement \""},{"line_number":194,"context_line":"                                  \"\u0027resize_cluster\u0027.\")"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    @property"},{"line_number":197,"context_line":"    def supported_master_sizes(self):"},{"line_number":198,"context_line":"        # Permitted control plane sizes are low uneven numbers."},{"line_number":199,"context_line":"        # This maintains best features of etcd\u0027s raft consensus."},{"line_number":200,"context_line":"        return [1, 3, 5, 7]"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def validate_master_size(self, node_count):"},{"line_number":203,"context_line":"        if node_count not in self.supported_master_sizes:"}],"source_content_type":"text/x-python","patch_set":2,"id":"8e734c73_1b7bdf54","line":200,"range":{"start_line":196,"start_character":0,"end_line":200,"end_character":27},"in_reply_to":"8bf74668_49810680","updated":"2024-07-15 01:47:27.000000000","message":"Hi Mohammend, thanks for the comment.\n\nI want to make this as usable as possible to all drivers, so your feedback is helpful - could you link to the relevant code in the Vexxhost driver?\n\nI\u0027m not sure what you mean by mod; do you mean changing the function signature so it takes the new value and instead of returning a list it will validate with a True/False value? Or do you mean just creating this list in a different way?","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"0f35fc476fad24b0e775a39a05b25f7c8f5d37fc","unresolved":true,"context_lines":[{"line_number":193,"context_line":"        raise NotImplementedError(\"Subclasses must implement \""},{"line_number":194,"context_line":"                                  \"\u0027resize_cluster\u0027.\")"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    @property"},{"line_number":197,"context_line":"    def supported_master_sizes(self):"},{"line_number":198,"context_line":"        # Permitted control plane sizes are low uneven numbers."},{"line_number":199,"context_line":"        # This maintains best features of etcd\u0027s raft consensus."},{"line_number":200,"context_line":"        return [1, 3, 5, 7]"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def validate_master_size(self, node_count):"},{"line_number":203,"context_line":"        if node_count not in self.supported_master_sizes:"}],"source_content_type":"text/x-python","patch_set":2,"id":"bd399919_836dc734","line":200,"range":{"start_line":196,"start_character":0,"end_line":200,"end_character":27},"in_reply_to":"8e734c73_1b7bdf54","updated":"2024-07-17 23:12:53.000000000","message":"So the relevant restrictions are written in Cluster API with a stacked etcd: https://github.com/kubernetes-sigs/cluster-api/blob/main/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go#L334\n\nI note the Vexxhost driver validates control plane size with the mod operator, which matches the above: https://github.com/vexxhost/magnum-cluster-api/blob/main/magnum_cluster_api/utils.py#L370 \n\nThese don\u0027t set any upper bounds, and while it\u0027s reasonable to have an upper bound we don\u0027t **need** to set one - so perhaps the question is: Should Magnum be opinionated and set any upper bounds on control plane size?\n\nA driver can override this function and return any list of valid sizes it likes, eg. `return [n for n in range(1,100) if n%2 !\u003d 0]`.","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"abc9ea597d6eac7eb46feea5d8f94f6d4ff0d166","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        raise NotImplementedError(\"Subclasses must implement \""},{"line_number":194,"context_line":"                                  \"\u0027resize_cluster\u0027.\")"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    @property"},{"line_number":197,"context_line":"    def supported_master_sizes(self):"},{"line_number":198,"context_line":"        # Permitted control plane sizes are low uneven numbers."},{"line_number":199,"context_line":"        # This maintains best features of etcd\u0027s raft consensus."},{"line_number":200,"context_line":"        return [1, 3, 5, 7]"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def validate_master_size(self, node_count):"},{"line_number":203,"context_line":"        if node_count not in self.supported_master_sizes:"}],"source_content_type":"text/x-python","patch_set":2,"id":"d180f7c7_32e3e9fd","line":200,"range":{"start_line":196,"start_character":0,"end_line":200,"end_character":27},"in_reply_to":"bd399919_836dc734","updated":"2024-09-23 01:41:28.000000000","message":"I\u0027ve refactored this patchset to remove the upper limit and only test for even numbers in the size validation function.\n\nThis matches the above linked code in Cluster API, and simplifies the logic to a modulo.","commit_id":"eb7d92e5027a506e21818549205e6171a3b28b60"}]}