{"action":"created","issue":{"url":"https://api.github.com/repos/saltstack/salt/issues/49811","repository_url":"https://api.github.com/repos/saltstack/salt","labels_url":"https://api.github.com/repos/saltstack/salt/issues/49811/labels{/name}","comments_url":"https://api.github.com/repos/saltstack/salt/issues/49811/comments","events_url":"https://api.github.com/repos/saltstack/salt/issues/49811/events","html_url":"https://github.com/saltstack/salt/pull/49811","id":364648712,"node_id":"MDExOlB1bGxSZXF1ZXN0MjE4Nzg1Mzk5","number":49811,"title":"Optimize firewalld state","user":{"login":"nhavens","id":754908,"node_id":"MDQ6VXNlcjc1NDkwOA==","avatar_url":"https://avatars1.githubusercontent.com/u/754908?v=4","gravatar_id":"","url":"https://api.github.com/users/nhavens","html_url":"https://github.com/nhavens","followers_url":"https://api.github.com/users/nhavens/followers","following_url":"https://api.github.com/users/nhavens/following{/other_user}","gists_url":"https://api.github.com/users/nhavens/gists{/gist_id}","starred_url":"https://api.github.com/users/nhavens/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nhavens/subscriptions","organizations_url":"https://api.github.com/users/nhavens/orgs","repos_url":"https://api.github.com/users/nhavens/repos","events_url":"https://api.github.com/users/nhavens/events{/privacy}","received_events_url":"https://api.github.com/users/nhavens/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-09-27T20:52:08Z","updated_at":"2018-10-05T14:58:08Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/saltstack/salt/pulls/49811","html_url":"https://github.com/saltstack/salt/pull/49811","diff_url":"https://github.com/saltstack/salt/pull/49811.diff","patch_url":"https://github.com/saltstack/salt/pull/49811.patch"},"body":"### What does this PR do?\r\nThis PR removes refactors the firewalld.present state function such that it only calls firewalld execution module functions when necessary. This significantly improves the firewalld.present state function performance.\r\n\r\n### What issues does this PR fix or reference?\r\nFixes #44979\r\n\r\n### Previous Behavior\r\nPreviously, the firewalld.present state function called many firewalld execution module functions even if the calls were unnecessary based on the sls file being processed.\r\n\r\n### New Behavior\r\nOnly call firewalld execution module functions if necessary based on the sls file being processed.\r\n\r\n### Tests written?\r\nNo - functionality should not have changed. Existing tests should suffice.\r\n\r\n### Commits signed with GPG?\r\nYes"},"comment":{"url":"https://api.github.com/repos/saltstack/salt/issues/comments/427394926","html_url":"https://github.com/saltstack/salt/pull/49811#issuecomment-427394926","issue_url":"https://api.github.com/repos/saltstack/salt/issues/49811","id":427394926,"node_id":"MDEyOklzc3VlQ29tbWVudDQyNzM5NDkyNg==","user":{"login":"nhavens","id":754908,"node_id":"MDQ6VXNlcjc1NDkwOA==","avatar_url":"https://avatars1.githubusercontent.com/u/754908?v=4","gravatar_id":"","url":"https://api.github.com/users/nhavens","html_url":"https://github.com/nhavens","followers_url":"https://api.github.com/users/nhavens/followers","following_url":"https://api.github.com/users/nhavens/following{/other_user}","gists_url":"https://api.github.com/users/nhavens/gists{/gist_id}","starred_url":"https://api.github.com/users/nhavens/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nhavens/subscriptions","organizations_url":"https://api.github.com/users/nhavens/orgs","repos_url":"https://api.github.com/users/nhavens/repos","events_url":"https://api.github.com/users/nhavens/events{/privacy}","received_events_url":"https://api.github.com/users/nhavens/received_events","type":"User","site_admin":false},"created_at":"2018-10-05T14:58:08Z","updated_at":"2018-10-05T14:58:08Z","author_association":"CONTRIBUTOR","body":"@hakoerber I apologize if I'm missing something obvious, but I can't seem to find your comments. If you've identified more opportunities for improvement, I'd be happy to incorporate your suggestions.\r\n\r\nFWIW, I tested by setting `log_level_logfile: info` in /etc/salt/minion on my salt master, which is also a minion of itself. This setting is sufficient to get all commands executed by a state.apply (by the firewalld execution module) in the minion logfile. I applied sls files like the following, and watched the logs to ensure only the necessary commands were executed:\r\n```\r\nbasic firewalld test to check block_icmp code improvements:\r\n  firewalld.present:\r\n    - name: public\r\n    - services:\r\n      - ssh\r\n    - block_icmp:\r\n      - echo-request\r\n      - echo-reply\r\n      - lkjfj\r\n```\r\n"}}
{"actor":{"display_login":"nhavens"}}