)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000038,"name":"Jan Schär","display_name":"Jan","email":"jan@monogon.tech","username":"jan","avatars":[{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"9de552e256728d8a14e600bf6d3c7c5342c4e0e0","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2bb6b90e_60e874d0","updated":"2024-10-02 08:23:21.000000000","message":"Why do we need to make the synchronized labels configurable? Is there a case where a user does not want a label to be synchronized to Kubernetes? It appears that this is only configurable for technical reasons, so that we can determine which labels to remove. But Kubernetes already keeps track of which labels have been added by our field manager. I would suggest to drop the NodeLabelsToSynchronizeToKubernetes config, and instead use the info in managedFields to determine when labels need to be removed.\n\nKubernetes actually does almost all the work for us, we only need to send an Apply which does not include the label anymore, and that will remove it from the Kubernetes object. The only thing we need to do is to determine whether we need to send an Apply request or not. You can try using [ExtractNode](https://pkg.go.dev/k8s.io/client-go@v0.31.1/applyconfigurations/core/v1#ExtractNode) for this. This would replace filterManaged.","commit_id":"689ee0ef782012ce356afcc79b615591001e7248"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"3d34ee21489aa83f15c9ec1dbba1fd1d02130c6f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"30f3ae6b_5f545d63","in_reply_to":"2bb6b90e_60e874d0","updated":"2024-10-07 10:19:56.000000000","message":"Because Metropolis node labels and Kubernetes node labels are two entirely separate worlds. Kubernetes is just one application running on top of Metropolis, and Metropolis clusters do not exist just for the purpose of maintaining a Kubernetes cluster. Labels on one might make no sense in the other, etc. Being prudent (and explicit) about which ones should be synchronized seems to be the correct thing to do.\n\nPrinciple of least surprise applies, too: as a user, I would not expect all Metropolis node labels to automatically become Kubernetes node labels, unless opted in.\n\nThere are also potential footguns: unknowingly creating Metropolis labels that might end up being acted upon by Kubernetes node-watching code, for example.","commit_id":"689ee0ef782012ce356afcc79b615591001e7248"},{"author":{"_account_id":1000038,"name":"Jan Schär","display_name":"Jan","email":"jan@monogon.tech","username":"jan","avatars":[{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"5a983eed906cea85ba011b02cceeb204a716f6aa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"45a9551b_08e2d009","in_reply_to":"30f3ae6b_5f545d63","updated":"2024-10-07 15:11:03.000000000","message":"Acknowledged","commit_id":"689ee0ef782012ce356afcc79b615591001e7248"}],"metropolis/node/kubernetes/labelmaker.go":[{"author":{"_account_id":1000038,"name":"Jan Schär","display_name":"Jan","email":"jan@monogon.tech","username":"jan","avatars":[{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"5a983eed906cea85ba011b02cceeb204a716f6aa","unresolved":true,"context_lines":[{"line_number":223,"context_line":"\t\treturn nil"},{"line_number":224,"context_line":"\t}"},{"line_number":225,"context_line":"\tintent \u003d intent.filterManaged(labelRegexes)"},{"line_number":226,"context_line":"\tstate \u003d state.filterManaged(labelRegexes)"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"\tif labelsEqual(intent, state) {"},{"line_number":229,"context_line":"\t\treturn nil"}],"source_content_type":"text/x-go","patch_set":1,"id":"d506369f_f3bfc111","line":226,"range":{"start_line":226,"start_character":15,"end_line":226,"end_character":28},"updated":"2024-10-07 15:11:03.000000000","message":"Even if we keep the regexes config, we should use [ExtractNode](https://pkg.go.dev/k8s.io/client-go@v0.31.1/applyconfigurations/core/v1#ExtractNode) to filter the current state instead of filterManaged.\n\n`state` is used to predict whether an Apply will cause any changes, and this prediction can be wrong when using filterManaged in these cases:\n\n1. If the user adds a label directly to the Kubernetes node, where the label is matched by a regex, but the label does not exist on the Metropolis node, then calling Apply will not remove the label, because the label is not managed by the metropolis-labelmaker FieldManager. But the code is not aware of that and will call Apply in each iteration.\n2. If the user changes NodeLabelsToSynchronizeToKubernetes and removes a regex, calling Apply will remove the labels that have previously been synchronized due to this regex. But the code doesn\u0027t notice this and does not call Apply. Thus, the labels will only be removed later, when there is some other change which triggers Apply to be called.","commit_id":"689ee0ef782012ce356afcc79b615591001e7248"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"f4d521b13aedbd39f65648e9e57d6422a3525640","unresolved":false,"context_lines":[{"line_number":223,"context_line":"\t\treturn nil"},{"line_number":224,"context_line":"\t}"},{"line_number":225,"context_line":"\tintent \u003d intent.filterManaged(labelRegexes)"},{"line_number":226,"context_line":"\tstate \u003d state.filterManaged(labelRegexes)"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"\tif labelsEqual(intent, state) {"},{"line_number":229,"context_line":"\t\treturn nil"}],"source_content_type":"text/x-go","patch_set":1,"id":"f16a6156_6065cef6","line":226,"range":{"start_line":226,"start_character":15,"end_line":226,"end_character":28},"in_reply_to":"d506369f_f3bfc111","updated":"2024-10-14 13:56:06.000000000","message":"Done","commit_id":"689ee0ef782012ce356afcc79b615591001e7248"},{"author":{"_account_id":1000038,"name":"Jan Schär","display_name":"Jan","email":"jan@monogon.tech","username":"jan","avatars":[{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"2e7de7d946ab5392fcf7340df685d3b3be63112d","unresolved":true,"context_lines":[{"line_number":182,"context_line":"\t\treturn err"},{"line_number":183,"context_line":"\t}"},{"line_number":184,"context_line":"\tif state \u003d\u003d nil {"},{"line_number":185,"context_line":"\t\tsupervisor.Logger(ctx).Infof(\"Node %s does not exist in Kubernetes, skipping\", node.Id)"},{"line_number":186,"context_line":"\t\treturn nil"},{"line_number":187,"context_line":"\t}"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-go","patch_set":8,"id":"c29b1001_631dd19e","line":185,"range":{"start_line":185,"start_character":32,"end_line":185,"end_character":78},"updated":"2024-10-15 09:08:47.000000000","message":"Remove this log, it\u0027s expected that not all nodes exist in Kubernetes.","commit_id":"ad449ecb6a2d4d57f953a94eb23c19e9ead525bc"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"f5094d1ed0c88d086a38a2a1b74c68871f97164e","unresolved":false,"context_lines":[{"line_number":182,"context_line":"\t\treturn err"},{"line_number":183,"context_line":"\t}"},{"line_number":184,"context_line":"\tif state \u003d\u003d nil {"},{"line_number":185,"context_line":"\t\tsupervisor.Logger(ctx).Infof(\"Node %s does not exist in Kubernetes, skipping\", node.Id)"},{"line_number":186,"context_line":"\t\treturn nil"},{"line_number":187,"context_line":"\t}"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-go","patch_set":8,"id":"fbc1c48f_d596520f","line":185,"range":{"start_line":185,"start_character":32,"end_line":185,"end_character":78},"in_reply_to":"c29b1001_631dd19e","updated":"2024-10-15 11:49:58.000000000","message":"Done","commit_id":"ad449ecb6a2d4d57f953a94eb23c19e9ead525bc"}],"metropolis/test/e2e/suites/kubernetes/run_test.go":[{"author":{"_account_id":1000038,"name":"Jan Schär","display_name":"Jan","email":"jan@monogon.tech","username":"jan","avatars":[{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/fd0e7f48847aa0e46c8f361df2d6c26b.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"5a983eed906cea85ba011b02cceeb204a716f6aa","unresolved":true,"context_lines":[{"line_number":178,"context_line":"\t\t\t{Key: \"test.monogon.dev/foo\", Value: \"bar\"},"},{"line_number":179,"context_line":"\t\t},"},{"line_number":180,"context_line":"\t})"},{"line_number":181,"context_line":"\ttime.Sleep(5 * time.Second)"},{"line_number":182,"context_line":"\tif err !\u003d nil {"},{"line_number":183,"context_line":"\t\tt.Fatalf(\"Could not add label to node: %v\", err)"},{"line_number":184,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":1,"id":"ebfa7e5c_2c126b0f","line":181,"range":{"start_line":181,"start_character":1,"end_line":181,"end_character":28},"updated":"2024-10-07 15:11:03.000000000","message":"I would use MustTestEventual to make this more robust, and also faster, because that retries every second.","commit_id":"689ee0ef782012ce356afcc79b615591001e7248"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"530252a6a2e959dec6c2ca4d3e73dcf1fb59781f","unresolved":false,"context_lines":[{"line_number":178,"context_line":"\t\t\t{Key: \"test.monogon.dev/foo\", Value: \"bar\"},"},{"line_number":179,"context_line":"\t\t},"},{"line_number":180,"context_line":"\t})"},{"line_number":181,"context_line":"\ttime.Sleep(5 * time.Second)"},{"line_number":182,"context_line":"\tif err !\u003d nil {"},{"line_number":183,"context_line":"\t\tt.Fatalf(\"Could not add label to node: %v\", err)"},{"line_number":184,"context_line":"\t}"}],"source_content_type":"text/x-go","patch_set":1,"id":"6f62399a_39fc19e4","line":181,"range":{"start_line":181,"start_character":1,"end_line":181,"end_character":28},"in_reply_to":"ebfa7e5c_2c126b0f","updated":"2024-10-07 17:26:45.000000000","message":"Done","commit_id":"689ee0ef782012ce356afcc79b615591001e7248"}]}