{"action":"created","issue":{"url":"https://api.github.com/repos/cilium/cilium/issues/9491","repository_url":"https://api.github.com/repos/cilium/cilium","labels_url":"https://api.github.com/repos/cilium/cilium/issues/9491/labels{/name}","comments_url":"https://api.github.com/repos/cilium/cilium/issues/9491/comments","events_url":"https://api.github.com/repos/cilium/cilium/issues/9491/events","html_url":"https://github.com/cilium/cilium/pull/9491","id":511691002,"node_id":"MDExOlB1bGxSZXF1ZXN0MzMxODQyMjIz","number":9491,"title":"Fix cilium-health node updates","user":{"login":"joestringer","id":1243336,"node_id":"MDQ6VXNlcjEyNDMzMzY=","avatar_url":"https://avatars2.githubusercontent.com/u/1243336?v=4","gravatar_id":"","url":"https://api.github.com/users/joestringer","html_url":"https://github.com/joestringer","followers_url":"https://api.github.com/users/joestringer/followers","following_url":"https://api.github.com/users/joestringer/following{/other_user}","gists_url":"https://api.github.com/users/joestringer/gists{/gist_id}","starred_url":"https://api.github.com/users/joestringer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/joestringer/subscriptions","organizations_url":"https://api.github.com/users/joestringer/orgs","repos_url":"https://api.github.com/users/joestringer/repos","events_url":"https://api.github.com/users/joestringer/events{/privacy}","received_events_url":"https://api.github.com/users/joestringer/received_events","type":"User","site_admin":false},"labels":[{"id":1006941483,"node_id":"MDU6TGFiZWwxMDA2OTQxNDgz","url":"https://api.github.com/repos/cilium/cilium/labels/area/health","name":"area/health","color":"cced93","default":false},{"id":1465562648,"node_id":"MDU6TGFiZWwxNDY1NTYyNjQ4","url":"https://api.github.com/repos/cilium/cilium/labels/needs-backport/1.6","name":"needs-backport/1.6","color":"cef444","default":false},{"id":403149902,"node_id":"MDU6TGFiZWw0MDMxNDk5MDI=","url":"https://api.github.com/repos/cilium/cilium/labels/pending-review","name":"pending-review","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2019-10-24T03:54:55Z","updated_at":"2019-11-04T17:31:21Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/cilium/cilium/pulls/9491","html_url":"https://github.com/cilium/cilium/pull/9491","diff_url":"https://github.com/cilium/cilium/pull/9491.diff","patch_url":"https://github.com/cilium/cilium/pull/9491.patch"},"body":"Fix the handling of updates for existing nodes in `cilium-health`. The first two commits are bugfixes; the last three are to unit-test the functionality. Read commit-by-commit for further details.\r\n\r\nBroadly this PR removes the previous \"mark-and-sweep\" GC logic for nodes that previously existed in the `cilium-health` prober, because since PR #8300 we have had the exact context of added/removed nodes, meaning that we should not need to sweep the entire list.\r\n\r\nThere were two subtle bugs in the previous implementation:\r\n* When a node is updated, `cilium-health` would continue to probe IPs that no longer correspond to the node\r\n* Removal of a node would not disable TCP probing for all IPs corresponding to the node, it would only stop probing the primary IP of the node\r\n\r\nFunctionally when a node is updated (eg the health endpoint IP changes), the Cilium nodes API will propagate notifications that include the node in the `removed` nodes list as well as the `added` nodes list, and in this PR this will cause a reset of the node's cached prober results. It's not clear to me right now that there's any advantage to ensuring that the internally cached node representation inside the `cilium-health` prober would benefit from actually retaining the existing entry in the nodes map if the individual IP doesn't change; it's simpler to just handle the update in this scenario by actually deleting and adding the node. The following probe cycle will subsequently update the results for the IPs corresponding to the changed node. Strictly speaking this means there may be intermittent periods when nodes are updated where the `cilium-health` instance won't have prober results for the updated nodes. \r\n\r\nRelated: #9103 \r\n\r\n<!-- Reviewable:start -->\r\n---\r\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/cilium/cilium/9491)\r\n<!-- Reviewable:end -->\r\n"},"comment":{"url":"https://api.github.com/repos/cilium/cilium/issues/comments/549462627","html_url":"https://github.com/cilium/cilium/pull/9491#issuecomment-549462627","issue_url":"https://api.github.com/repos/cilium/cilium/issues/9491","id":549462627,"node_id":"MDEyOklzc3VlQ29tbWVudDU0OTQ2MjYyNw==","user":{"login":"joestringer","id":1243336,"node_id":"MDQ6VXNlcjEyNDMzMzY=","avatar_url":"https://avatars2.githubusercontent.com/u/1243336?v=4","gravatar_id":"","url":"https://api.github.com/users/joestringer","html_url":"https://github.com/joestringer","followers_url":"https://api.github.com/users/joestringer/followers","following_url":"https://api.github.com/users/joestringer/following{/other_user}","gists_url":"https://api.github.com/users/joestringer/gists{/gist_id}","starred_url":"https://api.github.com/users/joestringer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/joestringer/subscriptions","organizations_url":"https://api.github.com/users/joestringer/orgs","repos_url":"https://api.github.com/users/joestringer/repos","events_url":"https://api.github.com/users/joestringer/events{/privacy}","received_events_url":"https://api.github.com/users/joestringer/received_events","type":"User","site_admin":false},"created_at":"2019-11-04T17:31:21Z","updated_at":"2019-11-04T17:31:21Z","author_association":"CONTRIBUTOR","body":"test-me-please"}}
{"actor":{"display_login":"joestringer"}}