{"action":"created","issue":{"url":"https://api.github.com/repos/saltstack/salt/issues/35008","repository_url":"https://api.github.com/repos/saltstack/salt","labels_url":"https://api.github.com/repos/saltstack/salt/issues/35008/labels{/name}","comments_url":"https://api.github.com/repos/saltstack/salt/issues/35008/comments","events_url":"https://api.github.com/repos/saltstack/salt/issues/35008/events","html_url":"https://github.com/saltstack/salt/pull/35008","id":167982195,"number":35008,"title":"Fix multimaster failover on more than two masters and failback behaviour","user":{"login":"hu-dabao","id":3854528,"avatar_url":"https://avatars.githubusercontent.com/u/3854528?v=3","gravatar_id":"","url":"https://api.github.com/users/hu-dabao","html_url":"https://github.com/hu-dabao","followers_url":"https://api.github.com/users/hu-dabao/followers","following_url":"https://api.github.com/users/hu-dabao/following{/other_user}","gists_url":"https://api.github.com/users/hu-dabao/gists{/gist_id}","starred_url":"https://api.github.com/users/hu-dabao/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hu-dabao/subscriptions","organizations_url":"https://api.github.com/users/hu-dabao/orgs","repos_url":"https://api.github.com/users/hu-dabao/repos","events_url":"https://api.github.com/users/hu-dabao/events{/privacy}","received_events_url":"https://api.github.com/users/hu-dabao/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/saltstack/salt/labels/Pending%20Discussion","name":"Pending Discussion","color":"c7def8"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2016-07-27T23:44:22Z","updated_at":"2016-07-29T07:13:59Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/saltstack/salt/pulls/35008","html_url":"https://github.com/saltstack/salt/pull/35008","diff_url":"https://github.com/saltstack/salt/pull/35008.diff","patch_url":"https://github.com/saltstack/salt/pull/35008.patch"},"body":"### What does this PR do?\r\nFix the broken failover behavior when minion is configured to achieve multi-master setup, failover and failback. Actually, there is a bug in the code which cause this. See the following section for details. \r\n\r\nSome config piece:\r\n```\r\nmaster:\r\n  - master1\r\n  - master2\r\n  - master3\r\nmaster_type: failover\r\nmaster_alive_interval: 30\r\nmaster_failback: True\r\nmaster_failback_interval: 30 \r\n```\r\n\r\n\r\n### What issues does this PR fix or reference?\r\nBasically, this PR is trying to fix the broken behavior for multi-master failover. As designed in [multi-master doc](https://docs.saltstack.com/en/latest/topics/tutorials/multimaster_pki.html), salt-minion is supposed to failover to the masters in the list from the first master to the second one and then third, etc. <br>\r\nFollowing sections have the details about what is the broken behavior right now. \r\n\r\n### Previous Behavior\r\nmasters:\r\n`[master1, master2, master3]`\r\nmaster1 is the master minion is currently connecting.\r\n\r\n1. shutdown salt-master service on master1\r\n2. minion connect to master2 based on order,  the local master list changes to <br>\r\n`[master2, master3, master1]`\r\n3. shutdown salt-master service on master2, the local master list changes to  <br>\r\n`[master1, master3, master2]`\r\n4. minion connect to master1 \r\nfor step 4, minion is supposed to switch to connect to master3 by design. The reason is <br>\r\n```python\r\nopts['master'] = [x for x in opts['master_list'] if opts['master'] != x]\r\n```\r\nmakes local master list still shuffles in original master list rather than the updated one in step 2 and so forth.\r\n5. minion fails to connect to master1 due to step 1 and it changes to connect to master2 again since it's local master list is wrongly changed to <br>\r\n`[master2, master3, master1]` <br>\r\nThe code in step 4 I mentioned is the same cause. \r\n6. Later on, minion keeps on trying to connect to master1 and master2 in a dead loop behavior. \r\n\r\n### New Behavior\r\n1. shutdown salt-master service on master1\r\n2. minion connect to master2 based on order,  the local master list changes to <br>\r\n`[master2, master3, master1]`\r\n3. shutdown salt-master service on master2, the local master list changes to <br>\r\n`[master3, master1, master2]`\r\n4. minion connect to master3 \r\n\r\n### Tests written?\r\n\r\nNo\r\n\r\nPlease review [Salt's Contributing Guide](https://docs.saltstack.com/en/latest/topics/development/contributing.html) for best practices.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/saltstack/salt/issues/comments/236111197","html_url":"https://github.com/saltstack/salt/pull/35008#issuecomment-236111197","issue_url":"https://api.github.com/repos/saltstack/salt/issues/35008","id":236111197,"user":{"login":"DmitryKuzmenko","id":4807387,"avatar_url":"https://avatars.githubusercontent.com/u/4807387?v=3","gravatar_id":"","url":"https://api.github.com/users/DmitryKuzmenko","html_url":"https://github.com/DmitryKuzmenko","followers_url":"https://api.github.com/users/DmitryKuzmenko/followers","following_url":"https://api.github.com/users/DmitryKuzmenko/following{/other_user}","gists_url":"https://api.github.com/users/DmitryKuzmenko/gists{/gist_id}","starred_url":"https://api.github.com/users/DmitryKuzmenko/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DmitryKuzmenko/subscriptions","organizations_url":"https://api.github.com/users/DmitryKuzmenko/orgs","repos_url":"https://api.github.com/users/DmitryKuzmenko/repos","events_url":"https://api.github.com/users/DmitryKuzmenko/events{/privacy}","received_events_url":"https://api.github.com/users/DmitryKuzmenko/received_events","type":"User","site_admin":false},"created_at":"2016-07-29T07:13:59Z","updated_at":"2016-07-29T07:13:59Z","body":"@cachedout this patch looks good for me!\r\n\r\n@hu-dabao :+1: Thank you for good catch and patch!"}}
{"actor":{"display_login":"DmitryKuzmenko"}}