)]}'
{"specs/ussuri/labels-override.rst":[{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Magnum Labels Override"},{"line_number":2,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"Problem Description"},{"line_number":5,"context_line":"-------------------"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Magnum accepts labels at cluster or nodegroup creation in the format of"}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_a5e26bd3","line":4,"updated":"2020-04-02 08:25:14.000000000","message":"section LGTM","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"7751b761112e2a9282f09fd096dceb75c37427b4","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Magnum Labels Override"},{"line_number":2,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"Problem Description"},{"line_number":5,"context_line":"-------------------"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Magnum accepts labels at cluster or nodegroup creation in the format of"}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_083ecbbf","line":4,"in_reply_to":"df33271e_a5e26bd3","updated":"2020-04-02 09:12:13.000000000","message":"+1","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"This behavior, forces users to provide the full set of configured labels when"},{"line_number":20,"context_line":"in order to change a subset of them."},{"line_number":21,"context_line":"At the same time, it is very difficult for operators to keep track of the"},{"line_number":22,"context_line":"provided user labels when a problem occurs."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"The proposal tries to address the problem described above."},{"line_number":25,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_c5916f49","line":22,"range":{"start_line":21,"start_character":0,"end_line":22,"end_character":43},"updated":"2020-04-02 08:25:14.000000000","message":"+10 here","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"7751b761112e2a9282f09fd096dceb75c37427b4","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"This behavior, forces users to provide the full set of configured labels when"},{"line_number":20,"context_line":"in order to change a subset of them."},{"line_number":21,"context_line":"At the same time, it is very difficult for operators to keep track of the"},{"line_number":22,"context_line":"provided user labels when a problem occurs."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"The proposal tries to address the problem described above."},{"line_number":25,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_884f7b69","line":22,"range":{"start_line":21,"start_character":0,"end_line":22,"end_character":43},"in_reply_to":"df33271e_c5916f49","updated":"2020-04-02 09:12:13.000000000","message":"+1, needs newline above.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":24,"context_line":"The proposal tries to address the problem described above."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Use Cases"},{"line_number":28,"context_line":"---------"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Below are some of the use cases:"}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_e5e8f3b3","line":27,"updated":"2020-04-02 08:25:14.000000000","message":"section LGTM","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"7751b761112e2a9282f09fd096dceb75c37427b4","unresolved":false,"context_lines":[{"line_number":24,"context_line":"The proposal tries to address the problem described above."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Use Cases"},{"line_number":28,"context_line":"---------"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Below are some of the use cases:"}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_483453de","line":27,"in_reply_to":"df33271e_e5e8f3b3","updated":"2020-04-02 09:12:13.000000000","message":"+1","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"The proposed change includes:"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"* A new field will be added in the Cluster and Nodegroup objects where the "},{"line_number":49,"context_line":"  labels provided by the user will be stored."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"* Drivers and the conductor, will be adapted to respect the labels inheritance."}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_c5ba0fbf","line":48,"range":{"start_line":48,"start_character":74,"end_line":48,"end_character":75},"updated":"2020-04-02 08:25:14.000000000","message":"whitespace","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":53,"context_line":"* The client and the API will be adapted, in order to allow users to provide"},{"line_number":54,"context_line":"  user labels at creation time."},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"* The key/value pairs provided as user_labels, will be used to update the"},{"line_number":57,"context_line":"  dictionary of labels configured in the above level."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"Check sections \u0027Data Model Impact\u0027 and \u0027REST API Impact\u0027 for more details."}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_c57b2ff2","line":56,"range":{"start_line":56,"start_character":34,"end_line":56,"end_character":45},"updated":"2020-04-02 08:25:14.000000000","message":"This is the first mention of this term. We either need to push explain it fully or leave it for the data plane.\n\nWe can drop this bullet point and explain in Data Model Impact.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"7751b761112e2a9282f09fd096dceb75c37427b4","unresolved":false,"context_lines":[{"line_number":62,"context_line":"Alternatives"},{"line_number":63,"context_line":"------------"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"(input needed)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Data Model Impact"}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_089a4bb7","line":65,"range":{"start_line":65,"start_character":0,"end_line":65,"end_character":14},"updated":"2020-04-02 09:12:13.000000000","message":"Alternatives we have discussed are appending the passed label with a prefix, e.g. --labels\u003d-key1\u003d,+key2\u003dvalue2 where \"-\" \u003d\u003d do not inherit this label, \"+\" \u003d\u003d merge this label. If there are any keys without a label, replace all labels as before and the +, - prefixes will have no effect. No doubt this is simpler but a much uglier solution.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":65,"context_line":"(input needed)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Data Model Impact"},{"line_number":69,"context_line":"-----------------"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"A new field will be added to the Cluster and Nodegroup objects. The new field"}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_a551cb38","line":68,"updated":"2020-04-02 08:25:14.000000000","message":"Needs more info with the new fields in DB. What are the going to store and names for the columns.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":78,"context_line":"* make the functionality backward compatible"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"REST API Impact"},{"line_number":82,"context_line":"---------------"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"This change leads to a minor version increase in the Magnum API. "}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_a51f0b6a","line":81,"updated":"2020-04-02 08:25:14.000000000","message":"This section can be limited in describing the new API microversion and API compatibility.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"eb6e4208d94400f33e48dbec4a0100109f0d953b","unresolved":false,"context_lines":[{"line_number":83,"context_line":""},{"line_number":84,"context_line":"This change leads to a minor version increase in the Magnum API. "},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"The post methods of Clusters and Nodegroups APIs will be adapted as shown"},{"line_number":87,"context_line":"below:"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"* Old APIs will raise an exception if user_labels are provided."}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_748f7229","line":86,"range":{"start_line":86,"start_character":2,"end_line":86,"end_character":16},"updated":"2020-04-02 12:46:05.000000000","message":"What is the impact of the update method if the user decides to specify kube_tag for example?","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"6eca5862f8f1d5b78f8723b4138f97cc924bee9b","unresolved":false,"context_lines":[{"line_number":83,"context_line":""},{"line_number":84,"context_line":"This change leads to a minor version increase in the Magnum API. "},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"The post methods of Clusters and Nodegroups APIs will be adapted as shown"},{"line_number":87,"context_line":"below:"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"* Old APIs will raise an exception if user_labels are provided."}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_5f1b6771","line":86,"range":{"start_line":86,"start_character":2,"end_line":86,"end_character":16},"in_reply_to":"df33271e_748f7229","updated":"2020-04-02 13:10:04.000000000","message":"There was never and will never be available a PATCH method for labels.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"* Old APIs will raise an exception if user_labels are provided."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"* New APIs will allow users to provide either labels or user_labels. In case"},{"line_number":92,"context_line":"  both fields are provided the API will raise an exception."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The commands will be adapted:"}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_e53d92bb","line":91,"range":{"start_line":91,"start_character":22,"end_line":91,"end_character":27},"updated":"2020-04-02 08:25:14.000000000","message":"let\u0027s call it client. Terraform may be a user or horizon.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"eb6e4208d94400f33e48dbec4a0100109f0d953b","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"* Old APIs will raise an exception if user_labels are provided."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"* New APIs will allow users to provide either labels or user_labels. In case"},{"line_number":92,"context_line":"  both fields are provided the API will raise an exception."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The commands will be adapted:"}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_74bdb2b5","line":91,"range":{"start_line":91,"start_character":46,"end_line":91,"end_character":67},"updated":"2020-04-02 12:46:05.000000000","message":"so, we are phasing out labels? Until a while ago i though labels and user_labels where the same thing...\nif yes, why not use set/unset?","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"6eca5862f8f1d5b78f8723b4138f97cc924bee9b","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"* Old APIs will raise an exception if user_labels are provided."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"* New APIs will allow users to provide either labels or user_labels. In case"},{"line_number":92,"context_line":"  both fields are provided the API will raise an exception."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The commands will be adapted:"}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_7fa0ebf4","line":91,"range":{"start_line":91,"start_character":46,"end_line":91,"end_character":67},"in_reply_to":"df33271e_74bdb2b5","updated":"2020-04-02 13:10:04.000000000","message":"Please don\u0027t highjack here the discuss with something that doesn\u0027t exists. set/unset was never used in magnum.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":89,"context_line":"* Old APIs will raise an exception if user_labels are provided."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"* New APIs will allow users to provide either labels or user_labels. In case"},{"line_number":92,"context_line":"  both fields are provided the API will raise an exception."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The commands will be adapted:"},{"line_number":95,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_a571caaa","line":92,"updated":"2020-04-02 08:25:14.000000000","message":"We need to describe the mode of operation. What the conductor will do and when.\n\nHow it will update the dict of labels. CT -\u003e cluster -\u003e nodegroup","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":91,"context_line":"* New APIs will allow users to provide either labels or user_labels. In case"},{"line_number":92,"context_line":"  both fields are provided the API will raise an exception."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The commands will be adapted:"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"* create cluster: create cluster overriding a specific set of labels::"},{"line_number":97,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_254e5a37","line":94,"updated":"2020-04-02 08:25:14.000000000","message":"We can break this to another section and ask reviewers for consensus from top tp bottom per section, to make sure we are all on the same page.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"cb77a96d56993f816a44586c6bb10bf428a53611","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"* create nodegroup: create a nodegroup overriding a specific set of labels::"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    openstack coe nodegroup create --user-labels label1\u003dvalue1 ...         "},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"The get methods of Clusters and Nodegroups APIs will be adapted to show the"},{"line_number":105,"context_line":"labels provided by the user."}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_4b08e426","line":102,"range":{"start_line":102,"start_character":35,"end_line":102,"end_character":48},"updated":"2020-04-01 13:54:53.000000000","message":"if this is user labels, what is simply --labels? is that admin label? i think we need to call this something like --merge-labels to make its purpose clear?","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"a607c4d9975e5192b383679895dc2546d110ae77","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"* create nodegroup: create a nodegroup overriding a specific set of labels::"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    openstack coe nodegroup create --user-labels label1\u003dvalue1 ...         "},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"The get methods of Clusters and Nodegroups APIs will be adapted to show the"},{"line_number":105,"context_line":"labels provided by the user."}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_8b3e6cbe","line":102,"range":{"start_line":102,"start_character":66,"end_line":102,"end_character":75},"updated":"2020-04-01 13:56:31.000000000","message":"s/ $//g","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"eb6e4208d94400f33e48dbec4a0100109f0d953b","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"* create nodegroup: create a nodegroup overriding a specific set of labels::"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    openstack coe nodegroup create --user-labels label1\u003dvalue1 ...         "},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"The get methods of Clusters and Nodegroups APIs will be adapted to show the"},{"line_number":105,"context_line":"labels provided by the user."}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_5447cebc","line":102,"range":{"start_line":102,"start_character":35,"end_line":102,"end_character":48},"in_reply_to":"df33271e_45971f49","updated":"2020-04-02 12:46:05.000000000","message":"-1 on something labels.\nI am confused.\n\nNeeds explanation:\nWhat is going to happen to labels and how it will behave\nWhat is going to happen to \u003cnew_parameter\u003e and how it will behave (i think the behave part is understood that it is going to be saved in a new db clumn)","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"* create nodegroup: create a nodegroup overriding a specific set of labels::"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    openstack coe nodegroup create --user-labels label1\u003dvalue1 ...         "},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"The get methods of Clusters and Nodegroups APIs will be adapted to show the"},{"line_number":105,"context_line":"labels provided by the user."}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_45971f49","line":102,"range":{"start_line":102,"start_character":35,"end_line":102,"end_character":48},"in_reply_to":"df33271e_4b08e426","updated":"2020-04-02 08:25:14.000000000","message":"-1 to merge labels. add-labels or extra-labels makes sense.\n\nI propose to start from the sections above this and have concensus step by step.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"6eca5862f8f1d5b78f8723b4138f97cc924bee9b","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"* create nodegroup: create a nodegroup overriding a specific set of labels::"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    openstack coe nodegroup create --user-labels label1\u003dvalue1 ...         "},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"The get methods of Clusters and Nodegroups APIs will be adapted to show the"},{"line_number":105,"context_line":"labels provided by the user."}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_dff13703","line":102,"range":{"start_line":102,"start_character":35,"end_line":102,"end_character":48},"in_reply_to":"df33271e_5447cebc","updated":"2020-04-02 13:10:04.000000000","message":"@Diogo, please ask somethis specific. One at a time.\n\nIt would help if you and reply to the sections above first. This way we are on the same page.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"eb6e4208d94400f33e48dbec4a0100109f0d953b","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    openstack coe nodegroup create --user-labels label1\u003dvalue1 ...         "},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"The get methods of Clusters and Nodegroups APIs will be adapted to show the"},{"line_number":105,"context_line":"labels provided by the user."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"Other Implementation Options"}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_54e34ea4","line":105,"range":{"start_line":104,"start_character":0,"end_line":105,"end_character":28},"updated":"2020-04-02 12:46:05.000000000","message":"the default get method should show the cluster/NG template+\u003cnew_parameter\u003e/Cluster+\u003cnew_parameter\u003e as the labels committed to the cluster/NG.\n\nSame thing for NG as cluster_labels+\u003cnew_parameter\u003e and not only the user defined labels.\n\nIf you would prefer this would be a option lake --\u003cnew_parameter_only\u003e *that can be added later\n\nref: https://helm.sh/docs/helm/helm_get_values/","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"6eca5862f8f1d5b78f8723b4138f97cc924bee9b","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    openstack coe nodegroup create --user-labels label1\u003dvalue1 ...         "},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"The get methods of Clusters and Nodegroups APIs will be adapted to show the"},{"line_number":105,"context_line":"labels provided by the user."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"Other Implementation Options"}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_5fe52737","line":105,"range":{"start_line":104,"start_character":0,"end_line":105,"end_character":28},"in_reply_to":"df33271e_54e34ea4","updated":"2020-04-02 13:10:04.000000000","message":"This comment seems out of context. Please read the sections above. We need to have concensus on the DB first. Let\u0027s review PatchSet 3.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"292b5a36a2d0037f39862b43e70a7fbca0412631","unresolved":false,"context_lines":[{"line_number":128,"context_line":"The old way of providing labels (via --labels) will still be supported."},{"line_number":129,"context_line":""},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"Implementation"},{"line_number":132,"context_line":"--------------"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"1. Add the new field to Cluster and Nodegroup objects."}],"source_content_type":"text/x-rst","patch_set":1,"id":"df33271e_853926a5","line":131,"updated":"2020-04-02 08:25:14.000000000","message":"this section is ok. It can live here with a few short bullet points, even if we describe inhetitance and objects above.","commit_id":"c9583abfd06bf6f7020fe6ff90188189fd460087"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"0264ebffe70a7479712238bfc50e6a4070de5ac1","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"3. As an operator, I want to be able to properly track the labels that the"},{"line_number":41,"context_line":"   user provided while creating a cluster or nodegroup."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"Proposed Changes"},{"line_number":45,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"df33271e_a87a2920","line":42,"updated":"2020-04-04 20:19:03.000000000","message":"Though this maybe out the scope of this spec, but I\u0027d like to see we may have the capability to disable a particular addon. For example, when creating the cluster, k8s_keystone_auth is enabled, but after created the cluster, I want to disable it.","commit_id":"aff480bf84211cb6ef890fd45789eba8f4d6b119"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"0264ebffe70a7479712238bfc50e6a4070de5ac1","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The proposed change includes:"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"* A new field will be added in the Cluster and Nodegroup objects where the"},{"line_number":50,"context_line":"  labels provided by the user will be stored."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"* Drivers and the conductor, will be adapted to respect the labels inheritance."},{"line_number":53,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"df33271e_689ae161","line":50,"range":{"start_line":49,"start_character":71,"end_line":50,"end_character":29},"updated":"2020-04-04 20:19:03.000000000","message":"This is not a clear description. I\u0027d like to see we mention that \u0027comparing with the labels defined in the template\u0027.","commit_id":"aff480bf84211cb6ef890fd45789eba8f4d6b119"},{"author":{"_account_id":9995,"name":"Ricardo Rocha","email":"rocha.porto@gmail.com","username":"rocha"},"change_message_id":"1a366e6b4ca6f56c94753789aaa882db5a5f2eee","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":"* create cluster: create cluster overriding a specific set of labels::"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    openstack coe cluster create --add-labels label1\u003dvalue1 ..."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"* create nodegroup: create a nodegroup overriding a specific set of labels::"},{"line_number":117,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"df33271e_5a1b7571","line":114,"updated":"2020-04-02 13:24:13.000000000","message":"Why not just --user-labels to be consistent with the db field as well? --add-labels is not very accurate, you might be overriding with null values.","commit_id":"aff480bf84211cb6ef890fd45789eba8f4d6b119"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"6b1e30e5f361c0ddb2db1df5b82a44ee978b06bc","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":"* create cluster: create cluster overriding a specific set of labels::"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    openstack coe cluster create --add-labels label1\u003dvalue1 ..."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"* create nodegroup: create a nodegroup overriding a specific set of labels::"},{"line_number":117,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"df33271e_fad7e9bd","line":114,"in_reply_to":"df33271e_5a1b7571","updated":"2020-04-02 13:42:04.000000000","message":"Is overriding with null value\u003d\u003dremoving the inherited label? I think this would be a good idea. What about --labels-override as is the name of the spec?","commit_id":"aff480bf84211cb6ef890fd45789eba8f4d6b119"},{"author":{"_account_id":9995,"name":"Ricardo Rocha","email":"rocha.porto@gmail.com","username":"rocha"},"change_message_id":"4f505ef07ad06e390e945e657e88733abb18a144","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":"* create cluster: create cluster overriding a specific set of labels::"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    openstack coe cluster create --add-labels label1\u003dvalue1 ..."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"* create nodegroup: create a nodegroup overriding a specific set of labels::"},{"line_number":117,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"df33271e_5f423410","line":114,"in_reply_to":"df33271e_fad7e9bd","updated":"2020-04-03 06:59:38.000000000","message":"Both --user-labels and --override-labels sound good.","commit_id":"aff480bf84211cb6ef890fd45789eba8f4d6b119"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"1627d0344df5313da70f71fa3732965f7c8a3405","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":"* create cluster: create cluster overriding a specific set of labels::"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    openstack coe cluster create --add-labels label1\u003dvalue1 ..."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"* create nodegroup: create a nodegroup overriding a specific set of labels::"},{"line_number":117,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"df33271e_8e23fea6","line":114,"in_reply_to":"df33271e_fad7e9bd","updated":"2020-04-02 14:24:58.000000000","message":"We should make it clear that this is a corner case that is not solved completely unless we want to add an additional --remove-label.\n\nAdditionally, here it is why it is not that imporant in reality.\n\nThe cluster_template has:\nfoo_enabled\u003dtrue\n\nOn cluster creation:\n--unset foo_enabled\nOR\n--override-label foo_enabled\u003d\"\"\nOR\n--override-label foo_enabled\u003dnull\n\nIn the mean time, the default in the conductor is:\nfoo_enabled\u003dtrue\n\nResult:\nyou did unset foo_enabled (which was true)\nBut, in the end you got foo_enabled\u003dtrue\n\n\nAt the same time, I reviewed all labels in https://docs.openstack.org/magnum/latest/user/index.html#labels\n\nAll of them don\u0027t reply on some empty default and it is up to us,\nto make sure it won\u0027t.\n\nIs there a specific use-case for this? We can have (--user-labels or --overwrite-labels) and another one (to --remove-label --unset-label) which can cause the issue above.\n\n\nWhat is important to all agree is the data model change.\n* add a new field to *track* what the client sent when the cluster or nodegroup was created (HTTP POST)\n* (decide if we add) new field to track what the client requested\nto be unset/not sent to heat. Unset is strange and should be battled. What unset means is, \"do not send a parameter to heat\". Then what exists in heat will be used. In some cases we have defaults in the python code. eg calico_cidr.\n* Describe the way labels are getting merged.","commit_id":"aff480bf84211cb6ef890fd45789eba8f4d6b119"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"dd4adb36836897c4be1f1f7d042b73222a745360","unresolved":false,"context_lines":[{"line_number":137,"context_line":"labels provided by the user."},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    .. note:: Clients will still be able to provide labels, and the API will"},{"line_number":140,"context_line":"              continue being supported as it is up till now. Meaning that by"},{"line_number":141,"context_line":"              providing labels as --labels will result to not ignoring labels"},{"line_number":142,"context_line":"              configured in the level above."},{"line_number":143,"context_line":""},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"CLI Impact"}],"source_content_type":"text/x-rst","patch_set":4,"id":"df33271e_2b78ce8d","line":142,"range":{"start_line":140,"start_character":60,"end_line":142,"end_character":44},"updated":"2020-04-03 10:04:56.000000000","message":"I do not understand this sentence. If labels work as they do now, providing --labels will ignore all labels from the level above.","commit_id":"e8f0225341bf9246af5bcc315ef0c49abc532439"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"43f18feee2d4df5388b755d95fab527b7b47487d","unresolved":false,"context_lines":[{"line_number":137,"context_line":"labels provided by the user."},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    .. note:: Clients will still be able to provide labels, and the API will"},{"line_number":140,"context_line":"              continue being supported as it is up till now. Meaning that by"},{"line_number":141,"context_line":"              providing labels as --labels will result to not ignoring labels"},{"line_number":142,"context_line":"              configured in the level above."},{"line_number":143,"context_line":""},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"CLI Impact"}],"source_content_type":"text/x-rst","patch_set":4,"id":"df33271e_ebaa666e","line":142,"range":{"start_line":140,"start_character":60,"end_line":142,"end_character":44},"in_reply_to":"df33271e_0b650ab5","updated":"2020-04-03 10:18:05.000000000","message":"we should make this explicit in the spec.","commit_id":"e8f0225341bf9246af5bcc315ef0c49abc532439"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"1ad8c6d31a3b11937a3a5ebf2a8f964d70574248","unresolved":false,"context_lines":[{"line_number":137,"context_line":"labels provided by the user."},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    .. note:: Clients will still be able to provide labels, and the API will"},{"line_number":140,"context_line":"              continue being supported as it is up till now. Meaning that by"},{"line_number":141,"context_line":"              providing labels as --labels will result to not ignoring labels"},{"line_number":142,"context_line":"              configured in the level above."},{"line_number":143,"context_line":""},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"CLI Impact"}],"source_content_type":"text/x-rst","patch_set":4,"id":"df33271e_0b650ab5","line":142,"range":{"start_line":140,"start_character":60,"end_line":142,"end_character":44},"in_reply_to":"df33271e_2b78ce8d","updated":"2020-04-03 10:08:27.000000000","message":"ignore both --labels and --override-labels from the level above?","commit_id":"e8f0225341bf9246af5bcc315ef0c49abc532439"},{"author":{"_account_id":27057,"name":"Theodoros Tsioutsias","email":"theodoros.tsioutsias@cern.ch","username":"ttsiouts"},"change_message_id":"266a51986a7fdfc52fdd31c731db7e60b9df99fe","unresolved":false,"context_lines":[{"line_number":137,"context_line":"labels provided by the user."},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    .. note:: Clients will still be able to provide labels, and the API will"},{"line_number":140,"context_line":"              continue being supported as it is up till now. Meaning that by"},{"line_number":141,"context_line":"              providing labels as --labels will result to not ignoring labels"},{"line_number":142,"context_line":"              configured in the level above."},{"line_number":143,"context_line":""},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"CLI Impact"}],"source_content_type":"text/x-rst","patch_set":4,"id":"df33271e_6bcad6a0","line":142,"range":{"start_line":140,"start_character":60,"end_line":142,"end_character":44},"in_reply_to":"df33271e_2b78ce8d","updated":"2020-04-03 10:07:23.000000000","message":"s/not//","commit_id":"e8f0225341bf9246af5bcc315ef0c49abc532439"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"47504660538dfdf4abe5ffec6404495b1c2b5e47","unresolved":false,"context_lines":[{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    .. note:: Clients will still be able to provide labels, and the API will"},{"line_number":140,"context_line":"              continue being supported as it is up till now. Meaning that by"},{"line_number":141,"context_line":"              providing labels as --labels will result to ignoring labels"},{"line_number":142,"context_line":"              configured in the level above."},{"line_number":143,"context_line":""},{"line_number":144,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"df33271e_6be5b6db","line":141,"range":{"start_line":141,"start_character":67,"end_line":141,"end_character":73},"updated":"2020-04-03 10:20:05.000000000","message":"labels and override labels","commit_id":"2e0ca280764223774375541df468d30b9f97de02"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Magnum Labels Override"},{"line_number":2,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"Problem Description"},{"line_number":5,"context_line":"-------------------"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Magnum accepts labels at cluster or nodegroup creation in the format of"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_06c341ee","line":4,"updated":"2020-04-03 11:35:50.000000000","message":"content lgtm\n\njust nits","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":13,"context_line":"* Nodegroups inherit the labels already configured in the Cluster where they"},{"line_number":14,"context_line":"  belong."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In case user provides labels at cluster/nodegroup creation, labels configured"},{"line_number":17,"context_line":"in the levels above are ignored."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"This behavior, forces users to provide the full set of configured labels when"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_8600f135","line":16,"range":{"start_line":16,"start_character":8,"end_line":16,"end_character":28},"updated":"2020-04-03 11:35:50.000000000","message":"a user provides labels OR labels are provided","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":16,"context_line":"In case user provides labels at cluster/nodegroup creation, labels configured"},{"line_number":17,"context_line":"in the levels above are ignored."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"This behavior, forces users to provide the full set of configured labels when"},{"line_number":20,"context_line":"in order to change a subset of them."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"At the same time, it is very difficult for operators to keep track of the"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_069a2112","line":19,"range":{"start_line":19,"start_character":73,"end_line":19,"end_character":77},"updated":"2020-04-03 11:35:50.000000000","message":"not needed?","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":22,"context_line":"At the same time, it is very difficult for operators to keep track of the"},{"line_number":23,"context_line":"user provided labels when a problem occurs."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"The proposal tries to address the problem described above."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Use Cases"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_c68b19bd","line":25,"range":{"start_line":25,"start_character":0,"end_line":25,"end_character":3},"updated":"2020-04-03 11:35:50.000000000","message":"This\n\nOr no sentence :)","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":25,"context_line":"The proposal tries to address the problem described above."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Use Cases"},{"line_number":29,"context_line":"---------"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Below are some of the use cases:"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_a6b37597","line":28,"updated":"2020-04-03 11:35:50.000000000","message":"lgtm","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":41,"context_line":"   user provided while creating a cluster or nodegroup."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"Proposed Changes"},{"line_number":45,"context_line":"----------------"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The proposed change includes:"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_c67959c5","line":44,"updated":"2020-04-03 11:35:50.000000000","message":"lgtm","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"   * + \u003d merge this label to the already configured labels"},{"line_number":68,"context_line":"   * - \u003d do not inherit the label"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"Data Model Impact"},{"line_number":72,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_e6573d25","line":69,"updated":"2020-04-03 11:35:50.000000000","message":"you can here that this is more error prone, code more complicated (although shorter), less intuitive.","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":68,"context_line":"   * - \u003d do not inherit the label"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"Data Model Impact"},{"line_number":72,"context_line":"-----------------"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"A new DB field will be added to the Cluster and Nodegroup objects. The new"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_e6745d00","line":71,"updated":"2020-04-03 11:35:50.000000000","message":"lgtm","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"7967185783a831d50ae650f47b38061d885ec225","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        cluster_template.labels \u003d {\u0027label1\u0027: \u0027value1\u0027, \u0027label2\u0027: \u0027value2\u0027}"},{"line_number":101,"context_line":"        cluster.override_labels \u003d {\u0027label1\u0027: \u0027value3\u0027, \u0027label2\u0027: \u0027value4\u0027}"},{"line_number":102,"context_line":"        nodegroup.labels \u003d {\u0027label5\u0027: \u0027value5\u0027}"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"This scenario shows that the ``labels`` at the nodegroup level have priority"},{"line_number":105,"context_line":"over the ``override_labels`` set at the cluster scope, maintaining the"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_a64095a6","line":102,"range":{"start_line":102,"start_character":18,"end_line":102,"end_character":24},"updated":"2020-04-03 11:19:05.000000000","message":"override_labels ?","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"6015c505091b523ae32d756924e3edf51666fc90","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        cluster_template.labels \u003d {\u0027label1\u0027: \u0027value1\u0027, \u0027label2\u0027: \u0027value2\u0027}"},{"line_number":101,"context_line":"        cluster.override_labels \u003d {\u0027label1\u0027: \u0027value3\u0027, \u0027label2\u0027: \u0027value4\u0027}"},{"line_number":102,"context_line":"        nodegroup.labels \u003d {\u0027label5\u0027: \u0027value5\u0027}"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"This scenario shows that the ``labels`` at the nodegroup level have priority"},{"line_number":105,"context_line":"over the ``override_labels`` set at the cluster scope, maintaining the"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_061981d5","line":102,"range":{"start_line":102,"start_character":18,"end_line":102,"end_character":24},"in_reply_to":"df33271e_a64095a6","updated":"2020-04-03 11:20:31.000000000","message":"no, got it.","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":123,"context_line":"* make the functionality backwards compatible"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"REST API Impact"},{"line_number":127,"context_line":"---------------"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"This change leads to a minor version increase in the Magnum API."}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_61079719","line":126,"updated":"2020-04-03 11:35:50.000000000","message":"lgtm","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":146,"context_line":"              level above."},{"line_number":147,"context_line":""},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"CLI Impact"},{"line_number":150,"context_line":"----------"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"The OpenStack client commands will be adapted:"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_21010f06","line":149,"updated":"2020-04-03 11:35:50.000000000","message":"lgtm","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":166,"context_line":"With the proposed implementation, users will not be able to remove a configured"},{"line_number":167,"context_line":"label. Although it would be possible by adding a --remove-label option, the"},{"line_number":168,"context_line":"result of this action is not clear. Meaning that from user/client perspective,"},{"line_number":169,"context_line":"it is not clear if the label will not be used or its default value will be"},{"line_number":170,"context_line":"propagated to Heat."},{"line_number":171,"context_line":""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"Other Implementation Options"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_410a531f","line":170,"range":{"start_line":169,"start_character":16,"end_line":170,"end_character":19},"updated":"2020-04-03 11:35:50.000000000","message":"lgtm","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":188,"context_line":"N/A"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"Other End User Impact"},{"line_number":192,"context_line":"---------------------"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"Users will be able to provide labels in a new way, using this functionality."}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_412373a7","line":191,"updated":"2020-04-03 11:35:50.000000000","message":"lgtm","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":195,"context_line":"The old way of providing labels (via --labels) will still be supported."},{"line_number":196,"context_line":""},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"Implementation"},{"line_number":199,"context_line":"--------------"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"The tasks can be found below:"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_e133a7f4","line":198,"updated":"2020-04-03 11:35:50.000000000","message":"do you think we can add links to task in storyboard?\n\ni.e. have a story and a task for each bullet? It would be perfect.","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0372d11566cec857ee5c0f89bcf85e7e1ca5cbad","unresolved":false,"context_lines":[{"line_number":220,"context_line":"--------------------"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"Magnum documentation for labels will be adapted to describe the new way of"},{"line_number":223,"context_line":"providing override labels."},{"line_number":224,"context_line":""},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"References"}],"source_content_type":"text/x-rst","patch_set":6,"id":"df33271e_012deb93","line":223,"updated":"2020-04-03 11:35:50.000000000","message":"One more task for api-ref?","commit_id":"ebe4b83fd07cfbae9732bd1aa30c995603864404"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"9a4eee321542c4ed64c3d9047f233d8a153ae9bb","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Magnum Labels Override"},{"line_number":2,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"Problem Description"},{"line_number":5,"context_line":"-------------------"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Magnum accepts labels at cluster or nodegroup creation in the format of"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_5d6b371e","line":4,"range":{"start_line":4,"start_character":0,"end_line":4,"end_character":19},"updated":"2020-04-05 16:14:41.000000000","message":"Yes, totally agree","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"9a4eee321542c4ed64c3d9047f233d8a153ae9bb","unresolved":false,"context_lines":[{"line_number":25,"context_line":"This proposal tries to address the problem described above."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Use Cases"},{"line_number":29,"context_line":"---------"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Below are some of the use cases:"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_3d68f31f","line":28,"range":{"start_line":28,"start_character":0,"end_line":28,"end_character":9},"updated":"2020-04-05 16:14:41.000000000","message":"Yes, totally agree","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"9a4eee321542c4ed64c3d9047f233d8a153ae9bb","unresolved":false,"context_lines":[{"line_number":41,"context_line":"   user provided while creating a cluster or nodegroup."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"Proposed Changes"},{"line_number":45,"context_line":"----------------"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The proposed change includes:"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_7d727b13","line":44,"range":{"start_line":44,"start_character":0,"end_line":44,"end_character":16},"updated":"2020-04-05 16:14:41.000000000","message":"Ok.\n\nGood to have, predicted impacted files.","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"0264ebffe70a7479712238bfc50e6a4070de5ac1","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The proposed change includes:"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"* A new field will be added in the Cluster and Nodegroup objects where the"},{"line_number":50,"context_line":"  labels provided by the user will be stored."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"* Drivers and the conductor, will be adapted to respect the labels inheritance."},{"line_number":53,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_38caca4e","line":50,"range":{"start_line":49,"start_character":71,"end_line":50,"end_character":29},"updated":"2020-04-04 20:19:03.000000000","message":"This is not a clear description. I\u0027d like to see we mention that \u0027comparing with the labels defined in the template\u0027.","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"a37442e8be409484c644da943e9f27c219407d1e","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The proposed change includes:"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"* A new field will be added in the Cluster and Nodegroup objects where the"},{"line_number":50,"context_line":"  labels provided by the user will be stored."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"* Drivers and the conductor, will be adapted to respect the labels inheritance."},{"line_number":53,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_c0cabc18","line":50,"range":{"start_line":49,"start_character":71,"end_line":50,"end_character":29},"in_reply_to":"df33271e_38caca4e","updated":"2020-04-05 19:01:20.000000000","message":"\u003e This is not a clear description. I\u0027d like to see we mention that\n \u003e \u0027comparing with the labels defined in the template\u0027.\n\nA new field will be added in the Cluster object and another for the Nodegroup object. They will store the labels provide to the API at creation time (HTTP POST) for cluster and nodegroup respectively.\n\nWould something like this is better?\n\n@Thodoris\nIn general, it would be better to not say \u0027provided by the user\u0027 but \u0027provided at creation time (HTTP POST)\u0027. Or something similar indicating the REST action.","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"bba589ef0477204948135507752730e791961652","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":"The proposed change includes:"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"* A new field will be added in the Cluster and Nodegroup objects where the"},{"line_number":50,"context_line":"  labels provided by the user will be stored."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"* Drivers and the conductor, will be adapted to respect the labels inheritance."},{"line_number":53,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_c38da632","line":50,"range":{"start_line":49,"start_character":71,"end_line":50,"end_character":29},"in_reply_to":"df33271e_c0cabc18","updated":"2020-04-05 19:58:06.000000000","message":"+1","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"9a4eee321542c4ed64c3d9047f233d8a153ae9bb","unresolved":false,"context_lines":[{"line_number":71,"context_line":"more complicated than the proposed solution."},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"Data Model Impact"},{"line_number":75,"context_line":"-----------------"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"A new DB field will be added to the Cluster and Nodegroup objects. The new"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_dd93c7ff","line":74,"range":{"start_line":74,"start_character":0,"end_line":74,"end_character":17},"updated":"2020-04-05 16:14:41.000000000","message":"Much easier to understand. I agree with this changes","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"0264ebffe70a7479712238bfc50e6a4070de5ac1","unresolved":false,"context_lines":[{"line_number":118,"context_line":"              supposed to coexist. The API needs to make sure that either"},{"line_number":119,"context_line":"              labels or override labels are provided at the creation time."},{"line_number":120,"context_line":"              See `REST API Impact`_ for more details."},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"We chose to add a new field so that we:"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"* minimize the impact on the current functionality"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_d8b59ecf","line":121,"updated":"2020-04-04 20:19:03.000000000","message":"What happened if both labels and override_labels are provided?","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"9a4eee321542c4ed64c3d9047f233d8a153ae9bb","unresolved":false,"context_lines":[{"line_number":118,"context_line":"              supposed to coexist. The API needs to make sure that either"},{"line_number":119,"context_line":"              labels or override labels are provided at the creation time."},{"line_number":120,"context_line":"              See `REST API Impact`_ for more details."},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"We chose to add a new field so that we:"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"* minimize the impact on the current functionality"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_ddee278a","line":121,"in_reply_to":"df33271e_d8b59ecf","updated":"2020-04-05 16:14:41.000000000","message":"Also, include what is the default value of the non included label","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"9a4eee321542c4ed64c3d9047f233d8a153ae9bb","unresolved":false,"context_lines":[{"line_number":142,"context_line":"The get methods of Clusters and Nodegroups APIs will be adapted to show the"},{"line_number":143,"context_line":"labels provided by the user."},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    .. note:: Clients will still be able to provide labels as --labels for"},{"line_number":146,"context_line":"              both clusters and nodegroups and the API will continue being"},{"line_number":147,"context_line":"              supported as it is up till now. Meaning that by providing labels"},{"line_number":148,"context_line":"              as --labels will result to ignoring labels configured in the"},{"line_number":149,"context_line":"              level above."},{"line_number":150,"context_line":""},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"CLI Impact"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_c02bdc7f","line":149,"range":{"start_line":145,"start_character":14,"end_line":149,"end_character":26},"updated":"2020-04-05 16:14:41.000000000","message":"This part is a bit confusing. I don\u0027t think this is actually needed here as it was explained above (under Data Model).\n\nAlso, this is the get method? I would rather see, or see explained, what is going to change on the get method because you said above (under the Use Cases section):\n\n\u0027\u0027\u0027\n3. As an operator, I want to be able to properly track the labels that the\n   user provided while creating a cluster or nodegroup.\n\u0027\u0027\u0027\n\nAnd, if i recall, no explanation was made to this yet.","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"fb568db2c76e8212cc01995cdd40eaf18a75d8b4","unresolved":false,"context_lines":[{"line_number":161,"context_line":"* create nodegroup: create a nodegroup overriding a specific set of labels::"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    openstack coe nodegroup create --override-labels label1\u003dvalue1 ..."},{"line_number":164,"context_line":""},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"Known Limitations"},{"line_number":167,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_403cecbe","line":164,"updated":"2020-04-05 16:23:23.000000000","message":"Impact of get here.\n\nHow will this new data will be showed (or not) to the user?","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":29425,"name":"Diogo Guerra","email":"diogo.filipe.tomas.guerra@cern.ch","username":"dioguerra"},"change_message_id":"fb568db2c76e8212cc01995cdd40eaf18a75d8b4","unresolved":false,"context_lines":[{"line_number":165,"context_line":""},{"line_number":166,"context_line":"Known Limitations"},{"line_number":167,"context_line":"-----------------"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"With the proposed implementation, users will not be able to remove a configured"},{"line_number":170,"context_line":"label. Although it would be possible by adding a --remove-label option, the"},{"line_number":171,"context_line":"result of this action is not clear. Meaning that from user/client perspective,"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_a0497861","line":168,"updated":"2020-04-05 16:23:23.000000000","message":"all bellow lgtm","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"0264ebffe70a7479712238bfc50e6a4070de5ac1","unresolved":false,"context_lines":[{"line_number":171,"context_line":"result of this action is not clear. Meaning that from user/client perspective,"},{"line_number":172,"context_line":"it is not clear if the label will not be used or its default value will be"},{"line_number":173,"context_line":"propagated to Heat."},{"line_number":174,"context_line":""},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"Other Implementation Options"},{"line_number":177,"context_line":"----------------------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_18cc2660","line":174,"updated":"2020-04-04 20:19:03.000000000","message":"That\u0027s the topic I discussed with Ricardo in the previous patch. Personally, I\u0027m OK to discuss this in a separate spec/patch, but we should keep this in mind otherwise we maybe in trouble in the near future. But this is not a blocker for this spec. Would you mind creating another following spec to start the discussion?\n\nMy another concern is, we all know the labels in Magnum are like the double edges of the sword. It gives us a lot of flexibility and meanwhile they\u0027re getting messed because we don\u0027t have a strong policy to manage the labels. For example, 1) labels are driver-based; 2) labels are respected by different layers, some of them are respected by magnum/heat, and some of them respected by the COE/k8s. 3) we don\u0027t have a well-defined name convention for labels.\n\nIn short, my point is as long as we add this new field, that means we have to put more care/rules for the labels.","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"a37442e8be409484c644da943e9f27c219407d1e","unresolved":false,"context_lines":[{"line_number":171,"context_line":"result of this action is not clear. Meaning that from user/client perspective,"},{"line_number":172,"context_line":"it is not clear if the label will not be used or its default value will be"},{"line_number":173,"context_line":"propagated to Heat."},{"line_number":174,"context_line":""},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"Other Implementation Options"},{"line_number":177,"context_line":"----------------------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_c3792694","line":174,"in_reply_to":"df33271e_18cc2660","updated":"2020-04-05 19:01:20.000000000","message":"Can we do something in the context of this spec? The rules of of merging the labels are described in the Data Model, after this spec, the existing rules apply.\n\nCan you link the discussion here? Does it help?","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"12b67aba8827b22b685b4c08a892212885664c00","unresolved":false,"context_lines":[{"line_number":171,"context_line":"result of this action is not clear. Meaning that from user/client perspective,"},{"line_number":172,"context_line":"it is not clear if the label will not be used or its default value will be"},{"line_number":173,"context_line":"propagated to Heat."},{"line_number":174,"context_line":""},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"Other Implementation Options"},{"line_number":177,"context_line":"----------------------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_2334524a","line":174,"in_reply_to":"df33271e_2398d268","updated":"2020-04-05 20:16:16.000000000","message":"Thanks for the link.\n\nI think the current \u0027merging policy\u0027 is described in the Data Model Impact verbosely. I believe the current proposal, as described by Thodoris in Data Model Impact, covers all use cases for the labels we have in: https://docs.openstack.org/magnum/latest/user/index.html#labels\n\nWhat we can make sure as reviewers (I\u0027m not sure how we can do it in the API) is to not allow \"\" be something different than Unset/None/Null. This is the only corner case that can lead to issues. etcd_volume_size and boot_volume_size, accept number so \"\" would type error.\n\nIMO the optimal option is to introduce driver label validation. This would be a method for the Driver class what the API would calland it would give feedback to the user very very fast.\n\n* They only *big* drawback is that heat can do this validation with constraints in parameters. Feedback from this mechanism comes from the conductor.\n\nThis can be done as you mention in another SPEC. Thoughts?\n\nTo conclude, this is not an issue at the moment for existing labels, at least to my knowledge.","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"bba589ef0477204948135507752730e791961652","unresolved":false,"context_lines":[{"line_number":171,"context_line":"result of this action is not clear. Meaning that from user/client perspective,"},{"line_number":172,"context_line":"it is not clear if the label will not be used or its default value will be"},{"line_number":173,"context_line":"propagated to Heat."},{"line_number":174,"context_line":""},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"Other Implementation Options"},{"line_number":177,"context_line":"----------------------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"df33271e_2398d268","line":174,"in_reply_to":"df33271e_c3792694","updated":"2020-04-05 19:58:06.000000000","message":"Here is the discussion we had https://review.opendev.org/#/c/621611/","commit_id":"f5fb92ca8a38f6b0010acc8011a60bd8d883ac08"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"639d2c2e7ace541d8225c87bce994980b8ecef4c","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"The proposed change includes:"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"* Intoruducing a new boolean flag, which will indicate whether the provided"},{"line_number":58,"context_line":"  labels will be used to override specific values of the parent labels or"},{"line_number":59,"context_line":"  replaceing them completely, which is the current behaviour."},{"line_number":60,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_608c287f","line":57,"range":{"start_line":57,"start_character":2,"end_line":57,"end_character":14},"updated":"2020-04-27 09:23:31.000000000","message":"Introducing","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":27057,"name":"Theodoros Tsioutsias","email":"theodoros.tsioutsias@cern.ch","username":"ttsiouts"},"change_message_id":"c62b33791be3e750a96b50b129c7318086f0939f","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"The proposed change includes:"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"* Intoruducing a new boolean flag, which will indicate whether the provided"},{"line_number":58,"context_line":"  labels will be used to override specific values of the parent labels or"},{"line_number":59,"context_line":"  replaceing them completely, which is the current behaviour."},{"line_number":60,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_94753535","line":57,"range":{"start_line":57,"start_character":2,"end_line":57,"end_character":14},"in_reply_to":"1f493fa4_608c287f","updated":"2020-04-27 10:55:01.000000000","message":"Done","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"639d2c2e7ace541d8225c87bce994980b8ecef4c","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"* Intoruducing a new boolean flag, which will indicate whether the provided"},{"line_number":58,"context_line":"  labels will be used to override specific values of the parent labels or"},{"line_number":59,"context_line":"  replaceing them completely, which is the current behaviour."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"* A new field will be added in the GET response for clusters and nodegroups."},{"line_number":62,"context_line":"  The field will contain the differences between the current labels and the"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_2096a094","line":59,"range":{"start_line":59,"start_character":2,"end_line":59,"end_character":12},"updated":"2020-04-27 09:23:31.000000000","message":"replacing","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":27057,"name":"Theodoros Tsioutsias","email":"theodoros.tsioutsias@cern.ch","username":"ttsiouts"},"change_message_id":"c62b33791be3e750a96b50b129c7318086f0939f","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"* Intoruducing a new boolean flag, which will indicate whether the provided"},{"line_number":58,"context_line":"  labels will be used to override specific values of the parent labels or"},{"line_number":59,"context_line":"  replaceing them completely, which is the current behaviour."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"* A new field will be added in the GET response for clusters and nodegroups."},{"line_number":62,"context_line":"  The field will contain the differences between the current labels and the"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_7472294b","line":59,"range":{"start_line":59,"start_character":2,"end_line":59,"end_character":12},"in_reply_to":"1f493fa4_2096a094","updated":"2020-04-27 10:55:01.000000000","message":"Done","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"639d2c2e7ace541d8225c87bce994980b8ecef4c","unresolved":false,"context_lines":[{"line_number":58,"context_line":"  labels will be used to override specific values of the parent labels or"},{"line_number":59,"context_line":"  replaceing them completely, which is the current behaviour."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"* A new field will be added in the GET response for clusters and nodegroups."},{"line_number":62,"context_line":"  The field will contain the differences between the current labels and the"},{"line_number":63,"context_line":"  parent labels. This field will be generated by the Magnum API when showing"},{"line_number":64,"context_line":"  a cluster or nodegroup."}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_e0803849","line":61,"range":{"start_line":61,"start_character":8,"end_line":61,"end_character":13},"updated":"2020-04-27 09:23:31.000000000","message":"one or multiple fields? we discussed adding labels_skipped, labels_overridden, labels_new, or adding an overarching field \n called something like labels_diff with keys overridden, new and skipped. You preferred the first option but I\u0027m happy with either option.","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"a8f8e0dc43b8ed532cf9aae213f58b0041eac96d","unresolved":false,"context_lines":[{"line_number":58,"context_line":"  labels will be used to override specific values of the parent labels or"},{"line_number":59,"context_line":"  replaceing them completely, which is the current behaviour."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"* A new field will be added in the GET response for clusters and nodegroups."},{"line_number":62,"context_line":"  The field will contain the differences between the current labels and the"},{"line_number":63,"context_line":"  parent labels. This field will be generated by the Magnum API when showing"},{"line_number":64,"context_line":"  a cluster or nodegroup."}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_894e8535","line":61,"range":{"start_line":61,"start_character":8,"end_line":61,"end_character":13},"in_reply_to":"1f493fa4_54dc4de3","updated":"2020-04-29 09:37:40.000000000","message":"+2 from me, slighty prefer the look of labels_* prefix but see that *_labels reads better so will leave that up to you.","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"61a6e7b0aa09f7bae9a9ba0ca4fa4e0aded25703","unresolved":false,"context_lines":[{"line_number":58,"context_line":"  labels will be used to override specific values of the parent labels or"},{"line_number":59,"context_line":"  replaceing them completely, which is the current behaviour."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"* A new field will be added in the GET response for clusters and nodegroups."},{"line_number":62,"context_line":"  The field will contain the differences between the current labels and the"},{"line_number":63,"context_line":"  parent labels. This field will be generated by the Magnum API when showing"},{"line_number":64,"context_line":"  a cluster or nodegroup."}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_49e47d0d","line":61,"range":{"start_line":61,"start_character":8,"end_line":61,"end_character":13},"in_reply_to":"1f493fa4_54dc4de3","updated":"2020-04-29 09:42:32.000000000","message":"\u003e I feel that it would be better to have more than one fields from UX\n \u003e pov. Actually 3 labels to be exact.\n \u003e \n \u003e The fields I would like to have would be:\n \u003e \n \u003e * overridden_labels: labels that exist in both parent and object\n \u003e labels but have different value\n \u003e \n \u003e * added_labels: labels that do not exist in the parent labels and\n \u003e were added in the object\n \u003e \n \u003e * skipped_labels: labels that exist in the parent dict but do not\n \u003e exist in the object\u0027s labels. Specifically, this field will be used\n \u003e when the user did not provide the --override-labels (used the\n \u003e current functionality) and did not provide some of the labels that\n \u003e exist in the parent.\n \u003e \n \u003e Would such approach be ok with everyone?\n\nLets include this description in the spec too.","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":27057,"name":"Theodoros Tsioutsias","email":"theodoros.tsioutsias@cern.ch","username":"ttsiouts"},"change_message_id":"c62b33791be3e750a96b50b129c7318086f0939f","unresolved":false,"context_lines":[{"line_number":58,"context_line":"  labels will be used to override specific values of the parent labels or"},{"line_number":59,"context_line":"  replaceing them completely, which is the current behaviour."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"* A new field will be added in the GET response for clusters and nodegroups."},{"line_number":62,"context_line":"  The field will contain the differences between the current labels and the"},{"line_number":63,"context_line":"  parent labels. This field will be generated by the Magnum API when showing"},{"line_number":64,"context_line":"  a cluster or nodegroup."}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_54dc4de3","line":61,"range":{"start_line":61,"start_character":8,"end_line":61,"end_character":13},"in_reply_to":"1f493fa4_e0803849","updated":"2020-04-27 10:55:01.000000000","message":"I feel that it would be better to have more than one fields from UX pov. Actually 3 labels to be exact.\n\nThe fields I would like to have would be:\n\n* overridden_labels: labels that exist in both parent and object labels but have different value\n\n* added_labels: labels that do not exist in the parent labels and were added in the object\n\n* skipped_labels: labels that exist in the parent dict but do not exist in the object\u0027s labels. Specifically, this field will be used when the user did not provide the --override-labels (used the current functionality) and did not provide some of the labels that exist in the parent.\n\nWould such approach be ok with everyone?","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"e11f3bf00bea09aea38d39ea93930ae8faf6906c","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    * If labels are provided then the parent labels will be ignored."},{"line_number":111,"context_line":"    * If labels are not provided then the parent labels will be copied over to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_599d3dba","line":108,"updated":"2020-04-30 23:00:59.000000000","message":"Could we have a config option to set the default behavior? Actually, almost all our users are assuming the default behavior is overridden. So it would be nice if we can set the default behavior on the server-side instead of asking user to specify the flag each time. Does that make sense?","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"012710e833457ba3a65ed00bb13722350ee628b9","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    * If labels are provided then the parent labels will be ignored."},{"line_number":111,"context_line":"    * If labels are not provided then the parent labels will be copied over to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_8d93e300","line":108,"in_reply_to":"1f493fa4_4aba6be1","updated":"2020-05-04 08:18:58.000000000","message":"\u003e Although it is not good as a design principle to change behaviour\n \u003e of an API via a config option, I see that we want to lift user\u0027s\n \u003e pain. Keep in mind that the clients (ui/cli) have defaults too. So\n \u003e to control this in the API only we should not send anything to the\n \u003e API, same as what an old client would do (the old client won\u0027t send\n \u003e override-labels since it doesn\u0027t know about it). e.g. here [0],\n \u003e even if you don\u0027t pass anything in the cli, the client will send\n \u003e the value that is default in the API.\n \u003e \n \u003e \n \u003e [0] https://github.com/openstack/python-magnumclient/blob/5b7a67131920f42b2add9ad0d063e5d9ff01b611/magnumclient/osc/v1/cluster_templates.py#L208\n \u003e \n \u003e \n \u003e A1.\n \u003e a. Make override-labels true by default (the cli doesn\u0027t send\n \u003e anything if nothing is specified).\n \u003e b. Big flashing reno that old client will change behaviour.\n \u003e c. Give deployments the option with a config option to preserve old\n \u003e default functionality. The minority of deployments will want to do\n \u003e this. It is still a breaking change, but the minority of users will\n \u003e want to change to the old behaviour.\n\nThis also sounds like the best option to me but only for default option when nothing is specified via CLI. This means we  will need, as well as --override-labels, also something like --do-not-override-labels option so that the user can request the exact behaviour if they need to. \n\n \u003e \n \u003e A2.\n \u003e a. Make override-labels false by default (the cli doesn\u0027t send\n \u003e anything if nothing is specified).\n \u003e c. Give deployments the option with a config option to change it.\n \u003e This is not a breaking change, but upstream api-ref won\u0027t be the\n \u003e source of truth for the API.\n \u003e \n\nI agree this would be the safest. But I can imaging it will be bad UX if users need to specify --override-labels flag most of the time.\n\n \u003e B.\n \u003e Control the default in the client (for environments where the the\n \u003e ops team controls the clients) or have a different default than the\n \u003e API.\n \u003e \n \u003e Surpisingly (even to myself), I prefer to break the API (A1) and\n \u003e allow operators to \"unbreak\" it. Most operators will want this new\n \u003e default. Also, not having a default in the cli is a move towards\n \u003e the right direction.\n \u003e \n \u003e Finally, what we can do, is hard code in the CLI and ui, what is\n \u003e the maximum API version they can use. i.e. Train client  will send\n \u003e by default the maximum train API microversion when it was released.\n \u003e ATM it sends latest.\n\nSure, I was wondering why we were using \"latest\" and have encountered issues because of this in the past.\n\n \u003e \n \u003e What do you think?","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"c24c057594b1b6bae6f75cc6ab92ddd73c9dcad2","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    * If labels are provided then the parent labels will be ignored."},{"line_number":111,"context_line":"    * If labels are not provided then the parent labels will be copied over to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_506cd09b","line":108,"in_reply_to":"1f493fa4_4aba6be1","updated":"2020-05-04 08:47:10.000000000","message":"Personally, I prefer A2, and leave the option to cloud provider. Because it\u0027s a config option, which mean the cloud provider can make the decision if they want to break the compatibility.\n\nHowever, if both of you guys are happy to go for A1, I\u0027m OK with that. And obviously, we need a big warning in the release note.","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":9995,"name":"Ricardo Rocha","email":"rocha.porto@gmail.com","username":"rocha"},"change_message_id":"4e347afa48ce8b1b010e0d67fabec26be9f14136","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    * If labels are provided then the parent labels will be ignored."},{"line_number":111,"context_line":"    * If labels are not provided then the parent labels will be copied over to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_70f3cc74","line":108,"in_reply_to":"1f493fa4_506cd09b","updated":"2020-05-04 09:20:05.000000000","message":"Both options look ok to me. The main thing is as mentioned above we force users to provide this flag every time they create a cluster.\n\nA minor thing as people will get use to it, but `override-labels` still does not sound like the best name - overriding sounds like replacing to me (not a native speaker), i would expect setting it to True would replace the cluster template labels completely. In reality we\u0027re merging the provided values with what\u0027s coming from the cluster template.\n\nSmall thing though, whatever you decide is ok.","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"1720aacc66c97fe3cb09361a9863e4d400ac16d9","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    * If labels are provided then the parent labels will be ignored."},{"line_number":111,"context_line":"    * If labels are not provided then the parent labels will be copied over to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_4aba6be1","line":108,"in_reply_to":"1f493fa4_599d3dba","updated":"2020-05-02 14:20:14.000000000","message":"Although it is not good as a design principle to change behaviour of an API via a config option, I see that we want to lift user\u0027s pain. Keep in mind that the clients (ui/cli) have defaults too. So to control this in the API only we should not send anything to the API, same as what an old client would do (the old client won\u0027t send override-labels since it doesn\u0027t know about it). e.g. here [0], even if you don\u0027t pass anything in the cli, the client will send the value that is default in the API.\n\n\n[0] https://github.com/openstack/python-magnumclient/blob/5b7a67131920f42b2add9ad0d063e5d9ff01b611/magnumclient/osc/v1/cluster_templates.py#L208\n\n\nA1.\na. Make override-labels true by default (the cli doesn\u0027t send anything if nothing is specified).\nb. Big flashing reno that old client will change behaviour.\nc. Give deployments the option with a config option to preserve old default functionality. The minority of deployments will want to do this. It is still a breaking change, but the minority of users will want to change to the old behaviour.\n\nA2.\na. Make override-labels false by default (the cli doesn\u0027t send anything if nothing is specified).\nc. Give deployments the option with a config option to change it. This is not a breaking change, but upstream api-ref won\u0027t be the source of truth for the API.\n\nB.\nControl the default in the client (for environments where the the ops team controls the clients) or have a different default than the API. \n\nSurpisingly (even to myself), I prefer to break the API (A1) and allow operators to \"unbreak\" it. Most operators will want this new default. Also, not having a default in the cli is a move towards the right direction.\n\nFinally, what we can do, is hard code in the CLI and ui, what is the maximum API version they can use. i.e. Train client will send by default the maximum train API microversion when it was released. ATM it sends latest.\n\nWhat do you think?","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"553c817c1aeaf763c193b5135e661fbdf90473e4","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    * If labels are provided then the parent labels will be ignored."},{"line_number":111,"context_line":"    * If labels are not provided then the parent labels will be copied over to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_9374a29e","line":108,"in_reply_to":"1f493fa4_70f3cc74","updated":"2020-05-04 09:43:32.000000000","message":"\u003e Both options look ok to me. The main thing is as mentioned above we\n \u003e force users to provide this flag every time they create a cluster.\n \u003e \n \u003e A minor thing as people will get use to it, but `override-labels`\n \u003e still does not sound like the best name - overriding sounds like\n \u003e replacing to me (not a native speaker), i would expect setting it\n \u003e to True would replace the cluster template labels completely. In\n \u003e reality we\u0027re merging the provided values with what\u0027s coming from\n \u003e the cluster template.\n \u003e \n \u003e Small thing though, whatever you decide is ok.\n\nThe naming of `override-labels` has been an ongoing debate but I think the team is now largely on the same page that override_labels\u003dtrue is the new behaviour and override_labels\u003dfalse is the old behavior. The definition of the old action is \"do no override but the inherited labels, instead replace them completely\" and the new action is \"override the inherited labels with provided labels\".","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":9995,"name":"Ricardo Rocha","email":"rocha.porto@gmail.com","username":"rocha"},"change_message_id":"d083922131dd68e8ca8a94f917ad574e7d4947e5","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    * If labels are provided then the parent labels will be ignored."},{"line_number":111,"context_line":"    * If labels are not provided then the parent labels will be copied over to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_b0a2744a","line":108,"in_reply_to":"1f493fa4_70f3cc74","updated":"2020-05-04 09:22:02.000000000","message":"s/we force users to provide this flag/we *do not* force users to provide this flag/. Bad bad typo.","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"706c292664d54dd50b47fac78033c3a8b1296e78","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    * If labels are provided then the parent labels will be ignored."},{"line_number":111,"context_line":"    * If labels are not provided then the parent labels will be copied over to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_f6223401","line":108,"in_reply_to":"1f493fa4_9374a29e","updated":"2020-05-04 11:59:31.000000000","message":"Let\u0027s do A2 then which is safe.\n\nI share the opinion that passing an the --override-labesl argument is major improvement actually and a minor annoyance from a UX perspective. eg At CERN we have 26 labels in our latest public CT for example that the users needs to carry.\n\n\nThe override verb used in the argment is not intuitive, but the logic is not that intuitive anyway. Even in software development the concept is not the most obvious :).\n\n\nRegardless of the name we choose, the help message and docs is the most imporant which clarifies what the API is going to do. Effectively a dict update.","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"a340251944fbac393148bac647d64d1517a42619","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    * If labels are provided then the parent labels will be ignored."},{"line_number":111,"context_line":"    * If labels are not provided then the parent labels will be copied over to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_fb5a104e","line":108,"in_reply_to":"1f493fa4_f6223401","updated":"2020-05-06 03:49:20.000000000","message":"I shared the same comments as Ricardo as a non-native English speaker. Is \u0027merged_labels\u0027 an option? But this is not a blocker.","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"afcf87dc995ed503a1910a3f5abf5cb351c3b6d3","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    * If labels are provided then the parent labels will be ignored."},{"line_number":111,"context_line":"    * If labels are not provided then the parent labels will be copied over to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_1fe188ff","line":108,"in_reply_to":"1f493fa4_fb5a104e","updated":"2020-05-06 06:31:32.000000000","message":"We can do merge. The important part will be the help message and API reference.\n\nIn Computer Science, method overriding [0][1] can replace completely the parent function or the new implementation could execute both the parent and child logic. In a way, the logic we have now [2] in the API, is overriding. In English, the logic we propose is not overriding according to the dictionary [7].\n\nWe can say that merge [6] is more accurate because this [3] code snippet is described [4] frequently as merge. The python documentation for dict.update mentions \"overwriting for existing keys\" [5], overwrite [8] applies only for existing keys.\n\n[0] https://en.wikipedia.org/wiki/Method_overriding#Python\n[1] https://docs.python.org/3/tutorial/classes.html#inheritance\n[2] https://opendev.org/openstack/magnum/src/commit/673e624270d5a1bc95c25263b6fd8eb22c666091/magnum/api/controllers/v1/cluster.py#L474\n[3]\n Python 3.8.2 (default, Feb 28 2020, 00:00:00) \n [GCC 10.0.1 20200216 (Red Hat 10.0.1-0.8)] on linux\n Type \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n \u003e\u003e\u003e \n \u003e\u003e\u003e labels \u003d {\u0027label1\u0027: \u0027value1\u0027, \u0027label2\u0027: \u0027value2\u0027}\n \u003e\u003e\u003e \n \u003e\u003e\u003e cluster_create_labels \u003d { \u0027label1\u0027: \u0027value3\u0027, \u0027label4\u0027: \u0027value4\u0027 }\n \u003e\u003e\u003e \n \u003e\u003e\u003e labels.update(cluster_create_labels)\n \u003e\u003e\u003e \n \u003e\u003e\u003e labels\n {\u0027label1\u0027: \u0027value3\u0027, \u0027label2\u0027: \u0027value2\u0027, \u0027label4\u0027: \u0027value4\u0027}\n \u003e\u003e\u003e \n \u003e\u003e\u003e ng_create_labels \u003d { \u0027label4\u0027: \u0027label5\u0027 }\n \u003e\u003e\u003e \n \u003e\u003e\u003e labels.update(ng_create_labels)\n \u003e\u003e\u003e \n \u003e\u003e\u003e labels\n {\u0027label1\u0027: \u0027value3\u0027, \u0027label2\u0027: \u0027value2\u0027, \u0027label4\u0027: \u0027label5\u0027}\n[4] https://treyhunner.com/2016/02/how-to-merge-dictionaries-in-python/\n[5] https://docs.python.org/3/library/stdtypes.html#dict.update\n[6] https://www.oxfordlearnersdictionaries.com/definition/english/merge\n[7] https://www.oxfordlearnersdictionaries.com/definition/english/override\n[8] https://www.oxfordlearnersdictionaries.com/definition/english/overwrite","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"639d2c2e7ace541d8225c87bce994980b8ecef4c","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    openstack coe nodegroup create --labels label4\u003dlabel5 \u003ccluster_name\u003e ng2"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"The current functionality will be used and the  resulting labels stored in the"},{"line_number":145,"context_line":"nodegroup will be::"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    labels \u003d {\u0027label4\u0027: \u0027value5\u0027}"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_006e04c8","line":144,"range":{"start_line":144,"start_character":46,"end_line":144,"end_character":48},"updated":"2020-04-27 09:23:31.000000000","message":"s/  / /g","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":27057,"name":"Theodoros Tsioutsias","email":"theodoros.tsioutsias@cern.ch","username":"ttsiouts"},"change_message_id":"c62b33791be3e750a96b50b129c7318086f0939f","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    openstack coe nodegroup create --labels label4\u003dlabel5 \u003ccluster_name\u003e ng2"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"The current functionality will be used and the  resulting labels stored in the"},{"line_number":145,"context_line":"nodegroup will be::"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    labels \u003d {\u0027label4\u0027: \u0027value5\u0027}"}],"source_content_type":"text/x-rst","patch_set":8,"id":"1f493fa4_34d9c1f2","line":144,"range":{"start_line":144,"start_character":46,"end_line":144,"end_character":48},"in_reply_to":"1f493fa4_006e04c8","updated":"2020-04-27 10:55:01.000000000","message":"Done","commit_id":"c085c2220b07713b0e9e2d3eefbc0c5311a7560a"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"cb6d067b91edbc7a8d69cdbde5e74d5b4827c3fb","unresolved":false,"context_lines":[{"line_number":159,"context_line":"The ``GET`` methods of Clusters and Nodegroups APIs will be adapted to show the"},{"line_number":160,"context_line":"differences between the provided and parent labels. The proposed fields are::"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    * overridden_labels: labels that exist in both parent and object labels but"},{"line_number":163,"context_line":"                         have different value"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    * added_labels: labels that do not exist in the parent labels and were"},{"line_number":166,"context_line":"                    added in the object"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    * skipped_labels: labels that exist in the parent dict but do not exist in"},{"line_number":169,"context_line":"                      the object\u0027s labels. Specifically, this field will be"},{"line_number":170,"context_line":"                      used when the user did not provide the --override-labels"},{"line_number":171,"context_line":"                      (used the current functionality) and did not provide some"},{"line_number":172,"context_line":"                      of the labels that exist in the parent."},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"CLI Impact"},{"line_number":175,"context_line":"----------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1f493fa4_e186d9db","line":172,"range":{"start_line":162,"start_character":0,"end_line":172,"end_character":61},"updated":"2020-05-05 09:37:28.000000000","message":"I thought we agreed on labels_*","commit_id":"148a097d25186492dc424f353c6d221681ab9814"},{"author":{"_account_id":27057,"name":"Theodoros Tsioutsias","email":"theodoros.tsioutsias@cern.ch","username":"ttsiouts"},"change_message_id":"f1803d77061ffd6e926d76459b0914031421a718","unresolved":false,"context_lines":[{"line_number":159,"context_line":"The ``GET`` methods of Clusters and Nodegroups APIs will be adapted to show the"},{"line_number":160,"context_line":"differences between the provided and parent labels. The proposed fields are::"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    * overridden_labels: labels that exist in both parent and object labels but"},{"line_number":163,"context_line":"                         have different value"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    * added_labels: labels that do not exist in the parent labels and were"},{"line_number":166,"context_line":"                    added in the object"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    * skipped_labels: labels that exist in the parent dict but do not exist in"},{"line_number":169,"context_line":"                      the object\u0027s labels. Specifically, this field will be"},{"line_number":170,"context_line":"                      used when the user did not provide the --override-labels"},{"line_number":171,"context_line":"                      (used the current functionality) and did not provide some"},{"line_number":172,"context_line":"                      of the labels that exist in the parent."},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"CLI Impact"},{"line_number":175,"context_line":"----------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1f493fa4_4154ed54","line":172,"range":{"start_line":162,"start_character":0,"end_line":172,"end_character":61},"in_reply_to":"1f493fa4_e186d9db","updated":"2020-05-05 09:38:44.000000000","message":"oops... indeed","commit_id":"148a097d25186492dc424f353c6d221681ab9814"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"a340251944fbac393148bac647d64d1517a42619","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    Cluster Template -\u003e Cluster -\u003e Nodegroup"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""}],"source_content_type":"text/x-rst","patch_set":10,"id":"1f493fa4_44ffc966","line":106,"updated":"2020-05-06 03:49:20.000000000","message":"I\u0027m comparing this part with patch set 8 and I can\u0027t see any difference. Did I miss something? Have we all agreed to set the flag as Flase and set True on the client side?","commit_id":"26d573ecaac2ff790c016783468876ab040c7cf6"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"afcf87dc995ed503a1910a3f5abf5cb351c3b6d3","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    Cluster Template -\u003e Cluster -\u003e Nodegroup"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""}],"source_content_type":"text/x-rst","patch_set":10,"id":"1f493fa4_840c3143","line":106,"in_reply_to":"1f493fa4_44ffc966","updated":"2020-05-06 06:31:32.000000000","message":"* The new flag will default to false in the API to not break.\n* The client won\u0027t send anything by default. So not reason to change the REST API Impact section.\n* A follow-up to patch to change the behaviour of the API with a config option can be introduced outside the scope of this spec. The spec will describe what users and other project developers whose project interact with the magnum API should expect.\n\nIs this ok?","commit_id":"26d573ecaac2ff790c016783468876ab040c7cf6"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"bdc65c7e191649a5f7844e123701dacc5e39b20a","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    Cluster Template -\u003e Cluster -\u003e Nodegroup"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"A new boolean flag will be added to the API. The flag\u0027s proposed name is"},{"line_number":107,"context_line":"``--override-labels``. The default value of this flag will be ``False`` meaning"},{"line_number":108,"context_line":"that we will maintain as default the current functionality::"},{"line_number":109,"context_line":""}],"source_content_type":"text/x-rst","patch_set":10,"id":"1f493fa4_76d1d657","line":106,"in_reply_to":"1f493fa4_840c3143","updated":"2020-05-06 08:58:09.000000000","message":"Sounds good to me.","commit_id":"26d573ecaac2ff790c016783468876ab040c7cf6"}]}