{"action":"created","comment":{"url":"https://api.github.com/repos/strimzi/strimzi/pulls/comments/190610530","pull_request_review_id":123022905,"id":190610530,"diff_hunk":"@@ -205,15 +208,53 @@ public void onClose(KubernetesClientException e) {\n         }\n     }\n \n+    private static ObjectMeta templateMetadata(StatefulSet resource) {\n+        return resource.getSpec().getTemplate().getMetadata();\n+    }\n+\n     public String getPodName(StatefulSet desired, int podId) {\n-        return desired.getMetadata().getName() + \"-\" + podId;\n+        return templateMetadata(desired).getName() + \"-\" + podId;\n+    }\n+\n+    private void setGeneration(StatefulSet desired, int nextGeneration) {\n+        templateMetadata(desired).getAnnotations().put(ANNOTATION_GENERATION, String.valueOf(nextGeneration));\n+    }\n+\n+    private static int getGeneration(ObjectMeta objectMeta) {\n+        if (objectMeta.getAnnotations().get(ANNOTATION_GENERATION) == null) {\n+            new RuntimeException().printStackTrace();\n+        }\n+        return Integer.parseInt(objectMeta.getAnnotations().getOrDefault(ANNOTATION_GENERATION, \"-2\"));\n+    }\n+\n+    protected void incrementGeneration(StatefulSet current, StatefulSet desired) {\n+        final int generation = Integer.parseInt(templateMetadata(current).getAnnotations().getOrDefault(ANNOTATION_GENERATION, \"1\"));\n+        final int nextGeneration = generation + 1;\n+        setGeneration(desired, nextGeneration);\n+    }\n+\n+    protected abstract boolean shouldIncrementGeneration(StatefulSet current, StatefulSet desired);\n+\n+    private static int getSsGeneration(StatefulSet resource) {\n+        if (resource == null) {\n+            return -3;\n+        }\n+        return getGeneration(templateMetadata(resource));\n+    }\n+\n+    private static int getPodGeneration(Pod resource) {\n+        if (resource == null) {\n+            return -4;\n+        }\n+        return getGeneration(resource.getMetadata());","path":"cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/resource/StatefulSetOperator.java","position":225,"original_position":225,"commit_id":"dc83c0521c7bdc44febd1cd9046f0308cb722da6","original_commit_id":"dc83c0521c7bdc44febd1cd9046f0308cb722da6","user":{"login":"ppatierno","id":5842311,"avatar_url":"https://avatars0.githubusercontent.com/u/5842311?v=4","gravatar_id":"","url":"https://api.github.com/users/ppatierno","html_url":"https://github.com/ppatierno","followers_url":"https://api.github.com/users/ppatierno/followers","following_url":"https://api.github.com/users/ppatierno/following{/other_user}","gists_url":"https://api.github.com/users/ppatierno/gists{/gist_id}","starred_url":"https://api.github.com/users/ppatierno/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ppatierno/subscriptions","organizations_url":"https://api.github.com/users/ppatierno/orgs","repos_url":"https://api.github.com/users/ppatierno/repos","events_url":"https://api.github.com/users/ppatierno/events{/privacy}","received_events_url":"https://api.github.com/users/ppatierno/received_events","type":"User","site_admin":false},"body":"Do these -3 and -4 values mean something useful?","created_at":"2018-05-24T14:36:20Z","updated_at":"2018-05-24T14:59:49Z","html_url":"https://github.com/strimzi/strimzi/pull/462#discussion_r190610530","pull_request_url":"https://api.github.com/repos/strimzi/strimzi/pulls/462","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/strimzi/strimzi/pulls/comments/190610530"},"html":{"href":"https://github.com/strimzi/strimzi/pull/462#discussion_r190610530"},"pull_request":{"href":"https://api.github.com/repos/strimzi/strimzi/pulls/462"}}},"pull_request":{"url":"https://api.github.com/repos/strimzi/strimzi/pulls/462","id":189730585,"html_url":"https://github.com/strimzi/strimzi/pull/462","diff_url":"https://github.com/strimzi/strimzi/pull/462.diff","patch_url":"https://github.com/strimzi/strimzi/pull/462.patch","issue_url":"https://api.github.com/repos/strimzi/strimzi/issues/462","number":462,"state":"open","locked":false,"title":"Better rolling","user":{"login":"tombentley","id":879487,"avatar_url":"https://avatars3.githubusercontent.com/u/879487?v=4","gravatar_id":"","url":"https://api.github.com/users/tombentley","html_url":"https://github.com/tombentley","followers_url":"https://api.github.com/users/tombentley/followers","following_url":"https://api.github.com/users/tombentley/following{/other_user}","gists_url":"https://api.github.com/users/tombentley/gists{/gist_id}","starred_url":"https://api.github.com/users/tombentley/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tombentley/subscriptions","organizations_url":"https://api.github.com/users/tombentley/orgs","repos_url":"https://api.github.com/users/tombentley/repos","events_url":"https://api.github.com/users/tombentley/events{/privacy}","received_events_url":"https://api.github.com/users/tombentley/received_events","type":"User","site_admin":false},"body":"### Type of change\r\n\r\n- Enhancement / new feature\r\n\r\n### Description\r\n\r\nThis PR fixes #363 by changing the way StatefulSets are updated/rolled so that if the CO crashes part way through it will complete the roll when it restarts.\r\n\r\nUsing the same logic as before, whenever we patch in a way that needs a rolling update we update the SS template, incrementing the value of a 'generation' annotation. This happens as part of the SS patch. Previously, we only did the rolling update if the patch operation indicated that it was needed, but now (because a rolling update may be necessary even if the last patch was a noop) we always call `maybeRollingUpdate()`, and let that method decide which pods to roll. Since rolled pods will pick up the new generation number from the SS's template we can distinguish rolled pods from unrolled ones. Thus we only actually rolls pods whose generation annotation does not match the StatefulSets, and so `maybeRollingUpdate()` will be a noop iff all the pods' generation numbers match.\r\n\r\nThe changes are very noisy because we no longer need to return a boolean from the reconcile method on the SS: We actually return the whole updated SS, wrapped in a `ReconciliationResult` (it would be enough to return the generation number), which means we no longer need the type parameter `P`, which makes for a lot of noise.\r\n\r\nIt would now be possible to combine the `reconcile()` and `maybeRollingUpdate()` operations for statefulsets, because we now always do them sequentially, but I've not done that in this PR because I thought it was already a big change.\r\n\r\n### Checklist\r\n\r\n- [x] Write tests\r\n- [x] Make sure all tests pass\r\n- [ ] ~Update documentation~\r\n- [ ] ~Check RBAC rights for Kubernetes / OpenShift roles~\r\n- [ ] ~Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally~\r\n- [ ] ~Reference relevant issue(s) and close them after merging~\r\n\r\n","created_at":"2018-05-22T16:59:48Z","updated_at":"2018-05-24T14:59:49Z","closed_at":null,"merged_at":null,"merge_commit_sha":"d86e7d7ec465c0561a42651fa0c11325546c6159","assignee":null,"assignees":[],"requested_reviewers":[{"login":"scholzj","id":5658439,"avatar_url":"https://avatars0.githubusercontent.com/u/5658439?v=4","gravatar_id":"","url":"https://api.github.com/users/scholzj","html_url":"https://github.com/scholzj","followers_url":"https://api.github.com/users/scholzj/followers","following_url":"https://api.github.com/users/scholzj/following{/other_user}","gists_url":"https://api.github.com/users/scholzj/gists{/gist_id}","starred_url":"https://api.github.com/users/scholzj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/scholzj/subscriptions","organizations_url":"https://api.github.com/users/scholzj/orgs","repos_url":"https://api.github.com/users/scholzj/repos","events_url":"https://api.github.com/users/scholzj/events{/privacy}","received_events_url":"https://api.github.com/users/scholzj/received_events","type":"User","site_admin":false}],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/strimzi/strimzi/pulls/462/commits","review_comments_url":"https://api.github.com/repos/strimzi/strimzi/pulls/462/comments","review_comment_url":"https://api.github.com/repos/strimzi/strimzi/pulls/comments{/number}","comments_url":"https://api.github.com/repos/strimzi/strimzi/issues/462/comments","statuses_url":"https://api.github.com/repos/strimzi/strimzi/statuses/dc83c0521c7bdc44febd1cd9046f0308cb722da6","head":{"label":"strimzi:better-rolling","ref":"better-rolling","sha":"dc83c0521c7bdc44febd1cd9046f0308cb722da6","user":{"login":"strimzi","id":34767428,"avatar_url":"https://avatars1.githubusercontent.com/u/34767428?v=4","gravatar_id":"","url":"https://api.github.com/users/strimzi","html_url":"https://github.com/strimzi","followers_url":"https://api.github.com/users/strimzi/followers","following_url":"https://api.github.com/users/strimzi/following{/other_user}","gists_url":"https://api.github.com/users/strimzi/gists{/gist_id}","starred_url":"https://api.github.com/users/strimzi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/strimzi/subscriptions","organizations_url":"https://api.github.com/users/strimzi/orgs","repos_url":"https://api.github.com/users/strimzi/repos","events_url":"https://api.github.com/users/strimzi/events{/privacy}","received_events_url":"https://api.github.com/users/strimzi/received_events","type":"Organization","site_admin":false},"repo":{"id":58194180,"name":"strimzi","full_name":"strimzi/strimzi","owner":{"login":"strimzi","id":34767428,"avatar_url":"https://avatars1.githubusercontent.com/u/34767428?v=4","gravatar_id":"","url":"https://api.github.com/users/strimzi","html_url":"https://github.com/strimzi","followers_url":"https://api.github.com/users/strimzi/followers","following_url":"https://api.github.com/users/strimzi/following{/other_user}","gists_url":"https://api.github.com/users/strimzi/gists{/gist_id}","starred_url":"https://api.github.com/users/strimzi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/strimzi/subscriptions","organizations_url":"https://api.github.com/users/strimzi/orgs","repos_url":"https://api.github.com/users/strimzi/repos","events_url":"https://api.github.com/users/strimzi/events{/privacy}","received_events_url":"https://api.github.com/users/strimzi/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/strimzi/strimzi","description":"Apache Kafka running on Kubernetes and OpenShift","fork":false,"url":"https://api.github.com/repos/strimzi/strimzi","forks_url":"https://api.github.com/repos/strimzi/strimzi/forks","keys_url":"https://api.github.com/repos/strimzi/strimzi/keys{/key_id}","collaborators_url":"https://api.github.com/repos/strimzi/strimzi/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/strimzi/strimzi/teams","hooks_url":"https://api.github.com/repos/strimzi/strimzi/hooks","issue_events_url":"https://api.github.com/repos/strimzi/strimzi/issues/events{/number}","events_url":"https://api.github.com/repos/strimzi/strimzi/events","assignees_url":"https://api.github.com/repos/strimzi/strimzi/assignees{/user}","branches_url":"https://api.github.com/repos/strimzi/strimzi/branches{/branch}","tags_url":"https://api.github.com/repos/strimzi/strimzi/tags","blobs_url":"https://api.github.com/repos/strimzi/strimzi/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/strimzi/strimzi/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/strimzi/strimzi/git/refs{/sha}","trees_url":"https://api.github.com/repos/strimzi/strimzi/git/trees{/sha}","statuses_url":"https://api.github.com/repos/strimzi/strimzi/statuses/{sha}","languages_url":"https://api.github.com/repos/strimzi/strimzi/languages","stargazers_url":"https://api.github.com/repos/strimzi/strimzi/stargazers","contributors_url":"https://api.github.com/repos/strimzi/strimzi/contributors","subscribers_url":"https://api.github.com/repos/strimzi/strimzi/subscribers","subscription_url":"https://api.github.com/repos/strimzi/strimzi/subscription","commits_url":"https://api.github.com/repos/strimzi/strimzi/commits{/sha}","git_commits_url":"https://api.github.com/repos/strimzi/strimzi/git/commits{/sha}","comments_url":"https://api.github.com/repos/strimzi/strimzi/comments{/number}","issue_comment_url":"https://api.github.com/repos/strimzi/strimzi/issues/comments{/number}","contents_url":"https://api.github.com/repos/strimzi/strimzi/contents/{+path}","compare_url":"https://api.github.com/repos/strimzi/strimzi/compare/{base}...{head}","merges_url":"https://api.github.com/repos/strimzi/strimzi/merges","archive_url":"https://api.github.com/repos/strimzi/strimzi/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/strimzi/strimzi/downloads","issues_url":"https://api.github.com/repos/strimzi/strimzi/issues{/number}","pulls_url":"https://api.github.com/repos/strimzi/strimzi/pulls{/number}","milestones_url":"https://api.github.com/repos/strimzi/strimzi/milestones{/number}","notifications_url":"https://api.github.com/repos/strimzi/strimzi/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/strimzi/strimzi/labels{/name}","releases_url":"https://api.github.com/repos/strimzi/strimzi/releases{/id}","deployments_url":"https://api.github.com/repos/strimzi/strimzi/deployments","created_at":"2016-05-06T08:52:33Z","updated_at":"2018-05-24T14:35:01Z","pushed_at":"2018-05-24T14:33:57Z","git_url":"git://github.com/strimzi/strimzi.git","ssh_url":"git@github.com:strimzi/strimzi.git","clone_url":"https://github.com/strimzi/strimzi.git","svn_url":"https://github.com/strimzi/strimzi","homepage":"http://strimzi.io/","size":2877,"stargazers_count":272,"watchers_count":272,"language":"Java","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":53,"mirror_url":null,"archived":false,"open_issues_count":68,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0"},"forks":53,"open_issues":68,"watchers":272,"default_branch":"master"}},"base":{"label":"strimzi:master","ref":"master","sha":"55de89cb876057fe05734721133aaee5fc2c0d1e","user":{"login":"strimzi","id":34767428,"avatar_url":"https://avatars1.githubusercontent.com/u/34767428?v=4","gravatar_id":"","url":"https://api.github.com/users/strimzi","html_url":"https://github.com/strimzi","followers_url":"https://api.github.com/users/strimzi/followers","following_url":"https://api.github.com/users/strimzi/following{/other_user}","gists_url":"https://api.github.com/users/strimzi/gists{/gist_id}","starred_url":"https://api.github.com/users/strimzi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/strimzi/subscriptions","organizations_url":"https://api.github.com/users/strimzi/orgs","repos_url":"https://api.github.com/users/strimzi/repos","events_url":"https://api.github.com/users/strimzi/events{/privacy}","received_events_url":"https://api.github.com/users/strimzi/received_events","type":"Organization","site_admin":false},"repo":{"id":58194180,"name":"strimzi","full_name":"strimzi/strimzi","owner":{"login":"strimzi","id":34767428,"avatar_url":"https://avatars1.githubusercontent.com/u/34767428?v=4","gravatar_id":"","url":"https://api.github.com/users/strimzi","html_url":"https://github.com/strimzi","followers_url":"https://api.github.com/users/strimzi/followers","following_url":"https://api.github.com/users/strimzi/following{/other_user}","gists_url":"https://api.github.com/users/strimzi/gists{/gist_id}","starred_url":"https://api.github.com/users/strimzi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/strimzi/subscriptions","organizations_url":"https://api.github.com/users/strimzi/orgs","repos_url":"https://api.github.com/users/strimzi/repos","events_url":"https://api.github.com/users/strimzi/events{/privacy}","received_events_url":"https://api.github.com/users/strimzi/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/strimzi/strimzi","description":"Apache Kafka running on Kubernetes and OpenShift","fork":false,"url":"https://api.github.com/repos/strimzi/strimzi","forks_url":"https://api.github.com/repos/strimzi/strimzi/forks","keys_url":"https://api.github.com/repos/strimzi/strimzi/keys{/key_id}","collaborators_url":"https://api.github.com/repos/strimzi/strimzi/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/strimzi/strimzi/teams","hooks_url":"https://api.github.com/repos/strimzi/strimzi/hooks","issue_events_url":"https://api.github.com/repos/strimzi/strimzi/issues/events{/number}","events_url":"https://api.github.com/repos/strimzi/strimzi/events","assignees_url":"https://api.github.com/repos/strimzi/strimzi/assignees{/user}","branches_url":"https://api.github.com/repos/strimzi/strimzi/branches{/branch}","tags_url":"https://api.github.com/repos/strimzi/strimzi/tags","blobs_url":"https://api.github.com/repos/strimzi/strimzi/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/strimzi/strimzi/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/strimzi/strimzi/git/refs{/sha}","trees_url":"https://api.github.com/repos/strimzi/strimzi/git/trees{/sha}","statuses_url":"https://api.github.com/repos/strimzi/strimzi/statuses/{sha}","languages_url":"https://api.github.com/repos/strimzi/strimzi/languages","stargazers_url":"https://api.github.com/repos/strimzi/strimzi/stargazers","contributors_url":"https://api.github.com/repos/strimzi/strimzi/contributors","subscribers_url":"https://api.github.com/repos/strimzi/strimzi/subscribers","subscription_url":"https://api.github.com/repos/strimzi/strimzi/subscription","commits_url":"https://api.github.com/repos/strimzi/strimzi/commits{/sha}","git_commits_url":"https://api.github.com/repos/strimzi/strimzi/git/commits{/sha}","comments_url":"https://api.github.com/repos/strimzi/strimzi/comments{/number}","issue_comment_url":"https://api.github.com/repos/strimzi/strimzi/issues/comments{/number}","contents_url":"https://api.github.com/repos/strimzi/strimzi/contents/{+path}","compare_url":"https://api.github.com/repos/strimzi/strimzi/compare/{base}...{head}","merges_url":"https://api.github.com/repos/strimzi/strimzi/merges","archive_url":"https://api.github.com/repos/strimzi/strimzi/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/strimzi/strimzi/downloads","issues_url":"https://api.github.com/repos/strimzi/strimzi/issues{/number}","pulls_url":"https://api.github.com/repos/strimzi/strimzi/pulls{/number}","milestones_url":"https://api.github.com/repos/strimzi/strimzi/milestones{/number}","notifications_url":"https://api.github.com/repos/strimzi/strimzi/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/strimzi/strimzi/labels{/name}","releases_url":"https://api.github.com/repos/strimzi/strimzi/releases{/id}","deployments_url":"https://api.github.com/repos/strimzi/strimzi/deployments","created_at":"2016-05-06T08:52:33Z","updated_at":"2018-05-24T14:35:01Z","pushed_at":"2018-05-24T14:33:57Z","git_url":"git://github.com/strimzi/strimzi.git","ssh_url":"git@github.com:strimzi/strimzi.git","clone_url":"https://github.com/strimzi/strimzi.git","svn_url":"https://github.com/strimzi/strimzi","homepage":"http://strimzi.io/","size":2877,"stargazers_count":272,"watchers_count":272,"language":"Java","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":53,"mirror_url":null,"archived":false,"open_issues_count":68,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0"},"forks":53,"open_issues":68,"watchers":272,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/strimzi/strimzi/pulls/462"},"html":{"href":"https://github.com/strimzi/strimzi/pull/462"},"issue":{"href":"https://api.github.com/repos/strimzi/strimzi/issues/462"},"comments":{"href":"https://api.github.com/repos/strimzi/strimzi/issues/462/comments"},"review_comments":{"href":"https://api.github.com/repos/strimzi/strimzi/pulls/462/comments"},"review_comment":{"href":"https://api.github.com/repos/strimzi/strimzi/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/strimzi/strimzi/pulls/462/commits"},"statuses":{"href":"https://api.github.com/repos/strimzi/strimzi/statuses/dc83c0521c7bdc44febd1cd9046f0308cb722da6"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"ppatierno"}}